Keep stored cycle functions and self-referencing helper copies refused

Two fixes for the CommonJS cycle check and the bundled-chunk splitter:
- During a cycle's initialization, a read of a partner's exported
  function is safe only where the value neither runs nor stays: an
  expression statement, typeof, void, !, an equality operand, or the test
  of if, while, do and ?:. A const alias (`const f = binding.fn`), an
  object or array literal, a return, or an argument can be called before
  the cycle settles, so these keep SC1016 (Node throws when the function
  reads a binding in its temporal dead zone). A top-level const that only
  publishes the function in an export statement, as the static rewrite
  spells an export-table re-export, stays a read.
- A copied helper whose initializer is a named function expression
  reaches itself through its own name. Each chunk gets its own copy, so
  that name must also be used only as a callee; otherwise the chunk keeps
  the package refused with the existing reason.

packages/compiler/src/frontend/npm-static-bundled-chunks.test.ts

@@ -150,11 +150,43 @@ console.log(e.outer, e.indirect, e.wrapped, e.nested, e.seen.join(","));
   expect(chunk).toContain("function wrap(n)");
 });
 
+test("a named function expression that only calls itself travels with a chunk", () => {
+  const answer = expectSameBehavior(`${keyedHelper}var fact = function self(n) {
+  return n <= 1 ? 1 : n * self(n - 1);
+};
+var kind = function self(self) {
+  return typeof self;
+};
+var outer = function self() {
+  var self = 2;
+  return self;
+};
+var require_g = __commonJS({
+  "g.js"(exports2, module2) {
+    module2.exports = [fact(5), kind(1), outer()];
+  }
+});
+console.log(require_g().join(","));
+`);
+  const chunk = answer.chunks[0]!.source;
+  expect(chunk).toContain("var fact = function self(n)");
+});
+
 test.each([
   ["exports it", `function tally() {}`, `module2.exports = { tally };`],
   ["reads its properties", `function tally() {}`, `exports2.calls = tally.calls;`],
   ["constructs it", `function tally() {}`, `exports2.made = new tally();`],
   ["stores an arrow helper", `var tally = () => 1;`, `exports2.fn = tally;`],
+  [
+    "calls a function that reads its own name as a value",
+    `var tally = function self() { self.n = (self.n || 0) + 1; return self.n; };`,
+    `exports2.count = tally();`,
+  ],
+  [
+    "calls a function whose parameter default reads its own name",
+    `var tally = function self(box = self) { box.n = (box.n || 0) + 1; return box.n; };`,
+    `exports2.count = tally();`,
+  ],
 ])("a chunk that %s keeps the package refused", (_, declaration, statement) => {
   const result = split(`${keyedHelper}${declaration}
 var require_f = __commonJS({

packages/compiler/src/frontend/npm-static-bundled-chunks.ts

@@ -209,6 +209,15 @@ class BundleScopes {
     return false;
   }
 
+  /** True when a scope inside `root` (exclusive) binds the reference's
+   * name. */
+  boundInside(id: ts.Identifier, root: ts.Node): boolean {
+    for (let n = id.parent; n !== undefined && n !== root; n = n.parent) {
+      if (this.of(n).has(id.text)) return true;
+    }
+    return false;
+  }
+
   /** True when the reference resolves to a top-level binding of `sf`. */
   topLevel(id: ts.Identifier, sf: ts.SourceFile): boolean {
     for (let n = id.parent; n !== undefined && n !== sf; n = n.parent) {
@@ -350,6 +359,25 @@ function isFunctionValued(node: ts.VariableDeclaration | ts.FunctionDeclaration)
   return init !== undefined && (ts.isArrowFunction(init) || ts.isFunctionExpression(init));
 }
 
+/** True when a named function expression initializer (`var f = function
+ * self() {}`) uses its own name only as a callee: `self` is the copied
+ * function, so it is as observable as the top-level binding. */
+function selfCalledOnly(
+  node: ts.VariableDeclaration | ts.FunctionDeclaration,
+  scopes: BundleScopes,
+): boolean {
+  if (!ts.isVariableDeclaration(node) || node.initializer === undefined) return true;
+  const fn = unparen(node.initializer);
+  if (!ts.isFunctionExpression(fn) || fn.name === undefined || fn.body === undefined) return true;
+  const self = fn.name.text;
+  // A parameter or `var` of the same name hides the function's own name.
+  const own = varScopedNames(fn.body);
+  for (const param of fn.parameters.nodes) addBindingNames(param.name, own);
+  if (own.has(self)) return true;
+  // Its parameter initializers see the name too.
+  return references(fn).every((r) => r.text !== self || scopes.boundInside(r, fn) || isCallee(r));
+}
+
 /** True when a reference is the function a call invokes: `f(...)`, or the
  * `(0, f)(...)` form that drops the receiver. */
 function isCallee(id: ts.Identifier): boolean {
@@ -748,7 +776,7 @@ function chunkSource(
       return why(`reads the bundle's top-level binding '${name}', which it cannot carry along`);
     // Each chunk gets its own copy, so a function's identity and its
     // properties must stay unobservable: the bundle only ever calls it.
-    if (isFunctionValued(node) && !calledOnly(name))
+    if (isFunctionValued(node) && (!calledOnly(name) || !selfCalledOnly(node, scopes)))
       return why(
         `uses the bundle's top-level function '${name}' as a value, which its chunks cannot share`,
       );

packages/compiler/src/frontend/program.ts

@@ -3199,11 +3199,66 @@ function cjsCycleRefusal7(
   return null;
 }
 
-/** True for a plain read `binding.name` (not a call, not a write) of the
- * cycle member `dep` whose export `name` is a top-level function
- * declaration nothing reassigns. Every member settles its exports before
- * entering the cycle, so Node reads that same function whenever the read
- * runs, and the compiled read resolves to the declaration itself. */
+/** True when `use` initializes a top-level const whose every other
+ * reference is a value of an export statement (`module.exports = { … }`
+ * or `exports.n = …`). */
+function publishedAlias7(use: ts.Node, decl: ts.Node | undefined): boolean {
+  if (
+    decl === undefined ||
+    !ts.isVariableDeclaration(decl) ||
+    decl.initializer !== use ||
+    !ts.isIdentifier(decl.name)
+  )
+    return false;
+  const list = decl.parent;
+  const stmt = list?.parent;
+  const sf = stmt?.parent;
+  if (
+    list === undefined ||
+    (list.flags & ts.NodeFlags.Const) === 0 ||
+    stmt === undefined ||
+    !ts.isVariableStatement(stmt) ||
+    sf === undefined ||
+    !ts.isSourceFile(sf)
+  )
+    return false;
+  const alias = decl.name;
+  const published = new Set<ts.Node>();
+  for (const st of sf.statements.nodes) {
+    const cjs = cjsExportAssignmentOf(st);
+    if (cjs === null) continue;
+    if (cjs.kind === "member") published.add(cjs.value);
+    else if (cjs.obj !== null) {
+      for (const p of cjs.obj.properties.nodes) {
+        if (ts.isPropertyAssignment(p) && !ts.isComputedPropertyName(p.name))
+          published.add(p.initializer);
+        else if (ts.isShorthandPropertyAssignment(p)) published.add(p.name);
+      }
+    }
+  }
+  let kept = false;
+  ts.walkPreorder(sf, (n) => {
+    if (kept) return "stop";
+    if (!ts.isIdentifier(n) || n === alias || n.text !== alias.text) return undefined;
+    const p = n.parent;
+    if (
+      p !== undefined &&
+      ((ts.isPropertyAccessExpression(p) && p.name === n) ||
+        (ts.isPropertyAssignment(p) && p.name === n))
+    )
+      return undefined;
+    if (!published.has(n)) kept = true;
+    return undefined;
+  });
+  return !kept;
+}
+
+/** True for a plain read `binding.name` (not a call, not a write, not a
+ * value that is stored or passed on) of the cycle member `dep` whose
+ * export `name` is a top-level function declaration nothing reassigns.
+ * Every member settles its exports before entering the cycle, so Node
+ * reads that same function whenever the read runs, and the compiled read
+ * resolves to the declaration itself. */
 function functionExportRead7(id: ts.Identifier, dep: ts.SourceFile): boolean {
   const access = id.parent;
   if (!ts.isPropertyAccessExpression(access) || access.expression !== id) return false;
@@ -3222,25 +3277,27 @@ function functionExportRead7(id: ts.Identifier, dep: ts.SourceFile): boolean {
     else break;
   }
   const parent = use.parent;
-  // A use that can run the function (a call, `new`, a tag, a member of it
-  // such as `.call`, or an argument another call receives) is not a read.
-  if (
-    (ts.isCallExpression(parent) && parent.expression === use) ||
-    (ts.isCallExpression(parent) && parent.arguments.nodes.includes(use as ts.Expression)) ||
-    ts.isNewExpression(parent) ||
-    (ts.isTaggedTemplateExpression(parent) && parent.tag === use) ||
-    (ts.isPropertyAccessExpression(parent) && parent.expression === use) ||
-    (ts.isElementAccessExpression(parent) && parent.expression === use) ||
-    ts.isDeleteExpression(parent) ||
-    ((ts.isPrefixUnaryExpression(parent) || ts.isPostfixUnaryExpression(parent)) &&
-      (parent.operator === ts.SyntaxKind.PlusPlusToken ||
-        parent.operator === ts.SyntaxKind.MinusMinusToken)) ||
+  // Only a use that neither runs the function nor keeps it is a read: a
+  // stored alias (`const f = binding.name`, `{ f: binding.name }`) or an
+  // argument can be called while the cycle initializes. A top-level const
+  // that only publishes it (`module.exports = { name: f }`, the static
+  // rewrite's re-export spelling) runs nothing: a module that reads the
+  // export back checks that read itself.
+  const observes =
+    publishedAlias7(use, parent) ||
+    ts.isExpressionStatement(parent) ||
+    ts.isTypeOfExpression(parent) ||
+    ts.isVoidExpression(parent) ||
+    (ts.isPrefixUnaryExpression(parent) && parent.operator === ts.SyntaxKind.ExclamationToken) ||
     (ts.isBinaryExpression(parent) &&
-      parent.left === use &&
-      parent.operatorToken.kind >= ts.SyntaxKind.FirstAssignment &&
-      parent.operatorToken.kind <= ts.SyntaxKind.LastAssignment)
-  )
-    return false;
+      (parent.operatorToken.kind === ts.SyntaxKind.EqualsEqualsEqualsToken ||
+        parent.operatorToken.kind === ts.SyntaxKind.ExclamationEqualsEqualsToken ||
+        parent.operatorToken.kind === ts.SyntaxKind.EqualsEqualsToken ||
+        parent.operatorToken.kind === ts.SyntaxKind.ExclamationEqualsToken)) ||
+    ((ts.isIfStatement(parent) || ts.isWhileStatement(parent) || ts.isDoStatement(parent)) &&
+      parent.expression === use) ||
+    (ts.isConditionalExpression(parent) && parent.condition === use);
+  if (!observes) return false;
   const name = access.name.text;
   // The exported value's identifier: a table entry or a member export.
   const values: ts.Expression[] = [];

packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2571-cjs-cycle-stored-function/main.ts.json

@@ -0,0 +1,6 @@
+{
+ "order": [
+  "<repo>/tests/fixtures/npm/cases/2571-cjs-cycle-stored-function/main.ts"
+ ],
+ "diags": []
+}

tests/fixtures/npm/cases/2571-cjs-cycle-stored-function/main.ts

@@ -0,0 +1,8 @@
+// Require cycles whose partner stores an export of index.js (in a const,
+// or in an object literal) and calls it while index is still
+// initializing: the export reads a const in its temporal dead zone, so
+// Node throws a ReferenceError, and scriptc refuses both cycles.
+import { show } from "gtcyclealias";
+import { show as showBox } from "gtcyclebox";
+
+console.log(show(), showBox());

tests/fixtures/npm/node_modules/gtcyclealias/dist/index.d.ts

@@ -0,0 +1,3 @@
+export declare const LIMIT = 3;
+export declare function show(): string;
+export declare function peek(): number;

tests/fixtures/npm/node_modules/gtcyclealias/dist/index.js

@@ -0,0 +1,37 @@
+var __defProp = Object.defineProperty;
+var __getOwnPropDesc = Object.getOwnPropertyDescriptor;
+var __getOwnPropNames = Object.getOwnPropertyNames;
+var __hasOwnProp = Object.prototype.hasOwnProperty;
+var __export = (target, all) => {
+  for (var name in all)
+    __defProp(target, name, { get: all[name], enumerable: true });
+};
+var __copyProps = (to, from, except, desc) => {
+  if (from && typeof from === "object" || typeof from === "function") {
+    for (let key of __getOwnPropNames(from))
+      if (!__hasOwnProp.call(to, key) && key !== except)
+        __defProp(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable });
+  }
+  return to;
+};
+var __toCommonJS = (mod) => __copyProps(__defProp({}, "__esModule", { value: true }), mod);
+var index_exports = {};
+__export(index_exports, {
+  LIMIT: () => LIMIT,
+  peek: () => peek,
+  show: () => show
+});
+module.exports = __toCommonJS(index_exports);
+var import_reader = require("./reader");
+const LIMIT = 3;
+function peek() {
+  return LIMIT;
+}
+function show() {
+  return (0, import_reader.describe)();
+}
+// Annotate the CommonJS export names for ESM import in node:
+0 && (module.exports = {
+  LIMIT,
+  show
+});

tests/fixtures/npm/node_modules/gtcyclealias/dist/reader.js

@@ -0,0 +1,34 @@
+var __defProp = Object.defineProperty;
+var __getOwnPropDesc = Object.getOwnPropertyDescriptor;
+var __getOwnPropNames = Object.getOwnPropertyNames;
+var __hasOwnProp = Object.prototype.hasOwnProperty;
+var __export = (target, all) => {
+  for (var name in all)
+    __defProp(target, name, { get: all[name], enumerable: true });
+};
+var __copyProps = (to, from, except, desc) => {
+  if (from && typeof from === "object" || typeof from === "function") {
+    for (let key of __getOwnPropNames(from))
+      if (!__hasOwnProp.call(to, key) && key !== except)
+        __defProp(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable });
+  }
+  return to;
+};
+var __toCommonJS = (mod) => __copyProps(__defProp({}, "__esModule", { value: true }), mod);
+var reader_exports = {};
+__export(reader_exports, {
+  captured: () => captured,
+  describe: () => describe
+});
+module.exports = __toCommonJS(reader_exports);
+var import_index = require("./index");
+const read = import_index.peek;
+const captured = read();
+function describe() {
+  return "limit " + captured;
+}
+// Annotate the CommonJS export names for ESM import in node:
+0 && (module.exports = {
+  captured,
+  describe
+});

tests/fixtures/npm/node_modules/gtcyclealias/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "gtcyclealias",
+  "version": "1.0.0",
+  "description": "esbuild 0.25.10 per-file CommonJS output shape of a cycle whose partner stores an exported function in a const and calls it while that function's const binding is uninitialized",
+  "main": "./dist/index.js",
+  "types": "./dist/index.d.ts"
+}

