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

Improve ESM compatibility #49

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jstasiak
Copy link
Contributor

This change follows the solution described in [1] to make the package
play nice with both CommonJS and ESM clients.

This includes:

  • Changing imports to use the .js extension explicitly
  • Modifying the dist/package.json file to refer to different directories
    depending on the context
  • Providing two package.json files that override the package type
    depending on the context

Fixes: GH-37

[1] https://www.sensedeep.com/blog/posts/2021/how-to-create-single-source-npm-module.html

This change follows the solution described in [1] to make the package
play nice with both CommonJS and ESM clients.

This includes:

* Changing imports to use the .js extension explicitly
* Modifying the dist/package.json file to refer to different directories
  depending on the context
* Providing two package.json files that override the package type
  depending on the context

Fixes: GH-37

[1] https://www.sensedeep.com/blog/posts/2021/how-to-create-single-source-npm-module.html
"build:ts": "tsc -p tsconfig.json && tsc -p tsconfig.json -m esnext --outDir dist/esm/ -d false -declarationMap false",
"build:copy": "copyfiles README.md LICENSE src/package.json dist --flat && copyfiles src/rxjs-operators/package.json dist --up 1",
"build:ts": "tsc -p tsconfig-cjs.json && tsc -p tsconfig-esm.json",
"build:copy": "copyfiles README.md LICENSE src/package.json dist --flat && copyfiles src/rxjs-operators/package.json dist/cjs --up 1 && cp package-cjs.json dist/cjs/package.json && cp package-esm.json dist/esm/package.json",
Copy link
Contributor

Choose a reason for hiding this comment

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

I have not seen this approach in other packages. Have you seen other packages do it like that?

Did you see this?
https://www.the-guild.dev/blog/support-nodejs-esm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, I'm new to JS/TS packaging and I can't say I really know what other packages do. I followed https://www.sensedeep.com/blog/posts/2021/how-to-create-single-source-npm-module.html

@tim-rohrer
Copy link

Looking forward to seeing this merged as I use ts-results for all my work, and I'm now trying to make more projects pure ESM.

Thanks!

@jstasiak
Copy link
Contributor Author

jstasiak commented Aug 5, 2022

We have https://github.com/lune-climate/ts-results-es (ts-results-es on NPM) that can be used until this PR is merged or the ESM compatibility is otherwise fixed.

@marvin-j97
Copy link

I was looking into double exporting CJS/ESM a couple of weeks ago and I found that https://github.com/egoist/tsup makes the whole tooling much simpler and less error-prone. It reduced my build script down to:

tsup src/index.ts --format cjs,esm --dts --clean

No need to copy files or defining multiple tsconfigs.

@misl-smlz
Copy link

misl-smlz commented Mar 18, 2024

@vultix Will this ever be merged or is this library dead (and we should use ts-results-es as a replacement)?

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.

Cannot import via ES Module
5 participants