search

adobe / leonardo

Generate colors based on a desired contrast ratio
http://www.leonardocolor.io
Apache License 2.0
1.97k stars 108 forks source link

Fix usage of contrast-colors with commonjs and esm #185

Open dschmidt opened 1 year ago

dschmidt commented 1 year ago

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

dschmidt commented 1 year ago

ping :)

dschmidt commented 1 year ago

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

dsmmcken commented 1 year ago

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.

dschmidt commented 1 year ago

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.

xaddict commented 1 year ago

This is still sorely needed