Fix dynamic import() resolving before TLA module finishes evaluating

src/bun.js/bindings/ZigGlobalObject.cpp

@@ -2711,6 +2711,19 @@ void GlobalObject::addBuiltinGlobals(JSC::VM& vm)
 
     putDirectBuiltinFunction(vm, this, builtinNames.overridableRequirePrivateName(), commonJSOverridableRequireCodeGenerator(vm), 0);
 
+    // Override the stock WebKit `requestImportModule` builtin on the
+    // JSModuleLoader with a version that correctly waits for an in-flight
+    // top-level-await evaluation before resolving. The stock builtin sets
+    // `entry.evaluated = true` synchronously at the start of async
+    // evaluation, which causes a concurrent dynamic `import()` of the same
+    // module to take a fast path and return the namespace before the TLA
+    // completes. See https://github.com/oven-sh/bun/issues/29221.
+    //
+    // The JSC C++ `JSModuleLoader::requestImportModule` looks this function
+    // up dynamically by its private-name property, so installing our version
+    // here replaces both C++-initiated and JS-initiated dynamic imports.
+    this->moduleLoader()->putDirectBuiltinFunction(vm, this, vm.propertyNames->builtinNames().requestImportModulePublicName(), moduleLoaderOverridesRequestImportModuleCodeGenerator(vm), PropertyAttribute::Builtin | PropertyAttribute::DontEnum);
+
     putDirectNativeFunction(vm, this, builtinNames.createUninitializedArrayBufferPrivateName(), 1, functionCreateUninitializedArrayBuffer, ImplementationVisibility::Public, NoIntrinsic, PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly);
     putDirectNativeFunction(vm, this, builtinNames.resolveSyncPrivateName(), 1, functionImportMeta__resolveSyncPrivate, ImplementationVisibility::Public, NoIntrinsic, PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly);
     putDirectNativeFunction(vm, this, builtinNames.createInternalModuleByIdPrivateName(), 1, InternalModuleRegistry::jsCreateInternalModuleById, ImplementationVisibility::Public, NoIntrinsic, PropertyAttribute::DontDelete | PropertyAttribute::ReadOnly);

src/js/builtins/ModuleLoaderOverrides.ts

