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

chore: added ESM support #828

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

chore: added ESM support #828

wants to merge 4 commits into from

Conversation

subzero10
Copy link
Member

@subzero10 subzero10 commented Jul 9, 2022

Status

READY

Description

Fixes #823.

(cherry picked from commit a86d4d3)
@subzero10 subzero10 requested a review from joshuap July 9, 2022 14:01
@subzero10 subzero10 self-assigned this Jul 9, 2022
@subzero10
Copy link
Member Author

Integration tests are failing. Looking into it.

package.json Outdated Show resolved Hide resolved
@bompus bompus mentioned this pull request Mar 10, 2023
2 tasks
# Conflicts:
#	README.md
#	package.json
#	rollup.config.js
#	tsconfig.base.json
@BethanyBerkowitz
Copy link
Contributor

@subzero10 what will happen if someone is using this package in an esm module in Node? Will they potentially end up with the browser version instead of the server version?

@subzero10
Copy link
Member Author

@subzero10 what will happen if someone is using this package in an esm module in Node? Will they potentially end up with the browser version instead of the server version?

I had the same question my self. It's very confusing to say the least:

  • esbuild says here that nodejs does not support the "module" field. nodejs does not mention the "module" in their docs either.
  • esbuild has a recommended section for package authors. So I went ahead and applied this. I tested with webpack. Apparently, webpack will always import the esm build, even if require is used (as in our example project). This broke the bundle, though I was able to fix it by changing the require to an import statement, or forcing webpack to resolve the cjs build.

That's why I opted for the simplest non-breaking change, even though it's recommended to avoid 🤷 .
Once again, having the package isomorphic makes things more complicated than they should. I am hoping that soon we'll be able to break the js package to separate packages for node vs browser.

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.

Build - ESM Support
5 participants