From 9c683204dbd63e9a516213d75cb32be1236adc8f Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Fri, 29 Sep 2023 22:01:55 -0700 Subject: [PATCH] esm: unflag extensionless javascript and wasm in module scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR-URL: https://github.com/nodejs/node/pull/49974 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Guy Bedford Reviewed-By: Michaël Zasso --- doc/api/esm.md | 8 +- lib/internal/errors.js | 8 +- lib/internal/modules/esm/get_format.js | 34 ++---- .../test-esm-extensionless-esm-and-wasm.mjs | 106 ++++++++++++++++++ .../test-esm-type-flag-package-scopes.mjs | 25 ++++- ...nsion.js => test-esm-unknown-extension.js} | 13 +-- .../noext-esm | 2 + .../noext-wasm | Bin 0 -> 136 bytes .../package.json | 8 ++ .../wasm-dep.mjs | 15 +++ .../dep.js | 0 .../noext-cjs | 0 .../package.json | 2 +- .../run.js | 0 14 files changed, 174 insertions(+), 47 deletions(-) create mode 100644 test/es-module/test-esm-extensionless-esm-and-wasm.mjs rename test/es-module/{test-esm-unknown-or-no-extension.js => test-esm-unknown-extension.js} (65%) create mode 100644 test/fixtures/es-modules/package-type-module/node_modules/dep-with-package-json-type-module/noext-esm create mode 100644 test/fixtures/es-modules/package-type-module/node_modules/dep-with-package-json-type-module/noext-wasm create mode 100644 test/fixtures/es-modules/package-type-module/node_modules/dep-with-package-json-type-module/package.json create mode 100644 test/fixtures/es-modules/package-type-module/node_modules/dep-with-package-json-type-module/wasm-dep.mjs rename test/fixtures/es-modules/package-type-module/node_modules/{dep-with-package-json => dep-with-package-json-without-type}/dep.js (100%) rename test/fixtures/es-modules/package-type-module/node_modules/{dep-with-package-json => dep-with-package-json-without-type}/noext-cjs (100%) rename test/fixtures/es-modules/package-type-module/node_modules/{dep-with-package-json => dep-with-package-json-without-type}/package.json (56%) rename test/fixtures/es-modules/package-type-module/node_modules/{dep-with-package-json => dep-with-package-json-without-type}/run.js (100%) diff --git a/doc/api/esm.md b/doc/api/esm.md index bfd40556e5fcab..fa2abac7029b3d 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -1009,8 +1009,12 @@ _isImports_, _conditions_) > 5. Let _packageURL_ be the result of **LOOKUP\_PACKAGE\_SCOPE**(_url_). > 6. Let _pjson_ be the result of **READ\_PACKAGE\_JSON**(_packageURL_). > 7. If _pjson?.type_ exists and is _"module"_, then -> 1. If _url_ ends in _".js"_, then -> 1. Return _"module"_. +> 1. If _url_ ends in _".js"_ or has no file extension, then +> 1. If `--experimental-wasm-modules` is enabled and the file at _url_ +> contains the header for a WebAssembly module, then +> 1. Return _"wasm"_. +> 2. Otherwise, +> 1. Return _"module"_. > 2. Return **undefined**. > 8. Otherwise, > 1. Return **undefined**. diff --git a/lib/internal/errors.js b/lib/internal/errors.js index d577f8efbd63ff..ea8138d8dca7eb 100644 --- a/lib/internal/errors.js +++ b/lib/internal/errors.js @@ -1791,13 +1791,7 @@ E('ERR_UNHANDLED_ERROR', E('ERR_UNKNOWN_BUILTIN_MODULE', 'No such built-in module: %s', Error); E('ERR_UNKNOWN_CREDENTIAL', '%s identifier does not exist: %s', Error); E('ERR_UNKNOWN_ENCODING', 'Unknown encoding: %s', TypeError); -E('ERR_UNKNOWN_FILE_EXTENSION', (ext, path, suggestion) => { - let msg = `Unknown file extension "${ext}" for ${path}`; - if (suggestion) { - msg += `. ${suggestion}`; - } - return msg; -}, TypeError); +E('ERR_UNKNOWN_FILE_EXTENSION', 'Unknown file extension "%s" for %s', TypeError); E('ERR_UNKNOWN_MODULE_FORMAT', 'Unknown module format: %s for URL %s', RangeError); E('ERR_UNKNOWN_SIGNAL', 'Unknown signal: %s', TypeError); diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js index 59ab89f6f76377..56d002ca0883ad 100644 --- a/lib/internal/modules/esm/get_format.js +++ b/lib/internal/modules/esm/get_format.js @@ -9,7 +9,6 @@ const { StringPrototypeCharCodeAt, StringPrototypeSlice, } = primordials; -const { basename, relative } = require('path'); const { getOptionValue } = require('internal/options'); const { extensionFormatMap, @@ -22,7 +21,7 @@ const experimentalNetworkImports = const defaultTypeFlag = getOptionValue('--experimental-default-type'); // The next line is where we flip the default to ES modules someday. const defaultType = defaultTypeFlag === 'module' ? 'module' : 'commonjs'; -const { getPackageType, getPackageScopeConfig } = require('internal/modules/esm/resolve'); +const { getPackageType } = require('internal/modules/esm/resolve'); const { fileURLToPath } = require('internal/url'); const { ERR_UNKNOWN_FILE_EXTENSION } = require('internal/errors').codes; @@ -112,17 +111,16 @@ function getFileProtocolModuleFormat(url, context, ignoreErrors) { if (defaultType === 'commonjs') { // Legacy behavior if (packageType === 'none' || packageType === 'commonjs') { return 'commonjs'; - } - // If package type is `module`, fall through to the error case below - } else { // Else defaultType === 'module' - if (underNodeModules(url)) { // Exception for package scopes under `node_modules` - return 'commonjs'; - } - if (packageType === 'none' || packageType === 'module') { - return getFormatOfExtensionlessFile(url); - } // Else packageType === 'commonjs' - return 'commonjs'; + } // Else packageType === 'module' + return getFormatOfExtensionlessFile(url); + } // Else defaultType === 'module' + if (underNodeModules(url)) { // Exception for package scopes under `node_modules` + return packageType === 'module' ? getFormatOfExtensionlessFile(url) : 'commonjs'; } + if (packageType === 'none' || packageType === 'module') { + return getFormatOfExtensionlessFile(url); + } // Else packageType === 'commonjs' + return 'commonjs'; } const format = extensionFormatMap[ext]; @@ -131,17 +129,7 @@ function getFileProtocolModuleFormat(url, context, ignoreErrors) { // Explicit undefined return indicates load hook should rerun format check if (ignoreErrors) { return undefined; } const filepath = fileURLToPath(url); - let suggestion = ''; - if (getPackageType(url) === 'module' && ext === '') { - const config = getPackageScopeConfig(url); - const fileBasename = basename(filepath); - const relativePath = StringPrototypeSlice(relative(config.pjsonPath, filepath), 1); - suggestion = 'Loading extensionless files is not supported inside of "type":"module" package.json contexts ' + - `without --experimental-default-type=module. The package.json file ${config.pjsonPath} caused this "type":"module" ` + - `context. Try changing ${filepath} to have a file extension. Note the "bin" field of package.json can point ` + - `to a file with an extension, for example {"type":"module","bin":{"${fileBasename}":"${relativePath}.js"}}`; - } - throw new ERR_UNKNOWN_FILE_EXTENSION(ext, filepath, suggestion); + throw new ERR_UNKNOWN_FILE_EXTENSION(ext, filepath); } /** diff --git a/test/es-module/test-esm-extensionless-esm-and-wasm.mjs b/test/es-module/test-esm-extensionless-esm-and-wasm.mjs new file mode 100644 index 00000000000000..db20bc047feec1 --- /dev/null +++ b/test/es-module/test-esm-extensionless-esm-and-wasm.mjs @@ -0,0 +1,106 @@ +// Flags: --experimental-wasm-modules +import { mustNotCall, spawnPromisified } from '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; +import { describe, it } from 'node:test'; +import { match, ok, strictEqual } from 'node:assert'; + +describe('extensionless ES modules within a "type": "module" package scope', { concurrency: true }, () => { + it('should run as the entry point', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + fixtures.path('es-modules/package-type-module/noext-esm'), + ]); + + strictEqual(stderr, ''); + strictEqual(stdout, 'executed\n'); + strictEqual(code, 0); + strictEqual(signal, null); + }); + + it('should be importable', async () => { + const { default: defaultExport } = + await import(fixtures.fileURL('es-modules/package-type-module/noext-esm')); + strictEqual(defaultExport, 'module'); + }); + + it('should be importable from a module scope under node_modules', async () => { + const { default: defaultExport } = + await import(fixtures.fileURL( + 'es-modules/package-type-module/node_modules/dep-with-package-json-type-module/noext-esm')); + strictEqual(defaultExport, 'module'); + }); +}); +describe('extensionless Wasm modules within a "type": "module" package scope', { concurrency: true }, () => { + it('should run as the entry point', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--experimental-wasm-modules', + '--no-warnings', + fixtures.path('es-modules/package-type-module/noext-wasm'), + ]); + + strictEqual(stderr, ''); + strictEqual(stdout, 'executed\n'); + strictEqual(code, 0); + strictEqual(signal, null); + }); + + it('should be importable', async () => { + const { add } = await import(fixtures.fileURL('es-modules/package-type-module/noext-wasm')); + strictEqual(add(1, 2), 3); + }); + + it('should be importable from a module scope under node_modules', async () => { + const { add } = await import(fixtures.fileURL( + 'es-modules/package-type-module/node_modules/dep-with-package-json-type-module/noext-wasm')); + strictEqual(add(1, 2), 3); + }); +}); + +describe('extensionless ES modules within no package scope', { concurrency: true }, () => { + // This succeeds with `--experimental-default-type=module` + it('should error as the entry point', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + fixtures.path('es-modules/noext-esm'), + ]); + + match(stderr, /SyntaxError/); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + }); + + // This succeeds with `--experimental-default-type=module` + it('should error on import', async () => { + try { + await import(fixtures.fileURL('es-modules/noext-esm')); + mustNotCall(); + } catch (err) { + ok(err instanceof SyntaxError); + } + }); +}); + +describe('extensionless Wasm within no package scope', { concurrency: true }, () => { + // This succeeds with `--experimental-default-type=module` + it('should error as the entry point', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--experimental-wasm-modules', + '--no-warnings', + fixtures.path('es-modules/noext-wasm'), + ]); + + match(stderr, /SyntaxError/); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + }); + + // This succeeds with `--experimental-default-type=module` + it('should error on import', async () => { + try { + await import(fixtures.fileURL('es-modules/noext-wasm')); + mustNotCall(); + } catch (err) { + ok(err instanceof SyntaxError); + } + }); +}); diff --git a/test/es-module/test-esm-type-flag-package-scopes.mjs b/test/es-module/test-esm-type-flag-package-scopes.mjs index 6b16e8a3ae223b..bf9d7d7ca4944c 100644 --- a/test/es-module/test-esm-type-flag-package-scopes.mjs +++ b/test/es-module/test-esm-type-flag-package-scopes.mjs @@ -24,6 +24,14 @@ describe('the type flag should change the interpretation of certain files within strictEqual(defaultExport, 'module'); }); + it('should import an extensionless JavaScript file within a "type": "module" scope under node_modules', + async () => { + const { default: defaultExport } = + await import(fixtures.fileURL( + 'es-modules/package-type-module/node_modules/dep-with-package-json-type-module/noext-esm')); + strictEqual(defaultExport, 'module'); + }); + it('should run as Wasm an extensionless Wasm file within a "type": "module" scope', async () => { const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ '--experimental-default-type=module', @@ -42,6 +50,13 @@ describe('the type flag should change the interpretation of certain files within const { add } = await import(fixtures.fileURL('es-modules/package-type-module/noext-wasm')); strictEqual(add(1, 2), 3); }); + + it('should import an extensionless Wasm file within a "type": "module" scope under node_modules', + async () => { + const { add } = await import(fixtures.fileURL( + 'es-modules/package-type-module/node_modules/dep-with-package-json-type-module/noext-wasm')); + strictEqual(add(1, 2), 3); + }); }); describe(`the type flag should change the interpretation of certain files within a package scope that lacks a @@ -112,7 +127,7 @@ describe(`the type flag should NOT change the interpretation of certain files wi async () => { const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ '--experimental-default-type=module', - fixtures.path('es-modules/package-type-module/node_modules/dep-with-package-json/run.js'), + fixtures.path('es-modules/package-type-module/node_modules/dep-with-package-json-without-type/run.js'), ]); strictEqual(stderr, ''); @@ -124,7 +139,8 @@ describe(`the type flag should NOT change the interpretation of certain files wi it(`should import as CommonJS a .js file within a package scope that has no defined "type" and is under node_modules`, async () => { const { default: defaultExport } = - await import(fixtures.fileURL('es-modules/package-type-module/node_modules/dep-with-package-json/run.js')); + await import(fixtures.fileURL( + 'es-modules/package-type-module/node_modules/dep-with-package-json-without-type/run.js')); strictEqual(defaultExport, 42); }); @@ -132,7 +148,7 @@ describe(`the type flag should NOT change the interpretation of certain files wi under node_modules`, async () => { const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ '--experimental-default-type=module', - fixtures.path('es-modules/package-type-module/node_modules/dep-with-package-json/noext-cjs'), + fixtures.path('es-modules/package-type-module/node_modules/dep-with-package-json-without-type/noext-cjs'), ]); strictEqual(stderr, ''); @@ -144,7 +160,8 @@ describe(`the type flag should NOT change the interpretation of certain files wi it(`should import as CommonJS an extensionless JavaScript file within a package scope that has no defined "type" and is under node_modules`, async () => { const { default: defaultExport } = - await import(fixtures.fileURL('es-modules/package-type-module/node_modules/dep-with-package-json/noext-cjs')); + await import(fixtures.fileURL( + 'es-modules/package-type-module/node_modules/dep-with-package-json-without-type/noext-cjs')); strictEqual(defaultExport, 42); }); }); diff --git a/test/es-module/test-esm-unknown-or-no-extension.js b/test/es-module/test-esm-unknown-extension.js similarity index 65% rename from test/es-module/test-esm-unknown-or-no-extension.js rename to test/es-module/test-esm-unknown-extension.js index 83ebcc6267bfc3..dae9568c523fe5 100644 --- a/test/es-module/test-esm-unknown-or-no-extension.js +++ b/test/es-module/test-esm-unknown-extension.js @@ -7,14 +7,11 @@ const { execPath } = require('node:process'); const { describe, it } = require('node:test'); -// In a "type": "module" package scope, files with unknown extensions or no -// extensions should throw; both when used as a main entry point and also when -// referenced via `import`. -describe('ESM: extensionless and unknown specifiers', { concurrency: true }, () => { +// In a "type": "module" package scope, files with unknown extensions should throw; +// both when used as a main entry point and also when referenced via `import`. +describe('ESM: unknown specifiers', { concurrency: true }, () => { for ( const fixturePath of [ - '/es-modules/package-type-module/noext-esm', - '/es-modules/package-type-module/imports-noext.mjs', '/es-modules/package-type-module/extension.unknown', '/es-modules/package-type-module/imports-unknownext.mjs', ] @@ -27,10 +24,6 @@ describe('ESM: extensionless and unknown specifiers', { concurrency: true }, () assert.strictEqual(signal, null); assert.strictEqual(stdout, ''); assert.match(stderr, /ERR_UNKNOWN_FILE_EXTENSION/); - if (fixturePath.includes('noext')) { - // Check for explanation to users - assert.match(stderr, /extensionless/); - } }); } }); diff --git a/test/fixtures/es-modules/package-type-module/node_modules/dep-with-package-json-type-module/noext-esm b/test/fixtures/es-modules/package-type-module/node_modules/dep-with-package-json-type-module/noext-esm new file mode 100644 index 00000000000000..251d6e538a1fcf --- /dev/null +++ b/test/fixtures/es-modules/package-type-module/node_modules/dep-with-package-json-type-module/noext-esm @@ -0,0 +1,2 @@ +export default 'module'; +console.log('executed'); diff --git a/test/fixtures/es-modules/package-type-module/node_modules/dep-with-package-json-type-module/noext-wasm b/test/fixtures/es-modules/package-type-module/node_modules/dep-with-package-json-type-module/noext-wasm new file mode 100644 index 0000000000000000000000000000000000000000..9e035904b2e4d0a30ce5a63bcf05cc3a2c8449db GIT binary patch literal 136 zcmZ9COA5k35Jam#kl=s>MAx~130^|TEhaEo*f23T0he=in=IYDDqa=lk_iA^G=gdb zBG>AL9Q@$(Fn;}VPs=uBD{AGr0)Mu(GOe%O7ZMd>X|61DN|4~3^7j7hOM