-
Notifications
You must be signed in to change notification settings - Fork 30k
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
module: drop support for extensionless files in ESM #31415
module: drop support for extensionless files in ESM #31415
Conversation
Note that it is still possible to require extensionless files with this PR though: require('./file/without-extension');
import('./file/without-extension'); In order to disallow CommonJS execution from allowing ES module imports, we should add an extension filter to the "pre-emptive caching" at https://github.com/nodejs/node/blob/master/lib/internal/modules/cjs/loader.js#L1072. |
To be clear, I think he means that once an extensionless file has been |
To be clear, landing this PR means |
No. If it's in a If that's your situation, you could create a |
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.
LGTM, this alleviates prior concerns of edge cased behavior and solves the problem of removing edge cases that caused #31021 to be seen as a fix by taking the alternate route. Historically when behavior lacks consensus we remove controversial parts so this PR is likely the best route forward.
I'm a bit bummed that this will mean that self-contained binaries at known locations will be stuck with CommonJS or symlinks forever. But not bummed enough to block this. :) |
Not forever, just until we find an alternative solution. 😄 There are options. |
CI is green. |
PR-URL: #31415 Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: #31415 Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
Landed in dc90f92...c692568 |
PR-URL: nodejs#31415 Backport-PR-URL: nodejs#32280 Reviewed-By: Guy Bedford <guybedford@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
What about .ts as file extension? I'm trying to import some typescript module w/o .js files
and got
|
This PR removes support for extensionless files in ES module contexts, per discussion in #31388 (comment). The current ESM implementation that shipped in 12.0.0 supported extensionless files only as main entry points to the Node process, e.g.
node extensionless-file
, and not viaimport
; support for extensionless files inimport
was added only last month, in #31021. This PR reverts that one, and goes further to drop support for extensionless files as main entry points. The docs are also updated.The prior support for extensionless main entry files was there as part of supporting bin files. As a practical matter, the “empty string” extension was designated as ES module JavaScript (equivalent to
.mjs
) within a"type": "module"
scope. This is problematic for adding future support for extensionless files of other formats, such as WASM/WASI.In the interest of preserving design space for such future support, this PR makes all extensionless files unknown (and therefore throwing) to the ESM loader. Another solution besides #31021 or the implementation’s earlier special casing of extensionless main entry points will need to be found for configuring Node’s ES module loader on how to treat extensionless files of varying formats.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passescc @nodejs/modules-active-members