@@ -0,0 +1,82 @@
+// Override(s) installed on the JSC `JSModuleLoader` object at Zig::GlobalObject
+// construction. Each builtin in this file is installed by
+// ZigGlobalObject::finishCreation via `moduleLoader->putDirectBuiltinFunction`.
+//
+// Why this file exists: the stock WebKit builtin `ModuleLoader.js`'s
+// `requestImportModule` has a fast path that returns the module namespace
+// synchronously when `entry.evaluated` is set, even though for a top-level
+// await (TLA) module `entry.evaluated` is set *before* evaluation has
+// actually completed. As a result, a second dynamic `import()` of the same
+// module while the first is still mid-TLA resolves its promise before the
+// module finishes evaluating — breaking ECMA262 ContinueDynamicImport and
+// diverging from Node/Deno. See https://github.com/oven-sh/bun/issues/29221.
+//
+// Bun ships prebuilt WebKit, so patching the vendored `ModuleLoader.js`
+// isn't enough — we install this override on top of the existing builtin.
+// JSC's C++ `JSModuleLoader::requestImportModule` looks the function up
+// dynamically by its private-name property every time, so overriding the
+// property is sufficient for both C++-initiated and JS-initiated imports.
+//
+// The fix: cache the evaluation promise on the registry entry so concurrent
+// dynamic imports can `await` it instead of taking the early-return path.
+
+// `this` is the JSModuleLoader; `requestImportModule` is a builtin method.
+$visibility = "Private";
+export async function requestImportModule(
+  this: any,
+  moduleName: string,
+  referrer: unknown,
+  parameters: unknown,
+  fetcher: unknown,
+) {
+  "use strict";
+
+  const key = moduleName;
+  let entry = this.ensureRegistered(key);
+  let mod: unknown;
+
+  // Fast path 1: entry already present with a module record.
+  //
+  // If evaluation is still in flight (TLA), `entry.evaluatingPromise` holds
+  // the async evaluation promise — wait on it before handing the namespace
+  // back. This is the key fix for issue #29221.
+  if (entry.evaluated && (mod = entry.module)) {
+    if (entry.evaluatingPromise) {
+      await entry.evaluatingPromise;
+    }
+    return this.getModuleNamespaceObject(mod);
+  }
+
+  entry = await this.requestSatisfy(entry, parameters, fetcher, new Set());
+
+  // Fast path 2: another caller raced us through requestSatisfy and already
+  // finished (or is in the middle of) evaluating.
+  if (entry.evaluated && (mod = entry.module)) {
+    if (entry.evaluatingPromise) {
+      await entry.evaluatingPromise;
+    }
+    return this.getModuleNamespaceObject(mod);
+  }
+
+  // Another concurrent import may have reached the eval step first.
+  if (entry.evaluatingPromise) {
+    await entry.evaluatingPromise;
+    return this.getModuleNamespaceObject(entry.module);
+  }
+
+  // First call to reach evaluation for this entry. `linkAndEvaluateModule`
+  // returns `moduleEvaluation(entry, fetcher)` directly, which for a TLA
+  // module is the promise returned by `asyncModuleEvaluation`. Cache it on
+  // the entry so any concurrent caller that slips through the fast paths
+  // can observe and await the same in-flight evaluation.
+  const evalResult = this.linkAndEvaluateModule(entry.key, fetcher);
+  if (evalResult && typeof (evalResult as any).then === "function") {
+    entry.evaluatingPromise = evalResult;
+    try {
+      await evalResult;
+    } finally {
+      entry.evaluatingPromise = undefined;
+    }
+  }
+  return this.getModuleNamespaceObject(entry.module);
+}

test/regression/issue/29221.test.ts

@@ -0,0 +1,54 @@
+// https://github.com/oven-sh/bun/issues/29221
+//
+// Dynamic `import()` of a module with top-level await must not resolve its
+// promise before the module finishes evaluating. Repeated `import()` calls
+// for the same module share one evaluation — both `.then()` handlers fire
+// AFTER the module's TLA settles, matching Node.js / Deno.
+//
+// Bug was in JSC's ModuleLoader.js builtin (`requestImportModule` /
+// `moduleEvaluation`): `entry.evaluated` was set synchronously at the start
+// of async evaluation, so a second `import()` in the same tick took a fast
+// path that returned the namespace without awaiting the pending TLA.
+
+import { expect, test } from "bun:test";
+import { bunEnv, bunExe, tempDir } from "harness";
+
+test("dynamic import waits for top-level await to settle (#29221)", async () => {
+  using dir = tempDir("issue-29221", {
+    "entry.mjs": `
+globalThis.order = [];
+const a = import("./tla.mjs").then(() => globalThis.order.push("then-a"));
+const b = import("./tla.mjs").then(() => globalThis.order.push("then-b"));
+await Promise.all([a, b]);
+console.log(JSON.stringify(globalThis.order));
+`,
+    "tla.mjs": `
+globalThis.order.push("tla-start");
+await new Promise((r) => setTimeout(r, 50));
+globalThis.order.push("tla-end");
+`,
+  });
+
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), String(dir) + "/entry.mjs"],
+    env: bunEnv,
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+
+  // Expected ordering: the TLA module runs to completion first (tla-start
+  // then tla-end), then BOTH .then() handlers fire in import order.
+  //
+  // Pre-fix, Bun produced ["tla-start","then-b","then-a","tla-end"] — the
+  // second import's `.then()` fired before the TLA even resumed, because
+  // the JSC builtin's fast path returned the namespace without awaiting
+  // the pending evaluation promise.
+  expect({
+    stdout: stdout.trim(),
+    exitCode,
+  }).toEqual({
+    stdout: `["tla-start","tla-end","then-a","then-b"]`,
+    exitCode: 0,
+  });
+});