leonardo icon indicating copy to clipboard operation
leonardo copied to clipboard

Fix usage of contrast-colors with commonjs and esm

Open dschmidt opened this issue 1 year ago • 5 comments

Description

Fix usage as commonjs and esm module and add dependencies of contrast-colors to its package.json.

As .mjs file extension is used for esm module files, the module type can be set to commonjs, which is what .js files are. The main property of package.json should now point to the commonjs file in dist/ - index.js was not working at all, as there is index.mjs in the package root and index.js only in the dist folder. The module property obviously can point to index.mjs. The exports property apparently needs import and require to make importing and requiring the module work, I tried all sorts of other combinations, but this was the only variant that worked for me.

I had to adjust the imports, I assume bundlers handle it without the file extension, but using it in vanilla node requires the file extension: .mjs in this case - independently of the type property in package.json.

I did not update the lock files as I'm not sure where and how exactly I should run npm or yarn - also my versions seemed newer than yours and caused a huge amount of changes in those files, so maybe it's better if you do it yourselves.

See https://antfu.me/posts/publish-esm-and-cjs for further reference

Motivation

With version alpha.16 we're not able to use leonardo-contrast-colors anymore in a NodeJS script that's still using commonjs modules.

Fixes https://github.com/adobe/leonardo/issues/173

Screenshots

To-do list

  • [x] I have read the CONTRIBUTING document.
  • [x] This pull request is ready to merge.

dschmidt avatar Nov 30 '22 20:11 dschmidt

ping :)

dschmidt avatar Jan 09 '23 15:01 dschmidt

Anything wrong with this? Otherwise it would be great to merge this before conflicts are introduced or it becomes incomplete

dschmidt avatar Feb 03 '23 07:02 dschmidt

I was exploring the @adobe/spectrum-adaptive-theme package to see how it works, and I found it's broken too, because it depends on this package and it's imports, so it would be nice to see this merged.

dsmmcken avatar Aug 02 '23 17:08 dsmmcken

Would be appreciated if you or anyone else could take this over. As this PR was completely ignored, while development and releases happened, I've given up.

dschmidt avatar Aug 02 '23 20:08 dschmidt

This is still sorely needed

xaddict avatar Aug 04 '23 13:08 xaddict