-
Notifications
You must be signed in to change notification settings - Fork 312
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
feat!: remove support for CommonJS #607
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Why in the world did you do this? |
To simplify maintenance, reduce package variants (and thus bugs), and to reduce the size of the package. Now that Node.js widely supports ESM there is no need to carry this around any longer. |
Meh, you could have automated it, most of that sounds like BS to me. I was going to use the package, but couldn't. Most people still use CommonJS believe it or not, and now that Node 23 supports loading ESM, it's never going away. |
@eldoy yes, core is solving this so that we don't. Hope it works out for you. |
I am curious as to how this would not work for you, considering basically all versions of Node.js that are actively supported can import ESM from CommonJS. |
I've locked the issue. This is not an appropriate forum for new complaints. For a long time we have had an official place to discuss this. Thanks for going above and beyond here @jonkoops :). |
Ah yes, I forgot we had that discussion, thanks for reminding me. |
Drops the CommonJS distribution in favor of going for a pure ESM package. This also removes the
main
andtypes
fields, opting to only support theexports
field.Closes #603
Closes #532