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

emit d.cts files in src/cjs #56

Merged
merged 1 commit into from
Dec 25, 2024
Merged

Conversation

Nesopie
Copy link
Collaborator

@Nesopie Nesopie commented Dec 21, 2024

  • Adds separate types for cjs and esm in package.json
  • tsconfig.base.json emits type declarations
  • changed the postbuild command to convert .d.ts files to .d.cts

@junderw
Copy link
Member

junderw commented Dec 21, 2024

Await review by @arijoon

Copy link
Member

@junderw junderw left a comment

Choose a reason for hiding this comment

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

LGTM...

No clue how to test these weird packaging/transpiling issues.

We would need to have some CI that uses combinations of nodeJS versions, TS versions, webpack versions, etc etc etc and make sure all the combinations work...

It seems like newer versions of NodeJS have some new package.json keys that help with hybrid packages, but we don't want to bump the minimum NodeJS version too high.

Copy link

@arijoon arijoon left a comment

Choose a reason for hiding this comment

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

Thanks a lot, much appreciated. Tested against this repo by building this and packing locally, then installing it and works perfectly well

This lead me to find the issue also doesn't exist if typescript flag importHelpers is set to false. But we had this as true which manifests this issue. Worth noting importHelpers requires addition of tslib on the consumer side

@junderw
Copy link
Member

junderw commented Dec 25, 2024

Thanks for the importHelpers tip.

@junderw junderw merged commit 4b06c43 into cryptocoinjs:master Dec 25, 2024
3 checks passed
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.

cjs export is broken when importHelpers is set to true in tsconfig on consumer side
3 participants