From 5752a6fe873697ff8f94cb60306cc01303f1bd4e Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sun, 19 Jan 2020 07:54:27 -0800 Subject: [PATCH 1/3] module: revert #31021 reverses baa3621bb1479e85a057a0d0df5b9c84ac4eb755 --- doc/api/esm.md | 17 ++-- lib/internal/modules/esm/get_format.js | 8 +- lib/internal/modules/esm/loader.js | 2 +- test/es-module/test-esm-unknown-main.js | 81 ------------------- .../package-type-module/imports-noext.mjs | 1 - .../imports-unknownext.mjs | 1 - 6 files changed, 14 insertions(+), 96 deletions(-) delete mode 100644 test/es-module/test-esm-unknown-main.js delete mode 100644 test/fixtures/es-modules/package-type-module/imports-noext.mjs delete mode 100644 test/fixtures/es-modules/package-type-module/imports-unknownext.mjs diff --git a/doc/api/esm.md b/doc/api/esm.md index e5271b5b343a84..1a85b57ceee19e 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -1042,7 +1042,8 @@ a URL should be interpreted. This can be one of the following: ```js /** * @param {string} url - * @param {object} context (currently empty) + * @param {object} context + * @param {string} context.parentURL * @param {function} defaultGetFormat * @returns {object} response * @returns {string} response.format @@ -1366,13 +1367,15 @@ updates. In the following algorithms, all subroutine errors are propagated as errors of these top-level routines unless stated otherwise. +_isMain_ is **true** when resolving the Node.js application entry point. + _defaultEnv_ is the conditional environment name priority array, `["node", "import"]`.
Resolver algorithm specification -**ESM_RESOLVE**(_specifier_, _parentURL_) +**ESM_RESOLVE**(_specifier_, _parentURL_, _isMain_) > 1. Let _resolvedURL_ be **undefined**. > 1. If _specifier_ is a valid URL, then @@ -1393,7 +1396,7 @@ _defaultEnv_ is the conditional environment name priority array, > 1. If the file at _resolvedURL_ does not exist, then > 1. Throw a _Module Not Found_ error. > 1. Set _resolvedURL_ to the real path of _resolvedURL_. -> 1. Let _format_ be the result of **ESM_FORMAT**(_resolvedURL_). +> 1. Let _format_ be the result of **ESM_FORMAT**(_resolvedURL_, _isMain_). > 1. Load _resolvedURL_ as module format, _format_. **PACKAGE_RESOLVE**(_packageSpecifier_, _parentURL_) @@ -1546,20 +1549,20 @@ _defaultEnv_ is the conditional environment name priority array, > 1. Return _resolved_. > 1. Throw a _Module Not Found_ error. -**ESM_FORMAT**(_url_) +**ESM_FORMAT**(_url_, _isMain_) -> 1. Assert: _url_ corresponds to an existing file pathname. +> 1. Assert: _url_ corresponds to an existing file. > 1. Let _pjson_ be the result of **READ_PACKAGE_SCOPE**(_url_). > 1. If _url_ ends in _".mjs"_, then > 1. Return _"module"_. > 1. If _url_ ends in _".cjs"_, then > 1. Return _"commonjs"_. > 1. If _pjson?.type_ exists and is _"module"_, then -> 1. If _url_ ends in _".js"_ or lacks a file extension, then +> 1. If _isMain_ is **true** or _url_ ends in _".js"_, then > 1. Return _"module"_. > 1. Throw an _Unsupported File Extension_ error. > 1. Otherwise, -> 1. If _url_ lacks a file extension, then +> 1. If _isMain_ is **true**, then > 1. Return _"commonjs"_. > 1. Throw an _Unsupported File Extension_ error. diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js index 2c215ab5378a40..5996e9fa9420a4 100644 --- a/lib/internal/modules/esm/get_format.js +++ b/lib/internal/modules/esm/get_format.js @@ -54,13 +54,11 @@ function defaultGetFormat(url, context, defaultGetFormat) { return { format }; } else if (parsed.protocol === 'file:') { const ext = extname(parsed.pathname); - let format; - if (ext === '.js' || ext === '') { + let format = extensionFormatMap[ext]; + const isMain = context.parentURL === undefined; + if (ext === '.js' || (!format && isMain)) format = getPackageType(parsed.href) === TYPE_MODULE ? 'module' : 'commonjs'; - } else { - format = extensionFormatMap[ext]; - } if (!format) { if (experimentalSpeciferResolution === 'node') { process.emitWarning( diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 6d9b267ffe5d67..bac2c37eb30098 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -96,7 +96,7 @@ class Loader { } const getFormatResponse = await this._getFormat( - url, {}, defaultGetFormat); + url, { parentURL }, defaultGetFormat); if (typeof getFormatResponse !== 'object') { throw new ERR_INVALID_RETURN_VALUE( 'object', 'loader getFormat', getFormatResponse); diff --git a/test/es-module/test-esm-unknown-main.js b/test/es-module/test-esm-unknown-main.js deleted file mode 100644 index e49adbdae3dcfa..00000000000000 --- a/test/es-module/test-esm-unknown-main.js +++ /dev/null @@ -1,81 +0,0 @@ -'use strict'; - -const common = require('../common'); -const fixtures = require('../common/fixtures'); -const { spawn } = require('child_process'); -const assert = require('assert'); - -{ - const entry = fixtures.path( - '/es-modules/package-type-module/extension.unknown' - ); - const child = spawn(process.execPath, [entry]); - let stdout = ''; - let stderr = ''; - child.stderr.setEncoding('utf8'); - child.stdout.setEncoding('utf8'); - child.stdout.on('data', (data) => { - stdout += data; - }); - child.stderr.on('data', (data) => { - stderr += data; - }); - child.on('close', common.mustCall((code, signal) => { - assert.strictEqual(code, 1); - assert.strictEqual(signal, null); - assert.strictEqual(stdout, ''); - assert.ok(stderr.indexOf('ERR_UNKNOWN_FILE_EXTENSION') !== -1); - })); -} -{ - const entry = fixtures.path( - '/es-modules/package-type-module/imports-unknownext.mjs' - ); - const child = spawn(process.execPath, [entry]); - let stdout = ''; - let stderr = ''; - child.stderr.setEncoding('utf8'); - child.stdout.setEncoding('utf8'); - child.stdout.on('data', (data) => { - stdout += data; - }); - child.stderr.on('data', (data) => { - stderr += data; - }); - child.on('close', common.mustCall((code, signal) => { - assert.strictEqual(code, 1); - assert.strictEqual(signal, null); - assert.strictEqual(stdout, ''); - assert.ok(stderr.indexOf('ERR_UNKNOWN_FILE_EXTENSION') !== -1); - })); -} -{ - const entry = fixtures.path('/es-modules/package-type-module/noext-esm'); - const child = spawn(process.execPath, [entry]); - let stdout = ''; - child.stdout.setEncoding('utf8'); - child.stdout.on('data', (data) => { - stdout += data; - }); - child.on('close', common.mustCall((code, signal) => { - assert.strictEqual(code, 0); - assert.strictEqual(signal, null); - assert.strictEqual(stdout, 'executed\n'); - })); -} -{ - const entry = fixtures.path( - '/es-modules/package-type-module/imports-noext.mjs' - ); - const child = spawn(process.execPath, [entry]); - let stdout = ''; - child.stdout.setEncoding('utf8'); - child.stdout.on('data', (data) => { - stdout += data; - }); - child.on('close', common.mustCall((code, signal) => { - assert.strictEqual(code, 0); - assert.strictEqual(signal, null); - assert.strictEqual(stdout, 'executed\n'); - })); -} diff --git a/test/fixtures/es-modules/package-type-module/imports-noext.mjs b/test/fixtures/es-modules/package-type-module/imports-noext.mjs deleted file mode 100644 index 96eca54521b9d3..00000000000000 --- a/test/fixtures/es-modules/package-type-module/imports-noext.mjs +++ /dev/null @@ -1 +0,0 @@ -import './noext-esm'; diff --git a/test/fixtures/es-modules/package-type-module/imports-unknownext.mjs b/test/fixtures/es-modules/package-type-module/imports-unknownext.mjs deleted file mode 100644 index 2178abee1145d6..00000000000000 --- a/test/fixtures/es-modules/package-type-module/imports-unknownext.mjs +++ /dev/null @@ -1 +0,0 @@ -import './extension.unknown'; From 969fa8fb96df9e11a1e16c78008a06f9fd5c883f Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sun, 19 Jan 2020 07:56:43 -0800 Subject: [PATCH 2/3] module: correct docs about when extensionless files are supported --- doc/api/esm.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/doc/api/esm.md b/doc/api/esm.md index 1a85b57ceee19e..1fdfd8c247b111 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -33,9 +33,9 @@ initial input, or when referenced by `import` statements within ES module code: * Files ending in `.mjs`. -* Files ending in `.js`, or extensionless files, when the nearest parent - `package.json` file contains a top-level field `"type"` with a value of - `"module"`. +* Files ending in `.js`, or extensionless files when run as main entry points on + the command line, when the nearest parent `package.json` file contains a + top-level field `"type"` with a value of `"module"`. * Strings passed in as an argument to `--eval` or `--print`, or piped to `node` via `STDIN`, with the flag `--input-type=module`. @@ -50,9 +50,9 @@ or when referenced by `import` statements within ES module code: * Files ending in `.cjs`. -* Files ending in `.js`, or extensionless files, when the nearest parent - `package.json` file contains a top-level field `"type"` with a value of - `"commonjs"`. +* Files ending in `.js`, or extensionless files when run as main entry points on + the command line, when the nearest parent `package.json` file contains a + top-level field `"type"` with a value of `"commonjs"`. * Strings passed in as an argument to `--eval` or `--print`, or piped to `node` via `STDIN`, with the flag `--input-type=commonjs`. From cdacf7ddc942661b20410211ca770966e7e46377 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sun, 19 Jan 2020 22:40:26 -0800 Subject: [PATCH 3/3] module: drop support for extensionless main entry points in esm --- doc/api/esm.md | 54 ++++++++----------- lib/internal/modules/esm/get_format.js | 8 +-- lib/internal/modules/esm/loader.js | 2 +- test/es-module/test-esm-no-extension.js | 24 --------- .../test-esm-unknown-or-no-extension.js | 36 +++++++++++++ .../package-type-module/imports-noext.mjs | 1 + .../imports-unknownext.mjs | 1 + 7 files changed, 66 insertions(+), 60 deletions(-) delete mode 100644 test/es-module/test-esm-no-extension.js create mode 100644 test/es-module/test-esm-unknown-or-no-extension.js create mode 100644 test/fixtures/es-modules/package-type-module/imports-noext.mjs create mode 100644 test/fixtures/es-modules/package-type-module/imports-unknownext.mjs diff --git a/doc/api/esm.md b/doc/api/esm.md index 1fdfd8c247b111..a09fffccfce454 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -33,8 +33,7 @@ initial input, or when referenced by `import` statements within ES module code: * Files ending in `.mjs`. -* Files ending in `.js`, or extensionless files when run as main entry points on - the command line, when the nearest parent `package.json` file contains a +* Files ending in `.js` when the nearest parent `package.json` file contains a top-level field `"type"` with a value of `"module"`. * Strings passed in as an argument to `--eval` or `--print`, or piped to @@ -50,8 +49,7 @@ or when referenced by `import` statements within ES module code: * Files ending in `.cjs`. -* Files ending in `.js`, or extensionless files when run as main entry points on - the command line, when the nearest parent `package.json` file contains a +* Files ending in `.js` when the nearest parent `package.json` file contains a top-level field `"type"` with a value of `"commonjs"`. * Strings passed in as an argument to `--eval` or `--print`, or piped to @@ -59,9 +57,9 @@ or when referenced by `import` statements within ES module code: ### `package.json` `"type"` field -Files ending with `.js` or lacking any extension will be loaded as ES modules -when the nearest parent `package.json` file contains a top-level field `"type"` -with a value of `"module"`. +Files ending with `.js` will be loaded as ES modules when the nearest parent +`package.json` file contains a top-level field `"type"` with a value of +`"module"`. The nearest parent `package.json` is defined as the first `package.json` found when searching in the current folder, that folder’s parent, and so on up @@ -81,14 +79,12 @@ node my-app.js # Runs as ES module ``` If the nearest parent `package.json` lacks a `"type"` field, or contains -`"type": "commonjs"`, extensionless and `.js` files are treated as CommonJS. -If the volume root is reached and no `package.json` is found, -Node.js defers to the default, a `package.json` with no `"type"` -field. "Extensionless" refers to file paths which do not contain -an extension as opposed to optionally dropping a file extension in a specifier. +`"type": "commonjs"`, `.js` files are treated as CommonJS. If the volume root is +reached and no `package.json` is found, Node.js defers to the default, a +`package.json` with no `"type"` field. -`import` statements of `.js` and extensionless files are treated as ES modules -if the nearest parent `package.json` contains `"type": "module"`. +`import` statements of `.js` files are treated as ES modules if the nearest +parent `package.json` contains `"type": "module"`. ```js // my-app.js, part of the same example as above @@ -106,14 +102,13 @@ as ES modules and `.cjs` files are always treated as CommonJS. ### Package Scope and File Extensions -A folder containing a `package.json` file, and all subfolders below that -folder down until the next folder containing another `package.json`, is -considered a _package scope_. The `"type"` field defines how `.js` and -extensionless files should be treated within a particular `package.json` file’s -package scope. Every package in a project’s `node_modules` folder contains its -own `package.json` file, so each project’s dependencies have their own package -scopes. A `package.json` lacking a `"type"` field is treated as if it contained -`"type": "commonjs"`. +A folder containing a `package.json` file, and all subfolders below that folder +down until the next folder containing another `package.json`, is considered a +_package scope_. The `"type"` field defines how `.js` files should be treated +within a particular `package.json` file’s package scope. Every package in a +project’s `node_modules` folder contains its own `package.json` file, so each +project’s dependencies have their own package scopes. A `package.json` lacking a +`"type"` field is treated as if it contained `"type": "commonjs"`. The package scope applies not only to initial entry points (`node my-app.js`) but also to files referenced by `import` statements and `import()` expressions. @@ -1042,8 +1037,7 @@ a URL should be interpreted. This can be one of the following: ```js /** * @param {string} url - * @param {object} context - * @param {string} context.parentURL + * @param {object} context (currently empty) * @param {function} defaultGetFormat * @returns {object} response * @returns {string} response.format @@ -1367,15 +1361,13 @@ updates. In the following algorithms, all subroutine errors are propagated as errors of these top-level routines unless stated otherwise. -_isMain_ is **true** when resolving the Node.js application entry point. - _defaultEnv_ is the conditional environment name priority array, `["node", "import"]`.
Resolver algorithm specification -**ESM_RESOLVE**(_specifier_, _parentURL_, _isMain_) +**ESM_RESOLVE**(_specifier_, _parentURL_) > 1. Let _resolvedURL_ be **undefined**. > 1. If _specifier_ is a valid URL, then @@ -1396,7 +1388,7 @@ _defaultEnv_ is the conditional environment name priority array, > 1. If the file at _resolvedURL_ does not exist, then > 1. Throw a _Module Not Found_ error. > 1. Set _resolvedURL_ to the real path of _resolvedURL_. -> 1. Let _format_ be the result of **ESM_FORMAT**(_resolvedURL_, _isMain_). +> 1. Let _format_ be the result of **ESM_FORMAT**(_resolvedURL_). > 1. Load _resolvedURL_ as module format, _format_. **PACKAGE_RESOLVE**(_packageSpecifier_, _parentURL_) @@ -1549,7 +1541,7 @@ _defaultEnv_ is the conditional environment name priority array, > 1. Return _resolved_. > 1. Throw a _Module Not Found_ error. -**ESM_FORMAT**(_url_, _isMain_) +**ESM_FORMAT**(_url_) > 1. Assert: _url_ corresponds to an existing file. > 1. Let _pjson_ be the result of **READ_PACKAGE_SCOPE**(_url_). @@ -1558,12 +1550,10 @@ _defaultEnv_ is the conditional environment name priority array, > 1. If _url_ ends in _".cjs"_, then > 1. Return _"commonjs"_. > 1. If _pjson?.type_ exists and is _"module"_, then -> 1. If _isMain_ is **true** or _url_ ends in _".js"_, then +> 1. If _url_ ends in _".js"_, then > 1. Return _"module"_. > 1. Throw an _Unsupported File Extension_ error. > 1. Otherwise, -> 1. If _isMain_ is **true**, then -> 1. Return _"commonjs"_. > 1. Throw an _Unsupported File Extension_ error. **READ_PACKAGE_SCOPE**(_url_) diff --git a/lib/internal/modules/esm/get_format.js b/lib/internal/modules/esm/get_format.js index 5996e9fa9420a4..69ba2398129908 100644 --- a/lib/internal/modules/esm/get_format.js +++ b/lib/internal/modules/esm/get_format.js @@ -54,11 +54,13 @@ function defaultGetFormat(url, context, defaultGetFormat) { return { format }; } else if (parsed.protocol === 'file:') { const ext = extname(parsed.pathname); - let format = extensionFormatMap[ext]; - const isMain = context.parentURL === undefined; - if (ext === '.js' || (!format && isMain)) + let format; + if (ext === '.js') { format = getPackageType(parsed.href) === TYPE_MODULE ? 'module' : 'commonjs'; + } else { + format = extensionFormatMap[ext]; + } if (!format) { if (experimentalSpeciferResolution === 'node') { process.emitWarning( diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index bac2c37eb30098..6d9b267ffe5d67 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -96,7 +96,7 @@ class Loader { } const getFormatResponse = await this._getFormat( - url, { parentURL }, defaultGetFormat); + url, {}, defaultGetFormat); if (typeof getFormatResponse !== 'object') { throw new ERR_INVALID_RETURN_VALUE( 'object', 'loader getFormat', getFormatResponse); diff --git a/test/es-module/test-esm-no-extension.js b/test/es-module/test-esm-no-extension.js deleted file mode 100644 index 392bb5638b0e34..00000000000000 --- a/test/es-module/test-esm-no-extension.js +++ /dev/null @@ -1,24 +0,0 @@ -'use strict'; - -const common = require('../common'); -const fixtures = require('../common/fixtures'); -const { spawn } = require('child_process'); -const assert = require('assert'); - -const entry = fixtures.path('/es-modules/package-type-module/noext-esm'); - -// Run a module that does not have extension. -// This is to ensure that "type": "module" applies to extensionless files. - -const child = spawn(process.execPath, [entry]); - -let stdout = ''; -child.stdout.setEncoding('utf8'); -child.stdout.on('data', (data) => { - stdout += data; -}); -child.on('close', common.mustCall((code, signal) => { - assert.strictEqual(code, 0); - assert.strictEqual(signal, null); - assert.strictEqual(stdout, 'executed\n'); -})); diff --git a/test/es-module/test-esm-unknown-or-no-extension.js b/test/es-module/test-esm-unknown-or-no-extension.js new file mode 100644 index 00000000000000..3b1802a4dcedbd --- /dev/null +++ b/test/es-module/test-esm-unknown-or-no-extension.js @@ -0,0 +1,36 @@ +'use strict'; + +const common = require('../common'); +const fixtures = require('../common/fixtures'); +const { spawn } = require('child_process'); +const assert = require('assert'); + +// 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`. + +[ + '/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', +].forEach((fixturePath) => { + const entry = fixtures.path(fixturePath); + const child = spawn(process.execPath, [entry]); + let stdout = ''; + let stderr = ''; + child.stderr.setEncoding('utf8'); + child.stdout.setEncoding('utf8'); + child.stdout.on('data', (data) => { + stdout += data; + }); + child.stderr.on('data', (data) => { + stderr += data; + }); + child.on('close', common.mustCall((code, signal) => { + assert.strictEqual(code, 1); + assert.strictEqual(signal, null); + assert.strictEqual(stdout, ''); + assert.ok(stderr.indexOf('ERR_UNKNOWN_FILE_EXTENSION') !== -1); + })); +}); diff --git a/test/fixtures/es-modules/package-type-module/imports-noext.mjs b/test/fixtures/es-modules/package-type-module/imports-noext.mjs new file mode 100644 index 00000000000000..96eca54521b9d3 --- /dev/null +++ b/test/fixtures/es-modules/package-type-module/imports-noext.mjs @@ -0,0 +1 @@ +import './noext-esm'; diff --git a/test/fixtures/es-modules/package-type-module/imports-unknownext.mjs b/test/fixtures/es-modules/package-type-module/imports-unknownext.mjs new file mode 100644 index 00000000000000..2178abee1145d6 --- /dev/null +++ b/test/fixtures/es-modules/package-type-module/imports-unknownext.mjs @@ -0,0 +1 @@ +import './extension.unknown';