Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for ESM added, then removed? #160

Open
tannerstern opened this issue Aug 30, 2023 · 2 comments
Open

Support for ESM added, then removed? #160

tannerstern opened this issue Aug 30, 2023 · 2 comments

Comments

@tannerstern
Copy link

tannerstern commented Aug 30, 2023

Hey @davidmerfield!

Thanks for this package, it's a neat and simple tool that I find really useful, especially for seeding colors based on inputs.

I wanted to ask a question though: I see ESM support was added in #134 but then shortly after was removed in ff971fa. I'm wondering, was that on account of the changes from the PR breaking use of this module in other environments? That seems like what was going on. Since the version of this package available on npm is Node-first (non-browser compatible), it makes sense that going fully ESM would cause issues.

Maybe I've answered my own question, but at any rate, ESM-native imports would be a nice thing to have, especially through npm. Do you plan on re-adding ESM support in the future (of course, this being open-source I am not asking or demanding anything, just curious if that's a direction you are planning to go in).

Thanks!

@davidmerfield
Copy link
Owner

You're correct about my motivation for rolling back the changes: this broke the script in other environments and I value backwards compatibility.

Please note that I haven't actually 'released' the current state of the repo to NPM so if I can work out how to add ESM support in a non-breaking fashion, I'm open to it.

My understanding is that ESM and CJS modules are fundamentally incompatible, though I could be wrong.

However, if you or someone else creates your own package which exposes this script as an ESM, I'll gladly link to it in the readme for this project.

The same principle applies to anything related to TypeScript, etc.

@tannerstern
Copy link
Author

tannerstern commented Aug 31, 2023

That makes sense. After reading your message, I did a bit of digging to see how others have attempted to solve this issue (because there appear to be a lot of packages that have solved this, with build processes of varying complexity). I found a few things that were helpful in understanding what a solution could look like:

TL;DR Node has a way to provide support for both require and import with the exports field in package.json (docs in link 3). Per the "SenseDeep" example, one could:

  1. Convert the codebase to ESM (@keithamus has already done this in his fork)
  2. Add a minimal build step to yield two endpoints, one for CJS and one for ESM
  3. Update package.json to point to the appropriate endpoint depending on the environment
  4. (Bonus: as a fallback for those running on Node versions before exports, the main field can be left in; exports takes precedence, but is ignored on older versions of Node)

I'm not sure what that build step would look like (Babel? Rollup? something else?) but it would surely be a simple configuration 👍

Edit: this is not a request for you to implement these changes, more of "this is what I found" kinda thing haha, I have nothing but respect for open-source maintainers such as yourself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants