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

feat!: remove support for CommonJS #607

Merged
merged 1 commit into from
Oct 30, 2023
Merged

Conversation

jonkoops
Copy link
Collaborator

Drops the CommonJS distribution in favor of going for a pure ESM package. This also removes the main and types fields, opting to only support the exports field.

Closes #603
Closes #532

@jonkoops jonkoops added the breaking Issues or PRs that contain breaking changes label Oct 29, 2023
@jonkoops jonkoops requested a review from jasonkuhrt October 29, 2023 16:27
Copy link
Member

@jasonkuhrt jasonkuhrt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@eldoy
Copy link

eldoy commented Jan 25, 2025

Why in the world did you do this?

@jonkoops
Copy link
Collaborator Author

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.

@eldoy
Copy link

eldoy commented Jan 25, 2025

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.

@jasonkuhrt
Copy link
Member

@eldoy yes, core is solving this so that we don't. Hope it works out for you.

@graffle-js graffle-js locked and limited conversation to collaborators Jan 25, 2025
@jonkoops
Copy link
Collaborator Author

I was going to use the package, but couldn't.

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.

@jasonkuhrt
Copy link
Member

jasonkuhrt commented Jan 26, 2025

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 :).

@jonkoops
Copy link
Collaborator Author

Ah yes, I forgot we had that discussion, thanks for reminding me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
breaking Issues or PRs that contain breaking changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove CommonJS build Remove main field of package.json in future
3 participants