Fix spurious "require() async module" TypeError when a CJS module requires an ESM sibling mid-transpile

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;
     }

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);