tests/fixtures/npm/node_modules/gtcyclebox/dist/index.d.ts

@@ -0,0 +1,3 @@
+export declare const LIMIT = 3;
+export declare function show(): string;
+export declare function peek(): number;

tests/fixtures/npm/node_modules/gtcyclebox/dist/index.js

@@ -0,0 +1,37 @@
+var __defProp = Object.defineProperty;
+var __getOwnPropDesc = Object.getOwnPropertyDescriptor;
+var __getOwnPropNames = Object.getOwnPropertyNames;
+var __hasOwnProp = Object.prototype.hasOwnProperty;
+var __export = (target, all) => {
+  for (var name in all)
+    __defProp(target, name, { get: all[name], enumerable: true });
+};
+var __copyProps = (to, from, except, desc) => {
+  if (from && typeof from === "object" || typeof from === "function") {
+    for (let key of __getOwnPropNames(from))
+      if (!__hasOwnProp.call(to, key) && key !== except)
+        __defProp(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable });
+  }
+  return to;
+};
+var __toCommonJS = (mod) => __copyProps(__defProp({}, "__esModule", { value: true }), mod);
+var index_exports = {};
+__export(index_exports, {
+  LIMIT: () => LIMIT,
+  peek: () => peek,
+  show: () => show
+});
+module.exports = __toCommonJS(index_exports);
+var import_reader = require("./reader");
+const LIMIT = 3;
+function peek() {
+  return LIMIT;
+}
+function show() {
+  return (0, import_reader.describe)();
+}
+// Annotate the CommonJS export names for ESM import in node:
+0 && (module.exports = {
+  LIMIT,
+  show
+});

