require(esm): throw ERR_REQUIRE_CYCLE_MODULE instead of returning TDZ namespace

src/bun.js/bindings/ErrorCode.ts

@@ -216,6 +216,7 @@ const errors: ErrorCodeMapping = [
   ["ERR_MYSQL_LIFETIME_TIMEOUT", Error, "MySQLError"],
   ["ERR_UNHANDLED_REJECTION", Error, "UnhandledPromiseRejection"],
   ["ERR_REQUIRE_ASYNC_MODULE", Error],
+  ["ERR_REQUIRE_CYCLE_MODULE", Error],
   ["ERR_S3_INVALID_ENDPOINT", Error],
   ["ERR_S3_INVALID_METHOD", Error],
   ["ERR_S3_INVALID_PATH", Error],

src/bun.js/bindings/ZigGlobalObject.cpp

@@ -740,8 +740,40 @@
     bool entryExistedBefore = false;
     if (auto* entry = loader->registryEntry(key)) {
         entryExistedBefore = true;
-        if (isModuleEvaluated(entry->record())) {
-            auto* ns = entry->record()->getModuleNamespace(globalObject, false);
+        auto* record = entry->record();
+        if (isModuleEvaluated(record)) {
+            auto* ns = record->getModuleNamespace(globalObject, false);
+            RETURN_IF_EXCEPTION(scope, {});
+            return JSValue::encode(ns);
+        }
+        // The record already exists at status Evaluating, i.e. it is on an
+        // outer InnerModuleEvaluation stack that we are re-entering. Calling
+        // loadModuleSync here would reach CyclicModuleRecord::evaluate() and
+        // violate its precondition (status must be LINKED, EVALUATING-ASYNC,
+        // or EVALUATED), so handle this case directly instead.
+        if (auto* cyclic = dynamicDowncast<JSC::CyclicModuleRecord>(record); cyclic && cyclic->status() == JSC::CyclicModuleRecord::Status::Evaluating) {
+            // ExecuteModule sets the generator State field from Init -> Executing
+            // before entering the body. State == Init therefore means the body
+            // has not started: the module is still in the requested-modules
+            // loop of InnerModuleEvaluation, and let/const/class exports are
+            // TDZ. Returning the namespace here is the regression that surfaces
+            // as a confusing "x is not a function" far from the cycle. Match
+            // Node's ERR_REQUIRE_CYCLE_MODULE instead.
+            JSValue state = record->internalField(JSC::AbstractModuleRecord::Field::State).get();
+            if (state.isInt32() && state.asInt32() == static_cast<int32_t>(JSC::AbstractModuleRecord::State::Init))
+                return Bun::throwError(globalObject, scope, Bun::ErrorCode::ERR_REQUIRE_CYCLE_MODULE, makeString("Cannot require() ES Module "_s, keyString, " in a cycle."_s));
+            // State != Init: ExecuteModule has been entered. For a non-TLA
+            // record that means the body ran to completion synchronously
+            // (status only flips to Evaluated once the SCC root pops the
+            // Tarjan stack). The namespace is fully populated; return it
+            // without re-driving the loader. For a TLA record reachable here
+            // the body is suspended at its first await (status flips to
+            // EvaluatingAsync only when the SCC root pops), so post-await
+            // bindings are TDZ — treat that the same as the EvaluatingAsync
+            // path below and surface the async-module error.
+            if (cyclic->hasTLA())
+                return throwVMTypeError(globalObject, scope, makeString("require() async module \""_s, keyString, "\" is unsupported. use \"await import()\" instead."_s));
+            auto* ns = record->getModuleNamespace(globalObject, false);
             RETURN_IF_EXCEPTION(scope, {});
             return JSValue::encode(ns);
         }

src/js/builtins/CommonJS.ts

@@ -103,17 +103,19 @@
 
   // -1 means we need to lookup the module from the ESM registry.
   if (out === -1) {
+    let namespace;
     try {
-      out = $requireESM(id);
+      namespace = $requireESM(id);
     } catch (exception) {
       // Since the ESM code is mostly JS, we need to handle exceptions here.
       $requireMap.$delete(id);
       throw exception;
     }
 
-    // If we can pull out a ModuleNamespaceObject, let's do it.
-    const namespace = $esmNamespaceForCjs(id);
+    // $requireESM returns the namespace object directly (it throws on
+    // undefined). Use that — re-reading via $esmNamespaceForCjs would miss the
+    // Evaluating-but-body-ran case that $esmLoadSync handles.
     if (namespace !== undefined) {
       // In Bun, when __esModule is not defined, it's a CustomAccessor on the prototype.
       // Various libraries expect __esModule to be set when using ESM from require().
       // We don't want to always inject the __esModule export into every module,
@@ -324,16 +326,18 @@
 
 export function requireESMFromHijackedExtension(this: JSCommonJSModule, id: string) {
   $assert(this);
+  let namespace;
   try {
-    $requireESM(id);
+    namespace = $requireESM(id);
   } catch (exception) {
     // Since the ESM code is mostly JS, we need to handle exceptions here.
     $requireMap.$delete(id);
     throw exception;
   }
 
-  // If we can pull out a ModuleNamespaceObject, let's do it.
-  const namespace = $esmNamespaceForCjs(id);
+  // $requireESM returns the namespace object directly (it throws on
+  // undefined). Use that — re-reading via $esmNamespaceForCjs would miss the
+  // Evaluating-but-body-ran case that $esmLoadSync handles.
   if (namespace !== undefined) {
     // In Bun, when __esModule is not defined, it's a CustomAccessor on the prototype.
     // Various libraries expect __esModule to be set when using ESM from require().

test/js/bun/resolve/require-esm-cycle.test.ts

@@ -0,0 +1,108 @@
+// require(esm) on a module that is already on the InnerModuleEvaluation stack
+// (status == Evaluating) used to fall through to loadModuleSync and return the
+// namespace object before the body ran, exposing TDZ let/const/class exports.
+// Now it must throw ERR_REQUIRE_CYCLE_MODULE — unless ExecuteModule has already
+// run for that record (an SCC sibling whose body completed but whose status
+// only flips to Evaluated once the SCC root pops), in which case the namespace
+// is fully populated and is still returned.
+import { expect, test } from "bun:test";
+import { bunEnv, bunExe, tempDir } from "harness";
+
+test("require(esm) on a module mid-evaluation throws ERR_REQUIRE_CYCLE_MODULE", async () => {
+  using dir = tempDir("require-esm-cycle-throw", {
+    "a.mjs": `
+      import "./b.mjs";
+      export const value = "ok";
+    `,
+    "b.mjs": `
+      import { createRequire } from "node:module";
+      const require = createRequire(import.meta.url);
+      let code = "no-throw";
+      try {
+        const a = require("./a.mjs");
+        // pre-fix: this access threw "Cannot access 'value' before initialization"
+        void a.value;
+      } catch (e) {
+        code = e.code ?? e.constructor.name;
+      }
+      console.log(code);
+    `,
+    "entry.mjs": `import "./a.mjs";`,
+  });
+
+  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.trim()).toBe("ERR_REQUIRE_CYCLE_MODULE");
+  expect(exitCode).toBe(0);
+});
+
+test("require(esm) on an SCC sibling whose body already ran returns the populated namespace", async () => {
+  // entry imports c then d. c imports entry back, so c and entry share an SCC;
+  // after c.execute() returns c stays at status Evaluating until entry (the
+  // SCC root) finishes. d's body then require()s c — c is at Evaluating with
+  // generator State != Init (body ran), so the const export is readable
+  // without re-driving the loader.
+  using dir = tempDir("require-esm-cycle-sibling", {
+    "entry.mjs": `
+      import "./c.mjs";
+      import "./d.mjs";
+      export const FROM_ENTRY = 1;
+    `,
+    "c.mjs": `
+      import "./entry.mjs";
+      export const FROM_C = "populated";
+    `,
+    "d.mjs": `
+      import { createRequire } from "node:module";
+      const require = createRequire(import.meta.url);
+      const c = require("./c.mjs");
+      console.log(c.FROM_C);
+    `,
+  });
+
+  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.trim()).toBe("populated");
+  expect(exitCode).toBe(0);
+});
+
+test("require(esm) cycle error names the requested module", async () => {
+  using dir = tempDir("require-esm-cycle-message", {
+    "a.mjs": `
+      import "./b.mjs";
+      export const value = 1;
+    `,
+    "b.mjs": `
+      import { createRequire } from "node:module";
+      const require = createRequire(import.meta.url);
+      try {
+        require("./a.mjs");
+      } catch (e) {
+        console.log(e.code, e.message.includes("a.mjs"));
+      }
+    `,
+  });
+
+  await using proc = Bun.spawn({
+    cmd: [bunExe(), "a.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.trim()).toBe("ERR_REQUIRE_CYCLE_MODULE true");
+  expect(exitCode).toBe(0);
+});