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

[Lib/JS] Compile svix package for ESM and CommonJS #1549

Merged
merged 1 commit into from
Dec 4, 2024
Merged

Conversation

svix-lucho
Copy link
Contributor

@svix-lucho svix-lucho commented Dec 4, 2024

Closes #1483

Compile the svix JavaScript package in ESM and CommonJS simultaneously. This should solve the issues with tree-shaking (since it can only be done when the package is compiled with ESM). Also keeps the CommonJS version for backwards compatibility.

Tested this locally and all the usual imports (import { EventTypeOut } from "svix") work as expected, with no changes whatsoever.

@svix-lucho svix-lucho marked this pull request as ready for review December 4, 2024 20:15
@svix-lucho svix-lucho requested a review from a team as a code owner December 4, 2024 20:15
Copy link
Contributor

@svix-onelson svix-onelson left a comment

Choose a reason for hiding this comment

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

Will this help to surface problems like this?
#1505

I'm still hoping we can track something in CI to catch this problem the next time it creeps in.

@svix-lucho
Copy link
Contributor Author

Will this help to surface problems like this? #1505

I'm still hoping we can track something in CI to catch this problem the next time it creeps in.

I don't think this PR solves errors like that, because there's still a chance svix is used in ESM and we accidentally import a commonJS dependency. But we should be able to write a test and make the test runner interpret the code in both modes. I need to investigate how to do that.

@svix-lucho svix-lucho merged commit 633015f into main Dec 4, 2024
2 checks passed
@svix-lucho svix-lucho deleted the lucho/svix-esm branch December 4, 2024 21:24
svix-lucho added a commit that referenced this pull request Dec 5, 2024
I accidentally changed the version in
#1549
@svix-jplatte svix-jplatte mentioned this pull request Dec 6, 2024
svix-jplatte added a commit that referenced this pull request Dec 6, 2024
## What's changed

* Libs/Go: Add convenient construction of messages with pre-serialized
payload ([#1538])
* Libs/Go: Reduce the use of `NullableX` types to where they actually
have a use ([#1543])
* Libs/JavaScript: Add convenient construction of messages with
pre-serialized payload ([#1539])
* Libs/Java: Add convenient construction of messages with pre-serialized
payload ([#1544])
* Libs/C#: Bump .NET target to 8.0 ([#1546])
* Libs/C#: Add convenient construction of messages with pre-serialized
payload ([#1545])
* Libs/Python: Add convenient construction of messages with
pre-serialized payload ([#1540])
* Libs/Ruby: Add convenient construction of messages with pre-serialized
payload ([#1541])
* Libs/JavaScript: Compile svix package for ESM and CommonJS, reducing
bundle sizes ([#1549])

[#1538]: #1538
[#1543]: #1543
[#1539]: #1539
[#1540]: #1540
[#1541]: #1541
[#1544]: #1544
[#1545]: #1545
[#1546]: #1546
[#1549]: #1549
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.

Adding svix for webhook validation adds almost 1 MB to the JS bundle
2 participants