tests/fixtures/npm/node_modules/gtcyclebox/dist/reader.js

@@ -0,0 +1,34 @@
+var __defProp = Object.defineProperty;
+var __getOwnPropDesc = Object.getOwnPropertyDescriptor;
+var __getOwnPropNames = Object.getOwnPropertyNames;
+var __hasOwnProp = Object.prototype.hasOwnProperty;
+var __export = (target, all) => {
+  for (var name in all)
+    __defProp(target, name, { get: all[name], enumerable: true });
+};
+var __copyProps = (to, from, except, desc) => {
+  if (from && typeof from === "object" || typeof from === "function") {
+    for (let key of __getOwnPropNames(from))
+      if (!__hasOwnProp.call(to, key) && key !== except)
+        __defProp(to, key, { get: () => from[key], enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable });
+  }
+  return to;
+};
+var __toCommonJS = (mod) => __copyProps(__defProp({}, "__esModule", { value: true }), mod);
+var reader_exports = {};
+__export(reader_exports, {
+  captured: () => captured,
+  describe: () => describe
+});
+module.exports = __toCommonJS(reader_exports);
+var import_index = require("./index");
+const box = { read: import_index.peek };
+const captured = box.read();
+function describe() {
+  return "limit " + captured;
+}
+// Annotate the CommonJS export names for ESM import in node:
+0 && (module.exports = {
+  captured,
+  describe
+});

