module: drop support for extensionless files in ESM #31415

GeoffreyBooth · 2020-01-20T06:49:09Z

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 via import; support for extensionless files in import 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), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

cc @nodejs/modules-active-members

reverses baa3621

nodejs-github-bot · 2020-01-20T06:52:14Z

CI: https://ci.nodejs.org/job/node-test-pull-request/28494/

guybedford · 2020-01-20T07:31:48Z

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.

nodejs-github-bot · 2020-01-20T08:24:11Z

CI: https://ci.nodejs.org/job/node-test-pull-request/28495/

GeoffreyBooth · 2020-01-20T08:25:31Z

Note that it is still possible to require extensionless files

To be clear, I think he means that once an extensionless file has been required, then import of that file will succeed when it shouldn’t, because of this preemptive caching. I think fixing that should be its own PR.

aduh95 · 2020-01-20T12:57:16Z

To be clear, landing this PR means node ./bin/js-file-without-extension will be treated as CJS, no matter the package.json's type value, correct?

GeoffreyBooth · 2020-01-20T14:19:06Z

To be clear, landing this PR means node ./bin/js-file-without-extension will be treated as CJS, no matter the package.json's type value, correct?

No. If it's in a type: module scope, an error will be thrown.

If that's your situation, you could create a ./bin/package.json containing type: commonjs, and then the extensionless file in ./bin will be treated as CommonJS and become executable.

bmeck

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.

nodejs-github-bot · 2020-01-20T14:55:39Z

CI: https://ci.nodejs.org/job/node-test-pull-request/28499/

jkrems · 2020-01-20T16:05:36Z

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. :)

GeoffreyBooth · 2020-01-20T17:21:46Z

this will mean that self-contained binaries at known locations will be stuck with CommonJS or symlinks forever.

Not forever, just until we find an alternative solution. 😄 There are options.

nodejs-github-bot · 2020-01-20T18:51:05Z

CI: https://ci.nodejs.org/job/node-test-pull-request/28514/

GeoffreyBooth · 2020-01-20T19:35:13Z

CI: https://ci.nodejs.org/job/node-test-pull-request/28514/

CI is green.

reverses baa3621 PR-URL: #31415 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Rich Trott <[email protected]>

PR-URL: #31415 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Rich Trott <[email protected]>

GeoffreyBooth · 2020-01-23T07:22:20Z

Landed in dc90f92...c692568

PR-URL: nodejs#31415 Backport-PR-URL: nodejs#32280 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Rich Trott <[email protected]>

Backport-PR-URL: #32280 PR-URL: #31415 Backport-PR-URL: #32280 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Rich Trott <[email protected]>

Ryuzaki42 · 2021-02-15T22:54:46Z

What about .ts as file extension? I'm trying to import some typescript module w/o .js files

node --loader ts-node/esm --experimental-specifier-resolution=node src/index.ts

and got

TypeError [ERR_INVALID_RETURN_PROPERTY_VALUE]: Expected string to be returned for the "format" from the "loader getFormat" function but got type object.
    at new NodeError (node:internal/errors:278:15)
    at Loader.getFormat (node:internal/modules/esm/loader:111:13)
    at Loader.getModuleJob (node:internal/modules/esm/loader:231:20)
    at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:58:21)
    at async Promise.all (index 1)
    at link (node:internal/modules/esm/module_job:63:9)

GeoffreyBooth added 3 commits January 19, 2020 22:42

module: revert nodejs#31021

5752a6f

reverses baa3621

module: correct docs about when extensionless files are supported

969fa8f

module: drop support for extensionless main entry points in esm

cdacf7d

GeoffreyBooth requested review from bmeck and guybedford January 20, 2020 06:49

nodejs-github-bot added the esm Issues and PRs related to the ECMAScript Modules implementation. label Jan 20, 2020

GeoffreyBooth added the modules-agenda Issues and PRs to discuss during the meetings of the Modules team. label Jan 20, 2020

guybedford approved these changes Jan 20, 2020

View reviewed changes

devsnek approved these changes Jan 20, 2020

View reviewed changes

bmeck approved these changes Jan 20, 2020

View reviewed changes

bmeck mentioned this pull request Jan 20, 2020

esm: add package type wasm #31388

Closed

4 tasks

BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jan 20, 2020

Trott approved these changes Jan 21, 2020

View reviewed changes

GeoffreyBooth closed this Jan 23, 2020

GeoffreyBooth deleted the drop-import-of-extensionless branch January 23, 2020 07:22

guybedford mentioned this pull request Mar 17, 2020

Consider something clever for extension-less executables #32316

Closed

MylesBorins added backported-to-v12.x and removed backport-requested-v12.x labels Apr 3, 2020

codebytere mentioned this pull request Apr 3, 2020

v12.16.2 proposal #32313

Merged

GeoffreyBooth mentioned this pull request May 3, 2020

Extenstionless main files and ES modules interaction #33223

Closed

evg656e mentioned this pull request May 3, 2020

Consider to add .js extenstion to main entry point (bin/mocha) for better ES modules compatibility mochajs/mocha#4267

Closed

This was referenced Jul 2, 2020

Add extensions to bin scripts. microsoft/TypeScript#39380

Closed

Extensionsless "bin" scripts has conflict with Node --experimental-specifier-resolution option microsoft/TypeScript#39381

Closed

bmeck mentioned this pull request Jul 3, 2020

esm: treat modules with no extension as .js #34177

Open

4 tasks

sosoba mentioned this pull request Jan 26, 2021

Crash when using NODE_OPTIONS=--experimental-specifier-resolution=node with some .bin executables #35518

Closed

This comment was marked as off-topic.

Sign in to view

LiviaMedeiros mentioned this pull request Aug 31, 2023

module: add --module option #49295

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

module: drop support for extensionless files in ESM #31415

module: drop support for extensionless files in ESM #31415

GeoffreyBooth commented Jan 20, 2020

nodejs-github-bot commented Jan 20, 2020

guybedford commented Jan 20, 2020

nodejs-github-bot commented Jan 20, 2020

GeoffreyBooth commented Jan 20, 2020

aduh95 commented Jan 20, 2020

GeoffreyBooth commented Jan 20, 2020 •

edited

Loading

bmeck left a comment

nodejs-github-bot commented Jan 20, 2020

jkrems commented Jan 20, 2020

GeoffreyBooth commented Jan 20, 2020

nodejs-github-bot commented Jan 20, 2020

GeoffreyBooth commented Jan 20, 2020

GeoffreyBooth commented Jan 23, 2020

Ryuzaki42 commented Feb 15, 2021

This comment was marked as off-topic.

module: drop support for extensionless files in ESM #31415

module: drop support for extensionless files in ESM #31415

Conversation

GeoffreyBooth commented Jan 20, 2020

Checklist

nodejs-github-bot commented Jan 20, 2020

guybedford commented Jan 20, 2020

nodejs-github-bot commented Jan 20, 2020

GeoffreyBooth commented Jan 20, 2020

aduh95 commented Jan 20, 2020

GeoffreyBooth commented Jan 20, 2020 • edited Loading

bmeck left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Jan 20, 2020

jkrems commented Jan 20, 2020

GeoffreyBooth commented Jan 20, 2020

nodejs-github-bot commented Jan 20, 2020

GeoffreyBooth commented Jan 20, 2020

GeoffreyBooth commented Jan 23, 2020

Ryuzaki42 commented Feb 15, 2021

This comment was marked as off-topic.

GeoffreyBooth commented Jan 20, 2020 •

edited

Loading