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

fix: Alias --es-module-specifier-resolution to --experimental-specifier-resolution for Node 12 compatibility #1122

Merged
merged 2 commits into from
Sep 15, 2020

Conversation

nguyensomniac
Copy link
Contributor

@nguyensomniac nguyensomniac commented Sep 10, 2020

Context here: #1007 (comment)

The --es-module-specifier-resolution flag was introduced in Node 12. With the release of Node 13, the flag was renamed to --experimental-specifier-resolution, with --es-module-specifier-resolution acting as an alias to the new name.

This fix aliases the --es-module-specifier-resolution flag in older versions of node, so that module resolution acts as expected. I also changed what seems to be a dead branch in the esm-usage-example directory.

I tested this with node 12, 13, and 14 and all seems to be working as expected.

@codecov
Copy link

codecov bot commented Sep 10, 2020

Codecov Report

Merging #1122 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1122   +/-   ##
=======================================
  Coverage   79.32%   79.32%           
=======================================
  Files           7        7           
  Lines         711      711           
  Branches      158      158           
=======================================
  Hits          564      564           
  Misses         90       90           
  Partials       57       57           
Flag Coverage Δ
#node_10 76.28% <ø> (ø)
#node_12_15 76.63% <ø> (ø)
#node_12_16 76.63% <ø> (ø)
#node_13 79.04% <ø> (ø)
#node_14 79.04% <ø> (ø)
#typescript_2_7 78.90% <ø> (ø)
#typescript_latest 78.19% <ø> (ø)
#typescript_next 78.05% <ø> (ø)
#ubuntu 79.04% <ø> (ø)
#windows 79.18% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
dist-raw/node-options.js 78.78% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 41e7109...bf7e824. Read the comment docs.

@coveralls
Copy link

coveralls commented Sep 10, 2020

Coverage Status

Coverage increased (+0.9%) to 86.95% when pulling bf7e824 on nguyensomniac:lily/esm-alias into 41e7109 on TypeStrong:master.

@cspotcode
Copy link
Collaborator

@nguyensomniac thanks, seems like a simple change, and thanks for adding a test.

Node's docs do not mention the older flag name: --es-module-specifier-resolution
https://nodejs.org/docs/latest-v12.x/api/esm.html#esm_resolution_algorithm

Was the flag renamed during node 12's lifetime?

@ExE-Boss
Copy link

Was the flag renamed during node 12's lifetime?

Yes, it was.

Probably even long‑ish before v12 became LTS.

@nguyensomniac
Copy link
Contributor Author

Ah, you're correct. Running node --help looks like it was renamed after 12.15

@cspotcode cspotcode merged commit 4dba8e8 into TypeStrong:master Sep 15, 2020
@cspotcode
Copy link
Collaborator

Thanks! I added a comment to remind me when this flag was changed.

This will be published to npm after I merge #1121; until then, you can install directly from git. I always forget the syntax, but I think it's npm install github:TypeStrong/ts-node#master

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.

4 participants