Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

esm: bypass CommonJS loader under --default-type=module #49986

Merged
merged 11 commits into from
Oct 5, 2023
14 changes: 9 additions & 5 deletions doc/api/cli.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down
22 changes: 14 additions & 8 deletions lib/internal/main/run_main_module.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
33 changes: 22 additions & 11 deletions lib/internal/modules/esm/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand All @@ -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');
Comment on lines +1161 to +1164
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
error.stack =
ArrayPrototypeShift(lines) + '\n' +
hint + '\n' +
ArrayPrototypeJoin(lines, '\n');
error.stack = `${ArrayPrototypeShift(lines)}\n${hint}\n${ArrayPrototypeJoin(lines, '\n')}`;

error.message += `\n${hint}`;
}
}

module.exports = {
decorateErrorWithCommonJSHints,
defaultResolve,
encodedSepRegEx,
getPackageScopeConfig,
Expand Down
36 changes: 27 additions & 9 deletions lib/internal/modules/run_main.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
GeoffreyBooth marked this conversation as resolved.
Show resolved Hide resolved
} 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;
Expand All @@ -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).
Expand Down Expand Up @@ -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 });
}));
}
Expand All @@ -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);
Expand Down
15 changes: 11 additions & 4 deletions lib/internal/process/pre_execution.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const {
} = require('internal/v8/startup_snapshot');

function prepareMainThreadExecution(expandArgv1 = false, initializeModules = true) {
prepareExecution({
return prepareExecution({
expandArgv1,
initializeModules,
isMainThread: true,
Expand All @@ -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();
Expand Down Expand Up @@ -131,6 +131,8 @@ function prepareExecution(options) {
if (initializeModules) {
setupUserModules();
}

return mainEntry;
}

function setupSymbolDisposePolyfill() {
Expand Down Expand Up @@ -202,14 +204,17 @@ 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] &&
!StringPrototypeStartsWith(process.argv[1], '-')) {
// 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.
}
Expand All @@ -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) {
Expand Down
92 changes: 92 additions & 0 deletions test/es-module/test-esm-type-flag-cli-entry.mjs
Original file line number Diff line number Diff line change
@@ -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);
});
});
1 change: 1 addition & 0 deletions test/fixtures/es-modules/package-without-type/file#1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log('file#1');