tests/fixtures/npm/node_modules/gtcyclebox/package.json

@@ -0,0 +1,7 @@
+{
+  "name": "gtcyclebox",
+  "version": "1.0.0",
+  "description": "esbuild 0.25.10 per-file CommonJS output shape of a cycle whose partner stores an exported function in an object literal and calls it while that function's const binding is uninitialized",
+  "main": "./dist/index.js",
+  "types": "./dist/index.d.ts"
+}

tests/harness/npm-cases.ts

@@ -19,12 +19,12 @@ export function npmCases(fixturesRoot: string): NpmCase[] {
   return [
     ...globSync(join(fixturesRoot, "npm/cases/*/main.ts"))
       .sort()
-      // 2465-2469 and 2556-2566 are the --npm-static bundler-emitted-CJS
+      // 2465-2469, 2556-2566 and 2571 are the --npm-static bundler-emitted-CJS
       // cases (npm-static.test.ts drives them with the opt-in): their
       // consumers exercise the static rewrite's surface (lexer-visible
       // names the shipped .d.ts never declares, the __toESM interop
       // family), so they stay out of the flagless island lane by design.
-      .filter((entry) => !/\/(246[5-9]|255[6-9]|256[0-24-6])-[^/]+\/main\.ts$/.test(entry))
+      .filter((entry) => !/\/(246[5-9]|255[6-9]|256[0-24-6]|2571)-[^/]+\/main\.ts$/.test(entry))
       .map((entry) => ({
         name: entry.split("/").at(-2)!,
         entry,

tests/harness/npm-static.test.ts

@@ -76,7 +76,7 @@ async function buildStatic(entry: string, npmStatic: string[] | "auto"): Promise
     ...globSync(
       join(fixturesRoot, "npm/node_modules/cryptozoo/**/*.{js,mjs,cjs,json,d.ts}"),
     ).sort(),
-    // the bundler-emitted-CJS mini packages (cases 2465-2469, 2556-2566)
+    // the bundler-emitted-CJS mini packages (cases 2465-2469, 2556-2566, 2571)
     ...globSync(join(fixturesRoot, "npm/node_modules/gt*/**/*.{js,cjs,json}")).sort(),
   ];
   for (const f of inputs) hash.update(f).update(readFileSync(f));
@@ -1482,6 +1482,21 @@ describe(`npm-static pilots${sanitize ? " (sanitized)" : ""}`, () => {
     ]);
   }, 120_000);
 
+  // 2571: the partner stores the export (in a const, or in an object
+  // literal) and calls it while the cycle initializes; both keep SC1016.
+  test("a CommonJS cycle that stores an export and calls it stays refused", () => {
+    const entry = join(fixturesRoot, "npm/cases/2571-cjs-cycle-stored-function/main.ts");
+    const { coverage } = analyze(entry, { npmStatic: ["gtcyclealias", "gtcyclebox"] });
+    for (const pkg of ["gtcyclealias", "gtcyclebox"])
+      expect(coverage.npmStatic).toContainEqual({
+        package: pkg,
+        status: "refused",
+        detail: expect.stringMatching(
+          /circular imports .*can use its binding 'import_index' of a module of the cycle/,
+        ) as string,
+      });
+  }, 120_000);
+
   // 2564: a chunk reads a top-level function whose name a nested scope of
   // the chunk also declares; the function still travels with the chunk.
   test("a chunk carries a top-level function its nested scopes shadow", async () => {