Skip to content

Commit

Permalink
esm: bypass CommonJS loader under --default-type
Browse files Browse the repository at this point in the history
PR-URL: nodejs#49986
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
  • Loading branch information
GeoffreyBooth authored and targos committed Nov 11, 2023
1 parent 08201aa commit 151e812
Show file tree
Hide file tree
Showing 8 changed files with 179 additions and 41 deletions.
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 @@ -2417,6 +2420,7 @@ done
[`"type"`]: packages.md#type
[`--cpu-prof-dir`]: #--cpu-prof-dir
[`--diagnostic-dir`]: #--diagnostic-dirdirectory
[`--experimental-default-type=module`]: #--experimental-default-typetype
[`--experimental-wasm-modules`]: #--experimental-wasm-modules
[`--heap-prof-dir`]: #--heap-prof-dir
[`--import`]: #--importmodule
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 @@ -1204,17 +1204,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 @@ -1227,7 +1217,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,
Expand Down
39 changes: 28 additions & 11 deletions lib/internal/modules/run_main.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
'use strict';

const {
ObjectCreate,
StringPrototypeEndsWith,
} = primordials;

Expand All @@ -13,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;
Expand All @@ -34,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 @@ -69,11 +86,10 @@ 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, ObjectCreate(null));
return esmLoader.import(main, undefined, { __proto__: null });
}));
}

Expand All @@ -97,8 +113,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 @@ -37,7 +37,7 @@ const {
} = require('internal/v8/startup_snapshot');

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

return mainEntry;
}

function setupSymbolDisposePolyfill() {
Expand Down Expand Up @@ -167,14 +169,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 @@ -201,6 +206,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);
});
});
4 changes: 2 additions & 2 deletions test/fixtures/errors/force_colors.snapshot
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Error: Should include grayed stack trace
 at Module._extensions..js (node:internal*modules*cjs*loader:1414:10)
 at Module.load (node:internal*modules*cjs*loader:1197:32)
 at Module._load (node:internal*modules*cjs*loader:1013:12)
 at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main:109:12)
 at node:internal*main*run_main_module:23:47
 at Function.executeUserEntryPoint [as runMain] (node:internal*modules*run_main:128:12)
 at node:internal*main*run_main_module:28:49

Node.js *
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');

0 comments on commit 151e812

Please sign in to comment.