From 19b470f8669542dadd8307ca0334c789161e4e72 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 5 Oct 2023 10:57:32 -0700 Subject: [PATCH] esm: bypass CommonJS loader under --default-type PR-URL: https://github.com/nodejs/node/pull/49986 Reviewed-By: Jacob Smith Reviewed-By: Antoine du Hamel --- doc/api/cli.md | 14 ++- lib/internal/main/run_main_module.js | 22 +++-- lib/internal/modules/esm/resolve.js | 33 ++++--- lib/internal/modules/run_main.js | 36 ++++++-- lib/internal/process/pre_execution.js | 15 ++- .../test-esm-type-flag-cli-entry.mjs | 92 +++++++++++++++++++ .../es-modules/package-without-type/file#1.js | 1 + 7 files changed, 176 insertions(+), 37 deletions(-) create mode 100644 test/es-module/test-esm-type-flag-cli-entry.mjs create mode 100644 test/fixtures/es-modules/package-without-type/file#1.js 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 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/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 0b58d348203bfe..a9828286a9c0e0 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -12,17 +12,33 @@ 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); + const defaultType = getOptionValue('--experimental-default-type'); + /** @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. + // 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'); if (!preserveSymlinksMain) { const { toRealPath } = require('internal/modules/helpers'); - mainPath = toRealPath(mainPath); + try { + mainPath = toRealPath(mainPath); + } catch (err) { + if (defaultType === 'module' && err?.code === 'ENOENT') { + const { decorateErrorWithCommonJSHints } = require('internal/modules/esm/resolve'); + const { getCWDURL } = require('internal/util'); + decorateErrorWithCommonJSHints(err, mainPath, getCWDURL()); + } + throw err; + } } return mainPath; @@ -33,6 +49,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). @@ -62,10 +80,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 }); })); } @@ -90,8 +107,9 @@ 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 + * @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 d066d12a553d6f..917ba90a1c8bbb 100644 --- a/lib/internal/process/pre_execution.js +++ b/lib/internal/process/pre_execution.js @@ -52,7 +52,7 @@ const { } = require('internal/v8/startup_snapshot'); function prepareMainThreadExecution(expandArgv1 = false, initializeModules = true) { - prepareExecution({ + return prepareExecution({ expandArgv1, initializeModules, isMainThread: true, @@ -73,8 +73,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 +131,8 @@ function prepareExecution(options) { if (initializeModules) { setupUserModules(); } + + return mainEntry; } function setupSymbolDisposePolyfill() { @@ -202,6 +204,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 +213,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 +241,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-cli-entry.mjs b/test/es-module/test-esm-type-flag-cli-entry.mjs new file mode 100644 index 00000000000000..002840751532ff --- /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', + 'file#1.js', + ], { + cwd: fixtures.path('es-modules/package-without-type'), + }); + + strictEqual(stderr, ''); + strictEqual(stdout, 'file#1\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/file#1.js', + ], { + cwd: fixtures.path('es-modules/package-without-type'), + }); + + strictEqual(stderr, ''); + strictEqual(stdout, 'file#1\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', + './file#1.js', + ], { + cwd: fixtures.path('es-modules/package-without-type'), + }); + + strictEqual(stderr, ''); + strictEqual(stdout, 'file#1\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', + 'file#1.js', + ], { + cwd: fixtures.path('es-modules/package-without-type'), + }); + + strictEqual(stderr, ''); + 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');