-
Notifications
You must be signed in to change notification settings - Fork 71
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: add .js extension to imports and fix package.json #148
fix: add .js extension to imports and fix package.json #148
Conversation
This fixes two problems with the ESM build of this module. 1. The `package.json` that contains `{ "type": "module" }` wasn't being included in the npm tarball 2. When running in an ESM environment, `import foo from './bar'` does not work, you have to specify the extension The fix for the first is simple, add the cjs/esm `package.json` files to the `files` array in the project `package.json`. The second fix is harder. If you just add the `.js` extension to the source files, typescript is happy but ts-node is not, and this project uses ts-node to run the tests without a compile step. Typescript does not support importing `*.ts` and will not support adding `*.js` to the transpiled output - microsoft/TypeScript#16577 ts-node thought this was a bug in Typescript but it turns out not. Their suggestion to use `ts-node/esm` breaks sourcemap support because `source-map-support/register` is not esm - TypeStrong/ts-node#783 There is a PR against ts-node to add support for resolving `./foo.js` if `./foo.ts` or `./foo` fails but it seems to have stalled - TypeStrong/ts-node#1361 Given all of the above, the most expedient way forward seemed to just be to add a shell script that rewrites the various `import` statements in the esm output to add the `.js` extension, then if the ts-node PR ever gets merged the script can be backed out. Fixes beaugunderson#147
Codecov Report
@@ Coverage Diff @@
## master #148 +/- ##
=======================================
Coverage 98.08% 98.08%
=======================================
Files 9 9
Lines 575 575
Branches 84 84
=======================================
Hits 564 564
Misses 3 3
Partials 8 8 Continue to review full report at Codecov.
|
@beaugunderson are there any changes you'd like made to this PR or can it be merged? |
#!/bin/sh | ||
|
||
# Search for "from './baz'" and transform to "from './baz.js'" | ||
find dist/esm -type f -name *.js -exec sed -i -e -E "s/from '(\.\/.*)'/from '\1.js'/g" {} \; |
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.
Tested on debian the -e
parameter doesn't work.
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.
also this won't catch any pure imports like import 'utils/myfile.js'
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.
Note for anyone else viewing this that I tried getting a rule into eslint that would resolve this for similar use-cases: import-js/eslint-plugin-import#2701
removed all the ESM-specific stuff in this project, tested that the new version works in CJS/ESM/TS without issues... publishing a new release momentarily |
This fixes two problems with the ESM build of this module.
package.json
that contains{ "type": "module" }
wasn't being included in the npm tarballimport foo from './bar'
does not work, you have to specify the extensionThe fix for the first is simple, add the cjs/esm
package.json
files to thefiles
array in the projectpackage.json
.The second fix is harder. If you just add the
.js
extension to the source files, typescript is happy but ts-node is not, and this project uses ts-node to run the tests without a compile step.Typescript does not support importing
*.ts
and will not support adding*.js
to the transpiled output - microsoft/TypeScript#16577ts-node thought this was a bug in Typescript but it turns out not. Their suggestion to use
ts-node/esm
breaks sourcemap support becausesource-map-support/register
is not esm - TypeStrong/ts-node#783There is a PR against ts-node to add support for resolving
./foo.js
if./foo.ts
or./foo
fails but it seems to have stalled - TypeStrong/ts-node#1361Given all of the above, the most expedient way forward seemed to just be to add a shell script that rewrites the various
import
statements in the esm output to add the.js
extension, then if the ts-node PR ever gets merged the script can be backed out.Fixes #147