From f6c9b25e30b3ae8cbe3c64111070438d50bbba45 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sat, 8 Aug 2026 05:31:21 +0000 Subject: [PATCH 1/9] require(esm): deliver sync-transpiled source to a mid-fetch registry entry A CommonJS module imported by an ESM graph has its body evaluated while the graph is still loading, and that body can require() an ESM sibling the graph already started fetching on the transpiler thread. The sibling's registry entry is then status Fetching with a pending fetch promise. fetchCommonJSModule re-transpiled the file synchronously but provideFetch() only accepts a New entry, so the fresh source was silently dropped, the synchronous load had nothing to drain, and require() threw a spurious TypeError: require() async module "..." is unsupported. use "await import()" instead. for a module with no top-level await. Intermittent on multi-file mixed ESM/CJS graphs (28/30 runs on the repro below on a loaded machine). Settle the entry's fetch promise with the just-transpiled source instead. The async transpiler's own result lands later on an already-settled promise and is dropped by the existing PromiseFulfillWithoutHandlerJob pending-target guard. --- src/jsc/bindings/ModuleLoader.cpp | 45 +++++++++++++-- .../require-esm-in-flight-sibling.test.ts | 55 +++++++++++++++++++ 2 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 test/js/bun/resolve/require-esm-in-flight-sibling.test.ts diff --git a/src/jsc/bindings/ModuleLoader.cpp b/src/jsc/bindings/ModuleLoader.cpp index 7c26804e8329..1872f7353221 100644 --- a/src/jsc/bindings/ModuleLoader.cpp +++ b/src/jsc/bindings/ModuleLoader.cpp @@ -641,6 +641,43 @@ void evaluateCommonJSCustomExtension( RETURN_IF_EXCEPTION(scope, ); } +// Hand freshly-transpiled source to the module registry ahead of a +// synchronous require(esm) load. provideFetch() only accepts a New entry; +// when the async transpiler already started fetching this specifier (it is a +// dependency of an ESM graph that is mid-load), settle the entry's fetch +// promise with this source instead so the synchronous load can proceed +// without yielding to the transpiler thread. The async result lands later on +// an already-settled promise and is dropped (PromiseFulfillWithoutHandlerJob +// checks for a pending target). +static void provideFetchForSyncLoad(Zig::GlobalObject* globalObject, const WTF::String& specifier, JSC::JSSourceCode* jsSourceCode) +{ + auto& vm = JSC::getVM(globalObject); + auto key = JSC::Identifier::fromString(vm, specifier); + auto* loader = globalObject->moduleLoader(); + if (auto* entry = loader->registryEntry(key)) { + if (entry->status() == JSC::ModuleRegistryEntry::Status::Fetching) { + // Guarantees the FetchSettled reaction is attached to the fetch + // promise (a no-op for entries hostLoadImportedModule created). + entry->ensureModulePromise(globalObject); + JSC::JSPromise* fetchPromise = entry->ensureFetchPromise(globalObject); + if (fetchPromise->status() == JSC::JSPromise::Status::Pending) { + // The async path pipeFrom()'d this promise, which set the + // first-resolving-function flag, so the guarded fulfill() + // would no-op; settle it directly. The FetchSettled reaction + // lands on the synchronous module queue the caller is about + // to drain. + fetchPromise->fulfillPromise(vm, jsSourceCode); + } + // Already-settled fetch: the FetchSettled reaction either ran or + // is stranded on the normal microtask queue, in which case + // hostLoadImportedModule's synchronous-replay path makes the + // module from the settled source inline. + return; + } + } + loader->provideFetch(globalObject, key, JSC::ScriptFetchParameters::Type::JavaScript, jsSourceCode); +} + JSValue fetchCommonJSModule( Zig::GlobalObject* globalObject, JSCommonJSModule* target, @@ -697,7 +734,7 @@ JSValue fetchCommonJSModule( JSC::VM::SynchronousModuleQueue queue; queue.prev = vm.m_synchronousModuleQueue; vm.m_synchronousModuleQueue = &queue; - globalObject->moduleLoader()->provideFetch(globalObject, JSC::Identifier::fromString(vm, specifierWtfString), JSC::ScriptFetchParameters::Type::JavaScript, jsSourceCode); + provideFetchForSyncLoad(globalObject, specifierWtfString, jsSourceCode); if (!scope.exception()) JSC::JSModuleLoader::drainSynchronousModuleQueue(globalObject); vm.m_synchronousModuleQueue = queue.prev; RETURN_IF_EXCEPTION(scope, {}); @@ -752,7 +789,7 @@ JSValue fetchCommonJSModule( JSC::VM::SynchronousModuleQueue queue; queue.prev = vm.m_synchronousModuleQueue; vm.m_synchronousModuleQueue = &queue; - globalObject->moduleLoader()->provideFetch(globalObject, JSC::Identifier::fromString(vm, specifierWtfString), JSC::ScriptFetchParameters::Type::JavaScript, jsSourceCode); + provideFetchForSyncLoad(globalObject, specifierWtfString, jsSourceCode); if (!scope.exception()) JSC::JSModuleLoader::drainSynchronousModuleQueue(globalObject); vm.m_synchronousModuleQueue = queue.prev; RETURN_IF_EXCEPTION(scope, {}); @@ -786,7 +823,7 @@ JSValue fetchCommonJSModule( JSC::VM::SynchronousModuleQueue queue; queue.prev = vm.m_synchronousModuleQueue; vm.m_synchronousModuleQueue = &queue; - globalObject->moduleLoader()->provideFetch(globalObject, JSC::Identifier::fromString(vm, specifierWtfString), JSC::ScriptFetchParameters::Type::JavaScript, JSC::SourceCode(Ref(*cached))); + provideFetchForSyncLoad(globalObject, specifierWtfString, JSC::JSSourceCode::create(vm, JSC::SourceCode(Ref(*cached)))); if (!scope.exception()) JSC::JSModuleLoader::drainSynchronousModuleQueue(globalObject); vm.m_synchronousModuleQueue = queue.prev; RETURN_IF_EXCEPTION(scope, {}); @@ -884,7 +921,7 @@ JSValue fetchCommonJSModuleNonBuiltin( JSC::VM::SynchronousModuleQueue queue; queue.prev = vm.m_synchronousModuleQueue; vm.m_synchronousModuleQueue = &queue; - globalObject->moduleLoader()->provideFetch(globalObject, JSC::Identifier::fromString(vm, specifierWtfString), JSC::ScriptFetchParameters::Type::JavaScript, JSC::SourceCode(provider)); + provideFetchForSyncLoad(globalObject, specifierWtfString, JSC::JSSourceCode::create(vm, JSC::SourceCode(provider))); if (!scope.exception()) JSC::JSModuleLoader::drainSynchronousModuleQueue(globalObject); vm.m_synchronousModuleQueue = queue.prev; } diff --git a/test/js/bun/resolve/require-esm-in-flight-sibling.test.ts b/test/js/bun/resolve/require-esm-in-flight-sibling.test.ts new file mode 100644 index 000000000000..231e58409ce5 --- /dev/null +++ b/test/js/bun/resolve/require-esm-in-flight-sibling.test.ts @@ -0,0 +1,55 @@ +// A CommonJS module imported by an ESM graph has its body evaluated while the +// graph is still loading (during the loader's makeModule step). That body can +// require() an ESM sibling the graph already started fetching on the +// transpiler thread, so the sibling's registry entry is mid-fetch: status +// Fetching with a pending fetch promise. require(esm)'s synchronous load used +// to leave that pending promise untouched and threw a spurious +// `require() async module "..." is unsupported. use "await import()" instead.` +// TypeError even though the sibling has no top-level await. +// +// e.mjs is padded with exports so its transpile reliably loses the race to +// d.cjs's require(). The race is probabilistic per run; concurrent runs make +// an unfixed bun fail with near certainty. +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +test("require() of an ESM sibling that is still transpiling completes synchronously", async () => { + const pad = Array.from({ length: 3000 }, (_, i) => `export const pad${i} = ${i};`).join("\n"); + using dir = tempDir("require-esm-in-flight", { + "entry.mjs": `import "./a.mjs"; +import "./c.mjs"; +console.log((globalThis.o ??= []).concat("entry").join(","));`, + "a.mjs": `import "./b.cjs"; +(globalThis.o ??= []).push("a"); +export const a = 1;`, + "b.cjs": `require("./c.mjs"); +(globalThis.o ??= []).push("b"); +module.exports = {};`, + "c.mjs": `import "./d.cjs"; +import "./e.mjs"; +(globalThis.o ??= []).push("c"); +export const c = 1;`, + "d.cjs": `require("./e.mjs"); +(globalThis.o ??= []).push("d"); +module.exports = {};`, + "e.mjs": `(globalThis.o ??= []).push("e"); +export const e = 1; +${pad}`, + }); + + await Promise.all( + Array.from({ length: 12 }, async () => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "entry.mjs"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(stdout).toBe("e,d,c,b,a,entry\n"); + expect(exitCode).toBe(0); + }), + ); + // 12 subprocess spawns under a debug/ASAN build need more than the 5s default. +}, 30_000); From c8bf72019bf8112461308ce3ee188085b70b9210 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 01:42:46 +0000 Subject: [PATCH 2/9] test: add regression test for #33180 A deterministic CJS/ESM cycle shape (a one-line CJS starter whose require chain reaches an ESM module the entry is also importing) and the issue's shape with several large siblings required in a row. Both fail on 1.3.14 through current main with the spurious require() async module TypeError. --- test/regression/issue/33180.test.ts | 91 +++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) create mode 100644 test/regression/issue/33180.test.ts diff --git a/test/regression/issue/33180.test.ts b/test/regression/issue/33180.test.ts new file mode 100644 index 000000000000..a3753bd1d2f3 --- /dev/null +++ b/test/regression/issue/33180.test.ts @@ -0,0 +1,91 @@ +// https://github.com/oven-sh/bun/issues/33180 +// +// A CommonJS module imported by an ESM graph has its body run while the graph +// is still loading. If that body require()s a plain ESM module (no top-level +// await) that the same graph is also importing, the module's registry entry is +// still mid-fetch, and 1.3.14 misreported it as +// TypeError: require() async module "..." is unsupported. use "await import()" instead. +// 1.3.11 and Node both load it synchronously. +import { expect, test } from "bun:test"; +import { bunEnv, bunExe, tempDir } from "harness"; + +test.concurrent("require() of an ESM module the surrounding import graph is still fetching", async () => { + // Deterministic: starter.cjs is a one-line module, so its body runs as soon + // as it is delivered, before a.cjs and b.mjs have finished fetching, on + // every run. b.mjs importing a.cjs back while a.cjs is still evaluating + // additionally checks that the synchronous load hands the ESM side the live + // exports of the half-evaluated CJS module and does not run it twice. Node + // prints the same JSON. + using dir = tempDir("issue-33180-cycle", { + "entry.mjs": ` + import "./starter.cjs"; + import a from "./a.cjs"; + import { lateViaESM } from "./b.mjs"; + console.log(JSON.stringify({ evaluations: globalThis.evaluations, late: a.late, lateViaESM: lateViaESM() })); + `, + "starter.cjs": `require("./a.cjs");`, + "a.cjs": ` + globalThis.evaluations = (globalThis.evaluations ?? 0) + 1; + exports.early = "early"; + require("./b.mjs"); + exports.late = "late"; + `, + "b.mjs": ` + import a from "./a.cjs"; + export function lateViaESM() { return a.late; } + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "entry.mjs"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ evaluations: 1, late: "late", lateViaESM: "late" }) + "\n", + stderr: "", + exitCode: 0, + }); +}); + +test.concurrent("consecutive require()s of several ESM siblings that are still transpiling", async () => { + // The shape from the issue: the siblings are large so their transpile on the + // transpiler thread is still in flight when the tiny CJS module's body runs + // and require()s each of them in turn. + const files: Record = { + "entry.mjs": ` + import "./e1.mjs"; + import "./e2.mjs"; + import "./e3.mjs"; + import "./e4.mjs"; + import "./k.cjs"; + console.log("entry-done"); + `, + "k.cjs": ` + const ids = [1, 2, 3, 4].map(i => require("./e" + i + ".mjs").id); + console.log("required " + JSON.stringify(ids)); + module.exports = {}; + `, + }; + for (let i = 1; i <= 4; i++) { + let source = ""; + for (let j = 0; j < 500; j++) source += `export const v${j} = ${j};\n`; + files[`e${i}.mjs`] = source + `export const id = ${i};\n`; + } + using dir = tempDir("issue-33180-siblings", files); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "entry.mjs"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: "required [1,2,3,4]\nentry-done\n", + stderr: "", + exitCode: 0, + }); +}); From e06da7d2b204e1a4d2d184faed8a2c39b8709e3e Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 02:51:16 +0000 Subject: [PATCH 3/9] require(esm): make the in-flight sibling test deterministic, trim comments The test now imports a package named reflect-metadata, which the runtime always transpiles on the main thread, so its body runs in the microtask checkpoint after the graph's fetches are issued and the ESM sibling is mid-fetch on every run instead of most runs. One spawn replaces twelve. provideFetchForSyncLoad keeps one comment on the fulfillPromise call and follows the neighbouring throw-scope convention. --- src/jsc/bindings/ModuleLoader.cpp | 45 ++++------- .../require-esm-in-flight-sibling.test.ts | 80 +++++++------------ test/regression/issue/33180.test.ts | 16 ---- 3 files changed, 47 insertions(+), 94 deletions(-) diff --git a/src/jsc/bindings/ModuleLoader.cpp b/src/jsc/bindings/ModuleLoader.cpp index eaff706eac45..b17b40db4c11 100644 --- a/src/jsc/bindings/ModuleLoader.cpp +++ b/src/jsc/bindings/ModuleLoader.cpp @@ -655,41 +655,28 @@ void evaluateCommonJSCustomExtension( RETURN_IF_EXCEPTION(scope, ); } -// Hand freshly-transpiled source to the module registry ahead of a -// synchronous require(esm) load. provideFetch() only accepts a New entry; -// when the async transpiler already started fetching this specifier (it is a -// dependency of an ESM graph that is mid-load), settle the entry's fetch -// promise with this source instead so the synchronous load can proceed -// without yielding to the transpiler thread. The async result lands later on -// an already-settled promise and is dropped (PromiseFulfillWithoutHandlerJob -// checks for a pending target). +// Registers source for a synchronous require(esm) load. An entry the +// surrounding import graph is still fetching on the transpiler thread is +// settled with this source (provideFetch() would ignore it); the transpiler's +// own result later lands on a settled promise and is dropped. static void provideFetchForSyncLoad(Zig::GlobalObject* globalObject, const WTF::String& specifier, JSC::JSSourceCode* jsSourceCode) { auto& vm = JSC::getVM(globalObject); + auto scope = DECLARE_THROW_SCOPE(vm); auto key = JSC::Identifier::fromString(vm, specifier); auto* loader = globalObject->moduleLoader(); - if (auto* entry = loader->registryEntry(key)) { - if (entry->status() == JSC::ModuleRegistryEntry::Status::Fetching) { - // Guarantees the FetchSettled reaction is attached to the fetch - // promise (a no-op for entries hostLoadImportedModule created). - entry->ensureModulePromise(globalObject); - JSC::JSPromise* fetchPromise = entry->ensureFetchPromise(globalObject); - if (fetchPromise->status() == JSC::JSPromise::Status::Pending) { - // The async path pipeFrom()'d this promise, which set the - // first-resolving-function flag, so the guarded fulfill() - // would no-op; settle it directly. The FetchSettled reaction - // lands on the synchronous module queue the caller is about - // to drain. - fetchPromise->fulfillPromise(vm, jsSourceCode); - } - // Already-settled fetch: the FetchSettled reaction either ran or - // is stranded on the normal microtask queue, in which case - // hostLoadImportedModule's synchronous-replay path makes the - // module from the settled source inline. - return; - } + auto* entry = loader->registryEntry(key); + if (!entry || entry->status() != JSC::ModuleRegistryEntry::Status::Fetching) { + scope.release(); + loader->provideFetch(globalObject, key, JSC::ScriptFetchParameters::Type::JavaScript, jsSourceCode); + return; } - loader->provideFetch(globalObject, key, JSC::ScriptFetchParameters::Type::JavaScript, jsSourceCode); + entry->ensureModulePromise(globalObject); + RETURN_IF_EXCEPTION(scope, void()); + JSC::JSPromise* fetchPromise = entry->ensureFetchPromise(globalObject); + // fulfillPromise, not fulfill(): pipeFrom() already claimed this promise's resolving functions. + if (fetchPromise->status() == JSC::JSPromise::Status::Pending) + fetchPromise->fulfillPromise(vm, jsSourceCode); } JSValue fetchCommonJSModule( diff --git a/test/js/bun/resolve/require-esm-in-flight-sibling.test.ts b/test/js/bun/resolve/require-esm-in-flight-sibling.test.ts index 231e58409ce5..cb8ba22c4573 100644 --- a/test/js/bun/resolve/require-esm-in-flight-sibling.test.ts +++ b/test/js/bun/resolve/require-esm-in-flight-sibling.test.ts @@ -1,55 +1,37 @@ -// A CommonJS module imported by an ESM graph has its body evaluated while the -// graph is still loading (during the loader's makeModule step). That body can -// require() an ESM sibling the graph already started fetching on the -// transpiler thread, so the sibling's registry entry is mid-fetch: status -// Fetching with a pending fetch promise. require(esm)'s synchronous load used -// to leave that pending promise untouched and threw a spurious -// `require() async module "..." is unsupported. use "await import()" instead.` -// TypeError even though the sibling has no top-level await. -// -// e.mjs is padded with exports so its transpile reliably loses the race to -// d.cjs's require(). The race is probabilistic per run; concurrent runs make -// an unfixed bun fail with near certainty. import { expect, test } from "bun:test"; import { bunEnv, bunExe, tempDir } from "harness"; -test("require() of an ESM sibling that is still transpiling completes synchronously", async () => { - const pad = Array.from({ length: 3000 }, (_, i) => `export const pad${i} = ${i};`).join("\n"); +// A CommonJS module imported by an ESM graph runs its body while the graph is +// still loading, so a require() in it can hit an ESM sibling whose transpile is +// still in flight on the transpiler thread. The CJS module here is named +// reflect-metadata because the runtime transpiles that package on the main +// thread (ALWAYS_SYNC_MODULES in src/runtime/jsc_hooks.rs): its body runs in the +// microtask checkpoint right after the graph's fetches are issued, before the +// thread pool can have delivered e.mjs, so the sibling is mid-fetch on every run. +test("require() of an ESM sibling the import graph is still fetching loads it synchronously", async () => { using dir = tempDir("require-esm-in-flight", { - "entry.mjs": `import "./a.mjs"; -import "./c.mjs"; -console.log((globalThis.o ??= []).concat("entry").join(","));`, - "a.mjs": `import "./b.cjs"; -(globalThis.o ??= []).push("a"); -export const a = 1;`, - "b.cjs": `require("./c.mjs"); -(globalThis.o ??= []).push("b"); -module.exports = {};`, - "c.mjs": `import "./d.cjs"; -import "./e.mjs"; -(globalThis.o ??= []).push("c"); -export const c = 1;`, - "d.cjs": `require("./e.mjs"); -(globalThis.o ??= []).push("d"); -module.exports = {};`, - "e.mjs": `(globalThis.o ??= []).push("e"); -export const e = 1; -${pad}`, + "entry.mjs": ` + import "./e.mjs"; + import "reflect-metadata"; + console.log(globalThis.order.concat("entry").join(",")); + `, + "e.mjs": ` + (globalThis.order ??= []).push("e"); + export const e = 1; + `, + "node_modules/reflect-metadata/package.json": JSON.stringify({ name: "reflect-metadata", main: "index.js" }), + "node_modules/reflect-metadata/index.js": ` + const { e } = require("../../e.mjs"); + (globalThis.order ??= []).push("polyfill:" + e); + `, }); - await Promise.all( - Array.from({ length: 12 }, async () => { - await using proc = Bun.spawn({ - cmd: [bunExe(), "entry.mjs"], - env: bunEnv, - cwd: String(dir), - stderr: "pipe", - }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect(stderr).toBe(""); - expect(stdout).toBe("e,d,c,b,a,entry\n"); - expect(exitCode).toBe(0); - }), - ); - // 12 subprocess spawns under a debug/ASAN build need more than the 5s default. -}, 30_000); + await using proc = Bun.spawn({ + cmd: [bunExe(), "entry.mjs"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "e,polyfill:1,entry\n", stderr: "", exitCode: 0 }); +}); diff --git a/test/regression/issue/33180.test.ts b/test/regression/issue/33180.test.ts index a3753bd1d2f3..04258d319770 100644 --- a/test/regression/issue/33180.test.ts +++ b/test/regression/issue/33180.test.ts @@ -1,21 +1,8 @@ // https://github.com/oven-sh/bun/issues/33180 -// -// A CommonJS module imported by an ESM graph has its body run while the graph -// is still loading. If that body require()s a plain ESM module (no top-level -// await) that the same graph is also importing, the module's registry entry is -// still mid-fetch, and 1.3.14 misreported it as -// TypeError: require() async module "..." is unsupported. use "await import()" instead. -// 1.3.11 and Node both load it synchronously. import { expect, test } from "bun:test"; import { bunEnv, bunExe, tempDir } from "harness"; test.concurrent("require() of an ESM module the surrounding import graph is still fetching", async () => { - // Deterministic: starter.cjs is a one-line module, so its body runs as soon - // as it is delivered, before a.cjs and b.mjs have finished fetching, on - // every run. b.mjs importing a.cjs back while a.cjs is still evaluating - // additionally checks that the synchronous load hands the ESM side the live - // exports of the half-evaluated CJS module and does not run it twice. Node - // prints the same JSON. using dir = tempDir("issue-33180-cycle", { "entry.mjs": ` import "./starter.cjs"; @@ -51,9 +38,6 @@ test.concurrent("require() of an ESM module the surrounding import graph is stil }); test.concurrent("consecutive require()s of several ESM siblings that are still transpiling", async () => { - // The shape from the issue: the siblings are large so their transpile on the - // transpiler thread is still in flight when the tiny CJS module's body runs - // and require()s each of them in turn. const files: Record = { "entry.mjs": ` import "./e1.mjs"; From f4bd0ba958a968914c440e68a1b75c86cec1187a Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 03:02:34 +0000 Subject: [PATCH 4/9] require(esm): shorten provideFetchForSyncLoad comment --- src/jsc/bindings/ModuleLoader.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/jsc/bindings/ModuleLoader.cpp b/src/jsc/bindings/ModuleLoader.cpp index b17b40db4c11..788e6b6ad4dd 100644 --- a/src/jsc/bindings/ModuleLoader.cpp +++ b/src/jsc/bindings/ModuleLoader.cpp @@ -655,10 +655,8 @@ void evaluateCommonJSCustomExtension( RETURN_IF_EXCEPTION(scope, ); } -// Registers source for a synchronous require(esm) load. An entry the -// surrounding import graph is still fetching on the transpiler thread is -// settled with this source (provideFetch() would ignore it); the transpiler's -// own result later lands on a settled promise and is dropped. +// provideFetch() ignores an entry the import graph is already fetching, so settle that entry's +// fetch promise directly; the transpiler thread's result later finds it settled and is dropped. static void provideFetchForSyncLoad(Zig::GlobalObject* globalObject, const WTF::String& specifier, JSC::JSSourceCode* jsSourceCode) { auto& vm = JSC::getVM(globalObject); From a671e799aa183dcbfb71a963fdbe0f7ef21592e5 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 03:11:20 +0000 Subject: [PATCH 5/9] require(esm): drop provideFetchForSyncLoad header comment --- src/jsc/bindings/ModuleLoader.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/jsc/bindings/ModuleLoader.cpp b/src/jsc/bindings/ModuleLoader.cpp index 788e6b6ad4dd..b1c5cb9b1a0e 100644 --- a/src/jsc/bindings/ModuleLoader.cpp +++ b/src/jsc/bindings/ModuleLoader.cpp @@ -655,8 +655,6 @@ void evaluateCommonJSCustomExtension( RETURN_IF_EXCEPTION(scope, ); } -// provideFetch() ignores an entry the import graph is already fetching, so settle that entry's -// fetch promise directly; the transpiler thread's result later finds it settled and is dropped. static void provideFetchForSyncLoad(Zig::GlobalObject* globalObject, const WTF::String& specifier, JSC::JSSourceCode* jsSourceCode) { auto& vm = JSC::getVM(globalObject); @@ -672,7 +670,8 @@ static void provideFetchForSyncLoad(Zig::GlobalObject* globalObject, const WTF:: entry->ensureModulePromise(globalObject); RETURN_IF_EXCEPTION(scope, void()); JSC::JSPromise* fetchPromise = entry->ensureFetchPromise(globalObject); - // fulfillPromise, not fulfill(): pipeFrom() already claimed this promise's resolving functions. + // fulfillPromise, not fulfill(): pipeFrom() already claimed this promise for the transpiler + // thread's result, which will find it settled and be dropped. if (fetchPromise->status() == JSC::JSPromise::Status::Pending) fetchPromise->fulfillPromise(vm, jsSourceCode); } From e05032d4fb5cef430f33e2a0539ca41814241f20 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 03:29:31 +0000 Subject: [PATCH 6/9] require(esm): symmetric exception checks in provideFetchForSyncLoad --- src/jsc/bindings/ModuleLoader.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/jsc/bindings/ModuleLoader.cpp b/src/jsc/bindings/ModuleLoader.cpp index b1c5cb9b1a0e..d7d8c4827179 100644 --- a/src/jsc/bindings/ModuleLoader.cpp +++ b/src/jsc/bindings/ModuleLoader.cpp @@ -670,8 +670,8 @@ static void provideFetchForSyncLoad(Zig::GlobalObject* globalObject, const WTF:: entry->ensureModulePromise(globalObject); RETURN_IF_EXCEPTION(scope, void()); JSC::JSPromise* fetchPromise = entry->ensureFetchPromise(globalObject); - // fulfillPromise, not fulfill(): pipeFrom() already claimed this promise for the transpiler - // thread's result, which will find it settled and be dropped. + RETURN_IF_EXCEPTION(scope, void()); + // fulfillPromise, not fulfill(): pipeFrom() already claimed this promise's resolving functions. if (fetchPromise->status() == JSC::JSPromise::Status::Pending) fetchPromise->fulfillPromise(vm, jsSourceCode); } From 2e2870f14da3e83d4803f8b7bb4ebcc828d7ad32 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 04:55:00 +0000 Subject: [PATCH 7/9] ci: retrigger From 5968a2f40e08715a60e572d55f00679a0bd551d0 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 10:36:37 +0000 Subject: [PATCH 8/9] require(esm): only settle the JavaScript-typed registry entry registryEntry() falls back to an entry of any type, so a require() racing an in-flight import of the same file with a type attribute fulfilled that typed entry with default-loader output (and scanned the whole module map whenever there was no entry at all). Look up the JavaScript-typed entry directly, which is the one provideFetch() would create and loadModuleSync() reads. Tests: the typed-import race, and the sync-transpiler configuration in which the sibling's fetch promise is already settled when the require() lands. --- src/jsc/bindings/ModuleLoader.cpp | 32 +++++--- .../require-esm-in-flight-sibling.test.ts | 79 ++++++++++++++++--- 2 files changed, 90 insertions(+), 21 deletions(-) diff --git a/src/jsc/bindings/ModuleLoader.cpp b/src/jsc/bindings/ModuleLoader.cpp index d7d8c4827179..9f4d357a3c02 100644 --- a/src/jsc/bindings/ModuleLoader.cpp +++ b/src/jsc/bindings/ModuleLoader.cpp @@ -660,20 +660,30 @@ static void provideFetchForSyncLoad(Zig::GlobalObject* globalObject, const WTF:: auto& vm = JSC::getVM(globalObject); auto scope = DECLARE_THROW_SCOPE(vm); auto key = JSC::Identifier::fromString(vm, specifier); - auto* loader = globalObject->moduleLoader(); - auto* entry = loader->registryEntry(key); - if (!entry || entry->status() != JSC::ModuleRegistryEntry::Status::Fetching) { + // Only the JavaScript-typed entry is ours: it is the one loadModuleSync() reads, and this source came + // from the default loader. An import of the same file with a type attribute has a separate entry. + auto* entry = globalObject->moduleLoader()->ensureRegistered(globalObject, key, JSC::ScriptFetchParameters::Type::JavaScript); + switch (entry->status()) { + case JSC::ModuleRegistryEntry::Status::New: scope.release(); - loader->provideFetch(globalObject, key, JSC::ScriptFetchParameters::Type::JavaScript, jsSourceCode); + entry->provideFetch(globalObject, jsSourceCode); + return; + case JSC::ModuleRegistryEntry::Status::Fetching: { + entry->ensureModulePromise(globalObject); + RETURN_IF_EXCEPTION(scope, void()); + JSC::JSPromise* fetchPromise = entry->ensureFetchPromise(globalObject); + RETURN_IF_EXCEPTION(scope, void()); + // fulfillPromise, not fulfill(): pipeFrom() already claimed this promise's resolving functions. + // A fetch promise that is already settled has its FetchSettled reaction queued elsewhere; + // hostLoadImportedModule replays that step inline when loadModuleSync() gets there. + if (fetchPromise->status() == JSC::JSPromise::Status::Pending) + fetchPromise->fulfillPromise(vm, jsSourceCode); + return; + } + default: + // Fetched, or failed earlier: loadModuleSync() reports whatever the entry already holds. return; } - entry->ensureModulePromise(globalObject); - RETURN_IF_EXCEPTION(scope, void()); - JSC::JSPromise* fetchPromise = entry->ensureFetchPromise(globalObject); - RETURN_IF_EXCEPTION(scope, void()); - // fulfillPromise, not fulfill(): pipeFrom() already claimed this promise's resolving functions. - if (fetchPromise->status() == JSC::JSPromise::Status::Pending) - fetchPromise->fulfillPromise(vm, jsSourceCode); } JSValue fetchCommonJSModule( diff --git a/test/js/bun/resolve/require-esm-in-flight-sibling.test.ts b/test/js/bun/resolve/require-esm-in-flight-sibling.test.ts index cb8ba22c4573..f1dac72935de 100644 --- a/test/js/bun/resolve/require-esm-in-flight-sibling.test.ts +++ b/test/js/bun/resolve/require-esm-in-flight-sibling.test.ts @@ -8,21 +8,76 @@ import { bunEnv, bunExe, tempDir } from "harness"; // thread (ALWAYS_SYNC_MODULES in src/runtime/jsc_hooks.rs): its body runs in the // microtask checkpoint right after the graph's fetches are issued, before the // thread pool can have delivered e.mjs, so the sibling is mid-fetch on every run. -test("require() of an ESM sibling the import graph is still fetching loads it synchronously", async () => { - using dir = tempDir("require-esm-in-flight", { +const inFlightSibling = { + "entry.mjs": ` + import "./e.mjs"; + import "reflect-metadata"; + console.log(globalThis.order.concat("entry").join(",")); + `, + "e.mjs": ` + (globalThis.order ??= []).push("e"); + export const e = 1; + `, + "node_modules/reflect-metadata/package.json": JSON.stringify({ name: "reflect-metadata", main: "index.js" }), + "node_modules/reflect-metadata/index.js": ` + const { e } = require("../../e.mjs"); + (globalThis.order ??= []).push("polyfill:" + e); + `, +}; + +test.concurrent("require() of an ESM sibling the import graph is still fetching loads it synchronously", async () => { + using dir = tempDir("require-esm-in-flight", inFlightSibling); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "entry.mjs"], + env: bunEnv, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "e,polyfill:1,entry\n", stderr: "", exitCode: 0 }); +}); + +// With the transpiler thread disabled the graph's fetches settle on the spot, +// so when the require() lands the sibling's entry is still Fetching but its +// fetch promise is already settled and only the reaction that builds the module +// is still queued. That is the one state the require() path must leave alone. +test.concurrent("require() of a sibling whose fetch has settled but not been processed yet", async () => { + using dir = tempDir("require-esm-settled-sibling", inFlightSibling); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "entry.mjs"], + env: { ...bunEnv, BUN_FEATURE_FLAG_DISABLE_ASYNC_TRANSPILER: "1" }, + cwd: String(dir), + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, stderr, exitCode }).toEqual({ stdout: "e,polyfill:1,entry\n", stderr: "", exitCode: 0 }); +}); + +// An import with a type attribute gets its own registry entry, keyed by that +// type, and is transpiled with that loader. A require() of the same file is a +// plain JavaScript-typed load and must not hand its default-loader output to +// the typed entry that happens to be in flight. `a < b > (c)` is a generic call +// under the ts loader and two comparisons under the default loader for a .js +// file, so each consumer reveals which transpile it received. +test.concurrent("require() racing a typed import of the same file does not overwrite the typed import", async () => { + using dir = tempDir("require-esm-typed-sibling", { "entry.mjs": ` - import "./e.mjs"; + import { y } from "./mod.js" with { type: "ts" }; import "reflect-metadata"; - console.log(globalThis.order.concat("entry").join(",")); + console.log(JSON.stringify({ typedImport: y, require: globalThis.required, evaluations: globalThis.evaluations })); `, - "e.mjs": ` - (globalThis.order ??= []).push("e"); - export const e = 1; + "mod.js": ` + globalThis.evaluations = (globalThis.evaluations ?? 0) + 1; + const a = value => "called(" + value + ")"; + const b = 1; + const c = 3; + export const y = a < b > (c); `, "node_modules/reflect-metadata/package.json": JSON.stringify({ name: "reflect-metadata", main: "index.js" }), "node_modules/reflect-metadata/index.js": ` - const { e } = require("../../e.mjs"); - (globalThis.order ??= []).push("polyfill:" + e); + globalThis.required = require("../../mod.js").y; `, }); @@ -33,5 +88,9 @@ test("require() of an ESM sibling the import graph is still fetching loads it sy stderr: "pipe", }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); - expect({ stdout, stderr, exitCode }).toEqual({ stdout: "e,polyfill:1,entry\n", stderr: "", exitCode: 0 }); + expect({ stdout, stderr, exitCode }).toEqual({ + stdout: JSON.stringify({ typedImport: "called(3)", require: false, evaluations: 2 }) + "\n", + stderr: "", + exitCode: 0, + }); }); From 9ac5986c3351764665269ff34662e906e7dfbb02 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Thu, 13 Aug 2026 11:17:19 +0000 Subject: [PATCH 9/9] require(esm): tighten provideFetchForSyncLoad comments --- src/jsc/bindings/ModuleLoader.cpp | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/jsc/bindings/ModuleLoader.cpp b/src/jsc/bindings/ModuleLoader.cpp index 9f4d357a3c02..90754ff7cd70 100644 --- a/src/jsc/bindings/ModuleLoader.cpp +++ b/src/jsc/bindings/ModuleLoader.cpp @@ -660,8 +660,7 @@ static void provideFetchForSyncLoad(Zig::GlobalObject* globalObject, const WTF:: auto& vm = JSC::getVM(globalObject); auto scope = DECLARE_THROW_SCOPE(vm); auto key = JSC::Identifier::fromString(vm, specifier); - // Only the JavaScript-typed entry is ours: it is the one loadModuleSync() reads, and this source came - // from the default loader. An import of the same file with a type attribute has a separate entry. + // The JavaScript-typed entry is the one loadModuleSync() reads; a typed import has its own entry. auto* entry = globalObject->moduleLoader()->ensureRegistered(globalObject, key, JSC::ScriptFetchParameters::Type::JavaScript); switch (entry->status()) { case JSC::ModuleRegistryEntry::Status::New: @@ -674,8 +673,6 @@ static void provideFetchForSyncLoad(Zig::GlobalObject* globalObject, const WTF:: JSC::JSPromise* fetchPromise = entry->ensureFetchPromise(globalObject); RETURN_IF_EXCEPTION(scope, void()); // fulfillPromise, not fulfill(): pipeFrom() already claimed this promise's resolving functions. - // A fetch promise that is already settled has its FetchSettled reaction queued elsewhere; - // hostLoadImportedModule replays that step inline when loadModuleSync() gets there. if (fetchPromise->status() == JSC::JSPromise::Status::Pending) fetchPromise->fulfillPromise(vm, jsSourceCode); return;