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: added cjs support via esbuild #1161

Open
wants to merge 1 commit into
base: explore-new-api
Choose a base branch
from

Conversation

titanism
Copy link

@LinusU we have added CJS support without breaking anything or making any drastic changes. can you please merge and publish a new rc? thank you! 🙏 🙇

@LinusU
Copy link
Member

LinusU commented Jan 11, 2023

Sorry for the late reply on this one.

As this change is right now, it will bundle all of our dependencies into the published package. Whilst still keeping this as dependencies.

I think that bundling dependencies isn't a good way to go, since the end user then might end up with multiple copies of the same module. It would also add an additional maintanence burden on us since we would need to cut new releases anytime a dependency is updated.

Even worse, it could also block users from getting upstream security patches, as we would need to cut a new release with that package upgraded before they could use it. This is in contrast to how it is now where they could simply do npm audit fix or npm update affected-package.

@titanism
Copy link
Author

This would only affect CJS users, and I think that is a risk worth taking to at least give them support to be able to use this in CJS... We would gladly help be a maintainer to push out security updates as necessary. Thanks, let us know @LinusU!

@titanism titanism mentioned this pull request Jan 11, 2023
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