From 4bdb414640e19df4a3a33eb12db390882a5196d6 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Tue, 31 Mar 2026 17:17:51 -0700 Subject: [PATCH 1/8] module: synchronously load most ES modules Signed-off-by: Geoffrey Booth --- benchmark/esm/startup-esm-graph.js | 59 +++++++++++++++++++ lib/internal/modules/esm/loader.js | 40 +++++++++++-- lib/internal/modules/esm/module_job.js | 56 +++++++++++------- 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, 210 insertions(+), 26 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 000000000000..ab1481bdb5a6 --- /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 f92e87785ce1..0aa92f9d0ff3 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -350,6 +350,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(). @@ -572,10 +598,15 @@ class ModuleLoader { assert(moduleOrModulePromise instanceof ModuleWrap, `Expected ModuleWrap for loading ${url}`); } - // 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, @@ -602,8 +633,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 c54706a9e0af..19887efbd166 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -54,7 +54,7 @@ const { getSourceMapsSupport, } = require('internal/source_map/source_map_cache'); const assert = require('internal/assert'); -const resolvedPromise = PromiseResolve(); +let resolvedPromise; const { setHasStartedUserESMExecution, urlToFilename, @@ -511,7 +511,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(); } } @@ -577,12 +577,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 { @@ -623,28 +625,33 @@ class ModuleJobSync extends ModuleJobBase { return PromiseResolve(this.module); } - async run() { + async run(isEntryPoint = false) { assert(this.shouldRunModule(this.phase)); - // 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 || status === kUninstantiated) { - // If we get here, the module was initially required and is now being imported. - // The require() module failed either because the graph has TLA (kInstantiated), - // or instantiation failed (kUninstantiated, e.g. missing named export). - // Try finishing the instantiation - if it succeeds, proceed to evaluation, - // otherwise the branch below re-throw any instantiation error. - if (status === kUninstantiated) { - this.module.instantiate(); - } + } + 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); @@ -652,9 +659,14 @@ 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 + try { + this.module.evaluateSync(); + } 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 50ae8c281b02..dbd61dd8f472 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 000000000000..306d8eab1066 --- /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 000000000000..f6bf88eb8f1d --- /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 000000000000..ce03a24bece9 --- /dev/null +++ b/test/fixtures/es-modules/require-esm-entry.cjs @@ -0,0 +1 @@ +require('./imported-esm.mjs'); From 116e1f01b6cddd7341c621da30fecb778f94575c Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Wed, 1 Apr 2026 14:41:33 -0700 Subject: [PATCH 2/8] Use --trace-promises to count promises created during sync ESM loading Signed-off-by: Geoffrey Booth --- 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 306d8eab1066..832dd161f3e3 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 f6bf88eb8f1d..000000000000 --- 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 1e50bf75aedd2ea0d2f5c31c26c23692799aa253 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 2 Apr 2026 07:18:00 -0700 Subject: [PATCH 3/8] Refactor startup-esm-graph benchmark to create a tree-shaped graph with a branching factor of 10 Signed-off-by: Geoffrey Booth --- 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 ab1481bdb5a6..03d3e6013694 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 9bdfc5c0980104260416809ebaeef9345b57682b Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 2 Apr 2026 08:26:10 -0700 Subject: [PATCH 4/8] Support --inspect-brk Signed-off-by: Geoffrey Booth --- lib/internal/modules/esm/loader.js | 2 +- lib/internal/modules/esm/module_job.js | 14 +++++- lib/internal/modules/run_main.js | 5 +- .../test-inspector-debug-brk-flag-esm.js | 47 +++++++++++++++++++ test/parallel/test-inspector-esm.js | 7 +-- 5 files changed, 67 insertions(+), 8 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 0aa92f9d0ff3..171505250330 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -602,7 +602,7 @@ class ModuleLoader { 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). + // (which uses ModuleJob so that inspectBrk is handled in ModuleJob.runSync()). // TODO(joyeecheung): use ModuleJobSync for kRequireInImportedCJS too. const ModuleJobCtor = (!this.isForAsyncLoaderHookWorker && !inspectBrk && requestType !== kRequireInImportedCJS) ? diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 19887efbd166..bd8f13cacf6b 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -536,7 +536,19 @@ class ModuleJob extends ModuleJobBase { this.throwIfAsyncGraph(parent); if (status === kInstantiated) { setHasStartedUserESMExecution(); - const namespace = this.module.evaluateSync(); + let namespace; + if (this.inspectBrk) { + // Pre-evaluate all direct dependencies so that V8 will skip them when we evaluate the entry point. + // This ensures `callAndPauseOnStart` pauses at the entry point's first line rather than a dependency's. + const deps = isPromise(this.linked) ? [] : this.linked; + for (const dep of deps) { + dep.module.evaluateSync(); + } + const { callAndPauseOnStart } = internalBinding('inspector'); + namespace = callAndPauseOnStart(this.module.evaluateSync, this.module); + } else { + namespace = this.module.evaluateSync(); + } return { __proto__: null, module: this.module, namespace }; } throw this.module.getError(); diff --git a/lib/internal/modules/run_main.js b/lib/internal/modules/run_main.js index dbd61dd8f472..3f56704a25e1 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 000000000000..3b2de70770bd --- /dev/null +++ b/test/parallel/test-inspector-debug-brk-flag-esm.js @@ -0,0 +1,47 @@ +'use strict'; +const common = require('../common'); + +// Test that --inspect-brk pauses at the first executable 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' }); + // Static import declarations are not executable statements, so the break + // happens at the first executable line (var t = 1) rather than line 0. + await session.waitForBreakOnLine(2, 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()); diff --git a/test/parallel/test-inspector-esm.js b/test/parallel/test-inspector-esm.js index 79025e80a76d..5462feaac6db 100644 --- a/test/parallel/test-inspector-esm.js +++ b/test/parallel/test-inspector-esm.js @@ -4,7 +4,6 @@ const common = require('../common'); common.skipIfInspectorDisabled(); const assert = require('assert'); -const { resolve: UrlResolve } = require('url'); const fixtures = require('../common/fixtures'); const { NodeInstance } = require('../common/inspector-helper.js'); @@ -40,8 +39,10 @@ async function testBreakpointOnStart(session) { await session.waitForNotification('NodeRuntime.waitingForDebugger'); await session.send(commands); await session.send({ method: 'NodeRuntime.disable' }); - await session.waitForBreakOnLine( - 0, UrlResolve(session.scriptURL().toString(), 'message.mjs')); + // --inspect-brk pauses at the first executable line of the entry point. + // Static import declarations are not executable, so the break happens at + // var t = 1 (line 2), not at the import on line 0. + await session.waitForBreakOnLine(2, session.scriptURL()); } async function testBreakpoint(session) { From e2627051ca6a49bb88dcb863ecb9f0ae4fe3f306 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Thu, 2 Apr 2026 11:36:23 -0700 Subject: [PATCH 5/8] Fix CJS named export error message for sync module jobs Signed-off-by: Geoffrey Booth --- 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 bd8f13cacf6b..9c6b10b0dec8 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -241,6 +241,58 @@ function getTopLevelAwaitLocations(root) { return locations; } +/** + * 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'); @@ -462,50 +514,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) { @@ -654,7 +666,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) From 1abd8ba51cf2fe492ff52bf8d5a21b1ed29a247d Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sun, 24 May 2026 22:48:09 -0700 Subject: [PATCH 6/8] Make inspector break on entry point of ESM when --inspect-brk is used Signed-off-by: Geoffrey Booth --- lib/internal/modules/esm/module_job.js | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 9c6b10b0dec8..39c14adf4639 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -65,7 +65,6 @@ const { ERR_REQUIRE_ASYNC_MODULE, ERR_REQUIRE_ESM_RACE_CONDITION, } = require('internal/errors').codes; -let hasPausedEntry = false; const CJSGlobalLike = [ 'require', @@ -507,13 +506,7 @@ class ModuleJob extends ModuleJobBase { await addJobsToDependencyGraph(this); try { - if (!hasPausedEntry && this.inspectBrk) { - hasPausedEntry = true; - const initWrapper = internalBinding('inspector').callAndPauseOnStart; - initWrapper(this.module.instantiate, this.module); - } else { - this.module.instantiate(); - } + this.module.instantiate(); } catch (error) { decorateErrorStack(error); handleCJSNamedExportError(error, this.module, this.commonJsDeps); @@ -590,7 +583,18 @@ class ModuleJob extends ModuleJobBase { const breakOnSigint = false; setHasStartedUserESMExecution(); try { - await this.module.evaluate(timeout, breakOnSigint); + if (this.inspectBrk) { + // Pre-evaluate all direct dependencies so that V8 will skip them when we evaluate the entry point. + // This ensures `callAndPauseOnStart` pauses at the entry point's first line rather than a dependency's. + const deps = isPromise(this.linked) ? await this.linked : this.linked; + for (const dep of deps) { + await dep.module.evaluate(timeout, breakOnSigint); + } + const { callAndPauseOnStart } = internalBinding('inspector'); + await callAndPauseOnStart(this.module.evaluate, this.module, timeout, breakOnSigint); + } else { + await this.module.evaluate(timeout, breakOnSigint); + } } catch (e) { explainCommonJSGlobalLikeNotDefinedError(e, this.module.url, this.module.hasTopLevelAwait); throw e; From e9f69b3cd5f059d26c50cd57021e9119416e6193 Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sat, 12 Sep 2026 13:36:14 -0700 Subject: [PATCH 7/8] Pause at evaluation of the root module for --inspect-brk Previously the pause was set up in ModuleLoader, which forced the entry point onto the async ModuleJob and paused during instantiation, landing the debugger on internal frames or on a dependency rather than on the entry point itself. Move the handling into ModuleJobSync, where the root module is actually evaluated. Direct dependencies are pre-evaluated first so that V8 skips them and stops at the entry point's first executable line. Both the synchronous path and the top-level await fallback are covered. Signed-off-by: Geoffrey Booth --- lib/internal/modules/esm/loader.js | 5 +- lib/internal/modules/esm/module_job.js | 62 ++++++++++--------- .../test-inspector-debug-brk-flag-esm-tla.js | 49 +++++++++++++++ 3 files changed, 84 insertions(+), 32 deletions(-) create mode 100644 test/parallel/test-inspector-debug-brk-flag-esm-tla.js diff --git a/lib/internal/modules/esm/loader.js b/lib/internal/modules/esm/loader.js index 171505250330..cae89b5f886e 100644 --- a/lib/internal/modules/esm/loader.js +++ b/lib/internal/modules/esm/loader.js @@ -601,10 +601,9 @@ class ModuleLoader { 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 uses ModuleJob so that inspectBrk is handled in ModuleJob.runSync()). + // except for kRequireInImportedCJS (TODO: consolidate that case too). // TODO(joyeecheung): use ModuleJobSync for kRequireInImportedCJS too. - const ModuleJobCtor = (!this.isForAsyncLoaderHookWorker && !inspectBrk && + const ModuleJobCtor = (!this.isForAsyncLoaderHookWorker && requestType !== kRequireInImportedCJS) ? ModuleJobSync : ModuleJob; job = new ModuleJobCtor( diff --git a/lib/internal/modules/esm/module_job.js b/lib/internal/modules/esm/module_job.js index 39c14adf4639..cf5a35d8682f 100644 --- a/lib/internal/modules/esm/module_job.js +++ b/lib/internal/modules/esm/module_job.js @@ -541,19 +541,7 @@ class ModuleJob extends ModuleJobBase { this.throwIfAsyncGraph(parent); if (status === kInstantiated) { setHasStartedUserESMExecution(); - let namespace; - if (this.inspectBrk) { - // Pre-evaluate all direct dependencies so that V8 will skip them when we evaluate the entry point. - // This ensures `callAndPauseOnStart` pauses at the entry point's first line rather than a dependency's. - const deps = isPromise(this.linked) ? [] : this.linked; - for (const dep of deps) { - dep.module.evaluateSync(); - } - const { callAndPauseOnStart } = internalBinding('inspector'); - namespace = callAndPauseOnStart(this.module.evaluateSync, this.module); - } else { - namespace = this.module.evaluateSync(); - } + const namespace = this.module.evaluateSync(); return { __proto__: null, module: this.module, namespace }; } throw this.module.getError(); @@ -583,18 +571,7 @@ class ModuleJob extends ModuleJobBase { const breakOnSigint = false; setHasStartedUserESMExecution(); try { - if (this.inspectBrk) { - // Pre-evaluate all direct dependencies so that V8 will skip them when we evaluate the entry point. - // This ensures `callAndPauseOnStart` pauses at the entry point's first line rather than a dependency's. - const deps = isPromise(this.linked) ? await this.linked : this.linked; - for (const dep of deps) { - await dep.module.evaluate(timeout, breakOnSigint); - } - const { callAndPauseOnStart } = internalBinding('inspector'); - await callAndPauseOnStart(this.module.evaluate, this.module, timeout, breakOnSigint); - } else { - await this.module.evaluate(timeout, breakOnSigint); - } + await this.module.evaluate(timeout, breakOnSigint); } catch (e) { explainCommonJSGlobalLikeNotDefinedError(e, this.module.url, this.module.hasTopLevelAwait); throw e; @@ -669,7 +646,10 @@ class ModuleJobSync extends ModuleJobBase { return { __proto__: null, module: this.module }; } if (status < kInstantiated) { - // Fresh module: instantiate it now (links were already resolved synchronously in constructor) + // Either a fresh module - links were already resolved synchronously in the constructor - + // or one that was initially require()'d and is now being imported after its instantiation + // failed (e.g. a missing named export). Try finishing the instantiation: if it succeeds, + // proceed to evaluation, otherwise re-throw the instantiation error. try { this.module.instantiate(); } catch (error) { @@ -688,14 +668,24 @@ class ModuleJobSync extends ModuleJobBase { // Has top-level `await`: fall back to async evaluation const timeout = -1; const breakOnSigint = false; - this.evaluationPromise = this.module.evaluate(timeout, breakOnSigint); + if (this.inspectBrk) { + // Pre-evaluate all direct dependencies so that V8 will skip them when we evaluate the entry point. + // This ensures `callAndPauseOnStart` pauses at the entry point's first line rather than a dependency's. + for (const dep of this.linked) { + await dep.module.evaluate(timeout, breakOnSigint); + } + const { callAndPauseOnStart } = internalBinding('inspector'); + this.evaluationPromise = callAndPauseOnStart(this.module.evaluate, this.module, timeout, breakOnSigint); + } else { + this.evaluationPromise = this.module.evaluate(timeout, breakOnSigint); + } await this.evaluationPromise; this.evaluationPromise = undefined; return { __proto__: null, module: this.module }; } // No top-level `await`: evaluate synchronously try { - this.module.evaluateSync(); + this.#evaluateSync(); } catch (evaluateError) { explainCommonJSGlobalLikeNotDefinedError(evaluateError, this.module.url, this.module.hasTopLevelAwait); throw evaluateError; @@ -703,6 +693,20 @@ class ModuleJobSync extends ModuleJobBase { return { __proto__: null, module: this.module }; } + // Evaluate this module synchronously, pausing at the first line if --inspect-brk is active. + // When pausing, direct dependencies are pre-evaluated first so V8 skips them and stops at + // this module's first line rather than a dependency's. + #evaluateSync() { + if (this.inspectBrk) { + for (const dep of this.linked) { + dep.module.evaluateSync(); + } + const { callAndPauseOnStart } = internalBinding('inspector'); + return callAndPauseOnStart(this.module.evaluateSync, this.module); + } + return this.module.evaluateSync(); + } + runSync(parent) { debug('ModuleJobSync.runSync()', this.module); assert(this.shouldRunModule(this.phase)); @@ -716,7 +720,7 @@ class ModuleJobSync extends ModuleJobBase { this.throwIfAsyncGraph(parent); setHasStartedUserESMExecution(); try { - const namespace = this.module.evaluateSync(); + const namespace = this.#evaluateSync(); return { __proto__: null, module: this.module, namespace }; } catch (e) { explainCommonJSGlobalLikeNotDefinedError(e, this.module.url, this.module.hasTopLevelAwait); diff --git a/test/parallel/test-inspector-debug-brk-flag-esm-tla.js b/test/parallel/test-inspector-debug-brk-flag-esm-tla.js new file mode 100644 index 000000000000..56f0840d8ac0 --- /dev/null +++ b/test/parallel/test-inspector-debug-brk-flag-esm-tla.js @@ -0,0 +1,49 @@ +'use strict'; +const common = require('../common'); + +// Test that --inspect-brk pauses at the first executable line of an ESM entry +// point that uses top-level await (which takes the async evaluation path). + +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' }); + // Static import declarations are not executable statements, so the break + // happens at the first executable line (await setTimeout(9), line index 4). + await session.waitForBreakOnLine(4, session.scriptURL()); +} + +async function runTests() { + const child = new NodeInstance(['--inspect-brk=0'], '', + fixtures.path('es-modules', 'esm-top-level-await.mjs')); + const session = await child.connectInspectorSession(); + + await testBreakpointOnStart(session); + await session.runToCompletion(); + + assert.strictEqual((await child.expectShutdown()).exitCode, 0); +} + +runTests().then(common.mustCall()); From f1dad35977a188c3c3e5db8bea9c29556065be1d Mon Sep 17 00:00:00 2001 From: Geoffrey Booth Date: Sat, 12 Sep 2026 13:36:25 -0700 Subject: [PATCH 8/8] Update source map snapshot for synchronous entry point evaluation An uncaught error thrown by an ES module entry point now escapes through ModuleJobSync's evaluateSync() instead of the async triggerUncaughtException path in run_main, so the internal frame shown above the error changes. Signed-off-by: Geoffrey Booth --- .../output/source_map_assert_no_source_map.snapshot | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/fixtures/source-map/output/source_map_assert_no_source_map.snapshot b/test/fixtures/source-map/output/source_map_assert_no_source_map.snapshot index 0a2ea1a45fe6..cc9ba4849109 100644 --- a/test/fixtures/source-map/output/source_map_assert_no_source_map.snapshot +++ b/test/fixtures/source-map/output/source_map_assert_no_source_map.snapshot @@ -1,6 +1,6 @@ -node:internal/modules/run_main: - triggerUncaughtException( - ^ +node:internal/modules/esm/module_job: + return this.module.evaluateSync(); + ^ AssertionError [ERR_ASSERTION]: The expression evaluated to a falsy value: