Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 132a5c1

Browse filesBrowse files
GeoffreyBoothmarco-ippolito
GeoffreyBooth
authored and
marco-ippolito
committed
module: eliminate performance cost of detection for cjs entry
PR-URL: #52093 Backport-PR-URL: #56927 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Richard Lau <rlau@redhat.com> Refs: #52697
1 parent 697a392 commit 132a5c1
Copy full SHA for 132a5c1

File tree

Expand file treeCollapse file tree

6 files changed

+192
-97
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

6 files changed

+192
-97
lines changed
Open diff view settings
Collapse file

‎benchmark/misc/startup-core.js‎

Copy file name to clipboardExpand all lines: benchmark/misc/startup-core.js
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const bench = common.createBenchmark(main, {
99
script: [
1010
'benchmark/fixtures/require-builtins',
1111
'test/fixtures/semicolon',
12+
'test/fixtures/snapshot/typescript',
1213
],
1314
mode: ['process', 'worker'],
1415
n: [30],
Collapse file

‎lib/internal/modules/cjs/loader.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/cjs/loader.js
+10-2Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,7 @@ module.exports = {
106106
kModuleExportNames,
107107
kModuleCircularVisited,
108108
initializeCJS,
109+
entryPointSource: undefined, // Set below.
109110
Module,
110111
wrapSafe,
111112
kIsMainSymbol,
@@ -1392,8 +1393,15 @@ function wrapSafe(filename, content, cjsModuleInstance, codeCache) {
13921393
return result;
13931394
} catch (err) {
13941395
if (process.mainModule === cjsModuleInstance) {
1395-
const { enrichCJSError } = require('internal/modules/esm/translators');
1396-
enrichCJSError(err, content, filename);
1396+
if (getOptionValue('--experimental-detect-module')) {
1397+
// For the main entry point, cache the source to potentially retry as ESM.
1398+
module.exports.entryPointSource = content;
1399+
} else {
1400+
// We only enrich the error (print a warning) if we're sure we're going to for-sure throw it; so if we're
1401+
// retrying as ESM, wait until we know whether we're going to retry before calling `enrichCJSError`.
1402+
const { enrichCJSError } = require('internal/modules/esm/translators');
1403+
enrichCJSError(err, content, filename);
1404+
}
13971405
}
13981406
throw err;
13991407
}
Collapse file

‎lib/internal/modules/helpers.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/helpers.js
+35Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,15 @@ const {
1919
} = require('internal/errors').codes;
2020
const { BuiltinModule } = require('internal/bootstrap/realm');
2121

22+
const {
23+
shouldRetryAsESM: contextifyShouldRetryAsESM,
24+
constants: {
25+
syntaxDetectionErrors: {
26+
esmSyntaxErrorMessages,
27+
throwsOnlyInCommonJSErrorMessages,
28+
},
29+
},
30+
} = internalBinding('contextify');
2231
const { validateString } = require('internal/validators');
2332
const fs = require('fs'); // Import all of `fs` so that it can be monkey-patched.
2433
const internalFS = require('internal/fs/utils');
@@ -329,6 +338,31 @@ function urlToFilename(url) {
329338
return url;
330339
}
331340

341+
let esmSyntaxErrorMessagesSet; // Declared lazily in shouldRetryAsESM
342+
let throwsOnlyInCommonJSErrorMessagesSet; // Declared lazily in shouldRetryAsESM
343+
/**
344+
* After an attempt to parse a module as CommonJS throws an error, should we try again as ESM?
345+
* We only want to try again as ESM if the error is due to syntax that is only valid in ESM; and if the CommonJS parse
346+
* throws on an error that would not have been a syntax error in ESM (like via top-level `await` or a lexical
347+
* redeclaration of one of the CommonJS variables) then we need to parse again to see if it would have thrown in ESM.
348+
* @param {string} errorMessage The string message thrown by V8 when attempting to parse as CommonJS
349+
* @param {string} source Module contents
350+
*/
351+
function shouldRetryAsESM(errorMessage, source) {
352+
esmSyntaxErrorMessagesSet ??= new SafeSet(esmSyntaxErrorMessages);
353+
if (esmSyntaxErrorMessagesSet.has(errorMessage)) {
354+
return true;
355+
}
356+
357+
throwsOnlyInCommonJSErrorMessagesSet ??= new SafeSet(throwsOnlyInCommonJSErrorMessages);
358+
if (throwsOnlyInCommonJSErrorMessagesSet.has(errorMessage)) {
359+
return /** @type {boolean} */(contextifyShouldRetryAsESM(source));
360+
}
361+
362+
return false;
363+
}
364+
365+
332366
// Whether we have started executing any user-provided CJS code.
333367
// This is set right before we call the wrapped CJS code (not after,
334368
// in case we are half-way in the execution when internals check this).
@@ -362,6 +396,7 @@ module.exports = {
362396
loadBuiltinModule,
363397
makeRequireFunction,
364398
normalizeReferrerURL,
399+
shouldRetryAsESM,
365400
stripBOM,
366401
toRealPath,
367402
hasStartedUserCJSExecution() {
Collapse file

‎lib/internal/modules/run_main.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/run_main.js
+32-10Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ const {
44
StringPrototypeEndsWith,
55
} = primordials;
66

7-
const { containsModuleSyntax } = internalBinding('contextify');
87
const { getOptionValue } = require('internal/options');
98
const path = require('path');
109
const { pathToFileURL } = require('internal/url');
@@ -85,10 +84,6 @@ function shouldUseESMLoader(mainPath) {
8584
case 'commonjs':
8685
return false;
8786
default: { // No package.json or no `type` field.
88-
if (getOptionValue('--experimental-detect-module')) {
89-
// If the first argument of `containsModuleSyntax` is undefined, it will read `mainPath` from the file system.
90-
return containsModuleSyntax(undefined, mainPath);
91-
}
9287
return false;
9388
}
9489
}
@@ -153,12 +148,43 @@ function runEntryPointWithESMLoader(callback) {
153148
* by `require('module')`) even when the entry point is ESM.
154149
* This monkey-patchable code is bypassed under `--experimental-default-type=module`.
155150
* Because of backwards compatibility, this function is exposed publicly via `import { runMain } from 'node:module'`.
151+
* When `--experimental-detect-module` is passed, this function will attempt to run ambiguous (no explicit extension, no
152+
* `package.json` type field) entry points as CommonJS first; under certain conditions, it will retry running as ESM.
156153
* @param {string} main - First positional CLI argument, such as `'entry.js'` from `node entry.js`
157154
*/
158155
function executeUserEntryPoint(main = process.argv[1]) {
159156
const resolvedMain = resolveMainPath(main);
160157
const useESMLoader = shouldUseESMLoader(resolvedMain);
161-
if (useESMLoader) {
158+
159+
// Unless we know we should use the ESM loader to handle the entry point per the checks in `shouldUseESMLoader`, first
160+
// try to run the entry point via the CommonJS loader; and if that fails under certain conditions, retry as ESM.
161+
let retryAsESM = false;
162+
if (!useESMLoader) {
163+
const cjsLoader = require('internal/modules/cjs/loader');
164+
const { Module } = cjsLoader;
165+
if (getOptionValue('--experimental-detect-module')) {
166+
try {
167+
// Module._load is the monkey-patchable CJS module loader.
168+
Module._load(main, null, true);
169+
} catch (error) {
170+
const source = cjsLoader.entryPointSource;
171+
const { shouldRetryAsESM } = require('internal/modules/helpers');
172+
retryAsESM = shouldRetryAsESM(error.message, source);
173+
// In case the entry point is a large file, such as a bundle,
174+
// ensure no further references can prevent it being garbage-collected.
175+
cjsLoader.entryPointSource = undefined;
176+
if (!retryAsESM) {
177+
const { enrichCJSError } = require('internal/modules/esm/translators');
178+
enrichCJSError(error, source, resolvedMain);
179+
throw error;
180+
}
181+
}
182+
} else { // `--experimental-detect-module` is not passed
183+
Module._load(main, null, true);
184+
}
185+
}
186+
187+
if (useESMLoader || retryAsESM) {
162188
const mainPath = resolvedMain || main;
163189
const mainURL = pathToFileURL(mainPath).href;
164190

@@ -167,10 +193,6 @@ function executeUserEntryPoint(main = process.argv[1]) {
167193
// even after the event loop stops running.
168194
return cascadedLoader.import(mainURL, undefined, { __proto__: null }, true);
169195
});
170-
} else {
171-
// Module._load is the monkey-patchable CJS module loader.
172-
const { Module } = require('internal/modules/cjs/loader');
173-
Module._load(main, null, true);
174196
}
175197
}
176198

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.