-
Notifications
You must be signed in to change notification settings - Fork 943
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
Node ESM compatibility #1942
Node ESM compatibility #1942
Conversation
Fixes #1940 |
Thanks for catching this! There's one package that isn't quite the same as everything else, can you make the same update in |
@mfedderly I thought I caught that one? |
Oh I see - not all of the changes in this PR came from the monorepolint changes. You can actually do all of this from the .monorepolint.config.js directly. I pushed a branch that I think does what you want. If you can verify that the change I made was correct and update your PR I can get this merged tomorrow. I'm pretty sure if you pull my own commit in it will block me from being an approver on a branch I contributed to, so its easiest if you make your own commit with those changes if possible. |
Are you saying that you make that change to |
Writes a package.json to each dist/es/ directory, which ensures that Node loads adjacent files as ES modules. close #1940
Okay I've incorporated your change. |
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.
Thanks! I mostly consume Turf through webpack, so I would've missed this entirely.
It is breaking on webpack v5 because of missing file extensions on imports, as described here: https://nodejs.org/api/esm.html#esm_mandatory_file_extensions Sample error:
|
@lcrosetto what version? 6.2.0 released today or another version? |
Yes, 6.2.0. Some of the imports from ./lib would need the file extension added, e.g.
|
Does two things to ensure turf ESM packages can be imported in Node:
"exports"
in package.json, a successor to the"module"
property.{"type":"module"}
indist/es/
. This ensures thatdist/es/index.js
is loaded as an ESM module, even if the package's package.json is not"type": "module"