Skip to content

Commit

Permalink
module: eliminate performance cost of detection for cjs entry
Browse files Browse the repository at this point in the history
PR-URL: nodejs#52093
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>
  • Loading branch information
GeoffreyBooth authored and rdw-msft committed Mar 26, 2024
1 parent cd30a9f commit 75bea2d
Show file tree
Hide file tree
Showing 6 changed files with 192 additions and 97 deletions.
1 change: 1 addition & 0 deletions benchmark/misc/startup-core.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const bench = common.createBenchmark(main, {
script: [
'benchmark/fixtures/require-builtins',
'test/fixtures/semicolon',
'test/fixtures/snapshot/typescript',
],
mode: ['process', 'worker'],
count: [30],
Expand Down
12 changes: 10 additions & 2 deletions lib/internal/modules/cjs/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ module.exports = {
cjsExportsCache,
cjsSourceCache,
initializeCJS,
entryPointSource: undefined, // Set below.
Module,
wrapSafe,
};
Expand Down Expand Up @@ -1337,8 +1338,15 @@ function wrapSafe(filename, content, cjsModuleInstance, codeCache) {
return result;
} catch (err) {
if (process.mainModule === cjsModuleInstance) {
const { enrichCJSError } = require('internal/modules/esm/translators');
enrichCJSError(err, content, filename);
if (getOptionValue('--experimental-detect-module')) {
// For the main entry point, cache the source to potentially retry as ESM.
module.exports.entryPointSource = content;
} else {
// We only enrich the error (print a warning) if we're sure we're going to for-sure throw it; so if we're
// retrying as ESM, wait until we know whether we're going to retry before calling `enrichCJSError`.
const { enrichCJSError } = require('internal/modules/esm/translators');
enrichCJSError(err, content, filename);
}
}
throw err;
}
Expand Down
35 changes: 35 additions & 0 deletions lib/internal/modules/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,15 @@ const {
} = require('internal/errors').codes;
const { BuiltinModule } = require('internal/bootstrap/realm');

const {
shouldRetryAsESM: contextifyShouldRetryAsESM,
constants: {
syntaxDetectionErrors: {
esmSyntaxErrorMessages,
throwsOnlyInCommonJSErrorMessages,
},
},
} = internalBinding('contextify');
const { validateString } = require('internal/validators');
const fs = require('fs'); // Import all of `fs` so that it can be monkey-patched.
const internalFS = require('internal/fs/utils');
Expand Down Expand Up @@ -320,6 +329,31 @@ function normalizeReferrerURL(referrerName) {
}


let esmSyntaxErrorMessagesSet; // Declared lazily in shouldRetryAsESM
let throwsOnlyInCommonJSErrorMessagesSet; // Declared lazily in shouldRetryAsESM
/**
* After an attempt to parse a module as CommonJS throws an error, should we try again as ESM?
* 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
* throws on an error that would not have been a syntax error in ESM (like via top-level `await` or a lexical
* redeclaration of one of the CommonJS variables) then we need to parse again to see if it would have thrown in ESM.
* @param {string} errorMessage The string message thrown by V8 when attempting to parse as CommonJS
* @param {string} source Module contents
*/
function shouldRetryAsESM(errorMessage, source) {
esmSyntaxErrorMessagesSet ??= new SafeSet(esmSyntaxErrorMessages);
if (esmSyntaxErrorMessagesSet.has(errorMessage)) {
return true;
}

throwsOnlyInCommonJSErrorMessagesSet ??= new SafeSet(throwsOnlyInCommonJSErrorMessages);
if (throwsOnlyInCommonJSErrorMessagesSet.has(errorMessage)) {
return /** @type {boolean} */(contextifyShouldRetryAsESM(source));
}

return false;
}


// Whether we have started executing any user-provided CJS code.
// This is set right before we call the wrapped CJS code (not after,
// in case we are half-way in the execution when internals check this).
Expand All @@ -339,6 +373,7 @@ module.exports = {
loadBuiltinModule,
makeRequireFunction,
normalizeReferrerURL,
shouldRetryAsESM,
stripBOM,
toRealPath,
hasStartedUserCJSExecution() {
Expand Down
42 changes: 32 additions & 10 deletions lib/internal/modules/run_main.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ const {
globalThis,
} = primordials;

const { containsModuleSyntax } = internalBinding('contextify');
const { getNearestParentPackageJSONType } = internalBinding('modules');
const { getOptionValue } = require('internal/options');
const { checkPackageJSONIntegrity } = require('internal/modules/package_json_reader');
Expand Down Expand Up @@ -87,10 +86,6 @@ function shouldUseESMLoader(mainPath) {

// No package.json or no `type` field.
if (response === undefined || response[0] === 'none') {
if (getOptionValue('--experimental-detect-module')) {
// If the first argument of `containsModuleSyntax` is undefined, it will read `mainPath` from the file system.
return containsModuleSyntax(undefined, mainPath);
}
return false;
}

Expand Down Expand Up @@ -157,12 +152,43 @@ function runEntryPointWithESMLoader(callback) {
* 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'`.
* When `--experimental-detect-module` is passed, this function will attempt to run ambiguous (no explicit extension, no
* `package.json` type field) entry points as CommonJS first; under certain conditions, it will retry running as ESM.
* @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);
const useESMLoader = shouldUseESMLoader(resolvedMain);
if (useESMLoader) {

// Unless we know we should use the ESM loader to handle the entry point per the checks in `shouldUseESMLoader`, first
// try to run the entry point via the CommonJS loader; and if that fails under certain conditions, retry as ESM.
let retryAsESM = false;
if (!useESMLoader) {
const cjsLoader = require('internal/modules/cjs/loader');
const { Module } = cjsLoader;
if (getOptionValue('--experimental-detect-module')) {
try {
// Module._load is the monkey-patchable CJS module loader.
Module._load(main, null, true);
} catch (error) {
const source = cjsLoader.entryPointSource;
const { shouldRetryAsESM } = require('internal/modules/helpers');
retryAsESM = shouldRetryAsESM(error.message, source);
// In case the entry point is a large file, such as a bundle,
// ensure no further references can prevent it being garbage-collected.
cjsLoader.entryPointSource = undefined;
if (!retryAsESM) {
const { enrichCJSError } = require('internal/modules/esm/translators');
enrichCJSError(error, source, resolvedMain);
throw error;
}
}
} else { // `--experimental-detect-module` is not passed
Module._load(main, null, true);
}
}

if (useESMLoader || retryAsESM) {
const mainPath = resolvedMain || main;
const mainURL = pathToFileURL(mainPath).href;

Expand All @@ -171,10 +197,6 @@ function executeUserEntryPoint(main = process.argv[1]) {
// even after the event loop stops running.
return cascadedLoader.import(mainURL, undefined, { __proto__: null }, true);
});
} else {
// Module._load is the monkey-patchable CJS module loader.
const { Module } = require('internal/modules/cjs/loader');
Module._load(main, null, true);
}
}

Expand Down
Loading

0 comments on commit 75bea2d

Please sign in to comment.