From 97b1966a2a2daa53f512e6d041ea6d31f147fb2b Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sat, 30 Sep 2023 20:38:24 -0700 Subject: [PATCH 01/11] esm: bypass CommonJS loader under --default-type --- lib/internal/main/run_main_module.js | 22 ++++++++------ lib/internal/modules/run_main.js | 18 ++++++++---- lib/internal/process/pre_execution.js | 16 ++++++++--- test/es-module/test-esm-type-flag-errors.mjs | 30 ++++++++++++++++++++ 4 files changed, 69 insertions(+), 17 deletions(-) diff --git a/lib/internal/main/run_main_module.js b/lib/internal/main/run_main_module.js index 51331270a2161f..5d09203b8c27ee 100644 --- a/lib/internal/main/run_main_module.js +++ b/lib/internal/main/run_main_module.js @@ -6,18 +6,24 @@ const { prepareMainThreadExecution, markBootstrapComplete, } = require('internal/process/pre_execution'); +const { getOptionValue } = require('internal/options'); -prepareMainThreadExecution(true); +const mainEntry = prepareMainThreadExecution(true); markBootstrapComplete(); // Necessary to reset RegExp statics before user code runs. RegExpPrototypeExec(/^/, ''); -// Note: this loads the module through the ESM loader if the module is -// determined to be an ES module. This hangs from the CJS module loader -// because we currently allow monkey-patching of the module loaders -// in the preloaded scripts through require('module'). -// runMain here might be monkey-patched by users in --require. -// XXX: the monkey-patchability here should probably be deprecated. -require('internal/modules/cjs/loader').Module.runMain(process.argv[1]); +if (getOptionValue('--experimental-default-type') === 'module') { + require('internal/modules/run_main').executeUserEntryPoint(mainEntry); +} else { + /** + * To support legacy monkey-patching of `Module.runMain`, we call `runMain` here to have the CommonJS loader begin + * the execution of the main entry point, even if the ESM loader immediately takes over because the main entry is an + * ES module or one of the other opt-in conditions (such as the use of `--import`) are met. Users can monkey-patch + * before the main entry point is loaded by doing so via scripts loaded through `--require`. This monkey-patchability + * is undesirable and is removed in `--experimental-default-type=module` mode. + */ + require('internal/modules/cjs/loader').Module.runMain(mainEntry); +} diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index 0b58d348203bfe..a2f34944218ab1 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -12,11 +12,16 @@ const path = require('path'); * @param {string} main - Entry point path */ function resolveMainPath(main) { - // Note extension resolution for the main entry point can be deprecated in a - // future major. - // Module._findPath is monkey-patchable here. - const { Module } = require('internal/modules/cjs/loader'); - let mainPath = Module._findPath(path.resolve(main), null, true); + /** @type {string} */ + let mainPath; + if (getOptionValue('--experimental-default-type') === 'module') { + mainPath = path.resolve(main); + } else { + // Extension searching for the main entry point is supported only in legacy mode. + // Module._findPath is monkey-patchable here. + const { Module } = require('internal/modules/cjs/loader'); + mainPath = Module._findPath(path.resolve(main), null, true); + } if (!mainPath) { return; } const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); @@ -33,6 +38,8 @@ function resolveMainPath(main) { * @param {string} mainPath - Absolute path to the main entry point */ function shouldUseESMLoader(mainPath) { + if (getOptionValue('--experimental-default-type') === 'module') { return true; } + /** * @type {string[]} userLoaders A list of custom loaders registered by the user * (or an empty list when none have been registered). @@ -90,6 +97,7 @@ async function handleMainPromise(promise) { * Parse the CLI main entry point string and run it. * For backwards compatibility, we have to run a bunch of monkey-patchable code that belongs to the CJS loader (exposed * by `require('module')`) even when the entry point is ESM. + * This monkey-patchable code is bypassed under `--experimental-default-type=module`. * Because of backwards compatibility, this function is exposed publicly via `import { runMain } from 'node:module'`. * @param {string} main - Resolved absolute path for the main entry point, if found */ diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index d066d12a553d6f..832a7eae5a6775 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -50,9 +50,10 @@ const { isBuildingSnapshot, }, } = require('internal/v8/startup_snapshot'); +const { resolve } = require('path'); function prepareMainThreadExecution(expandArgv1 = false, initializeModules = true) { - prepareExecution({ + return prepareExecution({ expandArgv1, initializeModules, isMainThread: true, @@ -73,8 +74,8 @@ function prepareExecution(options) { refreshRuntimeOptions(); reconnectZeroFillToggle(); - // Patch the process object with legacy properties and normalizations - patchProcessObject(expandArgv1); + // Patch the process object and get the resolved main entry point. + const mainEntry = patchProcessObject(expandArgv1); setupTraceCategoryState(); setupInspectorHooks(); setupWarningHandler(); @@ -131,6 +132,8 @@ function prepareExecution(options) { if (initializeModules) { setupUserModules(); } + + return mainEntry; } function setupSymbolDisposePolyfill() { @@ -202,6 +205,8 @@ function patchProcessObject(expandArgv1) { process._exiting = false; process.argv[0] = process.execPath; + /** @type {string} */ + let mainEntry; // If requested, update process.argv[1] to replace whatever the user provided with the resolved absolute file path of // the entry point. if (expandArgv1 && process.argv[1] && @@ -209,7 +214,8 @@ function patchProcessObject(expandArgv1) { // Expand process.argv[1] into a full path. const path = require('path'); try { - process.argv[1] = path.resolve(process.argv[1]); + mainEntry = path.resolve(process.argv[1]); + process.argv[1] = mainEntry; } catch { // Continue regardless of error. } @@ -236,6 +242,8 @@ function patchProcessObject(expandArgv1) { addReadOnlyProcessAlias('traceDeprecation', '--trace-deprecation'); addReadOnlyProcessAlias('_breakFirstLine', '--inspect-brk', false); addReadOnlyProcessAlias('_breakNodeFirstLine', '--inspect-brk-node', false); + + return mainEntry; } function addReadOnlyProcessAlias(name, option, enumerable = true) { diff --git a/test/es-module/test-esm-type-flag-errors.mjs b/test/es-module/test-esm-type-flag-errors.mjs index 6d54eff94763ef..ba316674b00354 100644 --- a/test/es-module/test-esm-type-flag-errors.mjs +++ b/test/es-module/test-esm-type-flag-errors.mjs @@ -3,6 +3,36 @@ import * as fixtures from '../common/fixtures.mjs'; import { describe, it } from 'node:test'; import { match, strictEqual } from 'node:assert'; +describe('--experimental-default-type=module should not support extension searching', { concurrency: true }, () => { + it('should support extension searching under --experimental-default-type=commonjs', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--experimental-default-type=commonjs', + './index', + ], { + cwd: fixtures.path('es-modules/package-without-type'), + }); + + strictEqual(stdout, 'package-without-type\n'); + strictEqual(stderr, ''); + strictEqual(code, 0); + strictEqual(signal, null); + }); + + it('should error with implicit extension under --experimental-default-type=module', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--experimental-default-type=module', + './index', + ], { + cwd: fixtures.path('es-modules/package-without-type'), + }); + + match(stderr, /ENOENT/); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + }); +}); + describe('--experimental-default-type=module should not affect the interpretation of files with unknown extensions', { concurrency: true }, () => { it('should error on an entry point with an unknown extension', async () => { From 30821dbd0cf8ba509254a2deb1f30d35656e13b1 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sun, 1 Oct 2023 10:31:59 -0700 Subject: [PATCH 02/11] Streamline --- lib/internal/modules/run_main.js | 26 ++++++++++---------- test/es-module/test-esm-type-flag-errors.mjs | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index a2f34944218ab1..80df40cafff930 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -9,19 +9,15 @@ const path = require('path'); /** * Get the absolute path to the main entry point. + * Only used in `--experimental-default-type=commonjs` mode. * @param {string} main - Entry point path */ function resolveMainPath(main) { - /** @type {string} */ - let mainPath; - if (getOptionValue('--experimental-default-type') === 'module') { - mainPath = path.resolve(main); - } else { - // Extension searching for the main entry point is supported only in legacy mode. - // Module._findPath is monkey-patchable here. - const { Module } = require('internal/modules/cjs/loader'); - mainPath = Module._findPath(path.resolve(main), null, true); - } + // Note extension resolution for the main entry point can be deprecated in a + // future major. + // Module._findPath is monkey-patchable here. + const { Module } = require('internal/modules/cjs/loader'); + let mainPath = Module._findPath(path.resolve(main), null, true); if (!mainPath) { return; } const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); @@ -35,11 +31,10 @@ function resolveMainPath(main) { /** * Determine whether the main entry point should be loaded through the ESM Loader. + * Only used in `--experimental-default-type=commonjs` mode. * @param {string} mainPath - Absolute path to the main entry point */ function shouldUseESMLoader(mainPath) { - if (getOptionValue('--experimental-default-type') === 'module') { return true; } - /** * @type {string[]} userLoaders A list of custom loaders registered by the user * (or an empty list when none have been registered). @@ -64,7 +59,7 @@ function shouldUseESMLoader(mainPath) { /** * Run the main entry point through the ESM Loader. - * @param {string} mainPath - Absolute path for the main entry point + * @param {string} mainPath - Path to the main entry point, either absolute or relative to CWD */ function runMainESM(mainPath) { const { loadESM } = require('internal/process/esm_loader'); @@ -102,6 +97,11 @@ async function handleMainPromise(promise) { * @param {string} main - Resolved absolute path for the main entry point, if found */ function executeUserEntryPoint(main = process.argv[1]) { + if (getOptionValue('--experimental-default-type') === 'module') { + runMainESM(main); + return; + } + const resolvedMain = resolveMainPath(main); const useESMLoader = shouldUseESMLoader(resolvedMain); if (useESMLoader) { diff --git a/test/es-module/test-esm-type-flag-errors.mjs b/test/es-module/test-esm-type-flag-errors.mjs index ba316674b00354..a970560c082f4f 100644 --- a/test/es-module/test-esm-type-flag-errors.mjs +++ b/test/es-module/test-esm-type-flag-errors.mjs @@ -26,7 +26,7 @@ describe('--experimental-default-type=module should not support extension search cwd: fixtures.path('es-modules/package-without-type'), }); - match(stderr, /ENOENT/); + match(stderr, /ERR_MODULE_NOT_FOUND/); strictEqual(stdout, ''); strictEqual(code, 1); strictEqual(signal, null); From 286c632d798497026813eb8b70e19e997d39a1f2 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sun, 1 Oct 2023 16:07:51 -0700 Subject: [PATCH 03/11] Revert "Streamline" This reverts commit 6c6aefec5ede1070b2a17864075b2cc3d60718ee. --- lib/internal/modules/run_main.js | 26 ++++++++++---------- test/es-module/test-esm-type-flag-errors.mjs | 2 +- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index 80df40cafff930..a2f34944218ab1 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -9,15 +9,19 @@ const path = require('path'); /** * Get the absolute path to the main entry point. - * Only used in `--experimental-default-type=commonjs` mode. * @param {string} main - Entry point path */ function resolveMainPath(main) { - // Note extension resolution for the main entry point can be deprecated in a - // future major. - // Module._findPath is monkey-patchable here. - const { Module } = require('internal/modules/cjs/loader'); - let mainPath = Module._findPath(path.resolve(main), null, true); + /** @type {string} */ + let mainPath; + if (getOptionValue('--experimental-default-type') === 'module') { + mainPath = path.resolve(main); + } else { + // Extension searching for the main entry point is supported only in legacy mode. + // Module._findPath is monkey-patchable here. + const { Module } = require('internal/modules/cjs/loader'); + mainPath = Module._findPath(path.resolve(main), null, true); + } if (!mainPath) { return; } const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); @@ -31,10 +35,11 @@ function resolveMainPath(main) { /** * Determine whether the main entry point should be loaded through the ESM Loader. - * Only used in `--experimental-default-type=commonjs` mode. * @param {string} mainPath - Absolute path to the main entry point */ function shouldUseESMLoader(mainPath) { + if (getOptionValue('--experimental-default-type') === 'module') { return true; } + /** * @type {string[]} userLoaders A list of custom loaders registered by the user * (or an empty list when none have been registered). @@ -59,7 +64,7 @@ function shouldUseESMLoader(mainPath) { /** * Run the main entry point through the ESM Loader. - * @param {string} mainPath - Path to the main entry point, either absolute or relative to CWD + * @param {string} mainPath - Absolute path for the main entry point */ function runMainESM(mainPath) { const { loadESM } = require('internal/process/esm_loader'); @@ -97,11 +102,6 @@ async function handleMainPromise(promise) { * @param {string} main - Resolved absolute path for the main entry point, if found */ function executeUserEntryPoint(main = process.argv[1]) { - if (getOptionValue('--experimental-default-type') === 'module') { - runMainESM(main); - return; - } - const resolvedMain = resolveMainPath(main); const useESMLoader = shouldUseESMLoader(resolvedMain); if (useESMLoader) { diff --git a/test/es-module/test-esm-type-flag-errors.mjs b/test/es-module/test-esm-type-flag-errors.mjs index a970560c082f4f..ba316674b00354 100644 --- a/test/es-module/test-esm-type-flag-errors.mjs +++ b/test/es-module/test-esm-type-flag-errors.mjs @@ -26,7 +26,7 @@ describe('--experimental-default-type=module should not support extension search cwd: fixtures.path('es-modules/package-without-type'), }); - match(stderr, /ERR_MODULE_NOT_FOUND/); + match(stderr, /ENOENT/); strictEqual(stdout, ''); strictEqual(code, 1); strictEqual(signal, null); From 462084278ac1f11cec8edcb577a57d64b9b02f55 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sun, 1 Oct 2023 16:45:55 -0700 Subject: [PATCH 04/11] Move/improve tests; decorate "not found" error to provide hint about extension searching; fix lint --- lib/internal/modules/esm/resolve.js | 33 ++++--- lib/internal/modules/run_main.js | 16 +++- lib/internal/process/pre_execution.js | 1 - .../test-esm-type-flag-cli-entry.mjs | 92 +++++++++++++++++++ test/es-module/test-esm-type-flag-errors.mjs | 30 ------ .../es-modules/package-without-type/wha?.js | 1 + 6 files changed, 128 insertions(+), 45 deletions(-) create mode 100644 test/es-module/test-esm-type-flag-cli-entry.mjs create mode 100644 test/fixtures/es-modules/package-without-type/wha?.js diff --git a/lib/internal/modules/esm/resolve.js b/lib/internal/modules/esm/resolve.js index 5aea5ca5460199..58e7df07ca5275 100644 --- a/lib/internal/modules/esm/resolve.js +++ b/lib/internal/modules/esm/resolve.js @@ -1132,17 +1132,7 @@ function defaultResolve(specifier, context = {}) { if (StringPrototypeStartsWith(specifier, 'file://')) { specifier = fileURLToPath(specifier); } - const found = resolveAsCommonJS(specifier, parentURL); - if (found) { - // Modify the stack and message string to include the hint - const lines = StringPrototypeSplit(error.stack, '\n'); - const hint = `Did you mean to import ${found}?`; - error.stack = - ArrayPrototypeShift(lines) + '\n' + - hint + '\n' + - ArrayPrototypeJoin(lines, '\n'); - error.message += `\n${hint}`; - } + decorateErrorWithCommonJSHints(error, specifier, parentURL); } throw error; } @@ -1156,7 +1146,28 @@ function defaultResolve(specifier, context = {}) { }; } +/** + * Decorates the given error with a hint for CommonJS modules. + * @param {Error} error - The error to decorate. + * @param {string} specifier - The specifier that was attempted to be imported. + * @param {string} parentURL - The URL of the parent module. + */ +function decorateErrorWithCommonJSHints(error, specifier, parentURL) { + const found = resolveAsCommonJS(specifier, parentURL); + if (found) { + // Modify the stack and message string to include the hint + const lines = StringPrototypeSplit(error.stack, '\n'); + const hint = `Did you mean to import ${found}?`; + error.stack = + ArrayPrototypeShift(lines) + '\n' + + hint + '\n' + + ArrayPrototypeJoin(lines, '\n'); + error.message += `\n${hint}`; + } +} + module.exports = { + decorateErrorWithCommonJSHints, defaultResolve, encodedSepRegEx, getPackageScopeConfig, diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index a2f34944218ab1..fda69f4b9caf59 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -12,9 +12,10 @@ const path = require('path'); * @param {string} main - Entry point path */ function resolveMainPath(main) { + const defaultType = getOptionValue('--experimental-default-type'); /** @type {string} */ let mainPath; - if (getOptionValue('--experimental-default-type') === 'module') { + if (defaultType === 'module') { mainPath = path.resolve(main); } else { // Extension searching for the main entry point is supported only in legacy mode. @@ -27,7 +28,16 @@ function resolveMainPath(main) { const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); if (!preserveSymlinksMain) { const { toRealPath } = require('internal/modules/helpers'); - mainPath = toRealPath(mainPath); + try { + mainPath = toRealPath(mainPath); + } catch (err) { + if (err.code === 'ENOENT' && defaultType === 'module') { + const { decorateErrorWithCommonJSHints } = require('internal/modules/esm/resolve'); + const { getCWDURL } = require('internal/util'); + decorateErrorWithCommonJSHints(err, mainPath, getCWDURL()); + } + throw err; + } } return mainPath; @@ -99,7 +109,7 @@ async function handleMainPromise(promise) { * by `require('module')`) even when the entry point is ESM. * This monkey-patchable code is bypassed under `--experimental-default-type=module`. * Because of backwards compatibility, this function is exposed publicly via `import { runMain } from 'node:module'`. - * @param {string} main - Resolved absolute path for the main entry point, if found + * @param {string} main - First positional CLI argument, such as `'entry.js'` from `node entry.js` */ function executeUserEntryPoint(main = process.argv[1]) { const resolvedMain = resolveMainPath(main); diff --git a/lib/internal/process/pre_execution.js b/lib/internal/process/pre_execution.js index 832a7eae5a6775..917ba90a1c8bbb 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -50,7 +50,6 @@ const { isBuildingSnapshot, }, } = require('internal/v8/startup_snapshot'); -const { resolve } = require('path'); function prepareMainThreadExecution(expandArgv1 = false, initializeModules = true) { return prepareExecution({ diff --git a/test/es-module/test-esm-type-flag-cli-entry.mjs b/test/es-module/test-esm-type-flag-cli-entry.mjs new file mode 100644 index 00000000000000..3e5e5a1efee117 --- /dev/null +++ b/test/es-module/test-esm-type-flag-cli-entry.mjs @@ -0,0 +1,92 @@ +import { spawnPromisified } from '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; +import { describe, it } from 'node:test'; +import { match, strictEqual } from 'node:assert'; + +describe('--experimental-default-type=module should not support extension searching', { concurrency: true }, () => { + it('should support extension searching under --experimental-default-type=commonjs', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--experimental-default-type=commonjs', + 'index', + ], { + cwd: fixtures.path('es-modules/package-without-type'), + }); + + strictEqual(stdout, 'package-without-type\n'); + strictEqual(stderr, ''); + strictEqual(code, 0); + strictEqual(signal, null); + }); + + it('should error with implicit extension under --experimental-default-type=module', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--experimental-default-type=module', + 'index', + ], { + cwd: fixtures.path('es-modules/package-without-type'), + }); + + match(stderr, /ENOENT.*Did you mean to import .*index\.js\?/s); + strictEqual(stdout, ''); + strictEqual(code, 1); + strictEqual(signal, null); + }); +}); + +describe('--experimental-default-type=module should not parse paths as URLs', { concurrency: true }, () => { + it('should not parse a `?` in a filename as starting a query string', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--experimental-default-type=module', + 'wha?.js', + ], { + cwd: fixtures.path('es-modules/package-without-type'), + }); + + strictEqual(stderr, ''); + strictEqual(stdout, 'wha?\n'); + strictEqual(code, 0); + strictEqual(signal, null); + }); + + it('should resolve `..`', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--experimental-default-type=module', + '../package-without-type/wha?.js', + ], { + cwd: fixtures.path('es-modules/package-without-type'), + }); + + strictEqual(stderr, ''); + strictEqual(stdout, 'wha?\n'); + strictEqual(code, 0); + strictEqual(signal, null); + }); + + it('should allow a leading `./`', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--experimental-default-type=module', + './wha?.js', + ], { + cwd: fixtures.path('es-modules/package-without-type'), + }); + + strictEqual(stderr, ''); + strictEqual(stdout, 'wha?\n'); + strictEqual(code, 0); + strictEqual(signal, null); + }); + + it('should not require a leading `./`', async () => { + const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ + '--experimental-default-type=module', + 'wha?.js', + ], { + cwd: fixtures.path('es-modules/package-without-type'), + }); + + strictEqual(stderr, ''); + strictEqual(stdout, 'wha?\n'); + strictEqual(code, 0); + strictEqual(signal, null); + }); +}); diff --git a/test/es-module/test-esm-type-flag-errors.mjs b/test/es-module/test-esm-type-flag-errors.mjs index ba316674b00354..6d54eff94763ef 100644 --- a/test/es-module/test-esm-type-flag-errors.mjs +++ b/test/es-module/test-esm-type-flag-errors.mjs @@ -3,36 +3,6 @@ import * as fixtures from '../common/fixtures.mjs'; import { describe, it } from 'node:test'; import { match, strictEqual } from 'node:assert'; -describe('--experimental-default-type=module should not support extension searching', { concurrency: true }, () => { - it('should support extension searching under --experimental-default-type=commonjs', async () => { - const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ - '--experimental-default-type=commonjs', - './index', - ], { - cwd: fixtures.path('es-modules/package-without-type'), - }); - - strictEqual(stdout, 'package-without-type\n'); - strictEqual(stderr, ''); - strictEqual(code, 0); - strictEqual(signal, null); - }); - - it('should error with implicit extension under --experimental-default-type=module', async () => { - const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ - '--experimental-default-type=module', - './index', - ], { - cwd: fixtures.path('es-modules/package-without-type'), - }); - - match(stderr, /ENOENT/); - strictEqual(stdout, ''); - strictEqual(code, 1); - strictEqual(signal, null); - }); -}); - describe('--experimental-default-type=module should not affect the interpretation of files with unknown extensions', { concurrency: true }, () => { it('should error on an entry point with an unknown extension', async () => { diff --git a/test/fixtures/es-modules/package-without-type/wha?.js b/test/fixtures/es-modules/package-without-type/wha?.js new file mode 100644 index 00000000000000..df02872289e013 --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/wha?.js @@ -0,0 +1 @@ +console.log('wha?'); From bdafa669acc1d9bac2d0d510d346bb80f68e34d9 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sun, 1 Oct 2023 17:14:44 -0700 Subject: [PATCH 05/11] Update docs --- doc/api/cli.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/doc/api/cli.md b/doc/api/cli.md index 2c4e26dc1e407b..8e41de3d8ebc82 100644 --- a/doc/api/cli.md +++ b/doc/api/cli.md @@ -25,14 +25,16 @@ For more info about `node inspect`, see the [debugger][] documentation. The program entry point is a specifier-like string. If the string is not an absolute path, it's resolved as a relative path from the current working -directory. That path is then resolved by [CommonJS][] module loader. If no -corresponding file is found, an error is thrown. +directory. That path is then resolved by [CommonJS][] module loader, or by the +[ES module loader][Modules loaders] if [`--experimental-default-type=module`][] +is passed. If no corresponding file is found, an error is thrown. If a file is found, its path will be passed to the [ES module loader][Modules loaders] under any of the following conditions: * The program was started with a command-line flag that forces the entry - point to be loaded with ECMAScript module loader. + point to be loaded with ECMAScript module loader, such as `--import` or + [`--experimental-default-type=module`][]. * The file has an `.mjs` extension. * The file does not have a `.cjs` extension, and the nearest parent `package.json` file contains a top-level [`"type"`][] field with a value of @@ -45,8 +47,9 @@ Otherwise, the file is loaded using the CommonJS module loader. See When loading, the [ES module loader][Modules loaders] loads the program entry point, the `node` command will accept as input only files with `.js`, -`.mjs`, or `.cjs` extensions; and with `.wasm` extensions when -[`--experimental-wasm-modules`][] is enabled. +`.mjs`, or `.cjs` extensions; with `.wasm` extensions when +[`--experimental-wasm-modules`][] is enabled; and with no extension when +[`--experimental-default-type=module`][] is passed. ## Options @@ -2741,6 +2744,7 @@ done [`--allow-worker`]: #--allow-worker [`--cpu-prof-dir`]: #--cpu-prof-dir [`--diagnostic-dir`]: #--diagnostic-dirdirectory +[`--experimental-default-type=module`]: #--experimental-default-typetype [`--experimental-sea-config`]: single-executable-applications.md#generating-single-executable-preparation-blobs [`--experimental-wasm-modules`]: #--experimental-wasm-modules [`--heap-prof-dir`]: #--heap-prof-dir From 83884e544012d8b4f61d274ce0f6d3fdbbb7e0b0 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sun, 1 Oct 2023 17:35:21 -0700 Subject: [PATCH 06/11] Fix for Windows --- test/es-module/test-esm-type-flag-cli-entry.mjs | 16 ++++++++-------- .../es-modules/package-without-type/file#1.js | 1 + .../es-modules/package-without-type/wha?.js | 1 - 3 files changed, 9 insertions(+), 9 deletions(-) create mode 100644 test/fixtures/es-modules/package-without-type/file#1.js delete mode 100644 test/fixtures/es-modules/package-without-type/wha?.js diff --git a/test/es-module/test-esm-type-flag-cli-entry.mjs b/test/es-module/test-esm-type-flag-cli-entry.mjs index 3e5e5a1efee117..002840751532ff 100644 --- a/test/es-module/test-esm-type-flag-cli-entry.mjs +++ b/test/es-module/test-esm-type-flag-cli-entry.mjs @@ -37,13 +37,13 @@ describe('--experimental-default-type=module should not parse paths as URLs', { it('should not parse a `?` in a filename as starting a query string', async () => { const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ '--experimental-default-type=module', - 'wha?.js', + 'file#1.js', ], { cwd: fixtures.path('es-modules/package-without-type'), }); strictEqual(stderr, ''); - strictEqual(stdout, 'wha?\n'); + strictEqual(stdout, 'file#1\n'); strictEqual(code, 0); strictEqual(signal, null); }); @@ -51,13 +51,13 @@ describe('--experimental-default-type=module should not parse paths as URLs', { it('should resolve `..`', async () => { const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ '--experimental-default-type=module', - '../package-without-type/wha?.js', + '../package-without-type/file#1.js', ], { cwd: fixtures.path('es-modules/package-without-type'), }); strictEqual(stderr, ''); - strictEqual(stdout, 'wha?\n'); + strictEqual(stdout, 'file#1\n'); strictEqual(code, 0); strictEqual(signal, null); }); @@ -65,13 +65,13 @@ describe('--experimental-default-type=module should not parse paths as URLs', { it('should allow a leading `./`', async () => { const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ '--experimental-default-type=module', - './wha?.js', + './file#1.js', ], { cwd: fixtures.path('es-modules/package-without-type'), }); strictEqual(stderr, ''); - strictEqual(stdout, 'wha?\n'); + strictEqual(stdout, 'file#1\n'); strictEqual(code, 0); strictEqual(signal, null); }); @@ -79,13 +79,13 @@ describe('--experimental-default-type=module should not parse paths as URLs', { it('should not require a leading `./`', async () => { const { code, signal, stdout, stderr } = await spawnPromisified(process.execPath, [ '--experimental-default-type=module', - 'wha?.js', + 'file#1.js', ], { cwd: fixtures.path('es-modules/package-without-type'), }); strictEqual(stderr, ''); - strictEqual(stdout, 'wha?\n'); + strictEqual(stdout, 'file#1\n'); strictEqual(code, 0); strictEqual(signal, null); }); diff --git a/test/fixtures/es-modules/package-without-type/file#1.js b/test/fixtures/es-modules/package-without-type/file#1.js new file mode 100644 index 00000000000000..6ab97dbf4b58cf --- /dev/null +++ b/test/fixtures/es-modules/package-without-type/file#1.js @@ -0,0 +1 @@ +console.log('file#1'); diff --git a/test/fixtures/es-modules/package-without-type/wha?.js b/test/fixtures/es-modules/package-without-type/wha?.js deleted file mode 100644 index df02872289e013..00000000000000 --- a/test/fixtures/es-modules/package-without-type/wha?.js +++ /dev/null @@ -1 +0,0 @@ -console.log('wha?'); From 162b6fa219552575ccca45ac4d8fc79beb750009 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Wed, 4 Oct 2023 08:29:56 -0700 Subject: [PATCH 07/11] Move out main re-resolution --- lib/internal/modules/run_main.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index fda69f4b9caf59..0a50813499d3c1 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -79,10 +79,9 @@ function shouldUseESMLoader(mainPath) { function runMainESM(mainPath) { const { loadESM } = require('internal/process/esm_loader'); const { pathToFileURL } = require('internal/url'); + const main = pathToFileURL(mainPath).href; handleMainPromise(loadESM((esmLoader) => { - const main = path.isAbsolute(mainPath) ? - pathToFileURL(mainPath).href : mainPath; return esmLoader.import(main, undefined, { __proto__: null }); })); } From 22bddbdd0c59ba8e39fa9decfa3f4759bddd01f8 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Wed, 4 Oct 2023 20:07:33 -0700 Subject: [PATCH 08/11] Optimize --- lib/internal/modules/run_main.js | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index 0a50813499d3c1..d130522af2d797 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -12,6 +12,8 @@ const path = require('path'); * @param {string} main - Entry point path */ function resolveMainPath(main) { + if (getOptionValue('--preserve-symlinks-main')) { return; } + const defaultType = getOptionValue('--experimental-default-type'); /** @type {string} */ let mainPath; @@ -25,21 +27,18 @@ function resolveMainPath(main) { } if (!mainPath) { return; } - const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); - if (!preserveSymlinksMain) { - const { toRealPath } = require('internal/modules/helpers'); - try { - mainPath = toRealPath(mainPath); - } catch (err) { - if (err.code === 'ENOENT' && defaultType === 'module') { - const { decorateErrorWithCommonJSHints } = require('internal/modules/esm/resolve'); - const { getCWDURL } = require('internal/util'); - decorateErrorWithCommonJSHints(err, mainPath, getCWDURL()); - } - throw err; + // Follow symlinks + const { toRealPath } = require('internal/modules/helpers'); + try { + mainPath = toRealPath(mainPath); + } catch (err) { + if (err.code === 'ENOENT' && defaultType === 'module') { + const { decorateErrorWithCommonJSHints } = require('internal/modules/esm/resolve'); + const { getCWDURL } = require('internal/util'); + decorateErrorWithCommonJSHints(err, mainPath, getCWDURL()); } + throw err; } - return mainPath; } From 6c97b9aed371abe6cbb0fbd6153b471305b83143 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 5 Oct 2023 07:41:12 -0700 Subject: [PATCH 09/11] Revert "Optimize" This reverts commit 6d1a6e51118d1c4bd45cb21fad337ac2f87c8aa1. --- lib/internal/modules/run_main.js | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index d130522af2d797..0a50813499d3c1 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -12,8 +12,6 @@ const path = require('path'); * @param {string} main - Entry point path */ function resolveMainPath(main) { - if (getOptionValue('--preserve-symlinks-main')) { return; } - const defaultType = getOptionValue('--experimental-default-type'); /** @type {string} */ let mainPath; @@ -27,18 +25,21 @@ function resolveMainPath(main) { } if (!mainPath) { return; } - // Follow symlinks - const { toRealPath } = require('internal/modules/helpers'); - try { - mainPath = toRealPath(mainPath); - } catch (err) { - if (err.code === 'ENOENT' && defaultType === 'module') { - const { decorateErrorWithCommonJSHints } = require('internal/modules/esm/resolve'); - const { getCWDURL } = require('internal/util'); - decorateErrorWithCommonJSHints(err, mainPath, getCWDURL()); + const preserveSymlinksMain = getOptionValue('--preserve-symlinks-main'); + if (!preserveSymlinksMain) { + const { toRealPath } = require('internal/modules/helpers'); + try { + mainPath = toRealPath(mainPath); + } catch (err) { + if (err.code === 'ENOENT' && defaultType === 'module') { + const { decorateErrorWithCommonJSHints } = require('internal/modules/esm/resolve'); + const { getCWDURL } = require('internal/util'); + decorateErrorWithCommonJSHints(err, mainPath, getCWDURL()); + } + throw err; } - throw err; } + return mainPath; } From 32aaabc824e07062676b1adbbc228a49800bc886 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 5 Oct 2023 07:41:45 -0700 Subject: [PATCH 10/11] Update lib/internal/modules/run_main.js Co-authored-by: Antoine du Hamel --- lib/internal/modules/run_main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index 0a50813499d3c1..2b4314099ae382 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -31,7 +31,7 @@ function resolveMainPath(main) { try { mainPath = toRealPath(mainPath); } catch (err) { - if (err.code === 'ENOENT' && defaultType === 'module') { + if (defaultType === 'module' && err?.code === 'ENOENT') { const { decorateErrorWithCommonJSHints } = require('internal/modules/esm/resolve'); const { getCWDURL } = require('internal/util'); decorateErrorWithCommonJSHints(err, mainPath, getCWDURL()); From 038dfb82281436eb5d59bca42496bae1b64d741c Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 5 Oct 2023 08:02:58 -0700 Subject: [PATCH 11/11] Return early if possible --- lib/internal/modules/run_main.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index 2b4314099ae382..a9828286a9c0e0 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -16,6 +16,7 @@ function resolveMainPath(main) { /** @type {string} */ let mainPath; if (defaultType === 'module') { + if (getOptionValue('--preserve-symlinks-main')) { return; } mainPath = path.resolve(main); } else { // Extension searching for the main entry point is supported only in legacy mode.