-
Notifications
You must be signed in to change notification settings - Fork 25
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(sdk-snippets): library modernization with ESM+CJS builds #1017
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!
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.
a few small suggestions but LGTM for the most part!
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.
💙
Given that this package is really only used in our main codebase, is this PR necessary? Historically we've really only done this dual-export thing for packages that are used by |
Not really I guess. I'll close it out |
@kanadgupta I'm remembering now that the reason I did this was to upgrade to the latest release of |
yep, agreed with this! |
🧰 Changes
This modernizes our
sdk-snippets
library in order to now export CJS and ESM dist builds.🧬 QA & Testing
Running
npm run attw
all of the types for this new library output line up as expected: