From 6d13ee41dab00acef353ad394e868eb448996a88 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Tue, 31 Mar 2026 17:17:51 -0700 Subject: [PATCH 1/5] module: synchronously load most ES modules --- benchmark/esm/startup-esm-graph.js | 59 +++++++++++++++++++ lib/internal/modules/esm/loader.js | 40 +++++++++++-- lib/internal/modules/esm/module_job.js | 51 +++++++++++----- lib/internal/modules/run_main.js | 15 +++++ test/es-module/test-esm-sync-import.mjs | 54 +++++++++++++++++ test/fixtures/es-modules/promise-counter.cjs | 11 ++++ .../fixtures/es-modules/require-esm-entry.cjs | 1 + 7 files changed, 211 insertions(+), 20 deletions(-) create mode 100644 benchmark/esm/startup-esm-graph.js create mode 100644 test/es-module/test-esm-sync-import.mjs create mode 100644 test/fixtures/es-modules/promise-counter.cjs create mode 100644 test/fixtures/es-modules/require-esm-entry.cjs diff --git a/benchmark/esm/startup-esm-graph.js b/benchmark/esm/startup-esm-graph.js new file mode 100644 index 00000000000000..ab1481bdb5a689 --- /dev/null +++ b/benchmark/esm/startup-esm-graph.js @@ -0,0 +1,59 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const path = require('path'); +const { spawnSync } = require('child_process'); +const tmpdir = require('../../test/common/tmpdir'); + +const bench = common.createBenchmark(main, { + modules: [250, 500, 1000, 2000], + n: [30], +}); + +function prepare(count) { + tmpdir.refresh(); + const dir = tmpdir.resolve('esm-graph'); + fs.mkdirSync(dir, { recursive: true }); + + // Create a flat ESM graph: entry imports all modules directly. + // Each module is independent, maximizing the number of resolve/load/link + // operations in the loader pipeline. + const imports = []; + for (let i = 0; i < count; i++) { + fs.writeFileSync( + path.join(dir, `mod${i}.mjs`), + `export const value${i} = ${i};\n`, + ); + imports.push(`import './mod${i}.mjs';`); + } + + const entry = path.join(dir, 'entry.mjs'); + fs.writeFileSync(entry, imports.join('\n') + '\n'); + return entry; +} + +function main({ n, modules }) { + const entry = prepare(modules); + const cmd = process.execPath || process.argv[0]; + const warmup = 3; + const state = { finished: -warmup }; + + while (state.finished < n) { + const child = spawnSync(cmd, [entry]); + if (child.status !== 0) { + console.log('---- STDOUT ----'); + console.log(child.stdout.toString()); + console.log('---- STDERR ----'); + console.log(child.stderr.toString()); + throw new Error(`Child process stopped with exit code ${child.status}`); + } + state.finished++; + if (state.finished === 0) { + bench.start(); + } + if (state.finished === n) { + bench.end(n); + } + } +} diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 8ae6761fba571a..ed1e44bcf21377 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -369,6 +369,32 @@ class ModuleLoader { return { wrap: job.module, namespace: job.runSync(parent).namespace }; } + /** + * Synchronously load and evaluate the entry point module. + * This avoids creating any promises when no TLA is present + * and no async customization hooks are registered. + * @param {string} url The URL of the entry point module. + * @returns {{ module: ModuleWrap, completed: boolean }} The entry module and whether + * evaluation completed synchronously. When false, the caller should fall back to + * async evaluation (TLA detected). + */ + importSyncForEntryPoint(url) { + return onImport.traceSync(() => { + const request = { specifier: url, phase: kEvaluationPhase, attributes: kEmptyObject, __proto__: null }; + const job = this.getOrCreateModuleJob(undefined, request, kImportInImportedESM); + job.module.instantiate(); + if (job.module.hasAsyncGraph) { + return { __proto__: null, module: job.module, completed: false }; + } + job.runSync(); + return { __proto__: null, module: job.module, completed: true }; + }, { + __proto__: null, + parentURL: undefined, + url, + }); + } + /** * Check invariants on a cached module job when require()'d from ESM. * @param {string} specifier The first parameter of require(). @@ -564,10 +590,15 @@ class ModuleLoader { } const { ModuleJob, ModuleJobSync } = require('internal/modules/esm/module_job'); - // TODO(joyeecheung): use ModuleJobSync for kRequireInImportedCJS too. - const ModuleJobCtor = (requestType === kImportInRequiredESM ? ModuleJobSync : ModuleJob); const isMain = (parentURL === undefined); const inspectBrk = (isMain && getOptionValue('--inspect-brk')); + // Use ModuleJobSync whenever we're on the main thread (not the async loader hook worker), + // except for kRequireInImportedCJS (TODO: consolidate that case too) and --inspect-brk + // (which needs the async ModuleJob to pause on the first line). + // TODO(joyeecheung): use ModuleJobSync for kRequireInImportedCJS too. + const ModuleJobCtor = (!this.isForAsyncLoaderHookWorker && !inspectBrk && + requestType !== kRequireInImportedCJS) ? + ModuleJobSync : ModuleJob; job = new ModuleJobCtor( this, url, @@ -594,8 +625,9 @@ class ModuleLoader { */ getOrCreateModuleJob(parentURL, request, requestType) { let maybePromise; - if (requestType === kRequireInImportedCJS || requestType === kImportInRequiredESM) { - // In these two cases, resolution must be synchronous. + if (!this.isForAsyncLoaderHookWorker) { + // On the main thread, always resolve synchronously; + // `resolveSync` coordinates with the async loader hook worker if needed. maybePromise = this.resolveSync(parentURL, request); assert(!isPromise(maybePromise)); } else { diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 22032f79e90d44..4a0297ece8074f 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -46,7 +46,7 @@ const { getSourceMapsSupport, } = require('internal/source_map/source_map_cache'); const assert = require('internal/assert'); -const resolvedPromise = PromiseResolve(); +let resolvedPromise; const { setHasStartedUserESMExecution, urlToFilename, @@ -374,7 +374,7 @@ class ModuleJob extends ModuleJobBase { for (const dependencyJob of jobsInGraph) { // Calling `this.module.instantiate()` instantiates not only the // ModuleWrap in this module, but all modules in the graph. - dependencyJob.instantiated = resolvedPromise; + dependencyJob.instantiated = resolvedPromise ??= PromiseResolve(); } } @@ -445,12 +445,14 @@ class ModuleJob extends ModuleJobBase { /** * This is a fully synchronous job and does not spawn additional threads in any way. - * All the steps are ensured to be synchronous and it throws on instantiating - * an asynchronous graph. It also disallows CJS <-> ESM cycles. + * Loading and linking are always synchronous. Evaluation via runSync() throws on an + * asynchronous graph; evaluation via run() falls back to async for top-level await. + * It also disallows CJS <-> ESM cycles. * - * This is used for ES modules loaded via require(esm). Modules loaded by require() in - * imported CJS are handled by ModuleJob with the isForRequireInImportedCJS set to true instead. - * The two currently have different caching behaviors. + * Used for all ES module imports on the main thread, regardless of how the import was + * triggered (entry point, import(), require(esm), --import, etc.). + * Modules loaded by require() in imported CJS are handled by ModuleJob with the + * isForRequireInImportedCJS set to true instead. The two currently have different caching behaviors. * TODO(joyeecheung): consolidate this with the isForRequireInImportedCJS variant of ModuleJob. */ class ModuleJobSync extends ModuleJobBase { @@ -491,22 +493,33 @@ class ModuleJobSync extends ModuleJobBase { return PromiseResolve(this.module); } - async run() { + async run(isEntryPoint = false) { assert(this.phase === kEvaluationPhase); - // This path is hit by a require'd module that is imported again. const status = this.module.getStatus(); debug('ModuleJobSync.run()', status, this.module); // If the module was previously required and errored, reject from import() again. if (status === kErrored) { throw this.module.getError(); - } else if (status > kInstantiated) { + } + if (status > kInstantiated) { + // Already evaluated (e.g. previously require()'d and now import()'d again). if (this.evaluationPromise) { await this.evaluationPromise; } return { __proto__: null, module: this.module }; - } else if (status === kInstantiated) { - // The evaluation may have been canceled because instantiate() detected TLA first. - // But when it is imported again, it's fine to re-evaluate it asynchronously. + } + if (status < kInstantiated) { + // Fresh module: instantiate it now (links were already resolved synchronously in constructor) + this.module.instantiate(); + } + // `status === kInstantiated`: either just instantiated above, or previously instantiated + // but evaluation was deferred (e.g. TLA detected by a prior `runSync()` call) + if (isEntryPoint) { + globalThis[entry_point_module_private_symbol] = this.module; + } + setHasStartedUserESMExecution(); + if (this.module.hasAsyncGraph) { + // Has top-level `await`: fall back to async evaluation const timeout = -1; const breakOnSigint = false; this.evaluationPromise = this.module.evaluate(timeout, breakOnSigint); @@ -514,9 +527,15 @@ class ModuleJobSync extends ModuleJobBase { this.evaluationPromise = undefined; return { __proto__: null, module: this.module }; } - - assert.fail('Unexpected status of a module that is imported again after being required. ' + - `Status = ${status}`); + // No top-level `await`: evaluate synchronously + const filename = urlToFilename(this.url); + try { + this.module.evaluateSync(filename, undefined); + } catch (evaluateError) { + explainCommonJSGlobalLikeNotDefinedError(evaluateError, this.module.url, this.module.hasTopLevelAwait); + throw evaluateError; + } + return { __proto__: null, module: this.module }; } runSync(parent) { diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index 2974459755ec25..bfff604522103a 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -19,6 +19,7 @@ const { const { privateSymbols: { entry_point_promise_private_symbol, + entry_point_module_private_symbol, }, } = internalBinding('util'); /** @@ -156,6 +157,20 @@ function executeUserEntryPoint(main = process.argv[1]) { const mainPath = resolvedMain || main; const mainURL = getOptionValue('--entry-url') ? new URL(mainPath, getCWDURL()) : pathToFileURL(mainPath); + // When no async hooks or --inspect-brk are needed, try the fully synchronous path first. + // This avoids creating any promises during startup. + if (!getOptionValue('--inspect-brk') && + getOptionValue('--experimental-loader').length === 0 && + getOptionValue('--import').length === 0) { + const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader(); + const { module: entryModule, completed } = cascadedLoader.importSyncForEntryPoint(mainURL.href); + globalThis[entry_point_module_private_symbol] = entryModule; + if (completed) { + return; + } + // TLA detected: fall through to async path. + } + runEntryPointWithESMLoader((cascadedLoader) => { // Note that if the graph contains unsettled TLA, this may never resolve // even after the event loop stops running. diff --git a/test/es-module/test-esm-sync-import.mjs b/test/es-module/test-esm-sync-import.mjs new file mode 100644 index 00000000000000..306d8eab1066da --- /dev/null +++ b/test/es-module/test-esm-sync-import.mjs @@ -0,0 +1,54 @@ +// Flags: --no-warnings +import { spawnPromisified } from '../common/index.mjs'; +import * as fixtures from '../common/fixtures.mjs'; +import { describe, it } from 'node:test'; +import assert from 'node:assert'; + + +describe('synchronous ESM loading', () => { + it('should create minimal promises for ESM importing ESM', async () => { + // import-esm.mjs imports imported-esm.mjs — a pure ESM graph. + const count = await getPromiseCount(fixtures.path('es-modules', 'import-esm.mjs')); + // V8's Module::Evaluate returns one promise for the entire graph. + assert.strictEqual(count, 1); + }); + + it('should create minimal promises for ESM importing CJS', async () => { + // builtin-imports-case.mjs imports node:assert (builtin) + dep1.js and dep2.js (CJS). + const count = await getPromiseCount(fixtures.path('es-modules', 'builtin-imports-case.mjs')); + // V8 creates one promise for the ESM entry evaluation, plus one per CJS module + // in the graph (each CJS namespace is wrapped in a promise). + // entry (ESM, 1) + node:assert (CJS, 1) + dep1.js (CJS, 1) + dep2.js (CJS, 1) = 4. + assert.strictEqual(count, 4); + }); + + it('should fall back to async evaluation for top-level await', async () => { + // tla/resolved.mjs uses top-level await, so the sync path detects TLA + // and falls back to async evaluation. + const count = await getPromiseCount(fixtures.path('es-modules', 'tla', 'resolved.mjs')); + // The async fallback creates more promises — just verify the module + // still runs successfully. The promise count will be higher than the + // sync path but should remain bounded. + assert(count > 1, `Expected TLA fallback to create multiple promises, got ${count}`); + }); + + it('should create minimal promises when entry point is CJS importing ESM', async () => { + // When a CJS entry point uses require(esm), the ESM module is loaded via + // ModuleJobSync, so the same promise minimization applies. + const count = await getPromiseCount(fixtures.path('es-modules', 'require-esm-entry.cjs')); + // V8's Module::Evaluate returns one promise for the ESM module. + assert.strictEqual(count, 1); + }); +}); + + +async function getPromiseCount(entry) { + const { stdout, stderr, code } = await spawnPromisified(process.execPath, [ + '--require', fixtures.path('es-modules', 'promise-counter.cjs'), + entry, + ]); + assert.strictEqual(code, 0, `child failed:\nstdout: ${stdout}\nstderr: ${stderr}`); + const match = stdout.match(/PROMISE_COUNT=(\d+)/); + assert(match, `Expected PROMISE_COUNT in output, got: ${stdout}`); + return Number(match[1]); +} diff --git a/test/fixtures/es-modules/promise-counter.cjs b/test/fixtures/es-modules/promise-counter.cjs new file mode 100644 index 00000000000000..f6bf88eb8f1d16 --- /dev/null +++ b/test/fixtures/es-modules/promise-counter.cjs @@ -0,0 +1,11 @@ +// Counts PROMISE async resources created during the process lifetime. +// Used by test-esm-sync-entry-point.mjs to verify the sync ESM loader +// path does not create unnecessary promises. +'use strict'; +let count = 0; +require('async_hooks').createHook({ + init(id, type) { if (type === 'PROMISE') count++; }, +}).enable(); +process.on('exit', () => { + process.stdout.write(`PROMISE_COUNT=${count}\n`); +}); diff --git a/test/fixtures/es-modules/require-esm-entry.cjs b/test/fixtures/es-modules/require-esm-entry.cjs new file mode 100644 index 00000000000000..ce03a24bece99d --- /dev/null +++ b/test/fixtures/es-modules/require-esm-entry.cjs @@ -0,0 +1 @@ +require('./imported-esm.mjs'); From 54e90019306bc2c7b58ef632d309dd45b53a3f7a Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Wed, 1 Apr 2026 14:41:33 -0700 Subject: [PATCH 2/5] Use --trace-promises to count promises created during sync ESM loading --- test/es-module/test-esm-sync-import.mjs | 6 ++---- test/fixtures/es-modules/promise-counter.cjs | 11 ----------- 2 files changed, 2 insertions(+), 15 deletions(-) delete mode 100644 test/fixtures/es-modules/promise-counter.cjs diff --git a/test/es-module/test-esm-sync-import.mjs b/test/es-module/test-esm-sync-import.mjs index 306d8eab1066da..832dd161f3e3f0 100644 --- a/test/es-module/test-esm-sync-import.mjs +++ b/test/es-module/test-esm-sync-import.mjs @@ -44,11 +44,9 @@ describe('synchronous ESM loading', () => { async function getPromiseCount(entry) { const { stdout, stderr, code } = await spawnPromisified(process.execPath, [ - '--require', fixtures.path('es-modules', 'promise-counter.cjs'), + '--trace-promises', entry, ]); assert.strictEqual(code, 0, `child failed:\nstdout: ${stdout}\nstderr: ${stderr}`); - const match = stdout.match(/PROMISE_COUNT=(\d+)/); - assert(match, `Expected PROMISE_COUNT in output, got: ${stdout}`); - return Number(match[1]); + return stderr.match(/created promise #/g)?.length ?? 0; } diff --git a/test/fixtures/es-modules/promise-counter.cjs b/test/fixtures/es-modules/promise-counter.cjs deleted file mode 100644 index f6bf88eb8f1d16..00000000000000 --- a/test/fixtures/es-modules/promise-counter.cjs +++ /dev/null @@ -1,11 +0,0 @@ -// Counts PROMISE async resources created during the process lifetime. -// Used by test-esm-sync-entry-point.mjs to verify the sync ESM loader -// path does not create unnecessary promises. -'use strict'; -let count = 0; -require('async_hooks').createHook({ - init(id, type) { if (type === 'PROMISE') count++; }, -}).enable(); -process.on('exit', () => { - process.stdout.write(`PROMISE_COUNT=${count}\n`); -}); From 0baaf01681f564f20ca7e494e4a93adea72f3872 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 2 Apr 2026 07:18:00 -0700 Subject: [PATCH 3/5] Refactor startup-esm-graph benchmark to create a tree-shaped graph with a branching factor of 10 --- benchmark/esm/startup-esm-graph.js | 37 ++++++++++++++++++------------ 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/benchmark/esm/startup-esm-graph.js b/benchmark/esm/startup-esm-graph.js index ab1481bdb5a689..03d3e60136947d 100644 --- a/benchmark/esm/startup-esm-graph.js +++ b/benchmark/esm/startup-esm-graph.js @@ -7,34 +7,41 @@ const { spawnSync } = require('child_process'); const tmpdir = require('../../test/common/tmpdir'); const bench = common.createBenchmark(main, { - modules: [250, 500, 1000, 2000], + modules: ['0250', '0500', '1000', '2000'], n: [30], }); +const BRANCHING_FACTOR = 10; + function prepare(count) { tmpdir.refresh(); const dir = tmpdir.resolve('esm-graph'); fs.mkdirSync(dir, { recursive: true }); - // Create a flat ESM graph: entry imports all modules directly. - // Each module is independent, maximizing the number of resolve/load/link - // operations in the loader pipeline. - const imports = []; - for (let i = 0; i < count; i++) { - fs.writeFileSync( - path.join(dir, `mod${i}.mjs`), - `export const value${i} = ${i};\n`, - ); - imports.push(`import './mod${i}.mjs';`); + // Create a tree-shaped ESM graph with a branching factor of 10. + // The root (mod0) plus `count` additional modules are created in BFS order. + // Module i imports modules BRANCHING_FACTOR*i+1 through + // BRANCHING_FACTOR*i+BRANCHING_FACTOR (capped at count), so the graph is a + // complete 10-ary tree rooted at mod0. + const total = count + 1; + for (let i = 0; i < total; i++) { + const children = []; + for (let c = 1; c <= BRANCHING_FACTOR; c++) { + const child = BRANCHING_FACTOR * i + c; + if (child < total) { + children.push(`import './mod${child}.mjs';`); + } + } + const content = children.join('\n') + (children.length ? '\n' : '') + + `export const value${i} = ${i};\n`; + fs.writeFileSync(path.join(dir, `mod${i}.mjs`), content); } - const entry = path.join(dir, 'entry.mjs'); - fs.writeFileSync(entry, imports.join('\n') + '\n'); - return entry; + return path.join(dir, 'mod0.mjs'); } function main({ n, modules }) { - const entry = prepare(modules); + const entry = prepare(Number(modules)); const cmd = process.execPath || process.argv[0]; const warmup = 3; const state = { finished: -warmup }; From a66268e5a5f88264dae0b9ec492eb9d2adc07289 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 2 Apr 2026 08:26:10 -0700 Subject: [PATCH 4/5] Support --inspect-brk --- lib/internal/modules/esm/loader.js | 7 ++- lib/internal/modules/run_main.js | 5 +-- .../test-inspector-debug-brk-flag-esm.js | 45 +++++++++++++++++++ 3 files changed, 53 insertions(+), 4 deletions(-) create mode 100644 test/parallel/test-inspector-debug-brk-flag-esm.js diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index ed1e44bcf21377..c9204178c34463 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -382,7 +382,12 @@ class ModuleLoader { return onImport.traceSync(() => { const request = { specifier: url, phase: kEvaluationPhase, attributes: kEmptyObject, __proto__: null }; const job = this.getOrCreateModuleJob(undefined, request, kImportInImportedESM); - job.module.instantiate(); + if (getOptionValue('--inspect-brk')) { + const { callAndPauseOnStart } = internalBinding('inspector'); + callAndPauseOnStart(job.module.instantiate, job.module); + } else { + job.module.instantiate(); + } if (job.module.hasAsyncGraph) { return { __proto__: null, module: job.module, completed: false }; } diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index bfff604522103a..2ea689d8fd4c4e 100644 --- a/lib/internal/modules/run_main.js +++ b/lib/internal/modules/run_main.js @@ -157,10 +157,9 @@ function executeUserEntryPoint(main = process.argv[1]) { const mainPath = resolvedMain || main; const mainURL = getOptionValue('--entry-url') ? new URL(mainPath, getCWDURL()) : pathToFileURL(mainPath); - // When no async hooks or --inspect-brk are needed, try the fully synchronous path first. + // When no async hooks are needed, try the fully synchronous path first. // This avoids creating any promises during startup. - if (!getOptionValue('--inspect-brk') && - getOptionValue('--experimental-loader').length === 0 && + if (getOptionValue('--experimental-loader').length === 0 && getOptionValue('--import').length === 0) { const cascadedLoader = require('internal/modules/esm/loader').getOrInitializeCascadedLoader(); const { module: entryModule, completed } = cascadedLoader.importSyncForEntryPoint(mainURL.href); diff --git a/test/parallel/test-inspector-debug-brk-flag-esm.js b/test/parallel/test-inspector-debug-brk-flag-esm.js new file mode 100644 index 00000000000000..93a24a37e13d66 --- /dev/null +++ b/test/parallel/test-inspector-debug-brk-flag-esm.js @@ -0,0 +1,45 @@ +'use strict'; +const common = require('../common'); + +// Test that --inspect-brk pauses at the first line of an ESM entry point. + +common.skipIfInspectorDisabled(); + +const assert = require('assert'); +const fixtures = require('../common/fixtures'); +const { NodeInstance } = require('../common/inspector-helper.js'); + +async function testBreakpointOnStart(session) { + const commands = [ + { 'method': 'Runtime.enable' }, + { 'method': 'Debugger.enable' }, + { 'method': 'Debugger.setPauseOnExceptions', + 'params': { 'state': 'none' } }, + { 'method': 'Debugger.setAsyncCallStackDepth', + 'params': { 'maxDepth': 0 } }, + { 'method': 'Profiler.enable' }, + { 'method': 'Profiler.setSamplingInterval', + 'params': { 'interval': 100 } }, + { 'method': 'Debugger.setBlackboxPatterns', + 'params': { 'patterns': [] } }, + { 'method': 'Runtime.runIfWaitingForDebugger' }, + ]; + + await session.send({ method: 'NodeRuntime.enable' }); + await session.waitForNotification('NodeRuntime.waitingForDebugger'); + await session.send(commands); + await session.send({ method: 'NodeRuntime.disable' }); + await session.waitForBreakOnLine(0, session.scriptURL()); +} + +async function runTests() { + const child = new NodeInstance(['--inspect-brk=0'], '', fixtures.path('es-modules', 'loop.mjs')); + const session = await child.connectInspectorSession(); + + await testBreakpointOnStart(session); + await session.runToCompletion(); + + assert.strictEqual((await child.expectShutdown()).exitCode, 55); +} + +runTests().then(common.mustCall()); From e5294fb1721c6a234a07b22fbff0290290009e27 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 2 Apr 2026 11:36:23 -0700 Subject: [PATCH 5/5] esm: fix CJS named export error message for sync module jobs --- lib/internal/modules/esm/module_job.js | 108 ++++++++++++++----------- 1 file changed, 63 insertions(+), 45 deletions(-) diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 4a0297ece8074f..b8dd28c75d074c 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -126,6 +126,58 @@ const explainCommonJSGlobalLikeNotDefinedError = (e, url, hasTopLevelAwait) => { } }; +/** + * If `error` is a SyntaxError from V8 for a missing named export on a CJS module, rewrite its message to the friendlier + * "Named export '...' not found..." form. Must be called after `decorateErrorStack(error)` so that the arrow (source + * context with the import statement text) has been prepended to `error.stack`. + * @param {Error} error + * @param {ModuleWrap} module The parent module that triggered the instantiation. + * @param {boolean[]} commonJsDeps Per-request array indicating whether each dependency is a CJS module, aligned with + * `module.getModuleRequests()`. + */ +const handleCJSNamedExportError = (error, module, commonJsDeps) => { + // TODO(@bcoe): Add source map support to exception that occurs as result + // of missing named export. This is currently not possible because + // stack trace originates in module_job, not the file itself. A hidden + // symbol with filename could be set in node_errors.cc to facilitate this. + if (!getSourceMapsSupport().enabled && + StringPrototypeIncludes(error.message, + ' does not provide an export named')) { + const splitStack = StringPrototypeSplit(error.stack, '\n', 2); + const { 1: childSpecifier, 2: name } = RegExpPrototypeExec( + /module '(.*)' does not provide an export named '(.+)'/, + error.message); + const moduleRequests = module.getModuleRequests(); + let isCommonJS = false; + for (let i = 0; i < moduleRequests.length; ++i) { + if (moduleRequests[i].specifier === childSpecifier) { + isCommonJS = commonJsDeps[i]; + break; + } + } + if (isCommonJS) { + const importStatement = splitStack[1]; + // TODO(@ctavan): The original error stack only provides the single + // line which causes the error. For multi-line import statements we + // cannot generate an equivalent object destructuring assignment by + // just parsing the error stack. + const oneLineNamedImports = RegExpPrototypeExec(/{.*}/, importStatement); + const destructuringAssignment = oneLineNamedImports && + RegExpPrototypeSymbolReplace(/\s+as\s+/g, oneLineNamedImports, ': '); + error.message = `Named export '${name}' not found. The requested module` + + ` '${childSpecifier}' is a CommonJS module, which may not support` + + ' all module.exports as named exports.\nCommonJS modules can ' + + 'always be imported via the default export, for example using:' + + `\n\nimport pkg from '${childSpecifier}';\n${ + destructuringAssignment ? + `const ${destructuringAssignment} = pkg;\n` : ''}`; + const newStack = StringPrototypeSplit(error.stack, '\n'); + newStack[3] = `SyntaxError: ${error.message}`; + error.stack = ArrayPrototypeJoin(newStack, '\n'); + } + } +}; + class ModuleJobBase { constructor(loader, url, importAttributes, phase, isMain, inspectBrk) { assert(typeof phase === 'number'); @@ -325,50 +377,10 @@ class ModuleJob extends ModuleJobBase { } else { this.module.instantiate(); } - } catch (e) { - decorateErrorStack(e); - // TODO(@bcoe): Add source map support to exception that occurs as result - // of missing named export. This is currently not possible because - // stack trace originates in module_job, not the file itself. A hidden - // symbol with filename could be set in node_errors.cc to facilitate this. - if (!getSourceMapsSupport().enabled && - StringPrototypeIncludes(e.message, - ' does not provide an export named')) { - const splitStack = StringPrototypeSplit(e.stack, '\n', 2); - const { 1: childSpecifier, 2: name } = RegExpPrototypeExec( - /module '(.*)' does not provide an export named '(.+)'/, - e.message); - const moduleRequests = this.module.getModuleRequests(); - let isCommonJS = false; - for (let i = 0; i < moduleRequests.length; ++i) { - if (moduleRequests[i].specifier === childSpecifier) { - isCommonJS = this.commonJsDeps[i]; - break; - } - } - - if (isCommonJS) { - const importStatement = splitStack[1]; - // TODO(@ctavan): The original error stack only provides the single - // line which causes the error. For multi-line import statements we - // cannot generate an equivalent object destructuring assignment by - // just parsing the error stack. - const oneLineNamedImports = RegExpPrototypeExec(/{.*}/, importStatement); - const destructuringAssignment = oneLineNamedImports && - RegExpPrototypeSymbolReplace(/\s+as\s+/g, oneLineNamedImports, ': '); - e.message = `Named export '${name}' not found. The requested module` + - ` '${childSpecifier}' is a CommonJS module, which may not support` + - ' all module.exports as named exports.\nCommonJS modules can ' + - 'always be imported via the default export, for example using:' + - `\n\nimport pkg from '${childSpecifier}';\n${ - destructuringAssignment ? - `const ${destructuringAssignment} = pkg;\n` : ''}`; - const newStack = StringPrototypeSplit(e.stack, '\n'); - newStack[3] = `SyntaxError: ${e.message}`; - e.stack = ArrayPrototypeJoin(newStack, '\n'); - } - } - throw e; + } catch (error) { + decorateErrorStack(error); + handleCJSNamedExportError(error, this.module, this.commonJsDeps); + throw error; } for (const dependencyJob of jobsInGraph) { @@ -510,7 +522,13 @@ class ModuleJobSync extends ModuleJobBase { } if (status < kInstantiated) { // Fresh module: instantiate it now (links were already resolved synchronously in constructor) - this.module.instantiate(); + try { + this.module.instantiate(); + } catch (error) { + decorateErrorStack(error); + handleCJSNamedExportError(error, this.module, this.commonJsDeps); + throw error; + } } // `status === kInstantiated`: either just instantiated above, or previously instantiated // but evaluation was deferred (e.g. TLA detected by a prior `runSync()` call)