From 298fdbe44280d9e3febdee4d789dc90e30ca2c34 Mon Sep 17 00:00:00 2001 From: Bradley Farias Date: Wed, 18 Dec 2019 09:28:11 -0600 Subject: [PATCH] esm: empty ext from pkg type/main doesnt affect format This ensures files with unknown extensions like foo.bar are not loaded as CJS/ESM when imported as a main entry point and makes sure that those files would maintain the same format even if loaded after the main entrypoint. PR-URL: https://github.com/nodejs/node/pull/31021 Reviewed-By: Guy Bedford --- doc/api/esm.md | 14 ++-- lib/internal/modules/esm/default_resolve.js | 7 +- test/es-module/test-esm-unknown-main.js | 81 +++++++++++++++++++ .../package-type-module/extension.unknown | 1 + .../package-type-module/imports-noext.mjs | 1 + .../imports-unknownext.mjs | 1 + 6 files changed, 95 insertions(+), 10 deletions(-) create mode 100644 test/es-module/test-esm-unknown-main.js create mode 100644 test/fixtures/es-modules/package-type-module/extension.unknown 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 a77e01350b596a..5ef8a616aa209d 100644 --- a/doc/api/esm.md +++ b/doc/api/esm.md @@ -1160,15 +1160,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 @@ -1189,7 +1187,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_) @@ -1340,20 +1338,20 @@ _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. Assert: _url_ corresponds to an existing file pathname. > 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 _isMain_ is **true** or _url_ ends in _".js"_, then +> 1. If _url_ ends in _".js"_ or lacks a file extension, then > 1. Return _"module"_. > 1. Throw an _Unsupported File Extension_ error. > 1. Otherwise, -> 1. If _isMain_ is **true**, then +> 1. If _url_ lacks a file extension, then > 1. Return _"commonjs"_. > 1. Throw an _Unsupported File Extension_ error. diff --git a/lib/internal/modules/esm/default_resolve.js b/lib/internal/modules/esm/default_resolve.js index 58ed8415b83b63..bd7f6421a8b579 100644 --- a/lib/internal/modules/esm/default_resolve.js +++ b/lib/internal/modules/esm/default_resolve.js @@ -104,9 +104,12 @@ function resolve(specifier, parentURL) { } const ext = extname(url.pathname); - let format = extensionFormatMap[ext]; - if (ext === '.js' || (!format && isMain)) + let format; + if (ext === '.js' || ext === '') { format = getPackageType(url.href) === TYPE_MODULE ? 'module' : 'commonjs'; + } else { + format = extensionFormatMap[ext]; + } if (!format) { if (experimentalSpeciferResolution === 'node') { process.emitWarning( diff --git a/test/es-module/test-esm-unknown-main.js b/test/es-module/test-esm-unknown-main.js new file mode 100644 index 00000000000000..12163efb659d81 --- /dev/null +++ b/test/es-module/test-esm-unknown-main.js @@ -0,0 +1,81 @@ +'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, ['--experimental-modules', 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, ['--experimental-modules', 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, ['--experimental-modules', 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, ['--experimental-modules', 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/extension.unknown b/test/fixtures/es-modules/package-type-module/extension.unknown new file mode 100644 index 00000000000000..bd2b1aaa1ebece --- /dev/null +++ b/test/fixtures/es-modules/package-type-module/extension.unknown @@ -0,0 +1 @@ +throw new Error('NO, NEVER'); 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';