Skip to content

feat: support commonjs distribution #133

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

Closed

Conversation

oderayi
Copy link

@oderayi oderayi commented Sep 30, 2024

This PR adds tsup to enable distribution in CommonJS format for wider support.

@@ -19,9 +19,6 @@
"skipLibCheck": true,
"removeComments": true,
"baseUrl": ".",
"paths": {
"*": ["node_modules/*"]
Copy link
Author

@oderayi oderayi Sep 30, 2024

Choose a reason for hiding this comment

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

Removed to allow tsup resolve .node packages correctly.

@kjellmorten
Copy link
Contributor

Thanks for this PR, @oderayi!

I've spent some time considering this, as I really want to make this package work for as many people as possible, but I've been hesitant for a few reasons:

  • The PR is using a different build system, and we have a standardized build setup that we use across many packages
  • Supporting both CommonJS and ESM involves going back to CommonJS for this package, moving the challenge of using other ESM packages into map-transform
  • Supporting both will increase the size of this package. That may not be a big problem, but everything adds up

I've been looking at different ways to serve both CommonJS and ESM without switching build system, but this hasn't been successful.

Also, I've been thinking about the way forward for the node community, and it seems to me that this is the time to keep moving in the ESM direction to get the entire community to a better place where we don't have to deal with the confusing situation we have now, with both CommonJS and ESM side by side. If find this article sums this up in a good way.

So for all these reasons, I'm closing this PR, but know that I really appreciate the time and effort you put into it, @oderayi. I also hope you find a way to make it work, perhaps with the synchronous require() making its way to more node versions these days.

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

Successfully merging this pull request may close these issues.

2 participants