Fix spurious "require() async module" TypeError when require() hits an ESM module the import graph is still fetching

src/jsc/bindings/ModuleLoader.cpp

@@ -655,6 +655,30 @@
     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.
+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();
+    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;
+    }
+    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(
     Zig::GlobalObject* globalObject,
     JSCommonJSModule* target,
@@ -711,7 +735,7 @@
                     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, {});
@@ -766,7 +790,7 @@
                     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, {});
@@ -800,7 +824,7 @@
                 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, {});
@@ -898,7 +922,7 @@
         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,37 @@
+import { expect, test } from "bun:test";
+import { bunEnv, bunExe, tempDir } from "harness";
+
+// 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 "./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 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 });
+});

test/regression/issue/33180.test.ts

@@ -0,0 +1,75 @@
+// https://github.com/oven-sh/bun/issues/33180
+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 () => {
+  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 () => {
+  const files: Record<string, string> = {
+    "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,
+  });
+});