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: Support Node.js ES Modules via conditional exports #87

Closed
wants to merge 2 commits into from

Conversation

ExE-Boss
Copy link
Contributor

I’ve also added exports mappings for tslib/tslib.js → tslib/tslib.cjs.


Supersedes and closes #44

Resolves #81; closes #84

Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

I'm unsure if we'll take this as-is, since it's a break to the API surface of the module (mostly the renaming of files). It might make sense to make a subfolder with a package.json containing type: module and a symlink to the es6 module in the root (or symlink the root to the subfolder, depending on symlink behavior), instead, this way the files are still in all the places consumers expect.

cc @rbuckton

package.json Outdated Show resolved Hide resolved
@ExE-Boss ExE-Boss requested a review from weswigham March 8, 2020 04:33
@dasa
Copy link

dasa commented Apr 3, 2020

ping 😄

@vikerman
Copy link

Symbolic links don't work since the final resolved path is considered. Also you can't publish symbolic links on npm.

A solution might be to add a prepare step that copies tslib.es6.js to tslib.mjs and the have the conditional export for import point to the .mjs file. I can create a PR if that's acceptable.

@ExE-Boss ExE-Boss force-pushed the feat/node-esm branch 2 times, most recently from 4b51c6f to 71887cc Compare May 29, 2020 17:17
@ExE-Boss
Copy link
Contributor Author

ExE-Boss commented May 29, 2020

A better option is to just do a ./tslib.js → ./tslib.cjs and ./tslib → ./tslib.cjs exports path mapping:

tslib/package.json

Lines 35 to 36 in 71887cc

"./tslib.js": "./tslib.cjs",
"./tslib": "./tslib.cjs",

@vikerman
Copy link

Changing the file extension from .js to .cjs is still a breaking change since conditional exports are used only by node and not other tools(and older node).

@ExE-Boss
Copy link
Contributor Author

That’s why I wanted to get this into v2.0.

@vikerman
Copy link

Ok - I shouldn't say breaking change. This change could break the usage of tslib with bundlers and other tools because of the .cjs file extension. Using .mjs for the conditional export would be a safer change.

@guybedford
Copy link
Contributor

Changing file names etc might be considered a breaking change? #121 would be fully backwards compatible I believe.

@orta
Copy link
Contributor

orta commented Oct 6, 2020

Thanks - ES Module support was shipped with #126

@orta orta closed this Oct 6, 2020
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.

rename tslib.es6.js to tslib.mjs Problem with tslib and ES modules
6 participants