Skip to content

Commit 132a5c1

Browse files
GeoffreyBoothmarco-ippolito
authored andcommitted
module: eliminate performance cost of detection for cjs entry
PR-URL: #52093 Backport-PR-URL: #56927 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: Jacob Smith <[email protected]> Reviewed-By: Richard Lau <[email protected]> Refs: #52697
1 parent 697a392 commit 132a5c1

File tree

6 files changed

+192
-97
lines changed

6 files changed

+192
-97
lines changed

‎benchmark/misc/startup-core.js‎

Lines 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],

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

Lines 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
returnresult;
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
throwerr;
13991407
}

‎lib/internal/modules/helpers.js‎

Lines 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
constfs=require('fs');// Import all of `fs` so that it can be monkey-patched.
2433
constinternalFS=require('internal/fs/utils');
@@ -329,6 +338,31 @@ function urlToFilename(url){
329338
returnurl;
330339
}
331340

341+
letesmSyntaxErrorMessagesSet;// Declared lazily in shouldRetryAsESM
342+
letthrowsOnlyInCommonJSErrorMessagesSet;// 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+
functionshouldRetryAsESM(errorMessage,source){
352+
esmSyntaxErrorMessagesSet??=newSafeSet(esmSyntaxErrorMessages);
353+
if(esmSyntaxErrorMessagesSet.has(errorMessage)){
354+
returntrue;
355+
}
356+
357+
throwsOnlyInCommonJSErrorMessagesSet??=newSafeSet(throwsOnlyInCommonJSErrorMessages);
358+
if(throwsOnlyInCommonJSErrorMessagesSet.has(errorMessage)){
359+
return/** @type{boolean} */(contextifyShouldRetryAsESM(source));
360+
}
361+
362+
returnfalse;
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(){

‎lib/internal/modules/run_main.js‎

Lines 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
constpath=require('path');
109
const{ pathToFileURL }=require('internal/url');
@@ -85,10 +84,6 @@ function shouldUseESMLoader(mainPath){
8584
case'commonjs':
8685
returnfalse;
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-
returncontainsModuleSyntax(undefined,mainPath);
91-
}
9287
returnfalse;
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
functionexecuteUserEntryPoint(main=process.argv[1]){
159156
constresolvedMain=resolveMainPath(main);
160157
constuseESMLoader=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+
letretryAsESM=false;
162+
if(!useESMLoader){
163+
constcjsLoader=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+
constsource=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+
throwerror;
180+
}
181+
}
182+
}else{// `--experimental-detect-module` is not passed
183+
Module._load(main,null,true);
184+
}
185+
}
186+
187+
if(useESMLoader||retryAsESM){
162188
constmainPath=resolvedMain||main;
163189
constmainURL=pathToFileURL(mainPath).href;
164190

@@ -167,10 +193,6 @@ function executeUserEntryPoint(main = process.argv[1]){
167193
// even after the event loop stops running.
168194
returncascadedLoader.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)