From 85608b1440f282dc2de1456d8a461483aa6bf19a Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Tue, 20 Oct 2026 10:15:50 -0700 Subject: [PATCH 01/22] Export settled CommonJS variables by value, and members of var-bound function exports `module.exports = x` and `module.exports = { x }` copy x's value at that statement, so every var or let export was fenced. esbuild hoists a bundled module's top-level const to a var in its __commonJS chunk, so a bundle's `module.exports = toText2` stopped the program at load. The export now compiles when the declaration and every write sit in top-level code above it, outside functions and classes: the copied value is then the binding's final value. A write that can run later keeps the fence. The package-file rewrite that reaches members through a whole-function export now also takes a var or const bound to a function expression or arrow above the export, and treats bindings of required modules before the export as inert. Without it the export's members (.default, .object) read as undefined. Co-Authored-By: Claude Opus 5.5 --- .../src/frontend/lowering/binding-analysis.ts | 67 +++++++++++++++++ .../src/frontend/lowering/lower-stmts.ts | 17 ++++- .../npm-static-cjs-exports-alias.test.ts | 37 +++++++++ .../frontend/npm-static-cjs-exports-alias.ts | 52 ++++++++++++- .../compiler/test/cjs-settled-exports.test.ts | 75 +++++++++++++++++++ .../corpus/6484-cjs-settled-exports/class.cjs | 11 +++ tests/corpus/6484-cjs-settled-exports/lib.cjs | 9 +++ .../corpus/6484-cjs-settled-exports/main.cjs | 16 ++++ .../corpus/6484-cjs-settled-exports/sema.cjs | 13 ++++ .../corpus/6484-cjs-settled-exports/table.cjs | 7 ++ .../npm/cases/2591-bundled-var-export/main.ts | 8 ++ .../node_modules/gtvarexport/dist/index.js | 61 +++++++++++++++ .../npm/node_modules/gtvarexport/index.d.ts | 1 + .../npm/node_modules/gtvarexport/package.json | 7 ++ tests/harness/npm-cases.ts | 4 +- tests/harness/npm-static.test.ts | 10 ++- 16 files changed, 387 insertions(+), 8 deletions(-) create mode 100644 packages/compiler/test/cjs-settled-exports.test.ts create mode 100644 tests/corpus/6484-cjs-settled-exports/class.cjs create mode 100644 tests/corpus/6484-cjs-settled-exports/lib.cjs create mode 100644 tests/corpus/6484-cjs-settled-exports/main.cjs create mode 100644 tests/corpus/6484-cjs-settled-exports/sema.cjs create mode 100644 tests/corpus/6484-cjs-settled-exports/table.cjs create mode 100644 tests/fixtures/npm/cases/2591-bundled-var-export/main.ts create mode 100644 tests/fixtures/npm/node_modules/gtvarexport/dist/index.js create mode 100644 tests/fixtures/npm/node_modules/gtvarexport/index.d.ts create mode 100644 tests/fixtures/npm/node_modules/gtvarexport/package.json diff --git a/packages/compiler/src/frontend/lowering/binding-analysis.ts b/packages/compiler/src/frontend/lowering/binding-analysis.ts index 88b0f91dc8..7975f39cc3 100644 --- a/packages/compiler/src/frontend/lowering/binding-analysis.ts +++ b/packages/compiler/src/frontend/lowering/binding-analysis.ts @@ -61,6 +61,73 @@ export function bindingNeverReassigned(lowerer: Lowerer, sym: ts.Symbol, decl: t return !written; } +/** True when a module-scope variable's value is settled before the + * top-level statement `at` runs, and stays that way: its declaration is a + * top-level statement above `at`, and every write (the initializer + * included) sits in top-level code above `at`, outside any function or + * class body. A CommonJS export of the binding at `at` then copies the + * value the binding keeps forever, so reading the binding itself is + * exact (esbuild hoists a bundled module's top-level `const` to such a + * `var`). */ +export function bindingSettledBefore(lowerer: Lowerer, sym: ts.Symbol, at: ts.Statement): boolean { + const sf = at.getSourceFile(); + if (at.parent !== sf) return false; + const decls = sym.declarations; + if (decls.length !== 1) return false; + const decl = decls[0]!; + if (!ts.isVariableDeclaration(decl) || !ts.isIdentifier(decl.name)) return false; + const stmt = decl.parent?.parent; + if (stmt === undefined || !ts.isVariableStatement(stmt) || stmt.parent !== sf) return false; + const limit = at.getStart(sf); + if (stmt.getEnd() > limit) return false; + const symText = sym.name; + const namesSym = (e: ts.Node): boolean => + ts.isIdentifier(e) && e.text === symText && lowerer.resolveValueSymbol(e) === sym; + let settled = true; + // `nested`: inside a function or class body, where code can run later. + const scanTarget = (t: ts.Expression, nested: boolean): void => { + let e: ts.Expression = t; + while (ts.isParenthesizedExpression(e)) e = e.expression; + let hit = namesSym(e); + if (!hit && (ts.isArrayLiteralExpression(e) || ts.isObjectLiteralExpression(e))) { + const walk = (n: ts.Node): void => { + if (namesSym(n)) hit = true; + else n.forEachChild(walk); + }; + walk(e); + } + if (hit && (nested || t.getEnd() > limit)) settled = false; + }; + const visit = (n: ts.Node, nested: boolean): void => { + if (!settled) return; + const inner = + nested || + ts.isFunctionLike(n) || + ts.isClassDeclaration(n) || + ts.isClassExpression(n) || + ts.isClassStaticBlockDeclaration(n); + if (ts.isBinaryExpression(n)) { + const k = n.operatorToken.kind; + if (k >= ts.SyntaxKind.FirstAssignment && k <= ts.SyntaxKind.LastAssignment) { + scanTarget(n.left, nested); + } + } else if ( + (ts.isPrefixUnaryExpression(n) || ts.isPostfixUnaryExpression(n)) && + (n.operator === ts.SyntaxKind.PlusPlusToken || n.operator === ts.SyntaxKind.MinusMinusToken) + ) { + scanTarget(n.operand as ts.Expression, nested); + } else if ( + (ts.isForOfStatement(n) || ts.isForInStatement(n)) && + !ts.isVariableDeclarationList(n.initializer) + ) { + scanTarget(n.initializer as ts.Expression, nested); + } + n.forEachChild((c) => visit(c, inner)); + }; + sf.forEachChild((c) => visit(c, false)); + return settled; +} + /** Strips the value-preserving wrappers off an expression: parens, * non-null assertions, `as`/`satisfies`/angle-bracket casts. What * remains is the expression that actually evaluates. */ diff --git a/packages/compiler/src/frontend/lowering/lower-stmts.ts b/packages/compiler/src/frontend/lowering/lower-stmts.ts index f004ac15f8..3efb0d1826 100644 --- a/packages/compiler/src/frontend/lowering/lower-stmts.ts +++ b/packages/compiler/src/frontend/lowering/lower-stmts.ts @@ -139,6 +139,7 @@ import { } from "./generic-functions.js"; import { bindingNeverReassigned, + bindingSettledBefore, deadUnmappableBinding, nullishExprUnitOf, nullishGenericBindingUnitOf, @@ -6577,11 +6578,16 @@ function lowerCjsExportStatement( // through to the declaration) — EXACT for const/function bindings, a // live-binding lie for `let`: Node copies the VALUE into the table at // this statement, so later reassignments are invisible to importers. - // Mutable bindings fence instead of diverging from Node. + // Mutable bindings fence instead of diverging from Node, unless the + // value is settled before this statement and never written again. + const exportStmt = cjs.expr.parent; const fenceMutable = (nameNode: ts.Identifier): void => { const sym = lowerer.resolveValueSymbol(nameNode); const g = sym ? lowerer.globalsBySymbol.get(sym) : undefined; - if (g?.mutable) { + if ( + g?.mutable && + !(ts.isExpressionStatement(exportStmt) && bindingSettledBefore(lowerer, sym!, exportStmt)) + ) { lowerer.unsupported( "SC1090", nameNode, @@ -6721,7 +6727,12 @@ function lowerCjsSingleValueExport( } const g = lowerer.globalsBySymbol.get(sym); if (g) { - if (g.mutable) { + // A binding settled before this statement and never written again + // holds the value Node copies here (esbuild's hoisted `var`). + const exportStmt = expr.parent; + const settled = + ts.isExpressionStatement(exportStmt) && bindingSettledBefore(lowerer, sym, exportStmt); + if (g.mutable && !settled) { lowerer.unsupported( "SC1090", rhs, diff --git a/packages/compiler/src/frontend/npm-static-cjs-exports-alias.test.ts b/packages/compiler/src/frontend/npm-static-cjs-exports-alias.test.ts index c3704ea290..6042c9ad5a 100644 --- a/packages/compiler/src/frontend/npm-static-cjs-exports-alias.test.ts +++ b/packages/compiler/src/frontend/npm-static-cjs-exports-alias.test.ts @@ -79,3 +79,40 @@ console.log(Object.keys(holder).join(","), holder.exports === Version, holder.ex ${report}`); expect(rewritten).toContain("{ exports: Version }"); }); + +test("a var or const bound to a function is a function export too", () => { + // esbuild's hoisted spelling of a bundled module's top-level const, + // below the bundle's require bindings. + const viaVar = expectSameBehavior(`"use strict"; +var path = require("path"); +var Version = (v) => ({ v, base: path.basename(v) }); +module.exports = Version; +module.exports.default = Version; +module.exports.object = (v) => ({ value: v }); +console.log(module.exports.object(1).value, module.exports.default === Version); +${report}`); + expect(viaVar).toContain("Version.object = "); + const viaConst = expectSameBehavior(`const Version = function (v) { return { v }; }; +module.exports = Version; +module.exports.tag = "t"; +${report}`); + expect(viaConst).toContain('Version.tag = "t"'); +}); + +test.each([ + ["a let binding", `let Version = () => 1;\nmodule.exports = Version;\nmodule.exports.x = 1;\n`], + [ + "a reassigned var", + `var Version = () => 1;\nmodule.exports = Version;\nmodule.exports.x = 1;\nVersion = () => 2;\n`, + ], + [ + "a var below the export", + `module.exports = Version;\nmodule.exports.x = 1;\nvar Version = () => 1;\n`, + ], + [ + "a call above the export", + `var Version = () => 1;\nconsole.log("early");\nmodule.exports = Version;\nmodule.exports.x = 1;\n`, + ], +])("%s keeps the module as it is", (_, source) => { + expect(rewrite(source)).toBeNull(); +}); diff --git a/packages/compiler/src/frontend/npm-static-cjs-exports-alias.ts b/packages/compiler/src/frontend/npm-static-cjs-exports-alias.ts index dfe45b8624..c86aad099a 100644 --- a/packages/compiler/src/frontend/npm-static-cjs-exports-alias.ts +++ b/packages/compiler/src/frontend/npm-static-cjs-exports-alias.ts @@ -11,6 +11,11 @@ * (members on a function declaration), and drops the `exports =` * rebinding the static export protocol has no storage for. * + * The function may also be a `var` or `const` bound to a function + * expression or arrow above the statement (esbuild hoists a bundled + * module's top-level `const` to such a `var`): members on it are the same + * expando form. + * * Exact only while nothing can observe the module before the statement * runs and nothing replaces the export object again: the statement is a * top-level statement preceded only by inert declarations, it is the @@ -41,11 +46,38 @@ function isModuleExports(e: ts.Expression): e is ts.PropertyAccessExpression { ); } +/** `require("x")`, or esbuild's interop wrapper around one + * (`__toESM(require("x"))`, `__toESM(require("x"), 1)`). */ +function isRequireValue(expr: ts.Expression): boolean { + const e = unparen(expr); + if (!ts.isCallExpression(e) || !ts.isIdentifier(e.expression)) return false; + const args = e.arguments.nodes; + if (e.expression.text === "require") + return ( + args.length === 1 && + (ts.isStringLiteral(args[0]!) || ts.isNoSubstitutionTemplateLiteral(args[0]!)) + ); + if (e.expression.text === "__toESM") + return (args.length === 1 || args.length === 2) && isRequireValue(args[0]!); + return false; +} + /** True for a statement whose evaluation runs no code that could observe * the export object: declarations and member writes (`F.prototype.m = - * function …`) without calls, and directives. */ + * function …`) without calls, directives, and bindings of required + * modules (`var x = require("x")`: another module can reach this one's + * export object only through a require cycle, which preflight's cycle + * rules already judge). */ function inert(stmt: ts.Statement): boolean { if (ts.isFunctionDeclaration(stmt) || ts.isEmptyStatement(stmt)) return true; + if ( + ts.isVariableStatement(stmt) && + stmt.declarationList.declarations.nodes.every( + (d) => + ts.isIdentifier(d.name) && d.initializer !== undefined && isRequireValue(d.initializer), + ) + ) + return true; if (ts.isExpressionStatement(stmt) && ts.isStringLiteral(stmt.expression)) return true; if (ts.isExpressionStatement(stmt)) { const e = unparen(stmt.expression); @@ -104,8 +136,11 @@ export function normalizeCjsFunctionExports(sf: ts.SourceFile): string | null { } if (whole === null) return null; const fnName = whole.fn; + const wholeAt = stmts.indexOf(whole.stmt); const fnDecls = stmts.filter( - (st) => ts.isFunctionDeclaration(st) && st.name?.text === fnName && st.body !== undefined, + (st, i) => + (ts.isFunctionDeclaration(st) && st.name?.text === fnName && st.body !== undefined) || + (i < wholeAt && functionValuedVar(st, fnName)), ); // The respelled reads name the function from wherever they sit, so no // other binding of its name (a parameter, a local, a nested function) @@ -183,6 +218,19 @@ export function normalizeCjsFunctionExports(sf: ts.SourceFile): string | null { return applyEdits(source, 0, source.length, edits); } +/** `var name = function …` / `const name = (…) => …`: one declarator + * whose initializer is a function expression or arrow. */ +function functionValuedVar(stmt: ts.Statement, name: string): boolean { + if (!ts.isVariableStatement(stmt)) return false; + if ((stmt.declarationList.flags & ts.NodeFlags.Let) !== 0) return false; + const decls = stmt.declarationList.declarations.nodes; + if (decls.length !== 1) return false; + const d = decls[0]!; + if (!ts.isIdentifier(d.name) || d.name.text !== name || d.initializer === undefined) return false; + const init = unparen(d.initializer); + return ts.isFunctionExpression(init) || ts.isArrowFunction(init); +} + function insideFunction(node: ts.Node): boolean { for (let p = node.parent; p !== undefined && !ts.isSourceFile(p); p = p.parent) { if (ts.isFunctionLike(p)) return true; diff --git a/packages/compiler/test/cjs-settled-exports.test.ts b/packages/compiler/test/cjs-settled-exports.test.ts new file mode 100644 index 0000000000..74fa598efd --- /dev/null +++ b/packages/compiler/test/cjs-settled-exports.test.ts @@ -0,0 +1,75 @@ +/* A CommonJS export of a variable copies its value at the export + * statement. The lowering reads the variable itself instead, which is + * exact only when the value is settled above the export and never written + * again (tests/corpus/6484-cjs-settled-exports runs that shape against + * Node). A write that can run later keeps the located fence: here a + * function's write, and top-level code below the export. + */ +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, expect, test } from "vitest"; +import { analyze } from "../src/index.js"; + +const scratch = mkdtempSync(join(tmpdir(), "scriptc-cjs-settled-")); +afterAll(() => rmSync(scratch, { recursive: true, force: true })); + +function fences(files: Record): string[] { + const dir = mkdtempSync(join(scratch, "case-")); + writeFileSync(join(dir, "package.json"), '{ "private": true }\n'); + for (const [name, text] of Object.entries(files)) writeFileSync(join(dir, name), text); + const { coverage } = analyze(join(dir, "main.cjs")); + expect(coverage.preflightFailed).toBe(false); + return [...coverage.diagnostics, ...(coverage.runtimeFences ?? [])] + .filter((d) => d.message.includes("by reference")) + .map((d) => `${d.loc.file.slice(dir.length + 1)}: ${d.message.split(" (")[0]}`); +} + +test("exports of variables written later keep their fence", () => { + expect( + fences({ + "main.cjs": `"use strict"; +const counter = require("./counter.cjs"); +const handler = require("./handler.cjs"); +console.log(counter.count, handler()); +`, + "counter.cjs": `"use strict"; +let count = 0; +function bump() { + count++; +} +module.exports = { count, bump }; +`, + "handler.cjs": `"use strict"; +var handler = function () { + return 1; +}; +module.exports = handler; +handler = function () { + return 2; +}; +`, + }), + ).toEqual([ + "counter.cjs: exporting the mutable 'let' binding 'count' by reference", + "handler.cjs: exporting the mutable 'let' binding 'handler' by reference", + ]); +}); + +test("exports of variables settled above the export compile", () => { + expect( + fences({ + "main.cjs": `"use strict"; +const table = require("./table.cjs"); +console.log(table.total, table.label); +`, + "table.cjs": `"use strict"; +let total = 0; +for (const n of [1, 2, 3]) total += n; +var label = "sum"; +if (total > 5) label = "big sum"; +module.exports = { total, label }; +`, + }), + ).toEqual([]); +}); diff --git a/tests/corpus/6484-cjs-settled-exports/class.cjs b/tests/corpus/6484-cjs-settled-exports/class.cjs new file mode 100644 index 0000000000..f00076e8a0 --- /dev/null +++ b/tests/corpus/6484-cjs-settled-exports/class.cjs @@ -0,0 +1,11 @@ +"use strict"; +// A bundled class keeps esbuild's `var X = class` spelling. +var Counter = class { + constructor(start) { + this.n = start; + } + next() { + return ++this.n; + } +}; +module.exports = Counter; diff --git a/tests/corpus/6484-cjs-settled-exports/lib.cjs b/tests/corpus/6484-cjs-settled-exports/lib.cjs new file mode 100644 index 0000000000..90b8ac0bec --- /dev/null +++ b/tests/corpus/6484-cjs-settled-exports/lib.cjs @@ -0,0 +1,9 @@ +"use strict"; +// esbuild hoists a bundled module's top-level const to a var like this. +var toText = function (input) { + return "text:" + input; +}; +toText.object = function (input) { + return { value: input }; +}; +module.exports = toText; diff --git a/tests/corpus/6484-cjs-settled-exports/main.cjs b/tests/corpus/6484-cjs-settled-exports/main.cjs new file mode 100644 index 0000000000..0677b007b1 --- /dev/null +++ b/tests/corpus/6484-cjs-settled-exports/main.cjs @@ -0,0 +1,16 @@ +// A CommonJS export copies a variable's value at the export statement. +// When every write to the variable sits in top-level code above that +// statement (esbuild's hoisted var, a let settled by a loop), the copied +// value is the variable's final value, and the module compiles. That +// includes classes bound to a var or let, as bundles spell them. +"use strict"; +const toText = require("./lib.cjs"); +const table = require("./table.cjs"); +const Counter = require("./class.cjs"); +const { Gate } = require("./sema.cjs"); +console.log(toText("a"), JSON.stringify(toText.object(2))); +console.log(table.label, table.total); +const counter = new Counter(1); +console.log(counter.next(), counter.next()); +const gate = new Gate(1); +console.log(gate.enter(), gate.enter()); diff --git a/tests/corpus/6484-cjs-settled-exports/sema.cjs b/tests/corpus/6484-cjs-settled-exports/sema.cjs new file mode 100644 index 0000000000..7b26c7fbdf --- /dev/null +++ b/tests/corpus/6484-cjs-settled-exports/sema.cjs @@ -0,0 +1,13 @@ +"use strict"; +let Gate = class { + constructor(limit) { + this.limit = limit; + this.open = 0; + } + enter() { + if (this.open === this.limit) return false; + this.open++; + return true; + } +}; +module.exports = { Gate }; diff --git a/tests/corpus/6484-cjs-settled-exports/table.cjs b/tests/corpus/6484-cjs-settled-exports/table.cjs new file mode 100644 index 0000000000..b62ea96be2 --- /dev/null +++ b/tests/corpus/6484-cjs-settled-exports/table.cjs @@ -0,0 +1,7 @@ +"use strict"; +// Written only above the export, in top-level code. +let total = 0; +for (const n of [1, 2, 3]) total += n; +var label = "sum"; +if (total > 5) label = "big sum"; +module.exports = { total, label }; diff --git a/tests/fixtures/npm/cases/2591-bundled-var-export/main.ts b/tests/fixtures/npm/cases/2591-bundled-var-export/main.ts new file mode 100644 index 0000000000..014af6d917 --- /dev/null +++ b/tests/fixtures/npm/cases/2591-bundled-var-export/main.ts @@ -0,0 +1,8 @@ +// A bundled CommonJS dependency exports a function binding as its whole +// module (`module.exports = toText2`, then members on it). esbuild hoists +// the dependency's top-level const to a `var` in its __commonJS chunk, so +// the export names a var that nothing reassigns after the export. +import { demo } from "gtvarexport"; + +console.log(demo(["a", "b"])); +console.log(demo([])); diff --git a/tests/fixtures/npm/node_modules/gtvarexport/dist/index.js b/tests/fixtures/npm/node_modules/gtvarexport/dist/index.js new file mode 100644 index 0000000000..b71c451e20 --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtvarexport/dist/index.js @@ -0,0 +1,61 @@ +var __create = Object.create; +var __defProp = Object.defineProperty; +var __getOwnPropDesc = Object.getOwnPropertyDescriptor; +var __getOwnPropNames = Object.getOwnPropertyNames; +var __getProtoOf = Object.getPrototypeOf; +var __hasOwnProp = Object.prototype.hasOwnProperty; +var __commonJS = (cb, mod) => function __require() { + return mod || (0, cb[__getOwnPropNames(cb)[0]])((mod = { exports: {} }).exports, mod), mod.exports; +}; +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 __toESM = (mod, isNodeMode, target) => (target = mod != null ? __create(__getProtoOf(mod)) : {}, __copyProps( + // If the importer is in node compatibility mode or this is not an ESM + // file that has been converted to a CommonJS file using a Babel- + // compatible transform (i.e. "__esModule" has not been set), then set + // "default" to the CommonJS "module.exports" for node compatibility. + isNodeMode || !mod || !mod.__esModule ? __defProp(target, "default", { value: mod, enumerable: true }) : target, + mod +)); +var __toCommonJS = (mod) => __copyProps(__defProp({}, "__esModule", { value: true }), mod); + +// node_modules/gttotext/index.js +var require_gttotext = __commonJS({ + "node_modules/gttotext/index.js"(exports2, module2) { + "use strict"; + var toText2 = (input) => { + if (Array.isArray(input)) { + input = input.join(","); + } + return `text:${input}`; + }; + module2.exports = toText2; + module2.exports.default = toText2; + module2.exports.object = (input) => ({ value: input }); + } +}); + +// src/index.ts +var index_exports = {}; +__export(index_exports, { + demo: () => demo +}); +module.exports = __toCommonJS(index_exports); +var import_gttotext = __toESM(require_gttotext()); +function demo(input) { + return `${(0, import_gttotext.default)(input)} ${JSON.stringify(import_gttotext.default.object(input.length))}`; +} +// Annotate the CommonJS export names for ESM import in node: +0 && (module.exports = { + demo +}); diff --git a/tests/fixtures/npm/node_modules/gtvarexport/index.d.ts b/tests/fixtures/npm/node_modules/gtvarexport/index.d.ts new file mode 100644 index 0000000000..18b866a07e --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtvarexport/index.d.ts @@ -0,0 +1 @@ +export declare function demo(input: string[]): string; diff --git a/tests/fixtures/npm/node_modules/gtvarexport/package.json b/tests/fixtures/npm/node_modules/gtvarexport/package.json new file mode 100644 index 0000000000..316ad07e0e --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtvarexport/package.json @@ -0,0 +1,7 @@ +{ + "name": "gtvarexport", + "version": "1.0.0", + "description": "real esbuild 0.27 output (--bundle --platform=node --format=cjs) of a TypeScript source that default-imports a CommonJS dependency exporting a const function by `module.exports = toText` plus members (the into-stream shape); esbuild hoists the const to `var toText2` inside the __commonJS chunk", + "main": "dist/index.js", + "types": "index.d.ts" +} diff --git a/tests/harness/npm-cases.ts b/tests/harness/npm-cases.ts index 9f995d1c89..766a8f2aaa 100644 --- a/tests/harness/npm-cases.ts +++ b/tests/harness/npm-cases.ts @@ -19,14 +19,14 @@ export function npmCases(fixturesRoot: string): NpmCase[] { return [ ...globSync(join(fixturesRoot, "npm/cases/*/main.ts")) .sort() - // 2465-2469, 2556-2567, 2571 and 2580-2590 are the --npm-static bundler-emitted-CJS + // 2465-2469, 2556-2567, 2571 and 2580-2591 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-7]|2571|258[0-9]|2590)-[^/]+\/main\.ts$/.test(entry), + !/\/(246[5-9]|255[6-9]|256[0-7]|2571|258[0-9]|259[01])-[^/]+\/main\.ts$/.test(entry), ) .map((entry) => ({ name: entry.split("/").at(-2)!, diff --git a/tests/harness/npm-static.test.ts b/tests/harness/npm-static.test.ts index 66f37f6ecd..125fe33679 100644 --- a/tests/harness/npm-static.test.ts +++ b/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-2567, 2571, 2580-2590) + // the bundler-emitted-CJS mini packages (cases 2465-2469, 2556-2567, 2571, 2580-2591) ...globSync(join(fixturesRoot, "npm/node_modules/gt*/**/*.{js,cjs,json}")).sort(), ...globSync(join(fixturesRoot, "npm/store/gt*/**/*.{js,cjs,json}")).sort(), ]; @@ -1627,6 +1627,14 @@ describe(`npm-static pilots${sanitize ? " (sanitized)" : ""}`, () => { ]); }, 180_000); + // 2591: a bundled dependency whose whole export is a var esbuild hoisted + // from its const, never reassigned (into-stream's shape). + test("a bundled var export 2591-bundled-var-export byte-matches Node", async () => { + await expectNodeParity(join(fixturesRoot, "npm/cases/2591-bundled-var-export/main.ts"), [ + "gtvarexport", + ]); + }, 180_000); + // 2562: a file with unverifiable interop that no compiled code can load // (the program imports a leaf data module and only a type from the // main entry) leaves its package static. From c673dc65b632a8b7b4e6d935791ce7188137fb71 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Tue, 20 Oct 2026 11:18:12 -0700 Subject: [PATCH 02/22] Split minified declarations of requires and the bindings read from them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A minifier merges a module's consecutive declarations, so its requires and the bindings read from them share one statement (glob's `var minimatch = require_minimatch(), Minimatch = minimatch.Minimatch, …` in a bundle). Require bindings are modeled per statement: inside a mixed statement the required module was never stored, so `minimatch.Minimatch` read undefined, and a later reference to `Minimatch` met "a binding form with no lowering". A package-file rewrite stage now splits a top-level declaration that mixes require() initializers with other declarators into one declaration per declarator, in order and on the same lines. Co-Authored-By: Claude Opus 5.5 --- .../npm-static-split-declarations.test.ts | 65 +++++++++++++++++++ .../frontend/npm-static-split-declarations.ts | 59 +++++++++++++++++ packages/compiler/src/frontend/npm-static.ts | 6 ++ packages/compiler/src/frontend/services.ts | 11 ++++ .../2592-bundled-merged-requires/main.ts | 8 +++ .../node_modules/gtmergedreq/dist/index.js | 1 + .../npm/node_modules/gtmergedreq/index.d.ts | 1 + .../npm/node_modules/gtmergedreq/package.json | 7 ++ tests/harness/npm-cases.ts | 4 +- tests/harness/npm-static.test.ts | 10 ++- 10 files changed, 169 insertions(+), 3 deletions(-) create mode 100644 packages/compiler/src/frontend/npm-static-split-declarations.test.ts create mode 100644 packages/compiler/src/frontend/npm-static-split-declarations.ts create mode 100644 tests/fixtures/npm/cases/2592-bundled-merged-requires/main.ts create mode 100644 tests/fixtures/npm/node_modules/gtmergedreq/dist/index.js create mode 100644 tests/fixtures/npm/node_modules/gtmergedreq/index.d.ts create mode 100644 tests/fixtures/npm/node_modules/gtmergedreq/package.json diff --git a/packages/compiler/src/frontend/npm-static-split-declarations.test.ts b/packages/compiler/src/frontend/npm-static-split-declarations.test.ts new file mode 100644 index 0000000000..f1ea06dab7 --- /dev/null +++ b/packages/compiler/src/frontend/npm-static-split-declarations.test.ts @@ -0,0 +1,65 @@ +import { spawnSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, expect, test } from "vitest"; +import { nodeFrontendServices } from "./services-node.js"; + +const dirs: string[] = []; +afterAll(() => { + for (const dir of dirs) rmSync(dir, { recursive: true, force: true }); +}); + +function rewrite(source: string): string | null { + return nodeFrontendServices().splitRequireDeclarations(source, "/pkg/index.js"); +} + +const lib = `"use strict"; +function Minimatch(p) { this.p = p; } +Minimatch.prototype.show = function () { return "mm:" + this.p; }; +module.exports = function minimatch(p) { return new Minimatch(p).show(); }; +module.exports.Minimatch = Minimatch; +`; + +/** The rewritten module prints what the original prints. */ +function expectSameBehavior(source: string): string { + const result = rewrite(source); + expect(typeof result).toBe("string"); + const dir = mkdtempSync(join(tmpdir(), "scriptc-split-declarations-")); + dirs.push(dir); + mkdirSync(join(dir, "lib")); + writeFileSync(join(dir, "lib", "index.js"), lib); + writeFileSync(join(dir, "original.cjs"), source); + writeFileSync(join(dir, "rewritten.cjs"), result!); + const run = (file: string) => spawnSync(process.execPath, [join(dir, file)], { encoding: "utf8" }); + const oracle = run("original.cjs"); + expect(oracle.status).toBe(0); + expect(run("rewritten.cjs").stdout).toBe(oracle.stdout); + expect(result!.split("\n").length).toBe(source.split("\n").length); + return result!; +} + +test("a minified declaration of requires and bindings read from them splits per binding", () => { + const out = expectSameBehavior(`"use strict"; +var path = require("path"), lib = require("./lib/index.js"), Minimatch = lib.Minimatch, sep = path.sep; +console.log(new Minimatch("a").show(), lib("b"), sep); +`); + expect(out).toContain( + 'var path = require("path"); var lib = require("./lib/index.js"); var Minimatch = lib.Minimatch; var sep = path.sep;', + ); +}); + +test("const declarations and a require in the last declarator split too", () => { + const out = expectSameBehavior(`const n = 1, + lib = require("./lib/index.js"); +console.log(n, lib("c")); +`); + expect(out).toContain("const n = 1; const \n"); +}); + +test("declarations without requires, nested ones and single declarators stay", () => { + expect(rewrite(`var a = 1, b = 2;\nvar c = require("x");\n`)).toBeNull(); + expect( + rewrite(`function f() { var a = require("x"), b = a.y; return b; }\nmodule.exports = f;\n`), + ).toBeNull(); +}); diff --git a/packages/compiler/src/frontend/npm-static-split-declarations.ts b/packages/compiler/src/frontend/npm-static-split-declarations.ts new file mode 100644 index 0000000000..777638e914 --- /dev/null +++ b/packages/compiler/src/frontend/npm-static-split-declarations.ts @@ -0,0 +1,59 @@ +/* A minifier merges consecutive declarations, so a module's requires and + * the bindings read from them share one statement: + * + * var rp = require_fs(), minimatch = require_minimatch(), + * Minimatch = minimatch.Minimatch, EE = require("events").EventEmitter; + * + * Require bindings and member aliases of them are modeled per statement, + * so a top-level declaration that mixes require() initializers with other + * declarators is respelled as one declaration statement per declarator, + * in the same order and on the same lines. Evaluation is unchanged: + * declarators of one statement already run left to right. */ +import * as ts from "./ts7/syntax.js"; +import { applyEdits, unparen, type Edit } from "./npm-static-bundled-chunks.js"; + +/** True when the text may hold such a declaration (a cheap probe). */ +export function mayMergeRequireDeclarations(source: string): boolean { + return ( + source.includes("require(") && + /\b(?:var|let|const)\s[^;]*(?:,[^;]*require\(|require\([^;]*,)/.test(source) + ); +} + +function isRequireValue(expr: ts.Expression): boolean { + const e = unparen(expr); + if (!ts.isCallExpression(e) || !ts.isIdentifier(e.expression)) return false; + const args = e.arguments.nodes; + if (e.expression.text === "require") + return ( + args.length === 1 && (ts.isStringLiteral(args[0]!) || ts.isNoSubstitutionTemplateLiteral(args[0]!)) + ); + if (e.expression.text === "__toESM") + return (args.length === 1 || args.length === 2) && isRequireValue(args[0]!); + return false; +} + +/** The module text with each mixed top-level declaration split, or null + * when there is none. */ +export function splitRequireDeclarations(sf: ts.SourceFile): string | null { + const edits: Edit[] = []; + for (const stmt of sf.statements.nodes) { + if (!ts.isVariableStatement(stmt)) continue; + if (stmt.modifiers !== undefined && stmt.modifiers.nodes.length > 0) continue; + const list = stmt.declarationList; + const decls = list.declarations.nodes; + if (decls.length < 2) continue; + if (!decls.some((d) => d.initializer !== undefined && isRequireValue(d.initializer))) continue; + const flags = list.flags; + const keyword = + (flags & ts.NodeFlags.Const) !== 0 ? "const" : (flags & ts.NodeFlags.Let) !== 0 ? "let" : "var"; + for (let i = 1; i < decls.length; i++) { + const start = decls[i - 1]!.getEnd(); + const end = decls[i]!.getStart(sf); + if (!sf.text.slice(start, end).includes(",")) return null; + edits.push({ start, end, text: `; ${keyword} ` }); + } + } + if (edits.length === 0) return null; + return applyEdits(sf.text, 0, sf.text.length, edits); +} diff --git a/packages/compiler/src/frontend/npm-static.ts b/packages/compiler/src/frontend/npm-static.ts index 1bf84e734d..9087c00a37 100644 --- a/packages/compiler/src/frontend/npm-static.ts +++ b/packages/compiler/src/frontend/npm-static.ts @@ -455,6 +455,11 @@ export function npmStaticFsShadow(services: FrontendServices): NpmStaticFsShadow // Polyfill packages that ARE a Node builtin compile as the builtin. const aliased = original === null ? null : services.builtinAliasRequires(original, path); if (aliased !== null) original = aliased; + // Minified merged declarations of requires and the bindings read + // from them become one declaration per binding. + const declSplit = + original === null ? null : services.splitRequireDeclarations(original, path); + if (declSplit !== null) original = declSplit; // A module exporting one of its functions reaches its members // through that function. const fnExports = original === null ? null : services.cjsFunctionExports(original, path); @@ -469,6 +474,7 @@ export function npmStaticFsShadow(services: FrontendServices): NpmStaticFsShadow if (exportsVar !== null) original = exportsVar; const respelled = split !== null || + declSplit !== null || esm !== null || aliased !== null || fnExports !== null || diff --git a/packages/compiler/src/frontend/services.ts b/packages/compiler/src/frontend/services.ts index be3881e137..689caddf89 100644 --- a/packages/compiler/src/frontend/services.ts +++ b/packages/compiler/src/frontend/services.ts @@ -13,6 +13,10 @@ import { rewriteBuiltinAliasRequires, } from "./npm-static-builtin-aliases.js"; import { mayHoldEnumIife, rewriteEnumIifes } from "./npm-static-enum-iife.js"; +import { + mayMergeRequireDeclarations, + splitRequireDeclarations, +} from "./npm-static-split-declarations.js"; import { builtinPackageAliasFor } from "./package-shims.js"; import * as syntax from "./npm-static-declaration-syntax.js"; import { normalizeCjsExportsVar } from "./npm-static-cjs-exports-var.js"; @@ -106,6 +110,13 @@ export class FrontendServices { if (!source.includes("module.exports")) return null; return normalizeCjsFunctionExports(this.parse(path, source, "js")); } + /** A package module with its top-level declarations that mix require() + * initializers with other declarators split per declarator (null: none). */ + splitRequireDeclarations(source: string, path: string): string | null { + this.ensureOpen(); + if (!mayMergeRequireDeclarations(source)) return null; + return splitRequireDeclarations(this.parse(path, source, "js")); + } /** A package module with its literal-valued TypeScript enum IIFEs * respelled as object literals (null: none). */ enumIifes(source: string, path: string): string | null { diff --git a/tests/fixtures/npm/cases/2592-bundled-merged-requires/main.ts b/tests/fixtures/npm/cases/2592-bundled-merged-requires/main.ts new file mode 100644 index 0000000000..7da3b3d4d1 --- /dev/null +++ b/tests/fixtures/npm/cases/2592-bundled-merged-requires/main.ts @@ -0,0 +1,8 @@ +// A minified bundle merges a module's requires and the bindings read from +// them into one declaration (`var minimatch=require_x(),Minimatch= +// minimatch.Minimatch,…`, glob's shape). Each binding reads the module +// value the earlier declarator required. +import { demo } from "gtmergedreq"; + +console.log(demo("/x/y.js")); +console.log(demo("pattern")); diff --git a/tests/fixtures/npm/node_modules/gtmergedreq/dist/index.js b/tests/fixtures/npm/node_modules/gtmergedreq/dist/index.js new file mode 100644 index 0000000000..4af92d0362 --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtmergedreq/dist/index.js @@ -0,0 +1 @@ +var __create=Object.create;var __defProp=Object.defineProperty;var __getOwnPropDesc=Object.getOwnPropertyDescriptor;var __getOwnPropNames=Object.getOwnPropertyNames;var __getProtoOf=Object.getPrototypeOf,__hasOwnProp=Object.prototype.hasOwnProperty;var __commonJS=(cb,mod)=>function(){return mod||(0,cb[__getOwnPropNames(cb)[0]])((mod={exports:{}}).exports,mod),mod.exports};var __export=(target,all)=>{for(var name in all)__defProp(target,name,{get:all[name],enumerable:!0})},__copyProps=(to,from,except,desc)=>{if(from&&typeof from=="object"||typeof from=="function")for(let key of __getOwnPropNames(from))!__hasOwnProp.call(to,key)&&key!==except&&__defProp(to,key,{get:()=>from[key],enumerable:!(desc=__getOwnPropDesc(from,key))||desc.enumerable});return to};var __toESM=(mod,isNodeMode,target)=>(target=mod!=null?__create(__getProtoOf(mod)):{},__copyProps(isNodeMode||!mod||!mod.__esModule?__defProp(target,"default",{value:mod,enumerable:!0}):target,mod)),__toCommonJS=mod=>__copyProps(__defProp({},"__esModule",{value:!0}),mod);var require_gtminilike=__commonJS({"node_modules/gtminilike/index.js"(exports2,module2){"use strict";function Minimatch(pattern){this.pattern=pattern}Minimatch.prototype.show=function(){return"mm:"+this.pattern};function minimatch(p){return new Minimatch(p).show()}minimatch.Minimatch=Minimatch;module2.exports=minimatch}});var require_gtglobish=__commonJS({"node_modules/gtglobish/index.js"(exports2){"use strict";var minimatch=require_gtminilike(),Minimatch=minimatch.Minimatch,path=require("path"),EE=require("events").EventEmitter;exports2.make=function(p){return new Minimatch(path.basename(p)).show()};exports2.quick=function(p){return minimatch(p)+(typeof EE=="function"?"":"?")}}});var index_exports={};__export(index_exports,{demo:()=>demo});module.exports=__toCommonJS(index_exports);var import_gtglobish=__toESM(require_gtglobish());function demo(p){return`${(0,import_gtglobish.make)(p)} ${(0,import_gtglobish.quick)(p)}`}0&&(module.exports={demo}); diff --git a/tests/fixtures/npm/node_modules/gtmergedreq/index.d.ts b/tests/fixtures/npm/node_modules/gtmergedreq/index.d.ts new file mode 100644 index 0000000000..403a2f0b88 --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtmergedreq/index.d.ts @@ -0,0 +1 @@ +export declare function demo(p: string): string; diff --git a/tests/fixtures/npm/node_modules/gtmergedreq/package.json b/tests/fixtures/npm/node_modules/gtmergedreq/package.json new file mode 100644 index 0000000000..0c1ce00394 --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtmergedreq/package.json @@ -0,0 +1,7 @@ +{ + "name": "gtmergedreq", + "version": "1.0.0", + "description": "real esbuild 0.27 output (--bundle --platform=node --format=cjs --minify-whitespace --minify-syntax) of a TypeScript source over a CommonJS dependency (glob's shape) whose separate require and member-alias declarations the minifier merges into one: `var minimatch=require_gtminilike(),Minimatch=minimatch.Minimatch,path=require(\"path\"),…`", + "main": "dist/index.js", + "types": "index.d.ts" +} diff --git a/tests/harness/npm-cases.ts b/tests/harness/npm-cases.ts index 766a8f2aaa..4a1083407d 100644 --- a/tests/harness/npm-cases.ts +++ b/tests/harness/npm-cases.ts @@ -19,14 +19,14 @@ export function npmCases(fixturesRoot: string): NpmCase[] { return [ ...globSync(join(fixturesRoot, "npm/cases/*/main.ts")) .sort() - // 2465-2469, 2556-2567, 2571 and 2580-2591 are the --npm-static bundler-emitted-CJS + // 2465-2469, 2556-2567, 2571 and 2580-2592 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-7]|2571|258[0-9]|259[01])-[^/]+\/main\.ts$/.test(entry), + !/\/(246[5-9]|255[6-9]|256[0-7]|2571|258[0-9]|259[0-2])-[^/]+\/main\.ts$/.test(entry), ) .map((entry) => ({ name: entry.split("/").at(-2)!, diff --git a/tests/harness/npm-static.test.ts b/tests/harness/npm-static.test.ts index 125fe33679..c0c63d2f2c 100644 --- a/tests/harness/npm-static.test.ts +++ b/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-2567, 2571, 2580-2591) + // the bundler-emitted-CJS mini packages (cases 2465-2469, 2556-2567, 2571, 2580-2592) ...globSync(join(fixturesRoot, "npm/node_modules/gt*/**/*.{js,cjs,json}")).sort(), ...globSync(join(fixturesRoot, "npm/store/gt*/**/*.{js,cjs,json}")).sort(), ]; @@ -1635,6 +1635,14 @@ describe(`npm-static pilots${sanitize ? " (sanitized)" : ""}`, () => { ]); }, 180_000); + // 2592: a minified declaration of requires and the bindings read from + // them (glob's `var minimatch = …, Minimatch = minimatch.Minimatch`). + test("merged require declarations 2592-bundled-merged-requires byte-match Node", async () => { + await expectNodeParity(join(fixturesRoot, "npm/cases/2592-bundled-merged-requires/main.ts"), [ + "gtmergedreq", + ]); + }, 180_000); + // 2562: a file with unverifiable interop that no compiled code can load // (the program imports a leaf data module and only a type from the // main entry) leaves its package static. From cd07bce7c49205e4c7b4e2145b29ded15357b762 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Tue, 20 Oct 2026 12:01:12 -0700 Subject: [PATCH 03/22] Keep semver's token table a plain object behind an early function export semver 6 starts with `exports = module.exports = SemVer`, then builds `var t = exports.tokens = {}` and fills it through `function tok(n) { t[n] = R++ }`. The hoisted SemVer reads `t`, and its export store counted as a reference that could call it before `t` was set, so `t` became a runtime-optional global and tok's computed write was fenced at load. A function stored as a CommonJS export, or given members, can only run before a binding settles if code re-enters the module first. When every top-level statement up to the settling one makes no call into user code, no construction, no require and no read of `module`/`exports`, such a reference now counts as settled (builtin methods of literals and standard globals cannot run user code). The declaration split also respells `var t = exports.tokens = {}` as `var t = {}; exports.tokens = t;`: writes through `t` were lost to the export otherwise (`exports.tokens` printed `{}`). Also records the order baselines of the new 6484 and 2591 fixtures. Co-Authored-By: Claude Opus 5.5 --- .../src/frontend/lowering/var-hoisting.ts | 118 +++++++++++++++++- .../npm-static-split-declarations.test.ts | 12 ++ .../frontend/npm-static-split-declarations.ts | 51 +++++++- .../6484-cjs-settled-exports/main.cjs.json | 10 ++ .../2591-bundled-var-export/main.ts.json | 6 + .../cases/2593-bundled-semver-tokens/main.ts | 9 ++ .../node_modules/gtsemverbundle/dist/index.js | 1 + .../node_modules/gtsemverbundle/index.d.ts | 1 + .../node_modules/gtsemverbundle/package.json | 7 ++ tests/harness/npm-cases.ts | 4 +- tests/harness/npm-static.test.ts | 9 +- 11 files changed, 219 insertions(+), 9 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6484-cjs-settled-exports/main.cjs.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2591-bundled-var-export/main.ts.json create mode 100644 tests/fixtures/npm/cases/2593-bundled-semver-tokens/main.ts create mode 100644 tests/fixtures/npm/node_modules/gtsemverbundle/dist/index.js create mode 100644 tests/fixtures/npm/node_modules/gtsemverbundle/index.d.ts create mode 100644 tests/fixtures/npm/node_modules/gtsemverbundle/package.json diff --git a/packages/compiler/src/frontend/lowering/var-hoisting.ts b/packages/compiler/src/frontend/lowering/var-hoisting.ts index c445f17517..0cd96ec4a7 100644 --- a/packages/compiler/src/frontend/lowering/var-hoisting.ts +++ b/packages/compiler/src/frontend/lowering/var-hoisting.ts @@ -240,7 +240,20 @@ class ScopeAnalysis { if (deferred !== null && ts.isFunctionDeclaration(deferred) && deferred.name !== undefined) return this.functionSettles(scope, settle, deferred); const index = statements.indexOf(top as ts.Statement); - return index > settle || (index === settle && settlesWithin(top, ref)); + if (index > settle || (index === settle && settlesWithin(top, ref))) return true; + // A function stored as a CommonJS export (`module.exports = F`), or + // given members (`F.x = …`), runs through that reference only when + // code re-enters the module before the binding settles: a require + // cycle, a call into user code, or a read of the module's exports. + // Top-level code up to the settling statement that does none of these + // leaves F uncalled until then. + return ( + !read && + deferred === null && + ts.isSourceFile(scope) && + nonInvokingReference(ref) && + statements.slice(index + 1, settle + 1).every(cannotReenter) + ); } private functionSettles(scope: Scope, settle: number, fn: ts.FunctionDeclaration): boolean { @@ -271,6 +284,109 @@ class ScopeAnalysis { } } +/** True when `ref` only stores or extends the function: the value stored + * by `module.exports = ref`, `exports = module.exports = ref` or + * `exports.name = ref`, or the object of a member store `ref.name = …`. */ +function nonInvokingReference(ref: ts.Identifier): boolean { + const member = ref.parent; + if ( + member !== undefined && + ts.isPropertyAccessExpression(member) && + member.expression === ref && + member.parent !== undefined && + ts.isBinaryExpression(member.parent) && + member.parent.operatorToken.kind === ts.SyntaxKind.EqualsToken && + member.parent.left === member + ) + return true; + let node: ts.Node = ref; + for (;;) { + const parent = node.parent; + if ( + parent === undefined || + !ts.isBinaryExpression(parent) || + parent.operatorToken.kind !== ts.SyntaxKind.EqualsToken || + parent.right !== node + ) + return node !== ref && ts.isExpressionStatement(node.parent!); + if (!isExportsTarget(parent.left)) return false; + node = parent; + } +} + +function isModuleExports(e: ts.Expression): boolean { + return ( + ts.isPropertyAccessExpression(e) && + ts.isIdentifier(e.expression) && + e.expression.text === "module" && + e.name.text === "exports" + ); +} + +function isExportsTarget(e: ts.Expression): boolean { + if (ts.isIdentifier(e)) return e.text === "exports"; + if (isModuleExports(e)) return true; + return ( + ts.isPropertyAccessExpression(e) && + ((ts.isIdentifier(e.expression) && e.expression.text === "exports") || + isModuleExports(e.expression)) + ); +} + +const PURE_GLOBALS = new Set(["Object", "Array", "Number", "String", "Math", "JSON", "Symbol"]); + +/** A call whose callee is a builtin method of a literal or a standard + * global: it cannot run user code. */ +function builtinCall(callee: ts.Expression): boolean { + if (!ts.isPropertyAccessExpression(callee)) return false; + let root: ts.Expression = callee.expression; + while (ts.isPropertyAccessExpression(root)) root = root.expression; + return ( + ts.isRegularExpressionLiteral(root) || + ts.isStringLiteral(root) || + ts.isNumericLiteral(root) || + (ts.isIdentifier(root) && PURE_GLOBALS.has(root.text)) + ); +} + +/** True when evaluating `stmt` cannot re-enter the module: no call into + * user code, no construction, no require, no read of `module`/`exports` + * beyond stores into them. Function and class bodies do not run here. */ +function cannotReenter(stmt: ts.Statement): boolean { + if (ts.isFunctionDeclaration(stmt) || ts.isEmptyStatement(stmt)) return true; + let quiet = true; + ts.walkPreorder(stmt, (n) => { + if (!quiet) return "stop"; + if ( + ts.isFunctionLike(n) || + ts.isClassDeclaration(n) || + ts.isClassExpression(n) || + ts.isClassStaticBlockDeclaration(n) + ) + return "skip"; + if (ts.isCallExpression(n)) { + if (!builtinCall(n.expression)) quiet = false; + } else if (ts.isNewExpression(n) || ts.isTaggedTemplateExpression(n)) { + quiet = false; + } else if (ts.isIdentifier(n) && (n.text === "module" || n.text === "exports")) { + // Only as the root of a store target. + let target: ts.Node = n; + while (target.parent !== undefined && ts.isPropertyAccessExpression(target.parent) && target.parent.expression === target) + target = target.parent; + const assign = target.parent; + if ( + assign === undefined || + !ts.isBinaryExpression(assign) || + assign.operatorToken.kind !== ts.SyntaxKind.EqualsToken || + assign.left !== target + ) + quiet = false; + } + return undefined; + }); + return quiet; +} + /** A loop statement that settles the binding settles the references in * its condition, update and body: its declarations run first. */ function settlesWithin(statement: ts.Node, ref: ts.Node): boolean { diff --git a/packages/compiler/src/frontend/npm-static-split-declarations.test.ts b/packages/compiler/src/frontend/npm-static-split-declarations.test.ts index f1ea06dab7..eda22b0e72 100644 --- a/packages/compiler/src/frontend/npm-static-split-declarations.test.ts +++ b/packages/compiler/src/frontend/npm-static-split-declarations.test.ts @@ -63,3 +63,15 @@ test("declarations without requires, nested ones and single declarators stay", ( rewrite(`function f() { var a = require("x"), b = a.y; return b; }\nmodule.exports = f;\n`), ).toBeNull(); }); + +test("a declarator that also stores an export becomes the declaration and the export", () => { + // semver 6's spelling, merged by a minifier. + const out = expectSameBehavior(`"use strict"; +var re = exports.re = [], t = module.exports.tokens = {}, R = 0; +function tok(n) { t[n] = R++; } +tok("A"); tok("B"); +re[t.B] = "b"; +console.log(JSON.stringify(exports.tokens), exports.re.length, t === exports.tokens); +`); + expect(out).toContain("var re = []; exports.re = re; var t = {}; module.exports.tokens = t; var R = 0;"); +}); diff --git a/packages/compiler/src/frontend/npm-static-split-declarations.ts b/packages/compiler/src/frontend/npm-static-split-declarations.ts index 777638e914..1abbba6279 100644 --- a/packages/compiler/src/frontend/npm-static-split-declarations.ts +++ b/packages/compiler/src/frontend/npm-static-split-declarations.ts @@ -8,18 +8,43 @@ * so a top-level declaration that mixes require() initializers with other * declarators is respelled as one declaration statement per declarator, * in the same order and on the same lines. Evaluation is unchanged: - * declarators of one statement already run left to right. */ + * declarators of one statement already run left to right. + * + * A declarator that also stores its value as an export + * (`t = exports.tokens = {}`, semver's spelling) becomes the declaration + * followed by the export of the binding (`var t = {}; exports.tokens = t;`): + * the value is evaluated once and both names hold it, as before, while the + * export is a plain member export of the binding. */ import * as ts from "./ts7/syntax.js"; import { applyEdits, unparen, type Edit } from "./npm-static-bundled-chunks.js"; /** True when the text may hold such a declaration (a cheap probe). */ export function mayMergeRequireDeclarations(source: string): boolean { return ( - source.includes("require(") && - /\b(?:var|let|const)\s[^;]*(?:,[^;]*require\(|require\([^;]*,)/.test(source) + (source.includes("require(") && + /\b(?:var|let|const)\s[^;]*(?:,[^;]*require\(|require\([^;]*,)/.test(source)) || + /=\s*(?:module\.)?exports2?\.[\w$]+\s*=[^=]/.test(source) ); } +/** `exports.X = E` / `module.exports.X = E`: the export target, or null. */ +function exportStoreOf(expr: ts.Expression): ts.PropertyAccessExpression | null { + const e = unparen(expr); + if (!ts.isBinaryExpression(e) || e.operatorToken.kind !== ts.SyntaxKind.EqualsToken) return null; + const left = e.left; + if (!ts.isPropertyAccessExpression(left) || !ts.isIdentifier(left.name)) return null; + const obj = left.expression; + if (ts.isIdentifier(obj) && obj.text === "exports") return left; + if ( + ts.isPropertyAccessExpression(obj) && + ts.isIdentifier(obj.expression) && + obj.expression.text === "module" && + obj.name.text === "exports" + ) + return left; + return null; +} + function isRequireValue(expr: ts.Expression): boolean { const e = unparen(expr); if (!ts.isCallExpression(e) || !ts.isIdentifier(e.expression)) return false; @@ -42,11 +67,27 @@ export function splitRequireDeclarations(sf: ts.SourceFile): string | null { if (stmt.modifiers !== undefined && stmt.modifiers.nodes.length > 0) continue; const list = stmt.declarationList; const decls = list.declarations.nodes; - if (decls.length < 2) continue; - if (!decls.some((d) => d.initializer !== undefined && isRequireValue(d.initializer))) continue; const flags = list.flags; const keyword = (flags & ts.NodeFlags.Const) !== 0 ? "const" : (flags & ts.NodeFlags.Let) !== 0 ? "let" : "var"; + const stores = decls.map((d) => + ts.isIdentifier(d.name) && d.initializer !== undefined ? exportStoreOf(d.initializer) : null, + ); + if (stores.some((st) => st !== null)) { + // Respell the whole statement: each declarator its own declaration, + // an export store after its binding's declaration. + const parts = decls.map((d, i) => { + const store = stores[i] ?? null; + if (store === null) return `${keyword} ${d.getText(sf)};`; + const name = (d.name as ts.Identifier).text; + const value = (unparen(d.initializer!) as ts.BinaryExpression).right.getText(sf); + return `${keyword} ${name} = ${value}; ${store.getText(sf)} = ${name};`; + }); + edits.push({ start: stmt.getStart(sf), end: stmt.getEnd(), text: parts.join(" ") }); + continue; + } + if (decls.length < 2) continue; + if (!decls.some((d) => d.initializer !== undefined && isRequireValue(d.initializer))) continue; for (let i = 1; i < decls.length; i++) { const start = decls[i - 1]!.getEnd(); const end = decls[i]!.getStart(sf); diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6484-cjs-settled-exports/main.cjs.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6484-cjs-settled-exports/main.cjs.json new file mode 100644 index 0000000000..7579a59564 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6484-cjs-settled-exports/main.cjs.json @@ -0,0 +1,10 @@ +{ + "order": [ + "/tests/corpus/6484-cjs-settled-exports/lib.cjs", + "/tests/corpus/6484-cjs-settled-exports/table.cjs", + "/tests/corpus/6484-cjs-settled-exports/class.cjs", + "/tests/corpus/6484-cjs-settled-exports/sema.cjs", + "/tests/corpus/6484-cjs-settled-exports/main.cjs" + ], + "diags": [] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2591-bundled-var-export/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2591-bundled-var-export/main.ts.json new file mode 100644 index 0000000000..94d74a8f4a --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2591-bundled-var-export/main.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/fixtures/npm/cases/2591-bundled-var-export/main.ts" + ], + "diags": [] +} diff --git a/tests/fixtures/npm/cases/2593-bundled-semver-tokens/main.ts b/tests/fixtures/npm/cases/2593-bundled-semver-tokens/main.ts new file mode 100644 index 0000000000..ea622beab2 --- /dev/null +++ b/tests/fixtures/npm/cases/2593-bundled-semver-tokens/main.ts @@ -0,0 +1,9 @@ +// semver 6's load-time shape in a bundle: the module exports its +// constructor first (`exports = module.exports = Version`), then fills a +// token table that the hoisted constructor reads +// (`t=exports.tokens={}` ... `function tok(n){t[n]=R++}`). Nothing can +// call the constructor before the table exists, so the table is a plain +// module object. +import { demo } from "gtsemverbundle"; + +console.log(demo()); diff --git a/tests/fixtures/npm/node_modules/gtsemverbundle/dist/index.js b/tests/fixtures/npm/node_modules/gtsemverbundle/dist/index.js new file mode 100644 index 0000000000..78c1c79ec1 --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtsemverbundle/dist/index.js @@ -0,0 +1 @@ +var __create=Object.create;var __defProp=Object.defineProperty;var __getOwnPropDesc=Object.getOwnPropertyDescriptor;var __getOwnPropNames=Object.getOwnPropertyNames;var __getProtoOf=Object.getPrototypeOf,__hasOwnProp=Object.prototype.hasOwnProperty;var __commonJS=(cb,mod)=>function(){return mod||(0,cb[__getOwnPropNames(cb)[0]])((mod={exports:{}}).exports,mod),mod.exports};var __export=(target,all)=>{for(var name in all)__defProp(target,name,{get:all[name],enumerable:!0})},__copyProps=(to,from,except,desc)=>{if(from&&typeof from=="object"||typeof from=="function")for(let key of __getOwnPropNames(from))!__hasOwnProp.call(to,key)&&key!==except&&__defProp(to,key,{get:()=>from[key],enumerable:!(desc=__getOwnPropDesc(from,key))||desc.enumerable});return to};var __toESM=(mod,isNodeMode,target)=>(target=mod!=null?__create(__getProtoOf(mod)):{},__copyProps(isNodeMode||!mod||!mod.__esModule?__defProp(target,"default",{value:mod,enumerable:!0}):target,mod)),__toCommonJS=mod=>__copyProps(__defProp({},"__esModule",{value:!0}),mod);var require_semver=__commonJS({"node_modules/gtsemverlike/semver.js"(exports2,module2){exports2=module2.exports=Version;var debug;typeof process=="object"&&process.env&&/\bsemverlike\b/i.test(process.env.NODE_DEBUG||"")?debug=function(){console.log.apply(console,["SEMVER"].concat(Array.prototype.slice.call(arguments)))}:debug=function(){};exports2.SPEC_VERSION="2.0.0";var re=exports2.re=[],src=exports2.src=[],t=exports2.tokens={},R=0;function tok(n){t[n]=R++}tok("NUMERIC");src[t.NUMERIC]="0|[1-9]\\d*";tok("MAIN");src[t.MAIN]="^("+src[t.NUMERIC]+")\\.("+src[t.NUMERIC]+")\\.("+src[t.NUMERIC]+")$";for(i=0;idemo});module.exports=__toCommonJS(index_exports);var import_gtsemverlike=__toESM(require_semver());function demo(){return`${import_gtsemverlike.default.valid("1.2.3")} ${import_gtsemverlike.default.valid("v1")} ${import_gtsemverlike.default.inc("1.2.3")} ${import_gtsemverlike.default.pattern().length}`}0&&(module.exports={demo}); diff --git a/tests/fixtures/npm/node_modules/gtsemverbundle/index.d.ts b/tests/fixtures/npm/node_modules/gtsemverbundle/index.d.ts new file mode 100644 index 0000000000..2a0bd84d6a --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtsemverbundle/index.d.ts @@ -0,0 +1 @@ +export declare function demo(): string; diff --git a/tests/fixtures/npm/node_modules/gtsemverbundle/package.json b/tests/fixtures/npm/node_modules/gtsemverbundle/package.json new file mode 100644 index 0000000000..3f08978c31 --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtsemverbundle/package.json @@ -0,0 +1,7 @@ +{ + "name": "gtsemverbundle", + "version": "1.0.0", + "description": "real esbuild 0.27 output (--bundle --platform=node --format=cjs --minify-whitespace --minify-syntax) of a TypeScript source over a CommonJS dependency with semver 6's load-time shape: `exports = module.exports = Version` first, then `var re=exports.re=[],src=exports.src=[],t=exports.tokens={},R=0;function tok(n){t[n]=R++}` and token calls, with the hoisted Version reading the token table", + "main": "dist/index.js", + "types": "index.d.ts" +} diff --git a/tests/harness/npm-cases.ts b/tests/harness/npm-cases.ts index 4a1083407d..691d1c100d 100644 --- a/tests/harness/npm-cases.ts +++ b/tests/harness/npm-cases.ts @@ -19,14 +19,14 @@ export function npmCases(fixturesRoot: string): NpmCase[] { return [ ...globSync(join(fixturesRoot, "npm/cases/*/main.ts")) .sort() - // 2465-2469, 2556-2567, 2571 and 2580-2592 are the --npm-static bundler-emitted-CJS + // 2465-2469, 2556-2567, 2571 and 2580-2593 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-7]|2571|258[0-9]|259[0-2])-[^/]+\/main\.ts$/.test(entry), + !/\/(246[5-9]|255[6-9]|256[0-7]|2571|258[0-9]|259[0-3])-[^/]+\/main\.ts$/.test(entry), ) .map((entry) => ({ name: entry.split("/").at(-2)!, diff --git a/tests/harness/npm-static.test.ts b/tests/harness/npm-static.test.ts index c0c63d2f2c..ce41f65e5a 100644 --- a/tests/harness/npm-static.test.ts +++ b/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-2567, 2571, 2580-2592) + // the bundler-emitted-CJS mini packages (cases 2465-2469, 2556-2567, 2571, 2580-2593) ...globSync(join(fixturesRoot, "npm/node_modules/gt*/**/*.{js,cjs,json}")).sort(), ...globSync(join(fixturesRoot, "npm/store/gt*/**/*.{js,cjs,json}")).sort(), ]; @@ -1643,6 +1643,13 @@ describe(`npm-static pilots${sanitize ? " (sanitized)" : ""}`, () => { ]); }, 180_000); + // 2593: semver 6's token table behind an early `module.exports = F`. + test("a bundled token table 2593-bundled-semver-tokens byte-matches Node", async () => { + await expectNodeParity(join(fixturesRoot, "npm/cases/2593-bundled-semver-tokens/main.ts"), [ + "gtsemverbundle", + ]); + }, 180_000); + // 2562: a file with unverifiable interop that no compiled code can load // (the program imports a leaf data module and only a type from the // main entry) leaves its package static. From c0224f268bd1e881c2284cfad140d9ff2dabb718 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Tue, 20 Oct 2026 12:08:10 -0700 Subject: [PATCH 04/22] Store fs.realpathSync as a function value fs.realpath (glob 7 and 8 reach the filesystem through it) keeps Node's own functions as values, `var origRealpathSync = fs.realpathSync`, and calls them with its legacy cache object as the options argument. Reading the function as a value was refused ("library functions as values"). The value is now an adapter over the native call. Node's second argument can change the result's encoding, so the adapter takes the call only when the path is a string and the options keep UTF-8 (none, "utf8", or an object without another `encoding`); any other shape throws a located SC1090 fence where the call runs instead of answering differently. Co-Authored-By: Claude Opus 5.5 --- .../limitations/builtin-function-values.mdx | 1 + .../frontend/lowering/lower-builtin-values.ts | 122 +++++++ .../compiler/src/frontend/lowering/lowerer.ts | 2 + .../npm/cases/2594-fs-realpath-values/main.ts | 17 + .../npm/node_modules/fs.realpath/LICENSE | 43 +++ .../npm/node_modules/fs.realpath/README.md | 33 ++ .../npm/node_modules/fs.realpath/index.js | 66 ++++ .../npm/node_modules/fs.realpath/old.js | 303 ++++++++++++++++++ .../npm/node_modules/fs.realpath/package.json | 26 ++ tests/harness/npm-cases.ts | 4 +- tests/harness/npm-static.test.ts | 9 +- 11 files changed, 623 insertions(+), 3 deletions(-) create mode 100644 tests/fixtures/npm/cases/2594-fs-realpath-values/main.ts create mode 100644 tests/fixtures/npm/node_modules/fs.realpath/LICENSE create mode 100644 tests/fixtures/npm/node_modules/fs.realpath/README.md create mode 100644 tests/fixtures/npm/node_modules/fs.realpath/index.js create mode 100644 tests/fixtures/npm/node_modules/fs.realpath/old.js create mode 100644 tests/fixtures/npm/node_modules/fs.realpath/package.json diff --git a/docs/content/limitations/builtin-function-values.mdx b/docs/content/limitations/builtin-function-values.mdx index 541c9fb2b7..a632b85446 100644 --- a/docs/content/limitations/builtin-function-values.mdx +++ b/docs/content/limitations/builtin-function-values.mdx @@ -4,6 +4,7 @@ Some builtins can be stored and passed as functions: - Supported zero-argument `node:os` functions and `querystring.escape`/`unescape`. - `util.isDeepStrictEqual`, `styleText`, `stripVTControlCharacters`, `toUSVString`, `inherits`, `deprecate`, `callbackify`, `debuglog`, and `debug`. - `fs.existsSync`, `unlinkSync`, `chmodSync`, `chownSync`, `renameSync`, and `closeSync`, and `fs/promises.unlink`, `chmod`, and `rename`. +- `fs.realpathSync`, called with a string path and options that keep the UTF-8 encoding (none, `"utf8"`, or an object without another `encoding`, such as fs.realpath's cache object). Other paths or encodings throw `SC1090` where the call runs. In JavaScript, `setTimeout`, `setInterval`, `setImmediate`, `clearTimeout`, `clearInterval`, `clearImmediate`, `queueMicrotask`, and `process.nextTick` are function values wherever they are stored, including conditional and logical expressions and assignments: `typeof` answers `"function"`, identity holds across spellings, and calls schedule as the direct calls do, returning the same numeric timer handles. Calls through these values bypass `node:test` mock timers. diff --git a/packages/compiler/src/frontend/lowering/lower-builtin-values.ts b/packages/compiler/src/frontend/lowering/lower-builtin-values.ts index 5e04ee83c1..47f0902363 100644 --- a/packages/compiler/src/frontend/lowering/lower-builtin-values.ts +++ b/packages/compiler/src/frontend/lowering/lower-builtin-values.ts @@ -11,7 +11,9 @@ import { type IrLibFn, type IrStmt, type SrcLoc, + funcOf, } from "../../ir/ir.js"; +import { unsupportedDiag } from "../../diagnostics/diagnostic.js"; import { strLit, varRef } from "../../ir/build.js"; import type { Lowerer } from "./lowerer.js"; import { @@ -710,3 +712,123 @@ export function builtinConstantsObject(lowerer: Lowerer, loc: SrcLoc): IrExpr { }; return { kind: "libCall", fn: "constants.object", args: [box(table)], type: DYN, loc }; } + +/** `fs.realpathSync` as a function value (fs.realpath's + * `var origRealpathSync = fs.realpathSync`). Node's second argument is an + * options object or encoding name; the native call answers the UTF-8 + * string, so the adapter takes the call whenever the options leave that + * encoding in place (absent, `"utf8"`, or an object whose `encoding` is + * absent or UTF-8) and the path is a string. Any other argument shape + * throws a located fence instead of a different result. */ +export function lowerFsRealpathSyncValue(lowerer: Lowerer, loc: SrcLoc): IrExpr { + const name = "%builtin.value.fs.realpathSync"; + if (!lowerer.builtinCallableValueFns.has(name)) { + lowerer.builtinCallableValueFns.set(name, name); + const path = varRef("path", DYN, loc); + const options = varRef("options", DYN, loc); + const fence = (what: string): IrStmt => { + const diag = unsupportedDiag( + "SC1090", + loc, + `fs.realpathSync as a value called with ${what}`, + ); + lowerer.runtimeFences.push(diag); + return { + kind: "runtimeFence", + code: diag.code, + message: `${diag.message} [${diag.code}]`, + loc, + }; + }; + const utf8 = (value: IrExpr): IrExpr => ({ + kind: "logical", + op: "||", + left: { kind: "dynScalarEq", left: value, right: strLit("utf8", loc), type: BOOL, loc }, + right: { kind: "dynScalarEq", left: value, right: strLit("utf-8", loc), type: BOOL, loc }, + type: BOOL, + loc, + }); + const encoding: IrExpr = { kind: "dynKeyGet", key: strLit("encoding", loc), value: options, type: DYN, loc }; + const defaultEncoding: IrExpr = { + kind: "logical", + op: "||", + left: { kind: "dynTest", test: "nullish", value: options, type: BOOL, loc }, + right: { + kind: "logical", + op: "||", + left: utf8(options), + right: { + kind: "logical", + op: "&&", + left: { kind: "dynTest", test: "object", value: options, type: BOOL, loc }, + right: { + kind: "logical", + op: "||", + left: { kind: "dynTest", test: "nullish", value: encoding, type: BOOL, loc }, + right: utf8(encoding), + type: BOOL, + loc, + }, + type: BOOL, + loc, + }, + type: BOOL, + loc, + }, + type: BOOL, + loc, + }; + lowerer.liftedFns.push({ + name, + params: [ + { localId: "path", name: "path", type: DYN }, + { localId: "options", name: "options", type: DYN }, + ], + returnType: DYN, + locals: [ + { id: "path", name: "path", type: DYN, mutable: false }, + { id: "options", name: "options", type: DYN, mutable: false }, + ], + loc, + body: [ + { + kind: "if", + cond: { kind: "dynTest", test: "string", negated: true, value: path, type: BOOL, loc }, + then: [fence("a path that is not a string")], + else_: [], + loc, + }, + { + kind: "if", + cond: { kind: "unary", op: "!", operand: defaultEncoding, type: BOOL, loc }, + then: [fence("an encoding other than UTF-8")], + else_: [], + loc, + }, + { + kind: "return", + value: { + kind: "dynFrom", + value: { + kind: "libCall", + fn: "fs.realpathSync", + args: [{ kind: "dynCheck", value: path, type: STRING, loc }], + type: STRING, + loc, + }, + type: DYN, + loc, + }, + loc, + }, + ], + }); + } + return { + kind: "dynFrom", + value: { kind: "closure", fnName: name, captures: [], type: funcOf([DYN, DYN], DYN), loc }, + fnName: "realpathSync", + type: DYN, + loc, + }; +} diff --git a/packages/compiler/src/frontend/lowering/lowerer.ts b/packages/compiler/src/frontend/lowering/lowerer.ts index ecd5114951..f2c808aa5b 100644 --- a/packages/compiler/src/frontend/lowering/lowerer.ts +++ b/packages/compiler/src/frontend/lowering/lowerer.ts @@ -93,6 +93,7 @@ import { deferredReadHelper, } from "./coercions/unions.js"; import { + lowerFsRealpathSyncValue, lowerUtilTypeValue, pathModuleValue, utilTypesModuleValue, @@ -12876,6 +12877,7 @@ export class Lowerer { return lowerUtilTypeValue(this, bi.member, loc); if (bi.module === "util" && bi.member === "getSystemErrorMap") return lowerSystemErrorMapValue(this, loc); + if (bi.module === "fs" && bi.member === "realpathSync") return lowerFsRealpathSyncValue(this, loc); if ( bi.module === "fs" && builtinModuleFnOf(this, bi.module, bi.member)?.fn === "fs.callbackCall" diff --git a/tests/fixtures/npm/cases/2594-fs-realpath-values/main.ts b/tests/fixtures/npm/cases/2594-fs-realpath-values/main.ts new file mode 100644 index 0000000000..df146d3ac8 --- /dev/null +++ b/tests/fixtures/npm/cases/2594-fs-realpath-values/main.ts @@ -0,0 +1,17 @@ +// fs.realpath 1.0.0 (vendored unmodified, ISC) keeps Node's own functions as +// values (`var origRealpathSync = fs.realpathSync`, `origRealpath = fs.realpath`) +// and calls them with its legacy cache object as the options argument. +// glob 7 and 8 reach the filesystem through it. +import { mkdirSync, mkdtempSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import rp from "fs.realpath"; + +const dir = mkdtempSync(join(tmpdir(), "scriptc-fs-realpath-")); +mkdirSync(join(dir, "real")); +writeFileSync(join(dir, "real", "f.txt"), "x"); +symlinkSync(join(dir, "real"), join(dir, "link")); +console.log(rp.realpathSync(join(dir, "link", "f.txt"), {}).endsWith("/real/f.txt")); +rp.realpath(join(dir, "link", "f.txt"), {}, (err: Error | null, resolved?: string) => { + console.log(err === null, String(resolved).endsWith("/real/f.txt")); +}); diff --git a/tests/fixtures/npm/node_modules/fs.realpath/LICENSE b/tests/fixtures/npm/node_modules/fs.realpath/LICENSE new file mode 100644 index 0000000000..5bd884c252 --- /dev/null +++ b/tests/fixtures/npm/node_modules/fs.realpath/LICENSE @@ -0,0 +1,43 @@ +The ISC License + +Copyright (c) Isaac Z. Schlueter and Contributors + +Permission to use, copy, modify, and/or distribute this software for any +purpose with or without fee is hereby granted, provided that the above +copyright notice and this permission notice appear in all copies. + +THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES +WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF +MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR +ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES +WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN +ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR +IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + +---- + +This library bundles a version of the `fs.realpath` and `fs.realpathSync` +methods from Node.js v0.10 under the terms of the Node.js MIT license. + +Node's license follows, also included at the header of `old.js` which contains +the licensed code: + + Copyright Joyent, Inc. and other Node contributors. + + Permission is hereby granted, free of charge, to any person obtaining a + copy of this software and associated documentation files (the "Software"), + to deal in the Software without restriction, including without limitation + the rights to use, copy, modify, merge, publish, distribute, sublicense, + and/or sell copies of the Software, and to permit persons to whom the + Software is furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + DEALINGS IN THE SOFTWARE. diff --git a/tests/fixtures/npm/node_modules/fs.realpath/README.md b/tests/fixtures/npm/node_modules/fs.realpath/README.md new file mode 100644 index 0000000000..a42ceac626 --- /dev/null +++ b/tests/fixtures/npm/node_modules/fs.realpath/README.md @@ -0,0 +1,33 @@ +# fs.realpath + +A backwards-compatible fs.realpath for Node v6 and above + +In Node v6, the JavaScript implementation of fs.realpath was replaced +with a faster (but less resilient) native implementation. That raises +new and platform-specific errors and cannot handle long or excessively +symlink-looping paths. + +This module handles those cases by detecting the new errors and +falling back to the JavaScript implementation. On versions of Node +prior to v6, it has no effect. + +## USAGE + +```js +var rp = require('fs.realpath') + +// async version +rp.realpath(someLongAndLoopingPath, function (er, real) { + // the ELOOP was handled, but it was a bit slower +}) + +// sync version +var real = rp.realpathSync(someLongAndLoopingPath) + +// monkeypatch at your own risk! +// This replaces the fs.realpath/fs.realpathSync builtins +rp.monkeypatch() + +// un-do the monkeypatching +rp.unmonkeypatch() +``` diff --git a/tests/fixtures/npm/node_modules/fs.realpath/index.js b/tests/fixtures/npm/node_modules/fs.realpath/index.js new file mode 100644 index 0000000000..b09c7c7e63 --- /dev/null +++ b/tests/fixtures/npm/node_modules/fs.realpath/index.js @@ -0,0 +1,66 @@ +module.exports = realpath +realpath.realpath = realpath +realpath.sync = realpathSync +realpath.realpathSync = realpathSync +realpath.monkeypatch = monkeypatch +realpath.unmonkeypatch = unmonkeypatch + +var fs = require('fs') +var origRealpath = fs.realpath +var origRealpathSync = fs.realpathSync + +var version = process.version +var ok = /^v[0-5]\./.test(version) +var old = require('./old.js') + +function newError (er) { + return er && er.syscall === 'realpath' && ( + er.code === 'ELOOP' || + er.code === 'ENOMEM' || + er.code === 'ENAMETOOLONG' + ) +} + +function realpath (p, cache, cb) { + if (ok) { + return origRealpath(p, cache, cb) + } + + if (typeof cache === 'function') { + cb = cache + cache = null + } + origRealpath(p, cache, function (er, result) { + if (newError(er)) { + old.realpath(p, cache, cb) + } else { + cb(er, result) + } + }) +} + +function realpathSync (p, cache) { + if (ok) { + return origRealpathSync(p, cache) + } + + try { + return origRealpathSync(p, cache) + } catch (er) { + if (newError(er)) { + return old.realpathSync(p, cache) + } else { + throw er + } + } +} + +function monkeypatch () { + fs.realpath = realpath + fs.realpathSync = realpathSync +} + +function unmonkeypatch () { + fs.realpath = origRealpath + fs.realpathSync = origRealpathSync +} diff --git a/tests/fixtures/npm/node_modules/fs.realpath/old.js b/tests/fixtures/npm/node_modules/fs.realpath/old.js new file mode 100644 index 0000000000..b40305e73f --- /dev/null +++ b/tests/fixtures/npm/node_modules/fs.realpath/old.js @@ -0,0 +1,303 @@ +// Copyright Joyent, Inc. and other Node contributors. +// +// Permission is hereby granted, free of charge, to any person obtaining a +// copy of this software and associated documentation files (the +// "Software"), to deal in the Software without restriction, including +// without limitation the rights to use, copy, modify, merge, publish, +// distribute, sublicense, and/or sell copies of the Software, and to permit +// persons to whom the Software is furnished to do so, subject to the +// following conditions: +// +// The above copyright notice and this permission notice shall be included +// in all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +// OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +// MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN +// NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, +// DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE +// USE OR OTHER DEALINGS IN THE SOFTWARE. + +var pathModule = require('path'); +var isWindows = process.platform === 'win32'; +var fs = require('fs'); + +// JavaScript implementation of realpath, ported from node pre-v6 + +var DEBUG = process.env.NODE_DEBUG && /fs/.test(process.env.NODE_DEBUG); + +function rethrow() { + // Only enable in debug mode. A backtrace uses ~1000 bytes of heap space and + // is fairly slow to generate. + var callback; + if (DEBUG) { + var backtrace = new Error; + callback = debugCallback; + } else + callback = missingCallback; + + return callback; + + function debugCallback(err) { + if (err) { + backtrace.message = err.message; + err = backtrace; + missingCallback(err); + } + } + + function missingCallback(err) { + if (err) { + if (process.throwDeprecation) + throw err; // Forgot a callback but don't know where? Use NODE_DEBUG=fs + else if (!process.noDeprecation) { + var msg = 'fs: missing callback ' + (err.stack || err.message); + if (process.traceDeprecation) + console.trace(msg); + else + console.error(msg); + } + } + } +} + +function maybeCallback(cb) { + return typeof cb === 'function' ? cb : rethrow(); +} + +var normalize = pathModule.normalize; + +// Regexp that finds the next partion of a (partial) path +// result is [base_with_slash, base], e.g. ['somedir/', 'somedir'] +if (isWindows) { + var nextPartRe = /(.*?)(?:[\/\\]+|$)/g; +} else { + var nextPartRe = /(.*?)(?:[\/]+|$)/g; +} + +// Regex to find the device root, including trailing slash. E.g. 'c:\\'. +if (isWindows) { + var splitRootRe = /^(?:[a-zA-Z]:|[\\\/]{2}[^\\\/]+[\\\/][^\\\/]+)?[\\\/]*/; +} else { + var splitRootRe = /^[\/]*/; +} + +exports.realpathSync = function realpathSync(p, cache) { + // make p is absolute + p = pathModule.resolve(p); + + if (cache && Object.prototype.hasOwnProperty.call(cache, p)) { + return cache[p]; + } + + var original = p, + seenLinks = {}, + knownHard = {}; + + // current character position in p + var pos; + // the partial path so far, including a trailing slash if any + var current; + // the partial path without a trailing slash (except when pointing at a root) + var base; + // the partial path scanned in the previous round, with slash + var previous; + + start(); + + function start() { + // Skip over roots + var m = splitRootRe.exec(p); + pos = m[0].length; + current = m[0]; + base = m[0]; + previous = ''; + + // On windows, check that the root exists. On unix there is no need. + if (isWindows && !knownHard[base]) { + fs.lstatSync(base); + knownHard[base] = true; + } + } + + // walk down the path, swapping out linked pathparts for their real + // values + // NB: p.length changes. + while (pos < p.length) { + // find the next part + nextPartRe.lastIndex = pos; + var result = nextPartRe.exec(p); + previous = current; + current += result[0]; + base = previous + result[1]; + pos = nextPartRe.lastIndex; + + // continue if not a symlink + if (knownHard[base] || (cache && cache[base] === base)) { + continue; + } + + var resolvedLink; + if (cache && Object.prototype.hasOwnProperty.call(cache, base)) { + // some known symbolic link. no need to stat again. + resolvedLink = cache[base]; + } else { + var stat = fs.lstatSync(base); + if (!stat.isSymbolicLink()) { + knownHard[base] = true; + if (cache) cache[base] = base; + continue; + } + + // read the link if it wasn't read before + // dev/ino always return 0 on windows, so skip the check. + var linkTarget = null; + if (!isWindows) { + var id = stat.dev.toString(32) + ':' + stat.ino.toString(32); + if (seenLinks.hasOwnProperty(id)) { + linkTarget = seenLinks[id]; + } + } + if (linkTarget === null) { + fs.statSync(base); + linkTarget = fs.readlinkSync(base); + } + resolvedLink = pathModule.resolve(previous, linkTarget); + // track this, if given a cache. + if (cache) cache[base] = resolvedLink; + if (!isWindows) seenLinks[id] = linkTarget; + } + + // resolve the link, then start over + p = pathModule.resolve(resolvedLink, p.slice(pos)); + start(); + } + + if (cache) cache[original] = p; + + return p; +}; + + +exports.realpath = function realpath(p, cache, cb) { + if (typeof cb !== 'function') { + cb = maybeCallback(cache); + cache = null; + } + + // make p is absolute + p = pathModule.resolve(p); + + if (cache && Object.prototype.hasOwnProperty.call(cache, p)) { + return process.nextTick(cb.bind(null, null, cache[p])); + } + + var original = p, + seenLinks = {}, + knownHard = {}; + + // current character position in p + var pos; + // the partial path so far, including a trailing slash if any + var current; + // the partial path without a trailing slash (except when pointing at a root) + var base; + // the partial path scanned in the previous round, with slash + var previous; + + start(); + + function start() { + // Skip over roots + var m = splitRootRe.exec(p); + pos = m[0].length; + current = m[0]; + base = m[0]; + previous = ''; + + // On windows, check that the root exists. On unix there is no need. + if (isWindows && !knownHard[base]) { + fs.lstat(base, function(err) { + if (err) return cb(err); + knownHard[base] = true; + LOOP(); + }); + } else { + process.nextTick(LOOP); + } + } + + // walk down the path, swapping out linked pathparts for their real + // values + function LOOP() { + // stop if scanned past end of path + if (pos >= p.length) { + if (cache) cache[original] = p; + return cb(null, p); + } + + // find the next part + nextPartRe.lastIndex = pos; + var result = nextPartRe.exec(p); + previous = current; + current += result[0]; + base = previous + result[1]; + pos = nextPartRe.lastIndex; + + // continue if not a symlink + if (knownHard[base] || (cache && cache[base] === base)) { + return process.nextTick(LOOP); + } + + if (cache && Object.prototype.hasOwnProperty.call(cache, base)) { + // known symbolic link. no need to stat again. + return gotResolvedLink(cache[base]); + } + + return fs.lstat(base, gotStat); + } + + function gotStat(err, stat) { + if (err) return cb(err); + + // if not a symlink, skip to the next path part + if (!stat.isSymbolicLink()) { + knownHard[base] = true; + if (cache) cache[base] = base; + return process.nextTick(LOOP); + } + + // stat & read the link if not read before + // call gotTarget as soon as the link target is known + // dev/ino always return 0 on windows, so skip the check. + if (!isWindows) { + var id = stat.dev.toString(32) + ':' + stat.ino.toString(32); + if (seenLinks.hasOwnProperty(id)) { + return gotTarget(null, seenLinks[id], base); + } + } + fs.stat(base, function(err) { + if (err) return cb(err); + + fs.readlink(base, function(err, target) { + if (!isWindows) seenLinks[id] = target; + gotTarget(err, target); + }); + }); + } + + function gotTarget(err, target, base) { + if (err) return cb(err); + + var resolvedLink = pathModule.resolve(previous, target); + if (cache) cache[base] = resolvedLink; + gotResolvedLink(resolvedLink); + } + + function gotResolvedLink(resolvedLink) { + // resolve the link, then start over + p = pathModule.resolve(resolvedLink, p.slice(pos)); + start(); + } +}; diff --git a/tests/fixtures/npm/node_modules/fs.realpath/package.json b/tests/fixtures/npm/node_modules/fs.realpath/package.json new file mode 100644 index 0000000000..3edc57d21c --- /dev/null +++ b/tests/fixtures/npm/node_modules/fs.realpath/package.json @@ -0,0 +1,26 @@ +{ + "name": "fs.realpath", + "version": "1.0.0", + "description": "Use node's fs.realpath, but fall back to the JS implementation if the native one fails", + "main": "index.js", + "dependencies": {}, + "devDependencies": {}, + "scripts": { + "test": "tap test/*.js --cov" + }, + "repository": { + "type": "git", + "url": "git+https://github.com/isaacs/fs.realpath.git" + }, + "keywords": [ + "realpath", + "fs", + "polyfill" + ], + "author": "Isaac Z. Schlueter (http://blog.izs.me/)", + "license": "ISC", + "files": [ + "old.js", + "index.js" + ] +} diff --git a/tests/harness/npm-cases.ts b/tests/harness/npm-cases.ts index 691d1c100d..c085cfe7a3 100644 --- a/tests/harness/npm-cases.ts +++ b/tests/harness/npm-cases.ts @@ -19,14 +19,14 @@ export function npmCases(fixturesRoot: string): NpmCase[] { return [ ...globSync(join(fixturesRoot, "npm/cases/*/main.ts")) .sort() - // 2465-2469, 2556-2567, 2571 and 2580-2593 are the --npm-static bundler-emitted-CJS + // 2465-2469, 2556-2567, 2571 and 2580-2594 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-7]|2571|258[0-9]|259[0-3])-[^/]+\/main\.ts$/.test(entry), + !/\/(246[5-9]|255[6-9]|256[0-7]|2571|258[0-9]|259[0-4])-[^/]+\/main\.ts$/.test(entry), ) .map((entry) => ({ name: entry.split("/").at(-2)!, diff --git a/tests/harness/npm-static.test.ts b/tests/harness/npm-static.test.ts index ce41f65e5a..5c39cfb2e2 100644 --- a/tests/harness/npm-static.test.ts +++ b/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-2567, 2571, 2580-2593) + // the bundler-emitted-CJS mini packages (cases 2465-2469, 2556-2567, 2571, 2580-2594) ...globSync(join(fixturesRoot, "npm/node_modules/gt*/**/*.{js,cjs,json}")).sort(), ...globSync(join(fixturesRoot, "npm/store/gt*/**/*.{js,cjs,json}")).sort(), ]; @@ -1650,6 +1650,13 @@ describe(`npm-static pilots${sanitize ? " (sanitized)" : ""}`, () => { ]); }, 180_000); + // 2594: fs.realpath keeps fs.realpathSync and fs.realpath as values. + test("fs.realpath 2594-fs-realpath-values byte-matches Node", async () => { + await expectNodeParity(join(fixturesRoot, "npm/cases/2594-fs-realpath-values/main.ts"), [ + "fs.realpath", + ]); + }, 180_000); + // 2562: a file with unverifiable interop that no compiled code can load // (the program imports a leaf data module and only a type from the // main entry) leaves its package static. From 2f14ef360186508757cb0aa90d4106318e3963b6 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Tue, 20 Oct 2026 12:13:07 -0700 Subject: [PATCH 05/22] Read and call members a module attaches to its whole export MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `module.exports = format; module.exports.shout = shout` (or the `exports = module.exports = format` spelling) attaches `shout` to the function. Destructured requires already found the member, but a read or call through the requirer's binding of the function (`format.shout`) answered undefined, and a call threw "is not a function". A read through a `const` require binding now resolves to the member's value when the module writes it once and the use site can read it again; a call resolves to the member function when it ignores `this` (Node passes the export as `this`) and otherwise fences by name. Applying the package-file rewrite (`module.exports.x` respelled as `F.x`) to program files was not the fix: it turns member exports into expandos of the function, so a requirer's `const { shout } = require(…)` found no member export and met "a binding form with no lowering" (corpus 6431). Co-Authored-By: Claude Opus 5.5 --- .../frontend/lowering/cjs-exports-object.ts | 64 +++++++++++++++++++ .../lowering/expressions/property-access.ts | 8 +++ .../src/frontend/lowering/lower-calls.ts | 28 +++++++- .../compiler/test/cjs-settled-exports.test.ts | 31 ++++++++- .../6485-cjs-whole-export-members/main.cjs | 11 ++++ .../6485-cjs-whole-export-members/plain.cjs | 10 +++ .../6485-cjs-whole-export-members/rebound.cjs | 10 +++ 7 files changed, 159 insertions(+), 3 deletions(-) create mode 100644 tests/corpus/6485-cjs-whole-export-members/main.cjs create mode 100644 tests/corpus/6485-cjs-whole-export-members/plain.cjs create mode 100644 tests/corpus/6485-cjs-whole-export-members/rebound.cjs diff --git a/packages/compiler/src/frontend/lowering/cjs-exports-object.ts b/packages/compiler/src/frontend/lowering/cjs-exports-object.ts index 5e72e4bf70..16a6eca170 100644 --- a/packages/compiler/src/frontend/lowering/cjs-exports-object.ts +++ b/packages/compiler/src/frontend/lowering/cjs-exports-object.ts @@ -533,6 +533,70 @@ export function cjsMemberAliasValueOf(lowerer: Lowerer, ref: ts.Identifier): ts. return memberAliasIn(lowerer, req.dep, key.text, 0); } +/** The module a `const fmt = require("./lib")` binding names, when `access` + * reads a member through that binding: `fmt.name`. */ +function requiredModuleOfMemberAccess( + lowerer: Lowerer, + access: ts.PropertyAccessExpression, +): ts.SourceFile | null { + if (access.questionDotToken !== undefined || !ts.isIdentifier(access.name)) return null; + if (!ts.isIdentifier(access.expression)) return null; + const sym = lowerer.checker.getSymbolAtLocation(access.expression); + const declaration = sym?.declarations.length === 1 ? sym.declarations[0] : undefined; + if ( + declaration === undefined || + !ts.isVariableDeclaration(declaration) || + !ts.isIdentifier(declaration.name) || + declaration.initializer === undefined || + (ts.getCombinedNodeFlags(declaration) & ts.NodeFlags.Const) === 0 + ) + return null; + return inlineProgramRequireOf(lowerer, declaration.initializer)?.dep ?? null; +} + +/** `fmt.shout` where `const fmt = require("./lib")` names a module whose + * whole export is a value (`module.exports = format`) and which writes + * `module.exports.shout = …` (or `exports.shout = …` after rebinding + * `exports`) once: Node reads the member off that same value, so the read + * is the member's value. A call through the member passes the export as + * `this`, so calls resolve only for functions that ignore it. */ +export function cjsWholeExportMemberValueOf( + lowerer: Lowerer, + access: ts.PropertyAccessExpression, + call = false, +): ts.Expression | null { + const parent = access.parent; + if (!call && parent !== undefined && ts.isCallExpression(parent) && parent.expression === access) + return null; + if ( + parent !== undefined && + ts.isBinaryExpression(parent) && + parent.left === access && + parent.operatorToken.kind >= ts.SyntaxKind.FirstAssignment && + parent.operatorToken.kind <= ts.SyntaxKind.LastAssignment + ) + return null; + const dep = requiredModuleOfMemberAccess(lowerer, access); + if (dep === null) return null; + const name = (access.name as ts.Identifier).text; + if (call && !cjsExportCallIgnoresThis(lowerer, dep, name)) return null; + return memberAliasIn(lowerer, dep, name, 0); +} + +/** True when `access` calls a member of a module's whole export value that + * may read `this` (Node binds it to the export). */ +export function cjsWholeExportMemberCallReadsThis( + lowerer: Lowerer, + access: ts.PropertyAccessExpression, +): boolean { + const dep = requiredModuleOfMemberAccess(lowerer, access); + if (dep === null) return false; + const name = (access.name as ts.Identifier).text; + return ( + memberAliasIn(lowerer, dep, name, 0) !== null && !cjsExportCallIgnoresThis(lowerer, dep, name) + ); +} + function memberAliasIn( lowerer: Lowerer, sf: ts.SourceFile, diff --git a/packages/compiler/src/frontend/lowering/expressions/property-access.ts b/packages/compiler/src/frontend/lowering/expressions/property-access.ts index 65c8fbcbcc..e9e1096409 100644 --- a/packages/compiler/src/frontend/lowering/expressions/property-access.ts +++ b/packages/compiler/src/frontend/lowering/expressions/property-access.ts @@ -74,6 +74,7 @@ import { } from "../lower-exprs.js"; import { cjsInFileExportGlobalOf, + cjsWholeExportMemberValueOf, isCjsExportsIdentifier, lowerCjsExportsObjectValue, } from "../cjs-exports-object.js"; @@ -292,6 +293,13 @@ export function lowerPropertyAccessExpr( const exportsValue = lowerCjsExportsObjectValue(lowerer, expr); if (exportsValue) return exportsValue; } + // A member a module attached to its whole export value + // (`module.exports = format; module.exports.shout = shout`), read + // through a requirer's binding of that value. + { + const member = cjsWholeExportMemberValueOf(lowerer, expr); + if (member !== null) return lowerExpr(lowerer, member); + } // tsc's `x_1.default` over a dependency whose whole export is one // value: that value (cjs-tsc-interop.ts). const interop = tscInteropDefaultValueOf(lowerer, expr); diff --git a/packages/compiler/src/frontend/lowering/lower-calls.ts b/packages/compiler/src/frontend/lowering/lower-calls.ts index b0b7884f28..ee8cbd903b 100644 --- a/packages/compiler/src/frontend/lowering/lower-calls.ts +++ b/packages/compiler/src/frontend/lowering/lower-calls.ts @@ -35,7 +35,12 @@ import { lowerUncompiledRequireCall, lowerWithInlineRequireInit, } from "./cjs-inline-require.js"; -import { cjsExportCallIgnoresThis, cjsInFileExportGlobalOf } from "./cjs-exports-object.js"; +import { + cjsExportCallIgnoresThis, + cjsInFileExportGlobalOf, + cjsWholeExportMemberCallReadsThis, + cjsWholeExportMemberValueOf, +} from "./cjs-exports-object.js"; import { lowerGenMethodCall } from "./lower-generators.js"; import { lowerClassSymbolMethodCall } from "./symbol-methods.js"; import { lowerReadlineDisposeCall } from "./builtins/readline.js"; @@ -1339,6 +1344,27 @@ export function lowerCall(lowerer: Lowerer, expr: ts.CallExpression): IrExpr { return nsUndefRead(lowerer, ambientRoot.text, expr, t); } } + // A function a module attached to its whole export value + // (`module.exports = format; module.exports.shout = shout`), called + // through a requirer's binding of that value: a call of the function. + if (ts.isPropertyAccessExpression(expr.expression)) { + const member = cjsWholeExportMemberValueOf(lowerer, expr.expression, true); + if (member !== null && ts.isIdentifier(member)) { + const sig = lowerer.fnSigOf(member); + if (sig) { + lowerer.noteEdge(sig.name); + const args = lowerer.completeArgs(expr.arguments.nodes, sig.params, loc, expr); + return { kind: "call", callee: sig.name, args, type: sig.returnType, loc }; + } + } + if (member === null && cjsWholeExportMemberCallReadsThis(lowerer, expr.expression)) { + lowerer.unsupported( + "SC1090", + expr, + `calling '${expr.expression.name.text}' through a module's whole export when the function may read 'this' (Node binds it to the export)`, + ); + } + } // CommonJS namespace member calls (`lib.double(5)` where lib is // `const lib = require("./lib.js")`): the export table is alias // plumbing, so the member call IS a call of the exporter's declaration diff --git a/packages/compiler/test/cjs-settled-exports.test.ts b/packages/compiler/test/cjs-settled-exports.test.ts index 74fa598efd..af7438b994 100644 --- a/packages/compiler/test/cjs-settled-exports.test.ts +++ b/packages/compiler/test/cjs-settled-exports.test.ts @@ -14,14 +14,14 @@ import { analyze } from "../src/index.js"; const scratch = mkdtempSync(join(tmpdir(), "scriptc-cjs-settled-")); afterAll(() => rmSync(scratch, { recursive: true, force: true })); -function fences(files: Record): string[] { +function fences(files: Record, match = "by reference"): string[] { const dir = mkdtempSync(join(scratch, "case-")); writeFileSync(join(dir, "package.json"), '{ "private": true }\n'); for (const [name, text] of Object.entries(files)) writeFileSync(join(dir, name), text); const { coverage } = analyze(join(dir, "main.cjs")); expect(coverage.preflightFailed).toBe(false); return [...coverage.diagnostics, ...(coverage.runtimeFences ?? [])] - .filter((d) => d.message.includes("by reference")) + .filter((d) => d.message.includes(match)) .map((d) => `${d.loc.file.slice(dir.length + 1)}: ${d.message.split(" (")[0]}`); } @@ -73,3 +73,30 @@ module.exports = { total, label }; }), ).toEqual([]); }); + +test("a call through a whole export's member that may read 'this' fences", () => { + expect( + fences( + { + "main.cjs": `"use strict"; +const Counter = require("./counter.cjs"); +console.log(Counter.bump(2), Counter.n); +`, + "counter.cjs": `"use strict"; +function Counter() { + this.n = 0; +} +function bump(by) { + this.n = (this.n || 0) + by; + return this.n; +} +module.exports = Counter; +module.exports.bump = bump; +`, + }, + "may read 'this'", + ), + ).toEqual([ + "main.cjs: calling 'bump' through a module's whole export when the function may read 'this'", + ]); +}); diff --git a/tests/corpus/6485-cjs-whole-export-members/main.cjs b/tests/corpus/6485-cjs-whole-export-members/main.cjs new file mode 100644 index 0000000000..762a53cbce --- /dev/null +++ b/tests/corpus/6485-cjs-whole-export-members/main.cjs @@ -0,0 +1,11 @@ +// A module whose whole export is a function and which attaches members to +// it (`module.exports = format; module.exports.shout = shout`, with or +// without rebinding `exports`): requirers read and call the members +// through the function, as Node attaches them to it. +"use strict"; +const format = require("./plain.cjs"); +const quote = require("./rebound.cjs"); +console.log(format("a"), format.shout("b"), format.version, typeof format.shout); +console.log(quote("c"), quote.whisper("D"), quote.kind, typeof quote.whisper); +const { shout } = require("./plain.cjs"); +console.log(shout === format.shout); diff --git a/tests/corpus/6485-cjs-whole-export-members/plain.cjs b/tests/corpus/6485-cjs-whole-export-members/plain.cjs new file mode 100644 index 0000000000..b0871dfbc8 --- /dev/null +++ b/tests/corpus/6485-cjs-whole-export-members/plain.cjs @@ -0,0 +1,10 @@ +"use strict"; +function format(value) { + return "[" + value + "]"; +} +function shout(value) { + return String(value).toUpperCase(); +} +module.exports = format; +module.exports.shout = shout; +module.exports.version = "1.2.3"; diff --git a/tests/corpus/6485-cjs-whole-export-members/rebound.cjs b/tests/corpus/6485-cjs-whole-export-members/rebound.cjs new file mode 100644 index 0000000000..d1ebca7b1e --- /dev/null +++ b/tests/corpus/6485-cjs-whole-export-members/rebound.cjs @@ -0,0 +1,10 @@ +"use strict"; +function quote(value) { + return '"' + value + '"'; +} +function whisper(value) { + return String(value).toLowerCase(); +} +exports = module.exports = quote; +exports.whisper = whisper; +exports.kind = "quote"; From 4a280500f590960822074270cf4d46bdfa26a70d Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Tue, 20 Oct 2026 12:26:45 -0700 Subject: [PATCH 06/22] Format and record the new fixtures' order baselines Co-Authored-By: Claude Opus 5.5 --- .../frontend/lowering/lower-builtin-values.ts | 14 ++++++++------ .../compiler/src/frontend/lowering/lowerer.ts | 3 ++- .../src/frontend/lowering/var-hoisting.ts | 6 +++++- .../npm-static-split-declarations.test.ts | 7 +++++-- .../frontend/npm-static-split-declarations.ts | 9 +++++++-- .../6485-cjs-whole-export-members/main.cjs.json | 8 ++++++++ .../2592-bundled-merged-requires/main.ts.json | 6 ++++++ .../2593-bundled-semver-tokens/main.ts.json | 6 ++++++ .../cases/2594-fs-realpath-values/main.ts.json | 16 ++++++++++++++++ 9 files changed, 63 insertions(+), 12 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6485-cjs-whole-export-members/main.cjs.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2592-bundled-merged-requires/main.ts.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2593-bundled-semver-tokens/main.ts.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2594-fs-realpath-values/main.ts.json diff --git a/packages/compiler/src/frontend/lowering/lower-builtin-values.ts b/packages/compiler/src/frontend/lowering/lower-builtin-values.ts index 47f0902363..4a41be8786 100644 --- a/packages/compiler/src/frontend/lowering/lower-builtin-values.ts +++ b/packages/compiler/src/frontend/lowering/lower-builtin-values.ts @@ -727,11 +727,7 @@ export function lowerFsRealpathSyncValue(lowerer: Lowerer, loc: SrcLoc): IrExpr const path = varRef("path", DYN, loc); const options = varRef("options", DYN, loc); const fence = (what: string): IrStmt => { - const diag = unsupportedDiag( - "SC1090", - loc, - `fs.realpathSync as a value called with ${what}`, - ); + const diag = unsupportedDiag("SC1090", loc, `fs.realpathSync as a value called with ${what}`); lowerer.runtimeFences.push(diag); return { kind: "runtimeFence", @@ -748,7 +744,13 @@ export function lowerFsRealpathSyncValue(lowerer: Lowerer, loc: SrcLoc): IrExpr type: BOOL, loc, }); - const encoding: IrExpr = { kind: "dynKeyGet", key: strLit("encoding", loc), value: options, type: DYN, loc }; + const encoding: IrExpr = { + kind: "dynKeyGet", + key: strLit("encoding", loc), + value: options, + type: DYN, + loc, + }; const defaultEncoding: IrExpr = { kind: "logical", op: "||", diff --git a/packages/compiler/src/frontend/lowering/lowerer.ts b/packages/compiler/src/frontend/lowering/lowerer.ts index f2c808aa5b..f07c3d6477 100644 --- a/packages/compiler/src/frontend/lowering/lowerer.ts +++ b/packages/compiler/src/frontend/lowering/lowerer.ts @@ -12877,7 +12877,8 @@ export class Lowerer { return lowerUtilTypeValue(this, bi.member, loc); if (bi.module === "util" && bi.member === "getSystemErrorMap") return lowerSystemErrorMapValue(this, loc); - if (bi.module === "fs" && bi.member === "realpathSync") return lowerFsRealpathSyncValue(this, loc); + if (bi.module === "fs" && bi.member === "realpathSync") + return lowerFsRealpathSyncValue(this, loc); if ( bi.module === "fs" && builtinModuleFnOf(this, bi.module, bi.member)?.fn === "fs.callbackCall" diff --git a/packages/compiler/src/frontend/lowering/var-hoisting.ts b/packages/compiler/src/frontend/lowering/var-hoisting.ts index 0cd96ec4a7..2a78e8fd30 100644 --- a/packages/compiler/src/frontend/lowering/var-hoisting.ts +++ b/packages/compiler/src/frontend/lowering/var-hoisting.ts @@ -371,7 +371,11 @@ function cannotReenter(stmt: ts.Statement): boolean { } else if (ts.isIdentifier(n) && (n.text === "module" || n.text === "exports")) { // Only as the root of a store target. let target: ts.Node = n; - while (target.parent !== undefined && ts.isPropertyAccessExpression(target.parent) && target.parent.expression === target) + while ( + target.parent !== undefined && + ts.isPropertyAccessExpression(target.parent) && + target.parent.expression === target + ) target = target.parent; const assign = target.parent; if ( diff --git a/packages/compiler/src/frontend/npm-static-split-declarations.test.ts b/packages/compiler/src/frontend/npm-static-split-declarations.test.ts index eda22b0e72..42fc0727d3 100644 --- a/packages/compiler/src/frontend/npm-static-split-declarations.test.ts +++ b/packages/compiler/src/frontend/npm-static-split-declarations.test.ts @@ -31,7 +31,8 @@ function expectSameBehavior(source: string): string { writeFileSync(join(dir, "lib", "index.js"), lib); writeFileSync(join(dir, "original.cjs"), source); writeFileSync(join(dir, "rewritten.cjs"), result!); - const run = (file: string) => spawnSync(process.execPath, [join(dir, file)], { encoding: "utf8" }); + const run = (file: string) => + spawnSync(process.execPath, [join(dir, file)], { encoding: "utf8" }); const oracle = run("original.cjs"); expect(oracle.status).toBe(0); expect(run("rewritten.cjs").stdout).toBe(oracle.stdout); @@ -73,5 +74,7 @@ tok("A"); tok("B"); re[t.B] = "b"; console.log(JSON.stringify(exports.tokens), exports.re.length, t === exports.tokens); `); - expect(out).toContain("var re = []; exports.re = re; var t = {}; module.exports.tokens = t; var R = 0;"); + expect(out).toContain( + "var re = []; exports.re = re; var t = {}; module.exports.tokens = t; var R = 0;", + ); }); diff --git a/packages/compiler/src/frontend/npm-static-split-declarations.ts b/packages/compiler/src/frontend/npm-static-split-declarations.ts index 1abbba6279..54714ab3d6 100644 --- a/packages/compiler/src/frontend/npm-static-split-declarations.ts +++ b/packages/compiler/src/frontend/npm-static-split-declarations.ts @@ -51,7 +51,8 @@ function isRequireValue(expr: ts.Expression): boolean { const args = e.arguments.nodes; if (e.expression.text === "require") return ( - args.length === 1 && (ts.isStringLiteral(args[0]!) || ts.isNoSubstitutionTemplateLiteral(args[0]!)) + args.length === 1 && + (ts.isStringLiteral(args[0]!) || ts.isNoSubstitutionTemplateLiteral(args[0]!)) ); if (e.expression.text === "__toESM") return (args.length === 1 || args.length === 2) && isRequireValue(args[0]!); @@ -69,7 +70,11 @@ export function splitRequireDeclarations(sf: ts.SourceFile): string | null { const decls = list.declarations.nodes; const flags = list.flags; const keyword = - (flags & ts.NodeFlags.Const) !== 0 ? "const" : (flags & ts.NodeFlags.Let) !== 0 ? "let" : "var"; + (flags & ts.NodeFlags.Const) !== 0 + ? "const" + : (flags & ts.NodeFlags.Let) !== 0 + ? "let" + : "var"; const stores = decls.map((d) => ts.isIdentifier(d.name) && d.initializer !== undefined ? exportStoreOf(d.initializer) : null, ); diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6485-cjs-whole-export-members/main.cjs.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6485-cjs-whole-export-members/main.cjs.json new file mode 100644 index 0000000000..fbacc3c7e1 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6485-cjs-whole-export-members/main.cjs.json @@ -0,0 +1,8 @@ +{ + "order": [ + "/tests/corpus/6485-cjs-whole-export-members/plain.cjs", + "/tests/corpus/6485-cjs-whole-export-members/rebound.cjs", + "/tests/corpus/6485-cjs-whole-export-members/main.cjs" + ], + "diags": [] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2592-bundled-merged-requires/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2592-bundled-merged-requires/main.ts.json new file mode 100644 index 0000000000..28ae665ca2 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2592-bundled-merged-requires/main.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/fixtures/npm/cases/2592-bundled-merged-requires/main.ts" + ], + "diags": [] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2593-bundled-semver-tokens/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2593-bundled-semver-tokens/main.ts.json new file mode 100644 index 0000000000..6ac215b61c --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2593-bundled-semver-tokens/main.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/fixtures/npm/cases/2593-bundled-semver-tokens/main.ts" + ], + "diags": [] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2594-fs-realpath-values/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2594-fs-realpath-values/main.ts.json new file mode 100644 index 0000000000..05a9b7e65c --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2594-fs-realpath-values/main.ts.json @@ -0,0 +1,16 @@ +{ + "order": [ + "/tests/fixtures/npm/cases/2594-fs-realpath-values/main.ts" + ], + "diags": [ + { + "code": "SC0001", + "message": "Could not find a declaration file for module 'fs.realpath'. '/tests/fixtures/npm/node_modules/fs.realpath/index.js' implicitly has an 'any' type.\n Try `npm i --save-dev @types/fs.realpath` if it exists or add a new declaration (.d.ts) file containing `declare module 'fs.realpath';`", + "loc": { + "file": "/tests/fixtures/npm/cases/2594-fs-realpath-values/main.ts", + "start": 443, + "end": 456 + } + } + ] +} From a80604c9d7b44b688e307d08e76e91c4aeb1a472 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Tue, 20 Oct 2026 13:27:17 -0700 Subject: [PATCH 07/22] Resolve whole-export members only for function exports the requirer does not write A member read through a binding of a factory-returned object (corpus 2923) or one the requirer writes later is a runtime slot: it keeps reading the object. Co-Authored-By: Claude Opus 5.5 --- .../frontend/lowering/cjs-exports-object.ts | 37 ++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/packages/compiler/src/frontend/lowering/cjs-exports-object.ts b/packages/compiler/src/frontend/lowering/cjs-exports-object.ts index 16a6eca170..c2910caab3 100644 --- a/packages/compiler/src/frontend/lowering/cjs-exports-object.ts +++ b/packages/compiler/src/frontend/lowering/cjs-exports-object.ts @@ -551,7 +551,42 @@ function requiredModuleOfMemberAccess( (ts.getCombinedNodeFlags(declaration) & ts.NodeFlags.Const) === 0 ) return null; - return inlineProgramRequireOf(lowerer, declaration.initializer)?.dep ?? null; + const dep = inlineProgramRequireOf(lowerer, declaration.initializer)?.dep ?? null; + if (dep === null || !wholeExportIsFunction(lowerer, dep)) return null; + // A member the requirer writes through its binding is a runtime slot. + const name = access.name.text; + let written = false; + ts.walkPreorder(access.getSourceFile(), (n) => { + if (written) return "stop"; + if ( + ts.isBinaryExpression(n) && + n.operatorToken.kind >= ts.SyntaxKind.FirstAssignment && + n.operatorToken.kind <= ts.SyntaxKind.LastAssignment && + ts.isPropertyAccessExpression(n.left) && + ts.isIdentifier(n.left.name) && + n.left.name.text === name && + ts.isIdentifier(n.left.expression) && + lowerer.checker.getSymbolAtLocation(n.left.expression) === sym + ) + written = true; + return undefined; + }); + return written ? null : dep; +} + +/** True when the module's whole export names a function: a function + * declaration, or a binding of a function expression or arrow. */ +function wholeExportIsFunction(lowerer: Lowerer, sf: ts.SourceFile): boolean { + const whole = cjsWholeExportValueOf(sf); + if (whole === null || !ts.isIdentifier(whole.rhs)) return false; + const sym = lowerer.resolveValueSymbol(whole.rhs); + const decl = sym?.declarations.length === 1 ? sym.declarations[0] : undefined; + if (decl === undefined) return false; + if (ts.isFunctionDeclaration(decl)) return true; + if (!ts.isVariableDeclaration(decl) || decl.initializer === undefined) return false; + let init: ts.Expression = decl.initializer; + while (ts.isParenthesizedExpression(init)) init = init.expression; + return ts.isFunctionExpression(init) || ts.isArrowFunction(init); } /** `fmt.shout` where `const fmt = require("./lib")` names a module whose From d60d1f20249ab962e620499939fb0efb089f1c94 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Tue, 20 Oct 2026 13:34:29 -0700 Subject: [PATCH 08/22] Unwrap tsc's interop helpers around builtin modules in packages tsc wraps every CommonJS import in its helpers: `const events_1 = __importDefault(require("events"))`, `const path = __importStar(require("path"))`. cjs-tsc-interop.ts reads such bindings of program modules as plain require bindings, so the helper declarations go dead, but a builtin module is not a program module: its binding kept the helpers live, and their probe of the module's top-level `this` (`(this && this.__importDefault) ||`) failed at load ("property access through top-level 'this'"). That is the shape of hundreds of tsc-compiled files in a typical dependency tree. A builtin carries no `__esModule`, so `events_1.default` is the module and the namespace copy reads the module's members. When every reference has that form, a package-file stage now respells the binding as the plain require() of the builtin and `x.default` as `x`. Co-Authored-By: Claude Opus 5.5 --- .../frontend/npm-static-tsc-helpers.test.ts | 62 ++++++++++ .../src/frontend/npm-static-tsc-helpers.ts | 109 ++++++++++++++++++ packages/compiler/src/frontend/npm-static.ts | 5 + packages/compiler/src/frontend/services.ts | 8 ++ .../npm/cases/tsc-builtin-interop/main.ts | 9 ++ .../node_modules/gttscinterop/lib/helper.d.ts | 1 + .../node_modules/gttscinterop/lib/helper.js | 6 + .../node_modules/gttscinterop/lib/index.d.ts | 5 + .../node_modules/gttscinterop/lib/index.js | 43 +++++++ .../node_modules/gttscinterop/package.json | 7 ++ 10 files changed, 255 insertions(+) create mode 100644 packages/compiler/src/frontend/npm-static-tsc-helpers.test.ts create mode 100644 packages/compiler/src/frontend/npm-static-tsc-helpers.ts create mode 100644 tests/fixtures/npm/cases/tsc-builtin-interop/main.ts create mode 100644 tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/lib/helper.d.ts create mode 100644 tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/lib/helper.js create mode 100644 tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/lib/index.d.ts create mode 100644 tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/lib/index.js create mode 100644 tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/package.json diff --git a/packages/compiler/src/frontend/npm-static-tsc-helpers.test.ts b/packages/compiler/src/frontend/npm-static-tsc-helpers.test.ts new file mode 100644 index 0000000000..95add609f3 --- /dev/null +++ b/packages/compiler/src/frontend/npm-static-tsc-helpers.test.ts @@ -0,0 +1,62 @@ +import { spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, expect, test } from "vitest"; +import { nodeFrontendServices } from "./services-node.js"; + +const dirs: string[] = []; +afterAll(() => { + for (const dir of dirs) rmSync(dir, { recursive: true, force: true }); +}); + +function rewrite(source: string): string | null { + return nodeFrontendServices().tscBuiltinImports(source, "/pkg/index.js"); +} + +const helpers = `"use strict"; +var __importDefault = (this && this.__importDefault) || function (mod) { + return (mod && mod.__esModule) ? mod : { "default": mod }; +}; +var __importStar = (this && this.__importStar) || function (mod) { + if (mod && mod.__esModule) return mod; + var result = {}; + if (mod != null) for (var k in mod) if (k !== "default" && Object.prototype.hasOwnProperty.call(mod, k)) result[k] = mod[k]; + result["default"] = mod; + return result; +}; +`; + +function expectSameBehavior(source: string): string { + const result = rewrite(source); + expect(typeof result).toBe("string"); + const dir = mkdtempSync(join(tmpdir(), "scriptc-tsc-builtin-imports-")); + dirs.push(dir); + writeFileSync(join(dir, "original.cjs"), source); + writeFileSync(join(dir, "rewritten.cjs"), result!); + const run = (file: string) => spawnSync(process.execPath, [join(dir, file)], { encoding: "utf8" }); + const oracle = run("original.cjs"); + expect(oracle.status).toBe(0); + expect(run("rewritten.cjs").stdout).toBe(oracle.stdout); + return result!; +} + +test("builtin modules behind tsc's interop helpers become plain requires", () => { + const out = expectSameBehavior(`${helpers}const events_1 = __importDefault(require("events")); +const path = __importStar(require("node:path")); +class Bus extends events_1.default.EventEmitter {} +console.log(new Bus() instanceof events_1.default, path.basename("/a/b.txt"), typeof path.join); +`); + expect(out).toContain('const events_1 = require("events");'); + expect(out).toContain("class Bus extends events_1.EventEmitter {}"); + expect(out).toContain('const path = require("node:path");'); +}); + +test("other references, program modules and default reads of a namespace keep their text", () => { + // The binding itself as a value. + expect(rewrite(`${helpers}const events_1 = __importDefault(require("events"));\nconsole.log(events_1);\n`)).toBeNull(); + // A program module (cjs-tsc-interop.ts owns it). + expect(rewrite(`${helpers}const m = __importDefault(require("./m"));\nm.default();\n`)).toBeNull(); + // `ns.default` reads the namespace copy's own default. + expect(rewrite(`${helpers}const p = __importStar(require("path"));\nconsole.log(p.default === p);\n`)).toBeNull(); +}); diff --git a/packages/compiler/src/frontend/npm-static-tsc-helpers.ts b/packages/compiler/src/frontend/npm-static-tsc-helpers.ts new file mode 100644 index 0000000000..19f69485a0 --- /dev/null +++ b/packages/compiler/src/frontend/npm-static-tsc-helpers.ts @@ -0,0 +1,109 @@ +/* tsc wraps every CommonJS import in its interop helpers: + * + * const events_1 = __importDefault(require("events")); // events_1.default.X + * const path = __importStar(require("path")); // path.basename + * + * For program modules cjs-tsc-interop.ts reads such a binding as a plain + * require binding, and the helper declarations (`var __importDefault = + * (this && this.__importDefault) || …`) go dead. A builtin module is not a + * program module, so its binding kept the helpers live, and their probe of + * the module's top-level `this` has no lowering. A builtin carries no + * `__esModule`: `events_1.default` is the module and the namespace copy + * reads the module's members. When every reference has that form + * (`x.default…` for the default helper, `x.` other than `default` + * for the namespace helper), the binding becomes the plain `require()` of + * the builtin and `x.default` becomes `x`. */ +import * as ts from "./ts7/syntax.js"; +import { applyEdits, unparen, type Edit } from "./npm-static-bundled-chunks.js"; +import { canonicalBuiltinModule } from "./builtin-modules.js"; + +/** True when the text may hold such a binding (a cheap probe). */ +export function mayHoldTscBuiltinImport(source: string): boolean { + return /__import(?:Default|Star)\(require\(/.test(source); +} + +/** `__importDefault(require(""))` / `__importStar(…)`: the helper + * and builtin spec, or null. */ +function builtinInterop(init: ts.Expression): { helper: string; spec: string } | null { + const e = unparen(init); + if (!ts.isCallExpression(e) || !ts.isIdentifier(e.expression) || e.arguments.nodes.length !== 1) + return null; + const helper = e.expression.text; + if (helper !== "__importDefault" && helper !== "__importStar") return null; + const req = unparen(e.arguments.nodes[0]!); + if ( + !ts.isCallExpression(req) || + !ts.isIdentifier(req.expression) || + req.expression.text !== "require" || + req.arguments.nodes.length !== 1 || + !ts.isStringLiteral(req.arguments.nodes[0]!) + ) + return null; + const spec = (req.arguments.nodes[0] as ts.StringLiteral).text; + return canonicalBuiltinModule(spec) === null ? null : { helper, spec }; +} + +/** The `.default` accesses to remove when every reference to `name` (other + * than its declaration) has the helper's form, or null. */ +function interopReferences( + sf: ts.SourceFile, + decl: ts.VariableDeclaration, + name: string, + helper: string, +): ts.PropertyAccessExpression[] | null { + const defaults: ts.PropertyAccessExpression[] = []; + let ok = true; + ts.walkPreorder(sf, (n) => { + if (!ok) return "stop"; + if (!ts.isIdentifier(n) || n.text !== name || n === decl.name) return undefined; + const parent = n.parent; + if ( + parent === undefined || + !ts.isPropertyAccessExpression(parent) || + parent.expression !== n || + parent.questionDotToken !== undefined + ) { + ok = false; + return "stop"; + } + const member = parent.name.text; + if (helper === "__importDefault") { + if (member !== "default") ok = false; + else defaults.push(parent); + } else if (member === "default") { + ok = false; + } + return undefined; + }); + return ok ? defaults : null; +} + + + + +/** The module text with its builtin interop bindings unwrapped, or null. */ +export function unwrapTscBuiltinImports(sf: ts.SourceFile): string | null { + if (ts.isExternalModule(sf) && !sf.fileName.endsWith(".cjs")) return null; + const edits: Edit[] = []; + const stmts = sf.statements.nodes; + // Builtin modules through the interop helpers. + for (const stmt of stmts) { + if (!ts.isVariableStatement(stmt)) continue; + for (const d of stmt.declarationList.declarations.nodes) { + if (!ts.isIdentifier(d.name) || d.initializer === undefined) continue; + const interop = builtinInterop(d.initializer); + if (interop === null) continue; + const defaults = interopReferences(sf, d, d.name.text, interop.helper); + if (defaults === null) continue; + edits.push({ + start: d.initializer.getStart(sf), + end: d.initializer.getEnd(), + text: `require(${JSON.stringify(interop.spec)})`, + }); + for (const access of defaults) + edits.push({ start: access.getStart(sf), end: access.getEnd(), text: d.name.text }); + } + } + if (edits.length === 0) return null; + return applyEdits(sf.text, 0, sf.text.length, edits); +} diff --git a/packages/compiler/src/frontend/npm-static.ts b/packages/compiler/src/frontend/npm-static.ts index 9087c00a37..d44c6c6994 100644 --- a/packages/compiler/src/frontend/npm-static.ts +++ b/packages/compiler/src/frontend/npm-static.ts @@ -455,6 +455,10 @@ export function npmStaticFsShadow(services: FrontendServices): NpmStaticFsShadow // Polyfill packages that ARE a Node builtin compile as the builtin. const aliased = original === null ? null : services.builtinAliasRequires(original, path); if (aliased !== null) original = aliased; + // tsc's interop helpers around builtin modules. + const tscHelpers = + original === null ? null : services.tscBuiltinImports(original, path); + if (tscHelpers !== null) original = tscHelpers; // Minified merged declarations of requires and the bindings read // from them become one declaration per binding. const declSplit = @@ -475,6 +479,7 @@ export function npmStaticFsShadow(services: FrontendServices): NpmStaticFsShadow const respelled = split !== null || declSplit !== null || + tscHelpers !== null || esm !== null || aliased !== null || fnExports !== null || diff --git a/packages/compiler/src/frontend/services.ts b/packages/compiler/src/frontend/services.ts index 689caddf89..5fe85f48c9 100644 --- a/packages/compiler/src/frontend/services.ts +++ b/packages/compiler/src/frontend/services.ts @@ -13,6 +13,7 @@ import { rewriteBuiltinAliasRequires, } from "./npm-static-builtin-aliases.js"; import { mayHoldEnumIife, rewriteEnumIifes } from "./npm-static-enum-iife.js"; +import { mayHoldTscBuiltinImport, unwrapTscBuiltinImports } from "./npm-static-tsc-helpers.js"; import { mayMergeRequireDeclarations, splitRequireDeclarations, @@ -117,6 +118,13 @@ export class FrontendServices { if (!mayMergeRequireDeclarations(source)) return null; return splitRequireDeclarations(this.parse(path, source, "js")); } + /** A package module with tsc's interop bindings of builtin modules + * unwrapped to plain requires (null: none). */ + tscBuiltinImports(source: string, path: string): string | null { + this.ensureOpen(); + if (!mayHoldTscBuiltinImport(source)) return null; + return unwrapTscBuiltinImports(this.parse(path, source, "js")); + } /** A package module with its literal-valued TypeScript enum IIFEs * respelled as object literals (null: none). */ enumIifes(source: string, path: string): string | null { diff --git a/tests/fixtures/npm/cases/tsc-builtin-interop/main.ts b/tests/fixtures/npm/cases/tsc-builtin-interop/main.ts new file mode 100644 index 0000000000..99bbe020a1 --- /dev/null +++ b/tests/fixtures/npm/cases/tsc-builtin-interop/main.ts @@ -0,0 +1,9 @@ +// A tsc-compiled dependency wraps builtin modules in its interop helpers +// (`const events_1 = __importDefault(require("events"))`, then +// `class Bus extends events_1.default.EventEmitter`; `const path = +// __importStar(require("path"))`). +import { Bus, where } from "gttscinterop"; + +const bus = new Bus(); +bus.on("ping", (name: string) => console.log("got", name)); +console.log(bus.ping("a"), where("/x/y.txt")); diff --git a/tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/lib/helper.d.ts b/tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/lib/helper.d.ts new file mode 100644 index 0000000000..cebef91199 --- /dev/null +++ b/tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/lib/helper.d.ts @@ -0,0 +1 @@ +export default function suffix(): string; diff --git a/tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/lib/helper.js b/tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/lib/helper.js new file mode 100644 index 0000000000..9c847c6ac5 --- /dev/null +++ b/tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/lib/helper.js @@ -0,0 +1,6 @@ +"use strict"; +Object.defineProperty(exports, "__esModule", { value: true }); +exports.default = suffix; +function suffix() { + return "!"; +} diff --git a/tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/lib/index.d.ts b/tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/lib/index.d.ts new file mode 100644 index 0000000000..f185ed2580 --- /dev/null +++ b/tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/lib/index.d.ts @@ -0,0 +1,5 @@ +import events from "events"; +export declare class Bus extends events.EventEmitter { + ping(name: string): string; +} +export declare function where(p: string): string; diff --git a/tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/lib/index.js b/tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/lib/index.js new file mode 100644 index 0000000000..96450e0e54 --- /dev/null +++ b/tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/lib/index.js @@ -0,0 +1,43 @@ +"use strict"; +var __createBinding = (this && this.__createBinding) || (Object.create ? (function(o, m, k, k2) { + if (k2 === undefined) k2 = k; + var desc = Object.getOwnPropertyDescriptor(m, k); + if (!desc || ("get" in desc ? !m.__esModule : desc.writable || desc.configurable)) { + desc = { enumerable: true, get: function() { return m[k]; } }; + } + Object.defineProperty(o, k2, desc); +}) : (function(o, m, k, k2) { + if (k2 === undefined) k2 = k; + o[k2] = m[k]; +})); +var __setModuleDefault = (this && this.__setModuleDefault) || (Object.create ? (function(o, v) { + Object.defineProperty(o, "default", { enumerable: true, value: v }); +}) : function(o, v) { + o["default"] = v; +}); +var __importStar = (this && this.__importStar) || function (mod) { + if (mod && mod.__esModule) return mod; + var result = {}; + if (mod != null) for (var k in mod) if (k !== "default" && Object.prototype.hasOwnProperty.call(mod, k)) __createBinding(result, mod, k); + __setModuleDefault(result, mod); + return result; +}; +var __importDefault = (this && this.__importDefault) || function (mod) { + return (mod && mod.__esModule) ? mod : { "default": mod }; +}; +Object.defineProperty(exports, "__esModule", { value: true }); +exports.Bus = void 0; +exports.where = where; +const events_1 = __importDefault(require("events")); +const path = __importStar(require("path")); +const helper_1 = __importDefault(require("./helper")); +class Bus extends events_1.default.EventEmitter { + ping(name) { + this.emit("ping", name); + return `pong ${name}${(0, helper_1.default)()}`; + } +} +exports.Bus = Bus; +function where(p) { + return path.basename(p) + (0, helper_1.default)(); +} diff --git a/tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/package.json b/tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/package.json new file mode 100644 index 0000000000..bc83710c82 --- /dev/null +++ b/tests/fixtures/npm/cases/tsc-builtin-interop/node_modules/gttscinterop/package.json @@ -0,0 +1,7 @@ +{ + "name": "gttscinterop", + "version": "1.0.0", + "description": "real tsc 5.6 output (module commonjs, esModuleInterop) of a source that default-imports `events` and namespace-imports `path`: the builtins are wrapped in tsc's __importDefault/__importStar helpers, which probe the module's top-level `this`", + "main": "lib/index.js", + "types": "lib/index.d.ts" +} From 24963c827fd9b631f8ee8bf7e62a098662a6c1b7 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Tue, 20 Oct 2026 14:10:44 -0700 Subject: [PATCH 09/22] Lower star re-exports of empty namespaces and reads of a bundle's entry namespace A bundled module that exports only types keeps `var x_exports = {}` with no __export call. Its star re-export into the entry, `__reExport(main, (init_x(), __toCommonJS(x_exports)), module.exports)`, was left in place because the rewrite only knew namespaces with an export table, and failed at load on the `module.exports` argument ("the CommonJS 'module' object in this position"). Such a namespace is now an empty table: the re-export runs the initializer and adds nothing. Two silent results on the same paths: a member read of a name a module does not export now answers undefined (`__esModule` true) where it was refused, and a read through the bundle's own entry namespace inside the bundle (`(init_index(), __toCommonJS(index_exports)).KIND`) answered undefined; it now reads the entry's export table. Co-Authored-By: Claude Opus 5.5 --- .../frontend/npm-static-bundled-esm.test.ts | 48 ++++++++ .../src/frontend/npm-static-bundled-esm.ts | 113 ++++++++++++++---- .../2595-bundled-entry-namespace/main.ts | 7 ++ .../npm/node_modules/gtemptyns/dist/index.js | 1 + .../npm/node_modules/gtemptyns/index.d.ts | 2 + .../npm/node_modules/gtemptyns/package.json | 7 ++ tests/harness/npm-cases.ts | 4 +- tests/harness/npm-static.test.ts | 10 +- 8 files changed, 169 insertions(+), 23 deletions(-) create mode 100644 tests/fixtures/npm/cases/2595-bundled-entry-namespace/main.ts create mode 100644 tests/fixtures/npm/node_modules/gtemptyns/dist/index.js create mode 100644 tests/fixtures/npm/node_modules/gtemptyns/index.d.ts create mode 100644 tests/fixtures/npm/node_modules/gtemptyns/package.json diff --git a/packages/compiler/src/frontend/npm-static-bundled-esm.test.ts b/packages/compiler/src/frontend/npm-static-bundled-esm.test.ts index c5b5568214..6e758e0d51 100644 --- a/packages/compiler/src/frontend/npm-static-bundled-esm.test.ts +++ b/packages/compiler/src/frontend/npm-static-bundled-esm.test.ts @@ -365,3 +365,51 @@ test("a minified initializer that ends with its class still hoists it", () => { ); expect(lowered).toContain("class Counter{"); }); + +test("an empty namespace and names a module does not export read undefined", () => { + const lowered = expectSameBehavior(`"use strict"; +${helpers}${lazyModule} +var manifest_exports = {}; +var init_manifest = __esm({ "src/manifest.ts"() { console.log("manifest init"); } }); +function read() { + const manifest = (init_manifest(), __toCommonJS(manifest_exports)); + const lazy = (init_lazy(), __toCommonJS(lazy_exports)); + return [manifest.version, lazy.missing, lazy.__esModule, lazy.VALUE].join(" "); +} +console.log(read()); +`); + expect(lowered).not.toContain("manifest_exports"); +}); + +test("a star re-export of a types-only module into the entry runs its initializer", () => { + const lowered = expectSameBehavior(`"use strict"; +${helpers} +var __reExport = (target, mod, secondTarget) => (__copyProps(target, mod, "default"), secondTarget && __copyProps(secondTarget, mod, "default")); +var index_exports = {}; +__export(index_exports, { run: () => run }); +module.exports = __toCommonJS(index_exports); +var manifest_exports = {}; +var init_manifest = __esm({ "src/manifest.ts"() { console.log("manifest init"); } }); +__reExport(index_exports, (init_manifest(), __toCommonJS(manifest_exports)), module.exports); +function run() { return "ran"; } +console.log(Object.keys(module.exports).join(","), module.exports.run()); +`); + expect(lowered).not.toContain("module.exports);"); +}); + +test("the bundle's own entry namespace answers member reads", () => { + expectSameBehavior(`"use strict"; +${helpers} +var index_exports = {}; +__export(index_exports, { KIND: () => KIND, late: () => late }); +module.exports = __toCommonJS(index_exports); +var KIND; +var init_index = __esm({ "src/index.ts"() { KIND = "lambda"; } }); +function late() { + const index = (init_index(), __toCommonJS(index_exports)); + return index.KIND + " " + index.missing; +} +init_index(); +console.log(late()); +`); +}); diff --git a/packages/compiler/src/frontend/npm-static-bundled-esm.ts b/packages/compiler/src/frontend/npm-static-bundled-esm.ts index d74b41a1d5..80fc23ec97 100644 --- a/packages/compiler/src/frontend/npm-static-bundled-esm.ts +++ b/packages/compiler/src/frontend/npm-static-bundled-esm.ts @@ -503,15 +503,49 @@ export function lowerBundledEsmInits( ) mainTarget = (rhs.arguments.nodes[0] as ts.Identifier).text; } - const tables = new Map(); + const tables = new Map< + string, + { stmt: ts.Statement | null; entries: [string, ts.Expression][] } + >(); + // The bundle's own entry namespace keeps its statements (the CommonJS + // rewrite builds the export table from them); its table only answers + // member reads through `__toCommonJS(main)` in the bundle's own code. + let mainTable: { stmt: ts.Statement | null; entries: [string, ts.Expression][] } | null = null; for (const stmt of sf.statements.nodes) { if (!ts.isExpressionStatement(stmt) || !ts.isCallExpression(stmt.expression)) continue; const table = exportTableOf(stmt.expression); - if (table !== null && table.target !== mainTarget) { + if (table !== null && table.target === mainTarget) { + mainTable = mainTable === null ? { stmt, entries: table.entries } : null; + } else if (table !== null) { if (tables.has(table.target)) return { degrade: ESM_SHAPE_DEGRADE }; tables.set(table.target, { stmt, entries: table.entries }); } } + // A module that exports only types keeps an empty namespace object + // (`var x_exports = {}`) with no __export call: an empty table, read + // only through `__toCommonJS(x_exports)`. + for (const [name, t] of top) { + if (tables.has(name) || name === mainTarget || writes.has(name)) continue; + const d = t.node; + if (!ts.isVariableDeclaration(d) || d.initializer === undefined) continue; + const init = unparen(d.initializer); + if (!ts.isObjectLiteralExpression(init) || init.properties.nodes.length !== 0) continue; + const uses = (refsByName.get(name) ?? []).filter((x) => x !== d.name); + if ( + uses.length > 0 && + uses.every((x) => { + const p = x.parent; + return ( + p !== undefined && + ts.isCallExpression(p) && + ts.isIdentifier(p.expression) && + helperBase(p.expression.text) === "__toCommonJS" && + p.arguments.nodes.length === 1 + ); + }) + ) + tables.set(name, { stmt: null, entries: [] }); + } const lowered = new Set(); /** Some binding between the use site and the top level rebinds a name * the table's getters read. */ @@ -673,11 +707,25 @@ export function lowerBundledEsmInits( } for (const r of allRefs) { - const table = tables.get(r.text); + const p = r.parent; + const isMain = + r.text === mainTarget && + mainTable !== null && + p !== undefined && + ts.isCallExpression(p) && + ts.isIdentifier(p.expression) && + helperBase(p.expression.text) === "__toCommonJS" && + p.arguments.nodes.length === 1 && + !( + p.parent !== undefined && + ts.isBinaryExpression(p.parent) && + p.parent.right === p && + ts.isPropertyAccessExpression(p.parent.left) + ); + const table = isMain ? mainTable! : tables.get(r.text); if (table === undefined) continue; if (loaderSpans.some((span) => r.getStart(sf) >= span.start && r.getEnd() <= span.end)) continue; - const p = r.parent; // The table's own declaration and __export call are handled below. if (p !== undefined && ts.isVariableDeclaration(p) && p.name === r) continue; if ( @@ -712,13 +760,29 @@ export function lowerBundledEsmInits( else break; } const values = new Map(table.entries); - const memberRead = (access: ts.Node | undefined, of: ts.Node): boolean => - access !== undefined && - ts.isPropertyAccessExpression(access) && - access.expression === of && - values.has(access.name.text) && - plainRead(access) && - (!calledWithReceiver(access) || receiverHidden(values, access.name.text)); + // A name the module does not export reads undefined on the namespace + // object (`__esModule` reads true); plain reads only, never calls. + const missing = new Set(); + const readNames = new Set(); + const memberRead = (access: ts.Node | undefined, of: ts.Node): boolean => { + if ( + access === undefined || + !ts.isPropertyAccessExpression(access) || + access.expression !== of || + !plainRead(access) + ) + return false; + const name = access.name.text; + if (!values.has(name)) { + const parent = access.parent; + if (parent !== undefined && ts.isCallExpression(parent) && parent.expression === access) + return false; + missing.add(name); + return true; + } + readNames.add(name); + return !calledWithReceiver(access) || receiverHidden(values, name); + }; const holder = ns.parent; // `__reExport(main, ns, module.exports)`: a star re-export of the // module into the bundle's own exports (`export * from` in its entry). @@ -752,7 +816,7 @@ export function lowerBundledEsmInits( end: holder.parent.getEnd(), text: `${init}__export(${mainTarget}, { ${entries} });`, }); - lowered.add(r.text); + if (!isMain) lowered.add(r.text); continue; } let readable = memberRead(holder, ns); @@ -793,17 +857,22 @@ export function lowerBundledEsmInits( }; return { degrade: `its shipped JS ${why}` }; } + // The entry namespace answers only the names read here. + const entries = isMain ? table.entries.filter(([n]) => readNames.has(n)) : table.entries; // The getters read top-level bindings: nothing between the use site // and the top level may rebind one of their names. - if (shadowed(p, table.entries)) return { degrade: `its shipped JS ${why}` }; - const getters = table.entries - .map( + if (shadowed(p, entries)) return { degrade: `its shipped JS ${why}` }; + const getters = [ + ...entries.map( ([n, value]) => `get ${/^[A-Za-z_$][\w$]*$/.test(n) ? n : JSON.stringify(n)}() { return ${getterText(value)}; }`, - ) - .join(", "); + ), + ...[...missing].map( + (n) => `get ${n}() { return ${n === "__esModule" ? "true" : "void 0"}; }`, + ), + ].join(", "); edits.push({ start: p.getStart(sf), end: p.getEnd(), text: `{ ${getters} }` }); - lowered.add(r.text); + if (!isMain) lowered.add(r.text); } // A table nothing reads (its module is never required) goes too: its // names are no exports of the bundle. @@ -812,7 +881,10 @@ export function lowerBundledEsmInits( const live = (refsByName.get(target) ?? []).some((x) => { const p = x.parent; if (p !== undefined && ts.isVariableDeclaration(p) && p.name === x) return false; - return !(x.getStart(sf) >= table.stmt.getStart(sf) && x.getEnd() <= table.stmt.getEnd()); + return ( + table.stmt === null || + !(x.getStart(sf) >= table.stmt.getStart(sf) && x.getEnd() <= table.stmt.getEnd()) + ); }); if (!live) lowered.add(target); } @@ -828,7 +900,8 @@ export function lowerBundledEsmInits( writes.has(target) ) return { degrade: ESM_SHAPE_DEGRADE }; - edits.push({ start: table.stmt.getStart(sf), end: table.stmt.getEnd(), text: "" }); + if (table.stmt !== null) + edits.push({ start: table.stmt.getStart(sf), end: table.stmt.getEnd(), text: "" }); deadDeclarators.add(decl); } // Every removed declarator in one pass: declarators of one statement diff --git a/tests/fixtures/npm/cases/2595-bundled-entry-namespace/main.ts b/tests/fixtures/npm/cases/2595-bundled-entry-namespace/main.ts new file mode 100644 index 0000000000..e79c9844c2 --- /dev/null +++ b/tests/fixtures/npm/cases/2595-bundled-entry-namespace/main.ts @@ -0,0 +1,7 @@ +// A bundled module requires the bundle's own entry and a module that exports +// only types: esbuild reads them as `(init_index(), __toCommonJS(index_exports))` +// and `(init_manifest(), __toCommonJS(manifest_exports))`, the second an +// empty namespace whose names all read undefined. +import { KIND, late } from "gtemptyns"; + +console.log(KIND, late()); diff --git a/tests/fixtures/npm/node_modules/gtemptyns/dist/index.js b/tests/fixtures/npm/node_modules/gtemptyns/dist/index.js new file mode 100644 index 0000000000..41a9c9a401 --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtemptyns/dist/index.js @@ -0,0 +1 @@ +var __defProp=Object.defineProperty;var __getOwnPropDesc=Object.getOwnPropertyDescriptor;var __getOwnPropNames=Object.getOwnPropertyNames;var __hasOwnProp=Object.prototype.hasOwnProperty;var __esm=(fn,res)=>function(){return fn&&(res=(0,fn[__getOwnPropNames(fn)[0]])(fn=0)),res};var __export=(target,all)=>{for(var name in all)__defProp(target,name,{get:all[name],enumerable:!0})},__copyProps=(to,from,except,desc)=>{if(from&&typeof from=="object"||typeof from=="function")for(let key of __getOwnPropNames(from))!__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:!0}),mod);var manifest_exports={};var init_manifest=__esm({"src/manifest.ts"(){}});var KIND,init_types=__esm({"src/types.ts"(){KIND="lambda"}});function late(){let index=(init_index(),__toCommonJS(index_exports)),manifest=(init_manifest(),__toCommonJS(manifest_exports));return`${index.KIND} ${manifest.version}`}var init_lazy=__esm({"src/lazy.ts"(){}});var index_exports={};__export(index_exports,{KIND:()=>KIND,late:()=>late});module.exports=__toCommonJS(index_exports);var init_index=__esm({"src/index.ts"(){init_manifest();init_types();init_lazy()}});init_index();0&&(module.exports={KIND,late}); diff --git a/tests/fixtures/npm/node_modules/gtemptyns/index.d.ts b/tests/fixtures/npm/node_modules/gtemptyns/index.d.ts new file mode 100644 index 0000000000..4ba5318a42 --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtemptyns/index.d.ts @@ -0,0 +1,2 @@ +export declare const KIND: string; +export declare function late(): string; diff --git a/tests/fixtures/npm/node_modules/gtemptyns/package.json b/tests/fixtures/npm/node_modules/gtemptyns/package.json new file mode 100644 index 0000000000..383b7a0767 --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtemptyns/package.json @@ -0,0 +1,7 @@ +{ + "name": "gtemptyns", + "version": "1.0.0", + "description": "real esbuild 0.27 output (--bundle --platform=node --format=cjs --minify-whitespace --minify-syntax) of a TypeScript entry whose module requires the entry itself and a types-only module: `(init_index(), __toCommonJS(index_exports)).KIND` reads the bundle's own entry namespace, and `(init_manifest(), __toCommonJS(manifest_exports))` an empty one", + "main": "dist/index.js", + "types": "index.d.ts" +} diff --git a/tests/harness/npm-cases.ts b/tests/harness/npm-cases.ts index c085cfe7a3..80e3bb55b2 100644 --- a/tests/harness/npm-cases.ts +++ b/tests/harness/npm-cases.ts @@ -19,14 +19,14 @@ export function npmCases(fixturesRoot: string): NpmCase[] { return [ ...globSync(join(fixturesRoot, "npm/cases/*/main.ts")) .sort() - // 2465-2469, 2556-2567, 2571 and 2580-2594 are the --npm-static bundler-emitted-CJS + // 2465-2469, 2556-2567, 2571 and 2580-2595 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-7]|2571|258[0-9]|259[0-4])-[^/]+\/main\.ts$/.test(entry), + !/\/(246[5-9]|255[6-9]|256[0-7]|2571|258[0-9]|259[0-5])-[^/]+\/main\.ts$/.test(entry), ) .map((entry) => ({ name: entry.split("/").at(-2)!, diff --git a/tests/harness/npm-static.test.ts b/tests/harness/npm-static.test.ts index 5c39cfb2e2..a981a892f0 100644 --- a/tests/harness/npm-static.test.ts +++ b/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-2567, 2571, 2580-2594) + // the bundler-emitted-CJS mini packages (cases 2465-2469, 2556-2567, 2571, 2580-2595) ...globSync(join(fixturesRoot, "npm/node_modules/gt*/**/*.{js,cjs,json}")).sort(), ...globSync(join(fixturesRoot, "npm/store/gt*/**/*.{js,cjs,json}")).sort(), ]; @@ -1657,6 +1657,14 @@ describe(`npm-static pilots${sanitize ? " (sanitized)" : ""}`, () => { ]); }, 180_000); + // 2595: a bundle's own entry namespace and an empty (types-only) one, + // read through `(init_x(), __toCommonJS(x_exports))`. + test("bundled namespace reads 2595-bundled-entry-namespace byte-match Node", async () => { + await expectNodeParity(join(fixturesRoot, "npm/cases/2595-bundled-entry-namespace/main.ts"), [ + "gtemptyns", + ]); + }, 180_000); + // 2562: a file with unverifiable interop that no compiled code can load // (the program imports a leaf data module and only a type from the // main entry) leaves its package static. From 278f6ca16c469312e254f3deb9dbb44180d871bd Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Tue, 20 Oct 2026 14:50:43 -0700 Subject: [PATCH 10/22] Format and record the new fixtures' order baselines Co-Authored-By: Claude Opus 5.5 --- .../src/frontend/npm-static-tsc-helpers.test.ts | 17 +++++++++++++---- .../src/frontend/npm-static-tsc-helpers.ts | 3 --- packages/compiler/src/frontend/npm-static.ts | 3 +-- .../2595-bundled-entry-namespace/main.ts.json | 6 ++++++ .../npm/cases/tsc-builtin-interop/main.ts.json | 6 ++++++ 5 files changed, 26 insertions(+), 9 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2595-bundled-entry-namespace/main.ts.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/tsc-builtin-interop/main.ts.json diff --git a/packages/compiler/src/frontend/npm-static-tsc-helpers.test.ts b/packages/compiler/src/frontend/npm-static-tsc-helpers.test.ts index 95add609f3..735c9d298b 100644 --- a/packages/compiler/src/frontend/npm-static-tsc-helpers.test.ts +++ b/packages/compiler/src/frontend/npm-static-tsc-helpers.test.ts @@ -34,7 +34,8 @@ function expectSameBehavior(source: string): string { dirs.push(dir); writeFileSync(join(dir, "original.cjs"), source); writeFileSync(join(dir, "rewritten.cjs"), result!); - const run = (file: string) => spawnSync(process.execPath, [join(dir, file)], { encoding: "utf8" }); + const run = (file: string) => + spawnSync(process.execPath, [join(dir, file)], { encoding: "utf8" }); const oracle = run("original.cjs"); expect(oracle.status).toBe(0); expect(run("rewritten.cjs").stdout).toBe(oracle.stdout); @@ -54,9 +55,17 @@ console.log(new Bus() instanceof events_1.default, path.basename("/a/b.txt"), ty test("other references, program modules and default reads of a namespace keep their text", () => { // The binding itself as a value. - expect(rewrite(`${helpers}const events_1 = __importDefault(require("events"));\nconsole.log(events_1);\n`)).toBeNull(); + expect( + rewrite( + `${helpers}const events_1 = __importDefault(require("events"));\nconsole.log(events_1);\n`, + ), + ).toBeNull(); // A program module (cjs-tsc-interop.ts owns it). - expect(rewrite(`${helpers}const m = __importDefault(require("./m"));\nm.default();\n`)).toBeNull(); + expect( + rewrite(`${helpers}const m = __importDefault(require("./m"));\nm.default();\n`), + ).toBeNull(); // `ns.default` reads the namespace copy's own default. - expect(rewrite(`${helpers}const p = __importStar(require("path"));\nconsole.log(p.default === p);\n`)).toBeNull(); + expect( + rewrite(`${helpers}const p = __importStar(require("path"));\nconsole.log(p.default === p);\n`), + ).toBeNull(); }); diff --git a/packages/compiler/src/frontend/npm-static-tsc-helpers.ts b/packages/compiler/src/frontend/npm-static-tsc-helpers.ts index 19f69485a0..fcb73be5b6 100644 --- a/packages/compiler/src/frontend/npm-static-tsc-helpers.ts +++ b/packages/compiler/src/frontend/npm-static-tsc-helpers.ts @@ -78,9 +78,6 @@ function interopReferences( return ok ? defaults : null; } - - - /** The module text with its builtin interop bindings unwrapped, or null. */ export function unwrapTscBuiltinImports(sf: ts.SourceFile): string | null { if (ts.isExternalModule(sf) && !sf.fileName.endsWith(".cjs")) return null; diff --git a/packages/compiler/src/frontend/npm-static.ts b/packages/compiler/src/frontend/npm-static.ts index d44c6c6994..2454b35469 100644 --- a/packages/compiler/src/frontend/npm-static.ts +++ b/packages/compiler/src/frontend/npm-static.ts @@ -456,8 +456,7 @@ export function npmStaticFsShadow(services: FrontendServices): NpmStaticFsShadow const aliased = original === null ? null : services.builtinAliasRequires(original, path); if (aliased !== null) original = aliased; // tsc's interop helpers around builtin modules. - const tscHelpers = - original === null ? null : services.tscBuiltinImports(original, path); + const tscHelpers = original === null ? null : services.tscBuiltinImports(original, path); if (tscHelpers !== null) original = tscHelpers; // Minified merged declarations of requires and the bindings read // from them become one declaration per binding. diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2595-bundled-entry-namespace/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2595-bundled-entry-namespace/main.ts.json new file mode 100644 index 0000000000..141e4c6385 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2595-bundled-entry-namespace/main.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/fixtures/npm/cases/2595-bundled-entry-namespace/main.ts" + ], + "diags": [] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/tsc-builtin-interop/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/tsc-builtin-interop/main.ts.json new file mode 100644 index 0000000000..cdb86be049 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/tsc-builtin-interop/main.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/fixtures/npm/cases/tsc-builtin-interop/main.ts" + ], + "diags": [] +} From 391175dbdfae9a96c399ed036a3f617131a47284 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Tue, 20 Oct 2026 15:44:39 -0700 Subject: [PATCH 11/22] Respell members of a whole function export behind requires only for var-bound functions #1558 let the function-export rewrite run when require bindings come before `module.exports = F`. For a function declaration that respelled its member exports as members of the function (`module.exports.sync = spawnSync` became `spawn.sync = spawnSync`), which need dynamic values: a typed member such as cross-spawn 7's spawnSync (returning SpawnSyncReturns) failed at load with SC1101. A declaration's member exports already compile as member exports, so behind requires only a function bound to a variable (esbuild's hoisted `var`, whose members have no other form) is respelled now. Co-Authored-By: Claude Opus 5.5 --- .../npm-static-cjs-exports-alias.test.ts | 15 +++++++++++++ .../frontend/npm-static-cjs-exports-alias.ts | 22 ++++++++++++++----- .../2596-function-export-members/main.ts | 5 +++++ .../npm/node_modules/gtspawnshape/index.js | 13 +++++++++++ .../node_modules/gtspawnshape/package.json | 1 + tests/harness/npm-cases.ts | 4 ++-- tests/harness/npm-static.test.ts | 10 ++++++++- 7 files changed, 62 insertions(+), 8 deletions(-) create mode 100644 tests/fixtures/npm/cases/2596-function-export-members/main.ts create mode 100644 tests/fixtures/npm/node_modules/gtspawnshape/index.js create mode 100644 tests/fixtures/npm/node_modules/gtspawnshape/package.json diff --git a/packages/compiler/src/frontend/npm-static-cjs-exports-alias.test.ts b/packages/compiler/src/frontend/npm-static-cjs-exports-alias.test.ts index 6042c9ad5a..2309918483 100644 --- a/packages/compiler/src/frontend/npm-static-cjs-exports-alias.test.ts +++ b/packages/compiler/src/frontend/npm-static-cjs-exports-alias.test.ts @@ -116,3 +116,18 @@ test.each([ ])("%s keeps the module as it is", (_, source) => { expect(rewrite(source)).toBeNull(); }); + +test("a function declaration export behind requires keeps its member exports", () => { + // cross-spawn 7: the member exports keep their declared types as member + // exports; members of the function would need dynamic values. + expect( + rewrite(`'use strict'; +const cp = require('child_process'); +function spawn(command, args, options) { return cp.spawn(command, args, options); } +function spawnSync(command, args, options) { return cp.spawnSync(command, args, options); } +module.exports = spawn; +module.exports.spawn = spawn; +module.exports.sync = spawnSync; +`), + ).toBeNull(); +}); diff --git a/packages/compiler/src/frontend/npm-static-cjs-exports-alias.ts b/packages/compiler/src/frontend/npm-static-cjs-exports-alias.ts index c86aad099a..b081a7e8d7 100644 --- a/packages/compiler/src/frontend/npm-static-cjs-exports-alias.ts +++ b/packages/compiler/src/frontend/npm-static-cjs-exports-alias.ts @@ -64,13 +64,18 @@ function isRequireValue(expr: ts.Expression): boolean { /** True for a statement whose evaluation runs no code that could observe * the export object: declarations and member writes (`F.prototype.m = - * function …`) without calls, directives, and bindings of required - * modules (`var x = require("x")`: another module can reach this one's - * export object only through a require cycle, which preflight's cycle - * rules already judge). */ + * function …`) without calls, and directives. */ function inert(stmt: ts.Statement): boolean { + return inertOrRequire(stmt, false); +} + +/** `inert`, and with `requires` also bindings of required modules (`var x + * = require("x")`: another module can reach this one's export object only + * through a require cycle, which preflight's cycle rules already judge). */ +function inertOrRequire(stmt: ts.Statement, requires: boolean): boolean { if (ts.isFunctionDeclaration(stmt) || ts.isEmptyStatement(stmt)) return true; if ( + requires && ts.isVariableStatement(stmt) && stmt.declarationList.declarations.nodes.every( (d) => @@ -131,12 +136,19 @@ export function normalizeCjsFunctionExports(sf: ts.SourceFile): string | null { if (!isModuleExports(e.left)) continue; const value = unparen(e.right); if (!ts.isIdentifier(value)) return null; - if (whole !== null || !stmts.slice(0, i).every(inert)) return null; + if (whole !== null || !stmts.slice(0, i).every((st) => inertOrRequire(st, true))) return null; whole = { stmt, fn: value.text, rebinds }; } if (whole === null) return null; const fnName = whole.fn; const wholeAt = stmts.indexOf(whole.stmt); + // A function declaration's member exports already compile as member + // exports, which keep their declared types; respelling them as members of + // the function needs dynamic values, so with requires above the export + // only a function bound to a variable (esbuild's hoisted `var`, whose + // member exports have no other form) is respelled. + const varBound = stmts.some((st, i) => i < wholeAt && functionValuedVar(st, fnName)); + if (!varBound && !stmts.slice(0, wholeAt).every(inert)) return null; const fnDecls = stmts.filter( (st, i) => (ts.isFunctionDeclaration(st) && st.name?.text === fnName && st.body !== undefined) || diff --git a/tests/fixtures/npm/cases/2596-function-export-members/main.ts b/tests/fixtures/npm/cases/2596-function-export-members/main.ts new file mode 100644 index 0000000000..57c4dbb7a3 --- /dev/null +++ b/tests/fixtures/npm/cases/2596-function-export-members/main.ts @@ -0,0 +1,5 @@ +// cross-spawn 7's export shape: `module.exports = spawn` behind require +// bindings, then typed function members (`module.exports.sync = spawnSync`). +import xs from "gtspawnshape"; + +console.log(typeof xs); diff --git a/tests/fixtures/npm/node_modules/gtspawnshape/index.js b/tests/fixtures/npm/node_modules/gtspawnshape/index.js new file mode 100644 index 0000000000..566b1a7da2 --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtspawnshape/index.js @@ -0,0 +1,13 @@ +'use strict'; +const cp = require('child_process'); +const path = require('path'); +function spawn(command, args, options) { + return cp.spawn(command, args, options); +} +function spawnSync(command, args, options) { + return cp.spawnSync(command, args, options); +} +module.exports = spawn; +module.exports.spawn = spawn; +module.exports.sync = spawnSync; +module.exports.sep = path.sep; diff --git a/tests/fixtures/npm/node_modules/gtspawnshape/package.json b/tests/fixtures/npm/node_modules/gtspawnshape/package.json new file mode 100644 index 0000000000..a33a31ccc7 --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtspawnshape/package.json @@ -0,0 +1 @@ +{"name":"gtspawnshape","version":"7.0.3","description":"cross-spawn 7`s export shape: a function declaration exported whole behind require bindings, with typed function members","main":"index.js"} diff --git a/tests/harness/npm-cases.ts b/tests/harness/npm-cases.ts index c085cfe7a3..6dc8e81470 100644 --- a/tests/harness/npm-cases.ts +++ b/tests/harness/npm-cases.ts @@ -19,14 +19,14 @@ export function npmCases(fixturesRoot: string): NpmCase[] { return [ ...globSync(join(fixturesRoot, "npm/cases/*/main.ts")) .sort() - // 2465-2469, 2556-2567, 2571 and 2580-2594 are the --npm-static bundler-emitted-CJS + // 2465-2469, 2556-2567, 2571 and 2580-2596 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-7]|2571|258[0-9]|259[0-4])-[^/]+\/main\.ts$/.test(entry), + !/\/(246[5-9]|255[6-9]|256[0-7]|2571|258[0-9]|259[0-6])-[^/]+\/main\.ts$/.test(entry), ) .map((entry) => ({ name: entry.split("/").at(-2)!, diff --git a/tests/harness/npm-static.test.ts b/tests/harness/npm-static.test.ts index 5c39cfb2e2..3a1942b096 100644 --- a/tests/harness/npm-static.test.ts +++ b/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-2567, 2571, 2580-2594) + // the bundler-emitted-CJS mini packages (cases 2465-2469, 2556-2567, 2571, 2580-2596) ...globSync(join(fixturesRoot, "npm/node_modules/gt*/**/*.{js,cjs,json}")).sort(), ...globSync(join(fixturesRoot, "npm/store/gt*/**/*.{js,cjs,json}")).sort(), ]; @@ -1657,6 +1657,14 @@ describe(`npm-static pilots${sanitize ? " (sanitized)" : ""}`, () => { ]); }, 180_000); + // 2596: cross-spawn 7's function export with typed function members + // behind require bindings. + test("function export members 2596-function-export-members byte-match Node", async () => { + await expectNodeParity(join(fixturesRoot, "npm/cases/2596-function-export-members/main.ts"), [ + "gtspawnshape", + ]); + }, 180_000); + // 2562: a file with unverifiable interop that no compiled code can load // (the program imports a leaf data module and only a type from the // main entry) leaves its package static. From 9d7e39843894fa7444bafe521fea471dc1059ef5 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Tue, 20 Oct 2026 16:38:22 -0700 Subject: [PATCH 12/22] Record the 2596 order baseline Co-Authored-By: Claude Opus 5.5 --- .../2596-function-export-members/main.ts.json | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2596-function-export-members/main.ts.json diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2596-function-export-members/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2596-function-export-members/main.ts.json new file mode 100644 index 0000000000..a0e236abd5 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2596-function-export-members/main.ts.json @@ -0,0 +1,16 @@ +{ + "order": [ + "/tests/fixtures/npm/cases/2596-function-export-members/main.ts" + ], + "diags": [ + { + "code": "SC0001", + "message": "Could not find a declaration file for module 'gtspawnshape'. '/tests/fixtures/npm/node_modules/gtspawnshape/index.js' implicitly has an 'any' type.\n Try `npm i --save-dev @types/gtspawnshape` if it exists or add a new declaration (.d.ts) file containing `declare module 'gtspawnshape';`", + "loc": { + "file": "/tests/fixtures/npm/cases/2596-function-export-members/main.ts", + "start": 166, + "end": 180 + } + } + ] +} From 5616526d2dd38d883c5ba5a7bdd1bd0a5e445898 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Tue, 20 Oct 2026 18:45:37 -0500 Subject: [PATCH 13/22] Count builtin calls with callbacks and every write of a whole export's members MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A builtin call above a function export's settling statement can call back into the module (`Array.from(xs, () => module.exports())`), so it counts as a possible early call unless each argument is a primitive: a literal, `undefined`, a read under `process`, or an operator over those. A member of a function whole export is a runtime property of the function when program code may write it through a require: an assignment, update or delete of `fmt.name`, `fmt["name"]` or `require("./lib").name` in any program file, the module's own writes through the function's name, and, for every member, a computed key or a use that hands the value elsewhere (`Object.assign(fmt, …)`). The module's `module.exports.name = …` then also stores the member on the function, so requirers read the value Node reads before and after those writes. --- .../frontend/lowering/cjs-exports-object.ts | 172 ++++++++++++++++-- .../src/frontend/lowering/lower-stmts.ts | 41 +++++ .../compiler/src/frontend/lowering/lowerer.ts | 4 + .../src/frontend/lowering/var-hoisting.ts | 58 +++++- .../compiler/test/cjs-settled-exports.test.ts | 30 +++ .../main.cjs.json | 2 + .../6485-cjs-whole-export-members/counted.cjs | 8 + .../6485-cjs-whole-export-members/main.cjs | 4 + .../6485-cjs-whole-export-members/writer.cjs | 6 + 9 files changed, 300 insertions(+), 25 deletions(-) create mode 100644 tests/corpus/6485-cjs-whole-export-members/counted.cjs create mode 100644 tests/corpus/6485-cjs-whole-export-members/writer.cjs diff --git a/packages/compiler/src/frontend/lowering/cjs-exports-object.ts b/packages/compiler/src/frontend/lowering/cjs-exports-object.ts index c2910caab3..ae2bcd96cf 100644 --- a/packages/compiler/src/frontend/lowering/cjs-exports-object.ts +++ b/packages/compiler/src/frontend/lowering/cjs-exports-object.ts @@ -553,25 +553,163 @@ function requiredModuleOfMemberAccess( return null; const dep = inlineProgramRequireOf(lowerer, declaration.initializer)?.dep ?? null; if (dep === null || !wholeExportIsFunction(lowerer, dep)) return null; - // A member the requirer writes through its binding is a runtime slot. - const name = access.name.text; - let written = false; - ts.walkPreorder(access.getSourceFile(), (n) => { - if (written) return "stop"; + // A member program code may write on the value is a runtime slot. + const writes = wholeExportWrites(lowerer, dep); + return writes === "any" || writes.has(access.name.text) ? null : dep; +} + +/** The function value of `sf`'s whole export (`module.exports = format`) + * when program code may write its member `name`, so reads of the member + * through a require are runtime reads of the function's properties: the + * module's own `module.exports.name = …` stores the member there too. */ +export function cjsWholeExportSlotOwner( + lowerer: Lowerer, + sf: ts.SourceFile, + name: string, +): ts.Identifier | null { + if (!wholeExportIsFunction(lowerer, sf)) return null; + const writes = wholeExportWrites(lowerer, sf); + if (writes !== "any" && !writes.has(name)) return null; + const whole = cjsWholeExportValueOf(sf); + return whole !== null && ts.isIdentifier(whole.rhs) ? whole.rhs : null; +} + +/** The member names program files may write on `dep`'s whole export value + * through a require of it: assignments, updates and deletes of `fmt.name`, + * `fmt["name"]` or `require("./lib").name`. "any" for a computed key, and + * for any use of the value other than a member access, a call or `typeof`, + * which hands it to code that can write it (`Object.assign(fmt, …)`, + * `mutate(fmt)`, a reassignment of the binding). */ +function wholeExportWrites(lowerer: Lowerer, dep: ts.SourceFile): ReadonlySet | "any" { + const cached = lowerer.cjsWholeExportWrites.get(dep); + if (cached !== undefined) return cached; + const names = new Set(); + let any = false; + const use = (value: ts.Node): void => { + while (ts.isParenthesizedExpression(value.parent)) value = value.parent; + const parent = value.parent; + if (ts.isPropertyAccessExpression(parent) && parent.expression === value) { + if (isWrittenMember(parent)) names.add(parent.name.text); + } else if (ts.isElementAccessExpression(parent) && parent.expression === value) { + if (!isWrittenMember(parent)) return; + const key = parent.argumentExpression; + if (ts.isStringLiteralLike(key)) names.add(key.text); + else any = true; + } else if ( + !(ts.isCallExpression(parent) && parent.expression === value) && + !ts.isTypeOfExpression(parent) + ) { + any = true; + } + }; + for (const sf of lowerer.program.getSourceFiles()) { + if (any) break; + if (sf.isDeclarationFile || !sf.text.includes("require")) continue; + const bindings = new Set(); + ts.walkPreorder(sf, (node) => { + if (any) return "stop"; + if (!ts.isCallExpression(node) || inlineProgramRequireOf(lowerer, node)?.dep !== dep) + return undefined; + let value: ts.Node = node; + while (ts.isParenthesizedExpression(value.parent)) value = value.parent; + const declaration = value.parent; + if (!ts.isVariableDeclaration(declaration) || declaration.initializer !== value) use(node); + else if (ts.isIdentifier(declaration.name)) { + const symbol = lowerer.checker.getSymbolAtLocation(declaration.name); + if (symbol === undefined) any = true; + else bindings.add(symbol); + } else if (!ts.isObjectBindingPattern(declaration.name)) any = true; + return undefined; + }); + if (any || bindings.size === 0) continue; + ts.walkPreorder(sf, (node) => { + if (any) return "stop"; + if (!ts.isIdentifier(node)) return undefined; + const parent = node.parent; + if (ts.isVariableDeclaration(parent) && parent.name === node) return undefined; + const symbol = lowerer.checker.getSymbolAtLocation(node); + if (symbol !== undefined && bindings.has(symbol)) use(node); + return undefined; + }); + } + // The module itself writes the value through the function's own binding + // (`format.version = 2`), or through `module.exports.name` inside a + // function, where memberWritesIn does not count the write. + const whole = cjsWholeExportValueOf(dep); + const fn = + whole !== null && ts.isIdentifier(whole.rhs) ? lowerer.resolveValueSymbol(whole.rhs) : null; + if (!any && whole !== null) { + ts.walkPreorder(dep, (node) => { + if (any) return "stop"; + if ( + ts.isPropertyAccessExpression(node) && + isWrittenMember(node) && + (isModuleExportsAccess(node.expression) || + (ts.isIdentifier(node.expression) && node.expression.text === "exports")) && + cjsExportAssignmentOf(topLevelStatementOf(node)) === null + ) + names.add(node.name.text); + if ( + fn === null || + !ts.isIdentifier(node) || + node === whole.rhs || + lowerer.resolveValueSymbol(node) !== fn + ) + return undefined; + const parent = node.parent; + if ( + (ts.isFunctionDeclaration(parent) || ts.isVariableDeclaration(parent)) && + parent.name === node + ) + return undefined; + use(node); + return undefined; + }); + } + const result = any ? "any" : names; + lowerer.cjsWholeExportWrites.set(dep, result); + return result; +} + +/** The top-level statement that contains `node`. */ +function topLevelStatementOf(node: ts.Node): ts.Statement { + while (node.parent !== undefined && !ts.isSourceFile(node.parent)) node = node.parent; + return node as ts.Statement; +} + +/** True when a member access is assigned (directly, or as a destructuring + * or for-in/of target), updated or deleted. */ +function isWrittenMember(access: ts.Expression): boolean { + let node: ts.Node = access; + while (ts.isParenthesizedExpression(node.parent)) node = node.parent; + const parent = node.parent; + if (ts.isPrefixUnaryExpression(parent) || ts.isPostfixUnaryExpression(parent)) + return ( + parent.operator === ts.SyntaxKind.PlusPlusToken || + parent.operator === ts.SyntaxKind.MinusMinusToken + ); + if (ts.isDeleteExpression(parent)) return true; + // The target of an assignment or a for-in/of head, directly or as an + // element or property value of a destructuring literal. + let target: ts.Node = node; + for (;;) { + const up = target.parent; + if (ts.isBinaryExpression(up)) + return ( + up.left === target && + up.operatorToken.kind >= ts.SyntaxKind.FirstAssignment && + up.operatorToken.kind <= ts.SyntaxKind.LastAssignment + ); if ( - ts.isBinaryExpression(n) && - n.operatorToken.kind >= ts.SyntaxKind.FirstAssignment && - n.operatorToken.kind <= ts.SyntaxKind.LastAssignment && - ts.isPropertyAccessExpression(n.left) && - ts.isIdentifier(n.left.name) && - n.left.name.text === name && - ts.isIdentifier(n.left.expression) && - lowerer.checker.getSymbolAtLocation(n.left.expression) === sym + ((ts.isPropertyAssignment(up) && up.initializer === target) || + ts.isSpreadElement(up) || + ts.isSpreadAssignment(up)) && + up.parent !== undefined ) - written = true; - return undefined; - }); - return written ? null : dep; + target = up.parent; + else if (ts.isArrayLiteralExpression(up) || ts.isObjectLiteralExpression(up)) target = up; + else return (ts.isForInStatement(up) || ts.isForOfStatement(up)) && up.initializer === target; + } } /** True when the module's whole export names a function: a function diff --git a/packages/compiler/src/frontend/lowering/lower-stmts.ts b/packages/compiler/src/frontend/lowering/lower-stmts.ts index af290326ad..8548709d16 100644 --- a/packages/compiler/src/frontend/lowering/lower-stmts.ts +++ b/packages/compiler/src/frontend/lowering/lower-stmts.ts @@ -35,6 +35,7 @@ import { } from "./cjs-inline-require.js"; import { cjsExportMemberWriteGlobalOf, + cjsWholeExportSlotOwner, isCjsExportsIdentifier, isCjsModuleExports, isFreshExportsReset, @@ -6460,6 +6461,46 @@ function lowerCjsExportStatement( lowerer.badType(value, lowerer.typeOf(value)); } const lowered = lowerer.lowerExprExpecting(value, g.type); + // A whole export that is a function, with a member program code writes + // through a require: requirers read that member from the function's + // own properties, so the export stores it there as well as in its + // export global. + const owner = ts.isIdentifier(nameNode) + ? cjsWholeExportSlotOwner(lowerer, cjs.expr.getSourceFile(), nameNode.text) + : null; + if (owner !== null && root?.type.kind !== "dyn" && ts.isIdentifier(nameNode)) { + const saved = lowerer.declareHiddenLocal("%cjsMember", g.type); + const savedRef = (): IrExpr => ({ + kind: "varRef", + localId: saved.id, + type: g.type, + loc: locOf(value), + }); + const fn = lowerer.lowerExpr(owner); + return { + kind: "block", + body: [ + { kind: "varDecl", localId: saved.id, init: lowered, loc: locOf(value) }, + { + kind: "exprStmt", + expr: { + kind: "libCall", + fn: "dyn.keySet", + args: [ + lowerer.coerceInto(owner, fn, DYN), + { kind: "strLit", value: nameNode.text, type: STRING, loc: locOf(nameNode) }, + lowerer.coerceInto(value, savedRef(), DYN), + ], + type: VOID, + loc, + }, + loc, + }, + { kind: "assign", localId: g.id, value: savedRef(), loc: locOf(value) }, + ], + loc, + }; + } if (root?.type.kind !== "dyn" || !ts.isIdentifier(nameNode)) { return { kind: "assign", localId: g.id, value: lowered, loc: locOf(value) }; } diff --git a/packages/compiler/src/frontend/lowering/lowerer.ts b/packages/compiler/src/frontend/lowering/lowerer.ts index f07c3d6477..b3e1bc3631 100644 --- a/packages/compiler/src/frontend/lowering/lowerer.ts +++ b/packages/compiler/src/frontend/lowering/lowerer.ts @@ -2103,6 +2103,10 @@ export class Lowerer { /** Per CommonJS file using top-level `this`: the global holding the * object that stands for its original exports (lowerCjsTopLevelThis). */ readonly cjsExportsThis = new Map(); + /** Per CommonJS module whose whole export is a function: the member + * names program code may write on that value through a require of the + * module, or "any" (cjs-exports-object.ts, wholeExportWrites). */ + readonly cjsWholeExportWrites = new Map | "any">(); /** Inline program-module require calls whose %init the enclosing member * read or member call is already sequencing (cjs-inline-require.ts). */ readonly inlineRequireInitDone = new Set(); diff --git a/packages/compiler/src/frontend/lowering/var-hoisting.ts b/packages/compiler/src/frontend/lowering/var-hoisting.ts index 2a78e8fd30..8e9e52f4cb 100644 --- a/packages/compiler/src/frontend/lowering/var-hoisting.ts +++ b/packages/compiler/src/frontend/lowering/var-hoisting.ts @@ -335,20 +335,62 @@ function isExportsTarget(e: ts.Expression): boolean { const PURE_GLOBALS = new Set(["Object", "Array", "Number", "String", "Math", "JSON", "Symbol"]); -/** A call whose callee is a builtin method of a literal or a standard - * global: it cannot run user code. */ -function builtinCall(callee: ts.Expression): boolean { +/** A call of a builtin method of a literal or a standard global whose + * arguments are primitives: it cannot run user code. An argument that may + * be a function or an object (`Array.from(xs, () => …)`, `JSON.parse(s, + * reviver)`) can reach user code through a callback, an iterator or a + * conversion. */ +function builtinCall(call: ts.CallExpression): boolean { + const callee = call.expression; if (!ts.isPropertyAccessExpression(callee)) return false; let root: ts.Expression = callee.expression; while (ts.isPropertyAccessExpression(root)) root = root.expression; return ( - ts.isRegularExpressionLiteral(root) || - ts.isStringLiteral(root) || - ts.isNumericLiteral(root) || - (ts.isIdentifier(root) && PURE_GLOBALS.has(root.text)) + (ts.isRegularExpressionLiteral(root) || + ts.isStringLiteral(root) || + ts.isNumericLiteral(root) || + (ts.isIdentifier(root) && PURE_GLOBALS.has(root.text))) && + call.arguments.nodes.every(primitiveArgument) ); } +/** A literal, `undefined`, a read under `process` (`process.env.NODE_DEBUG`), + * or an operator over those. */ +function primitiveArgument(e: ts.Expression): boolean { + if ( + ts.isStringLiteral(e) || + ts.isNumericLiteral(e) || + ts.isNoSubstitutionTemplateLiteral(e) || + e.kind === ts.SyntaxKind.TrueKeyword || + e.kind === ts.SyntaxKind.FalseKeyword || + e.kind === ts.SyntaxKind.NullKeyword + ) + return true; + if (ts.isIdentifier(e)) return e.text === "undefined"; + if (ts.isParenthesizedExpression(e) || ts.isTypeOfExpression(e)) + return primitiveArgument(e.expression); + if (ts.isPrefixUnaryExpression(e)) return primitiveArgument(e.operand); + if (ts.isBinaryExpression(e)) + return ( + (e.operatorToken.kind < ts.SyntaxKind.FirstAssignment || + e.operatorToken.kind > ts.SyntaxKind.LastAssignment) && + primitiveArgument(e.left) && + primitiveArgument(e.right) + ); + if (ts.isConditionalExpression(e)) + return ( + primitiveArgument(e.condition) && + primitiveArgument(e.whenTrue) && + primitiveArgument(e.whenFalse) + ); + if (ts.isPropertyAccessExpression(e)) { + let root: ts.Expression = e; + while (ts.isPropertyAccessExpression(root)) root = root.expression; + return ts.isIdentifier(root) && root.text === "process"; + } + return false; +} + /** True when evaluating `stmt` cannot re-enter the module: no call into * user code, no construction, no require, no read of `module`/`exports` * beyond stores into them. Function and class bodies do not run here. */ @@ -365,7 +407,7 @@ function cannotReenter(stmt: ts.Statement): boolean { ) return "skip"; if (ts.isCallExpression(n)) { - if (!builtinCall(n.expression)) quiet = false; + if (!builtinCall(n)) quiet = false; } else if (ts.isNewExpression(n) || ts.isTaggedTemplateExpression(n)) { quiet = false; } else if (ts.isIdentifier(n) && (n.text === "module" || n.text === "exports")) { diff --git a/packages/compiler/test/cjs-settled-exports.test.ts b/packages/compiler/test/cjs-settled-exports.test.ts index af7438b994..f3c58bb69c 100644 --- a/packages/compiler/test/cjs-settled-exports.test.ts +++ b/packages/compiler/test/cjs-settled-exports.test.ts @@ -100,3 +100,33 @@ module.exports.bump = bump; "main.cjs: calling 'bump' through a module's whole export when the function may read 'this'", ]); }); + +test("a builtin call with a callback above a function export's table keeps the table optional", () => { + // Array.from runs the callback, which can call the exported function + // before `var t` is assigned, so `t` stays runtime-optional and its + // computed write keeps the fence semver's shape had before. + expect( + fences( + { + "main.cjs": `"use strict"; +const version = require("./version.cjs"); +console.log(version()); +`, + "version.cjs": `"use strict"; +exports = module.exports = Version; +var early = Array.from([1], () => module.exports()); +var t = {}; +var R = 0; +function tok(n) { + t[n] = R++; +} +tok("A"); +function Version() { + return typeof t; +} +`, + }, + "runtime-optional", + ), + ).toEqual(["version.cjs: a computed read from a runtime-optional global"]); +}); diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6485-cjs-whole-export-members/main.cjs.json b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6485-cjs-whole-export-members/main.cjs.json index fbacc3c7e1..0e466a0506 100644 --- a/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6485-cjs-whole-export-members/main.cjs.json +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/corpus/6485-cjs-whole-export-members/main.cjs.json @@ -2,6 +2,8 @@ "order": [ "/tests/corpus/6485-cjs-whole-export-members/plain.cjs", "/tests/corpus/6485-cjs-whole-export-members/rebound.cjs", + "/tests/corpus/6485-cjs-whole-export-members/counted.cjs", + "/tests/corpus/6485-cjs-whole-export-members/writer.cjs", "/tests/corpus/6485-cjs-whole-export-members/main.cjs" ], "diags": [] diff --git a/tests/corpus/6485-cjs-whole-export-members/counted.cjs b/tests/corpus/6485-cjs-whole-export-members/counted.cjs new file mode 100644 index 0000000000..65f812d769 --- /dev/null +++ b/tests/corpus/6485-cjs-whole-export-members/counted.cjs @@ -0,0 +1,8 @@ +"use strict"; +function counted() { + return counted.total; +} +module.exports = counted; +module.exports.total = 1; +module.exports.label = "one"; +module.exports.unit = "item"; diff --git a/tests/corpus/6485-cjs-whole-export-members/main.cjs b/tests/corpus/6485-cjs-whole-export-members/main.cjs index 762a53cbce..5e2d9e1527 100644 --- a/tests/corpus/6485-cjs-whole-export-members/main.cjs +++ b/tests/corpus/6485-cjs-whole-export-members/main.cjs @@ -9,3 +9,7 @@ console.log(format("a"), format.shout("b"), format.version, typeof format.shout) console.log(quote("c"), quote.whisper("D"), quote.kind, typeof quote.whisper); const { shout } = require("./plain.cjs"); console.log(shout === format.shout); +// Members another module writes through its own require of the export. +const counted = require("./counted.cjs"); +require("./writer.cjs"); +console.log(counted.total, counted.label, counted.unit, counted()); diff --git a/tests/corpus/6485-cjs-whole-export-members/writer.cjs b/tests/corpus/6485-cjs-whole-export-members/writer.cjs new file mode 100644 index 0000000000..5304a9cbb6 --- /dev/null +++ b/tests/corpus/6485-cjs-whole-export-members/writer.cjs @@ -0,0 +1,6 @@ +"use strict"; +// Writes the members of counted.cjs's export through this module's own +// require of it: a computed key, Object.assign, and an update. +const counted = require("./counted.cjs"); +counted["total"] = 2; +Object.assign(counted, { label: "two" }); From 739f15094d4053f3d1b98a923d7160de572e8e45 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Tue, 20 Oct 2026 17:00:56 -0700 Subject: [PATCH 14/22] Pass the exports object as `this` to exported functions that call sibling exports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit retry 0.13.1's `exports.timeouts` reads `this.createTimeout`, and `exports.operation` calls it through the exports object; async-retry wraps every request in an operation. A call through the exports object passes that object as `this`, which compiles to a static table, so the call was fenced ("calling 'exports.timeouts' through the module's exports object when the function may read 'this'"). When every `this` in an exported function expression names another export the module writes (`this.X` with `exports.X = …`), the exports object is never replaced, and the module calls the function only through `exports.f(…)`, a package-file stage now respells `this.X` as `exports.X`: the member Node reads for those calls. Only a caller that detaches the function or rebinds its `this` would observe a difference. Co-Authored-By: Claude Opus 5.5 --- .../frontend/npm-static-exports-this.test.ts | 64 +++++ .../src/frontend/npm-static-exports-this.ts | 121 ++++++++++ packages/compiler/src/frontend/npm-static.ts | 5 + packages/compiler/src/frontend/services.ts | 8 + .../npm/cases/2597-exports-this/main.ts | 6 + .../npm/cases/2598-retry-timeouts/main.ts | 12 + .../npm/node_modules/gtexportsthis/index.js | 14 ++ .../node_modules/gtexportsthis/package.json | 1 + tests/fixtures/npm/node_modules/retry/License | 21 ++ .../fixtures/npm/node_modules/retry/README.md | 227 ++++++++++++++++++ .../npm/node_modules/retry/example/dns.js | 31 +++ .../npm/node_modules/retry/example/stop.js | 40 +++ .../fixtures/npm/node_modules/retry/index.js | 1 + .../npm/node_modules/retry/lib/retry.js | 100 ++++++++ .../node_modules/retry/lib/retry_operation.js | 162 +++++++++++++ .../npm/node_modules/retry/package.json | 36 +++ tests/harness/npm-cases.ts | 4 +- tests/harness/npm-static.test.ts | 13 +- 18 files changed, 863 insertions(+), 3 deletions(-) create mode 100644 packages/compiler/src/frontend/npm-static-exports-this.test.ts create mode 100644 packages/compiler/src/frontend/npm-static-exports-this.ts create mode 100644 tests/fixtures/npm/cases/2597-exports-this/main.ts create mode 100644 tests/fixtures/npm/cases/2598-retry-timeouts/main.ts create mode 100644 tests/fixtures/npm/node_modules/gtexportsthis/index.js create mode 100644 tests/fixtures/npm/node_modules/gtexportsthis/package.json create mode 100644 tests/fixtures/npm/node_modules/retry/License create mode 100644 tests/fixtures/npm/node_modules/retry/README.md create mode 100644 tests/fixtures/npm/node_modules/retry/example/dns.js create mode 100644 tests/fixtures/npm/node_modules/retry/example/stop.js create mode 100644 tests/fixtures/npm/node_modules/retry/index.js create mode 100644 tests/fixtures/npm/node_modules/retry/lib/retry.js create mode 100644 tests/fixtures/npm/node_modules/retry/lib/retry_operation.js create mode 100644 tests/fixtures/npm/node_modules/retry/package.json diff --git a/packages/compiler/src/frontend/npm-static-exports-this.test.ts b/packages/compiler/src/frontend/npm-static-exports-this.test.ts new file mode 100644 index 0000000000..462ee6a197 --- /dev/null +++ b/packages/compiler/src/frontend/npm-static-exports-this.test.ts @@ -0,0 +1,64 @@ +import { spawnSync } from "node:child_process"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { afterAll, expect, test } from "vitest"; +import { nodeFrontendServices } from "./services-node.js"; + +const dirs: string[] = []; +afterAll(() => { + for (const dir of dirs) rmSync(dir, { recursive: true, force: true }); +}); + +function rewrite(source: string): string | null { + return nodeFrontendServices().exportsThis(source, "/pkg/index.js"); +} + +function expectSameBehavior(source: string, use: string): string { + const result = rewrite(source); + expect(typeof result).toBe("string"); + const dir = mkdtempSync(join(tmpdir(), "scriptc-exports-this-")); + dirs.push(dir); + writeFileSync(join(dir, "original.cjs"), source); + writeFileSync(join(dir, "rewritten.cjs"), result!); + const run = (file: string) => { + writeFileSync(join(dir, "use.cjs"), use.replace("LIB", `./${file}`)); + return spawnSync(process.execPath, [join(dir, "use.cjs")], { encoding: "utf8" }); + }; + const oracle = run("original.cjs"); + expect(oracle.status).toBe(0); + expect(run("rewritten.cjs").stdout).toBe(oracle.stdout); + return result!; +} + +test("exported functions reach sibling exports through `this`", () => { + const out = expectSameBehavior( + `'use strict'; +exports.operation = function (o) { return exports.timeouts(o).length; }; +exports.timeouts = function (o) { + var t = []; + for (var i = 0; i < o.retries; i++) t.push(this.createTimeout(i)); + [1].forEach(() => t.push(this.createTimeout(9))); + return t; +}; +exports.createTimeout = function (i) { return 10 * Math.pow(2, i); }; +`, + `const lib = require("LIB"); console.log(lib.timeouts({ retries: 3 }).join(","), lib.operation({ retries: 2 }));`, + ); + expect(out).toContain("exports.createTimeout(i)"); +}); + +test("other uses of `this`, replaced exports and detached functions keep their text", () => { + // `this` as a value. + expect(rewrite(`exports.f = function () { return this; };\nexports.g = 1;\n`)).toBeNull(); + // A member the module does not export. + expect(rewrite(`exports.f = function () { return this.missing; };\n`)).toBeNull(); + // The exports object replaced. + expect( + rewrite(`exports.f = function () { return this.g(); };\nexports.g = function () { return 1; };\nmodule.exports = { f: exports.f };\n`), + ).toBeNull(); + // The function handed elsewhere inside the module. + expect( + rewrite(`exports.f = function () { return this.g(); };\nexports.g = function () { return 1; };\nsetTimeout(exports.f, 0);\n`), + ).toBeNull(); +}); diff --git a/packages/compiler/src/frontend/npm-static-exports-this.ts b/packages/compiler/src/frontend/npm-static-exports-this.ts new file mode 100644 index 0000000000..dc74d39661 --- /dev/null +++ b/packages/compiler/src/frontend/npm-static-exports-this.ts @@ -0,0 +1,121 @@ +/* A CommonJS module whose exported functions call each other through + * `this` (retry 0.13: `exports.timeouts = function (options) { … + * this.createTimeout(i, opts) … }`). A call `exports.timeouts()` or + * `retry.timeouts()` passes the exports object as `this`, which compiles to + * a static table with no runtime object to pass. When every `this` in such + * a function names another export the module writes (`this.X` where the + * module has `exports.X = …`), the exports object is never replaced, and + * the module itself calls the function only through `exports.f(…)`, `this.X` + * reads `exports.X`: the same member Node reads for those calls. Only a + * caller that detaches the function or rebinds its `this` would observe + * the difference. */ +import * as ts from "./ts7/syntax.js"; +import { applyEdits, unparen, type Edit } from "./npm-static-bundled-chunks.js"; + +/** True when the text may hold such a function (a cheap probe). */ +export function mayHoldExportsThis(source: string): boolean { + return source.includes("this.") && source.includes("exports."); +} + +function exportsMember(e: ts.Expression): string | null { + const u = unparen(e); + if (!ts.isPropertyAccessExpression(u) || !ts.isIdentifier(u.name)) return null; + const obj = u.expression; + if (ts.isIdentifier(obj) && obj.text === "exports") return u.name.text; + if ( + ts.isPropertyAccessExpression(obj) && + ts.isIdentifier(obj.expression) && + obj.expression.text === "module" && + obj.name.text === "exports" + ) + return u.name.text; + return null; +} + +/** The module text with `this.X` respelled `exports.X` in qualifying + * exported functions, or null. */ +export function rewriteExportsThis(sf: ts.SourceFile): string | null { + if (ts.isExternalModule(sf) && !sf.fileName.endsWith(".cjs")) return null; + // Names the module exports by member assignment, and whether it ever + // replaces or rebinds the exports object. + const exported = new Set(); + const functions = new Map(); + let replaced = false; + for (const stmt of sf.statements.nodes) { + if (!ts.isExpressionStatement(stmt)) continue; + const e = unparen(stmt.expression); + if (!ts.isBinaryExpression(e) || e.operatorToken.kind !== ts.SyntaxKind.EqualsToken) continue; + const name = exportsMember(e.left); + if (name !== null) { + if (exported.has(name)) functions.delete(name); + exported.add(name); + const value = unparen(e.right); + if (ts.isFunctionExpression(value) && !functions.has(name)) functions.set(name, value); + } + } + ts.walkPreorder(sf, (n) => { + if (replaced) return "stop"; + if (ts.isBinaryExpression(n) && n.operatorToken.kind >= ts.SyntaxKind.FirstAssignment && n.operatorToken.kind <= ts.SyntaxKind.LastAssignment) { + const left = unparen(n.left); + if (ts.isIdentifier(left) && left.text === "exports") replaced = true; + if ( + ts.isPropertyAccessExpression(left) && + ts.isIdentifier(left.expression) && + left.expression.text === "module" && + left.name.text === "exports" + ) + replaced = true; + } + return undefined; + }); + if (replaced) return null; + const edits: Edit[] = []; + for (const [name, fn] of functions) { + // `this` uses directly in the body (arrow functions share it). + const uses: ts.ThisExpression[] = []; + let ok = true; + const walk = (n: ts.Node): void => { + if (!ok) return; + if (n !== fn && (ts.isFunctionLike(n) && !ts.isArrowFunction(n))) return; + if (ts.isClassDeclaration(n) || ts.isClassExpression(n)) return; + if (n.kind === ts.SyntaxKind.ThisKeyword) { + const parent = n.parent; + if ( + parent === undefined || + !ts.isPropertyAccessExpression(parent) || + parent.expression !== n || + !ts.isIdentifier(parent.name) || + !exported.has(parent.name.text) + ) { + ok = false; + return; + } + uses.push(n as ts.ThisExpression); + return; + } + n.forEachChild(walk); + }; + fn.forEachChild(walk); + if (!ok || uses.length === 0) continue; + // Inside the module the function is only called through the exports. + let detached = false; + ts.walkPreorder(sf, (n) => { + if (detached) return "stop"; + if (!ts.isPropertyAccessExpression(n) || !ts.isIdentifier(n.name) || n.name.text !== name) return undefined; + if (exportsMember(n) !== name) return undefined; + const parent = n.parent; + const isStore = + parent !== undefined && + ts.isBinaryExpression(parent) && + parent.left === n && + parent.operatorToken.kind === ts.SyntaxKind.EqualsToken; + const isCall = parent !== undefined && ts.isCallExpression(parent) && parent.expression === n; + if (!isStore && !isCall) detached = true; + return undefined; + }); + if (detached) continue; + for (const use of uses) edits.push({ start: use.getStart(sf), end: use.getEnd(), text: "exports" }); + } + if (edits.length === 0) return null; + return applyEdits(sf.text, 0, sf.text.length, edits); +} diff --git a/packages/compiler/src/frontend/npm-static.ts b/packages/compiler/src/frontend/npm-static.ts index 2454b35469..5975f3679b 100644 --- a/packages/compiler/src/frontend/npm-static.ts +++ b/packages/compiler/src/frontend/npm-static.ts @@ -455,6 +455,10 @@ export function npmStaticFsShadow(services: FrontendServices): NpmStaticFsShadow // Polyfill packages that ARE a Node builtin compile as the builtin. const aliased = original === null ? null : services.builtinAliasRequires(original, path); if (aliased !== null) original = aliased; + // Exported functions that reach sibling exports through `this`. + const exportsThis = + original === null ? null : services.exportsThis(original, path); + if (exportsThis !== null) original = exportsThis; // tsc's interop helpers around builtin modules. const tscHelpers = original === null ? null : services.tscBuiltinImports(original, path); if (tscHelpers !== null) original = tscHelpers; @@ -479,6 +483,7 @@ export function npmStaticFsShadow(services: FrontendServices): NpmStaticFsShadow split !== null || declSplit !== null || tscHelpers !== null || + exportsThis !== null || esm !== null || aliased !== null || fnExports !== null || diff --git a/packages/compiler/src/frontend/services.ts b/packages/compiler/src/frontend/services.ts index 5fe85f48c9..ad0d108a0e 100644 --- a/packages/compiler/src/frontend/services.ts +++ b/packages/compiler/src/frontend/services.ts @@ -14,6 +14,7 @@ import { } from "./npm-static-builtin-aliases.js"; import { mayHoldEnumIife, rewriteEnumIifes } from "./npm-static-enum-iife.js"; import { mayHoldTscBuiltinImport, unwrapTscBuiltinImports } from "./npm-static-tsc-helpers.js"; +import { mayHoldExportsThis, rewriteExportsThis } from "./npm-static-exports-this.js"; import { mayMergeRequireDeclarations, splitRequireDeclarations, @@ -118,6 +119,13 @@ export class FrontendServices { if (!mayMergeRequireDeclarations(source)) return null; return splitRequireDeclarations(this.parse(path, source, "js")); } + /** A package module with `this.X` respelled `exports.X` in exported + * functions that only call other exports through `this` (null: none). */ + exportsThis(source: string, path: string): string | null { + this.ensureOpen(); + if (!mayHoldExportsThis(source)) return null; + return rewriteExportsThis(this.parse(path, source, "js")); + } /** A package module with tsc's interop bindings of builtin modules * unwrapped to plain requires (null: none). */ tscBuiltinImports(source: string, path: string): string | null { diff --git a/tests/fixtures/npm/cases/2597-exports-this/main.ts b/tests/fixtures/npm/cases/2597-exports-this/main.ts new file mode 100644 index 0000000000..2134e01f6b --- /dev/null +++ b/tests/fixtures/npm/cases/2597-exports-this/main.ts @@ -0,0 +1,6 @@ +// Exported functions that call sibling exports through `this`: a call +// through the exports object (`lib.total()`, `exports.scale()` inside the +// module) passes that object as `this`. +import lib from "gtexportsthis"; + +console.log(lib.scale(2), lib.total([1, 2, 3])); diff --git a/tests/fixtures/npm/cases/2598-retry-timeouts/main.ts b/tests/fixtures/npm/cases/2598-retry-timeouts/main.ts new file mode 100644 index 0000000000..6621bff68e --- /dev/null +++ b/tests/fixtures/npm/cases/2598-retry-timeouts/main.ts @@ -0,0 +1,12 @@ +// retry 0.13.1 (vendored unmodified, MIT): `exports.timeouts` reads +// `this.createTimeout`, and `exports.operation` calls it through the exports +// object. async-retry wraps every request in such an operation. +import retry from "retry"; + +console.log(retry.timeouts({ retries: 4, minTimeout: 10, maxTimeout: 100, factor: 2 }).join(",")); +const op = retry.operation({ retries: 2, minTimeout: 1, maxTimeout: 2 }); +op.attempt((attempt: number) => { + console.log("attempt", attempt); + if (op.retry(attempt < 3 ? new Error(`fail ${attempt}`) : null)) return; + console.log("done", op.attempts(), op.errors().length); +}); diff --git a/tests/fixtures/npm/node_modules/gtexportsthis/index.js b/tests/fixtures/npm/node_modules/gtexportsthis/index.js new file mode 100644 index 0000000000..e9a10cfe09 --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtexportsthis/index.js @@ -0,0 +1,14 @@ +'use strict'; +exports.scale = function (n) { + return n * this.factor(); +}; +exports.factor = function () { + return 3; +}; +exports.total = function (xs) { + var sum = 0; + xs.forEach((x) => { + sum += this.scale(x); + }); + return exports.scale(0) + sum; +}; diff --git a/tests/fixtures/npm/node_modules/gtexportsthis/package.json b/tests/fixtures/npm/node_modules/gtexportsthis/package.json new file mode 100644 index 0000000000..aeb53858da --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtexportsthis/package.json @@ -0,0 +1 @@ +{"name":"gtexportsthis","version":"1.0.0","main":"index.js"} diff --git a/tests/fixtures/npm/node_modules/retry/License b/tests/fixtures/npm/node_modules/retry/License new file mode 100644 index 0000000000..0b58de379f --- /dev/null +++ b/tests/fixtures/npm/node_modules/retry/License @@ -0,0 +1,21 @@ +Copyright (c) 2011: +Tim Koschützki (tim@debuggable.com) +Felix Geisendörfer (felix@debuggable.com) + + Permission is hereby granted, free of charge, to any person obtaining a copy + of this software and associated documentation files (the "Software"), to deal + in the Software without restriction, including without limitation the rights + to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + copies of the Software, and to permit persons to whom the Software is + furnished to do so, subject to the following conditions: + + The above copyright notice and this permission notice shall be included in + all copies or substantial portions of the Software. + + THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + THE SOFTWARE. diff --git a/tests/fixtures/npm/node_modules/retry/README.md b/tests/fixtures/npm/node_modules/retry/README.md new file mode 100644 index 0000000000..6d5415300f --- /dev/null +++ b/tests/fixtures/npm/node_modules/retry/README.md @@ -0,0 +1,227 @@ + +[![Build Status](https://secure.travis-ci.org/tim-kos/node-retry.svg?branch=master)](http://travis-ci.org/tim-kos/node-retry "Check this project's build status on TravisCI") +[![codecov](https://codecov.io/gh/tim-kos/node-retry/branch/master/graph/badge.svg)](https://codecov.io/gh/tim-kos/node-retry) + + +# retry + +Abstraction for exponential and custom retry strategies for failed operations. + +## Installation + + npm install retry + +## Current Status + +This module has been tested and is ready to be used. + +## Tutorial + +The example below will retry a potentially failing `dns.resolve` operation +`10` times using an exponential backoff strategy. With the default settings, this +means the last attempt is made after `17 minutes and 3 seconds`. + +``` javascript +var dns = require('dns'); +var retry = require('retry'); + +function faultTolerantResolve(address, cb) { + var operation = retry.operation(); + + operation.attempt(function(currentAttempt) { + dns.resolve(address, function(err, addresses) { + if (operation.retry(err)) { + return; + } + + cb(err ? operation.mainError() : null, addresses); + }); + }); +} + +faultTolerantResolve('nodejs.org', function(err, addresses) { + console.log(err, addresses); +}); +``` + +Of course you can also configure the factors that go into the exponential +backoff. See the API documentation below for all available settings. +currentAttempt is an int representing the number of attempts so far. + +``` javascript +var operation = retry.operation({ + retries: 5, + factor: 3, + minTimeout: 1 * 1000, + maxTimeout: 60 * 1000, + randomize: true, +}); +``` + +## API + +### retry.operation([options]) + +Creates a new `RetryOperation` object. `options` is the same as `retry.timeouts()`'s `options`, with three additions: + +* `forever`: Whether to retry forever, defaults to `false`. +* `unref`: Whether to [unref](https://nodejs.org/api/timers.html#timers_unref) the setTimeout's, defaults to `false`. +* `maxRetryTime`: The maximum time (in milliseconds) that the retried operation is allowed to run. Default is `Infinity`. + +### retry.timeouts([options]) + +Returns an array of timeouts. All time `options` and return values are in +milliseconds. If `options` is an array, a copy of that array is returned. + +`options` is a JS object that can contain any of the following keys: + +* `retries`: The maximum amount of times to retry the operation. Default is `10`. Seting this to `1` means `do it once, then retry it once`. +* `factor`: The exponential factor to use. Default is `2`. +* `minTimeout`: The number of milliseconds before starting the first retry. Default is `1000`. +* `maxTimeout`: The maximum number of milliseconds between two retries. Default is `Infinity`. +* `randomize`: Randomizes the timeouts by multiplying with a factor between `1` to `2`. Default is `false`. + +The formula used to calculate the individual timeouts is: + +``` +Math.min(random * minTimeout * Math.pow(factor, attempt), maxTimeout) +``` + +Have a look at [this article][article] for a better explanation of approach. + +If you want to tune your `factor` / `times` settings to attempt the last retry +after a certain amount of time, you can use wolfram alpha. For example in order +to tune for `10` attempts in `5 minutes`, you can use this equation: + +![screenshot](https://github.com/tim-kos/node-retry/raw/master/equation.gif) + +Explaining the various values from left to right: + +* `k = 0 ... 9`: The `retries` value (10) +* `1000`: The `minTimeout` value in ms (1000) +* `x^k`: No need to change this, `x` will be your resulting factor +* `5 * 60 * 1000`: The desired total amount of time for retrying in ms (5 minutes) + +To make this a little easier for you, use wolfram alpha to do the calculations: + + + +[article]: http://dthain.blogspot.com/2009/02/exponential-backoff-in-distributed.html + +### retry.createTimeout(attempt, opts) + +Returns a new `timeout` (integer in milliseconds) based on the given parameters. + +`attempt` is an integer representing for which retry the timeout should be calculated. If your retry operation was executed 4 times you had one attempt and 3 retries. If you then want to calculate a new timeout, you should set `attempt` to 4 (attempts are zero-indexed). + +`opts` can include `factor`, `minTimeout`, `randomize` (boolean) and `maxTimeout`. They are documented above. + +`retry.createTimeout()` is used internally by `retry.timeouts()` and is public for you to be able to create your own timeouts for reinserting an item, see [issue #13](https://github.com/tim-kos/node-retry/issues/13). + +### retry.wrap(obj, [options], [methodNames]) + +Wrap all functions of the `obj` with retry. Optionally you can pass operation options and +an array of method names which need to be wrapped. + +``` +retry.wrap(obj) + +retry.wrap(obj, ['method1', 'method2']) + +retry.wrap(obj, {retries: 3}) + +retry.wrap(obj, {retries: 3}, ['method1', 'method2']) +``` +The `options` object can take any options that the usual call to `retry.operation` can take. + +### new RetryOperation(timeouts, [options]) + +Creates a new `RetryOperation` where `timeouts` is an array where each value is +a timeout given in milliseconds. + +Available options: +* `forever`: Whether to retry forever, defaults to `false`. +* `unref`: Wether to [unref](https://nodejs.org/api/timers.html#timers_unref) the setTimeout's, defaults to `false`. + +If `forever` is true, the following changes happen: +* `RetryOperation.errors()` will only output an array of one item: the last error. +* `RetryOperation` will repeatedly use the `timeouts` array. Once all of its timeouts have been used up, it restarts with the first timeout, then uses the second and so on. + +#### retryOperation.errors() + +Returns an array of all errors that have been passed to `retryOperation.retry()` so far. The +returning array has the errors ordered chronologically based on when they were passed to +`retryOperation.retry()`, which means the first passed error is at index zero and the last is +at the last index. + +#### retryOperation.mainError() + +A reference to the error object that occured most frequently. Errors are +compared using the `error.message` property. + +If multiple error messages occured the same amount of time, the last error +object with that message is returned. + +If no errors occured so far, the value is `null`. + +#### retryOperation.attempt(fn, timeoutOps) + +Defines the function `fn` that is to be retried and executes it for the first +time right away. The `fn` function can receive an optional `currentAttempt` callback that represents the number of attempts to execute `fn` so far. + +Optionally defines `timeoutOps` which is an object having a property `timeout` in miliseconds and a property `cb` callback function. +Whenever your retry operation takes longer than `timeout` to execute, the timeout callback function `cb` is called. + + +#### retryOperation.try(fn) + +This is an alias for `retryOperation.attempt(fn)`. This is deprecated. Please use `retryOperation.attempt(fn)` instead. + +#### retryOperation.start(fn) + +This is an alias for `retryOperation.attempt(fn)`. This is deprecated. Please use `retryOperation.attempt(fn)` instead. + +#### retryOperation.retry(error) + +Returns `false` when no `error` value is given, or the maximum amount of retries +has been reached. + +Otherwise it returns `true`, and retries the operation after the timeout for +the current attempt number. + +#### retryOperation.stop() + +Allows you to stop the operation being retried. Useful for aborting the operation on a fatal error etc. + +#### retryOperation.reset() + +Resets the internal state of the operation object, so that you can call `attempt()` again as if this was a new operation object. + +#### retryOperation.attempts() + +Returns an int representing the number of attempts it took to call `fn` before it was successful. + +## License + +retry is licensed under the MIT license. + + +# Changelog + +0.10.0 Adding `stop` functionality, thanks to @maxnachlinger. + +0.9.0 Adding `unref` functionality, thanks to @satazor. + +0.8.0 Implementing retry.wrap. + +0.7.0 Some bug fixes and made retry.createTimeout() public. Fixed issues [#10](https://github.com/tim-kos/node-retry/issues/10), [#12](https://github.com/tim-kos/node-retry/issues/12), and [#13](https://github.com/tim-kos/node-retry/issues/13). + +0.6.0 Introduced optional timeOps parameter for the attempt() function which is an object having a property timeout in milliseconds and a property cb callback function. Whenever your retry operation takes longer than timeout to execute, the timeout callback function cb is called. + +0.5.0 Some minor refactoring. + +0.4.0 Changed retryOperation.try() to retryOperation.attempt(). Deprecated the aliases start() and try() for it. + +0.3.0 Added retryOperation.start() which is an alias for retryOperation.try(). + +0.2.0 Added attempts() function and parameter to retryOperation.try() representing the number of attempts it took to call fn(). diff --git a/tests/fixtures/npm/node_modules/retry/example/dns.js b/tests/fixtures/npm/node_modules/retry/example/dns.js new file mode 100644 index 0000000000..446729b6f9 --- /dev/null +++ b/tests/fixtures/npm/node_modules/retry/example/dns.js @@ -0,0 +1,31 @@ +var dns = require('dns'); +var retry = require('../lib/retry'); + +function faultTolerantResolve(address, cb) { + var opts = { + retries: 2, + factor: 2, + minTimeout: 1 * 1000, + maxTimeout: 2 * 1000, + randomize: true + }; + var operation = retry.operation(opts); + + operation.attempt(function(currentAttempt) { + dns.resolve(address, function(err, addresses) { + if (operation.retry(err)) { + return; + } + + cb(operation.mainError(), operation.errors(), addresses); + }); + }); +} + +faultTolerantResolve('nodejs.org', function(err, errors, addresses) { + console.warn('err:'); + console.log(err); + + console.warn('addresses:'); + console.log(addresses); +}); \ No newline at end of file diff --git a/tests/fixtures/npm/node_modules/retry/example/stop.js b/tests/fixtures/npm/node_modules/retry/example/stop.js new file mode 100644 index 0000000000..e1ceafeeba --- /dev/null +++ b/tests/fixtures/npm/node_modules/retry/example/stop.js @@ -0,0 +1,40 @@ +var retry = require('../lib/retry'); + +function attemptAsyncOperation(someInput, cb) { + var opts = { + retries: 2, + factor: 2, + minTimeout: 1 * 1000, + maxTimeout: 2 * 1000, + randomize: true + }; + var operation = retry.operation(opts); + + operation.attempt(function(currentAttempt) { + failingAsyncOperation(someInput, function(err, result) { + + if (err && err.message === 'A fatal error') { + operation.stop(); + return cb(err); + } + + if (operation.retry(err)) { + return; + } + + cb(operation.mainError(), operation.errors(), result); + }); + }); +} + +attemptAsyncOperation('test input', function(err, errors, result) { + console.warn('err:'); + console.log(err); + + console.warn('result:'); + console.log(result); +}); + +function failingAsyncOperation(input, cb) { + return setImmediate(cb.bind(null, new Error('A fatal error'))); +} diff --git a/tests/fixtures/npm/node_modules/retry/index.js b/tests/fixtures/npm/node_modules/retry/index.js new file mode 100644 index 0000000000..ee62f3a112 --- /dev/null +++ b/tests/fixtures/npm/node_modules/retry/index.js @@ -0,0 +1 @@ +module.exports = require('./lib/retry'); \ No newline at end of file diff --git a/tests/fixtures/npm/node_modules/retry/lib/retry.js b/tests/fixtures/npm/node_modules/retry/lib/retry.js new file mode 100644 index 0000000000..5e85e79197 --- /dev/null +++ b/tests/fixtures/npm/node_modules/retry/lib/retry.js @@ -0,0 +1,100 @@ +var RetryOperation = require('./retry_operation'); + +exports.operation = function(options) { + var timeouts = exports.timeouts(options); + return new RetryOperation(timeouts, { + forever: options && (options.forever || options.retries === Infinity), + unref: options && options.unref, + maxRetryTime: options && options.maxRetryTime + }); +}; + +exports.timeouts = function(options) { + if (options instanceof Array) { + return [].concat(options); + } + + var opts = { + retries: 10, + factor: 2, + minTimeout: 1 * 1000, + maxTimeout: Infinity, + randomize: false + }; + for (var key in options) { + opts[key] = options[key]; + } + + if (opts.minTimeout > opts.maxTimeout) { + throw new Error('minTimeout is greater than maxTimeout'); + } + + var timeouts = []; + for (var i = 0; i < opts.retries; i++) { + timeouts.push(this.createTimeout(i, opts)); + } + + if (options && options.forever && !timeouts.length) { + timeouts.push(this.createTimeout(i, opts)); + } + + // sort the array numerically ascending + timeouts.sort(function(a,b) { + return a - b; + }); + + return timeouts; +}; + +exports.createTimeout = function(attempt, opts) { + var random = (opts.randomize) + ? (Math.random() + 1) + : 1; + + var timeout = Math.round(random * Math.max(opts.minTimeout, 1) * Math.pow(opts.factor, attempt)); + timeout = Math.min(timeout, opts.maxTimeout); + + return timeout; +}; + +exports.wrap = function(obj, options, methods) { + if (options instanceof Array) { + methods = options; + options = null; + } + + if (!methods) { + methods = []; + for (var key in obj) { + if (typeof obj[key] === 'function') { + methods.push(key); + } + } + } + + for (var i = 0; i < methods.length; i++) { + var method = methods[i]; + var original = obj[method]; + + obj[method] = function retryWrapper(original) { + var op = exports.operation(options); + var args = Array.prototype.slice.call(arguments, 1); + var callback = args.pop(); + + args.push(function(err) { + if (op.retry(err)) { + return; + } + if (err) { + arguments[0] = op.mainError(); + } + callback.apply(this, arguments); + }); + + op.attempt(function() { + original.apply(obj, args); + }); + }.bind(obj, original); + obj[method].options = options; + } +}; diff --git a/tests/fixtures/npm/node_modules/retry/lib/retry_operation.js b/tests/fixtures/npm/node_modules/retry/lib/retry_operation.js new file mode 100644 index 0000000000..105ce72b2b --- /dev/null +++ b/tests/fixtures/npm/node_modules/retry/lib/retry_operation.js @@ -0,0 +1,162 @@ +function RetryOperation(timeouts, options) { + // Compatibility for the old (timeouts, retryForever) signature + if (typeof options === 'boolean') { + options = { forever: options }; + } + + this._originalTimeouts = JSON.parse(JSON.stringify(timeouts)); + this._timeouts = timeouts; + this._options = options || {}; + this._maxRetryTime = options && options.maxRetryTime || Infinity; + this._fn = null; + this._errors = []; + this._attempts = 1; + this._operationTimeout = null; + this._operationTimeoutCb = null; + this._timeout = null; + this._operationStart = null; + this._timer = null; + + if (this._options.forever) { + this._cachedTimeouts = this._timeouts.slice(0); + } +} +module.exports = RetryOperation; + +RetryOperation.prototype.reset = function() { + this._attempts = 1; + this._timeouts = this._originalTimeouts.slice(0); +} + +RetryOperation.prototype.stop = function() { + if (this._timeout) { + clearTimeout(this._timeout); + } + if (this._timer) { + clearTimeout(this._timer); + } + + this._timeouts = []; + this._cachedTimeouts = null; +}; + +RetryOperation.prototype.retry = function(err) { + if (this._timeout) { + clearTimeout(this._timeout); + } + + if (!err) { + return false; + } + var currentTime = new Date().getTime(); + if (err && currentTime - this._operationStart >= this._maxRetryTime) { + this._errors.push(err); + this._errors.unshift(new Error('RetryOperation timeout occurred')); + return false; + } + + this._errors.push(err); + + var timeout = this._timeouts.shift(); + if (timeout === undefined) { + if (this._cachedTimeouts) { + // retry forever, only keep last error + this._errors.splice(0, this._errors.length - 1); + timeout = this._cachedTimeouts.slice(-1); + } else { + return false; + } + } + + var self = this; + this._timer = setTimeout(function() { + self._attempts++; + + if (self._operationTimeoutCb) { + self._timeout = setTimeout(function() { + self._operationTimeoutCb(self._attempts); + }, self._operationTimeout); + + if (self._options.unref) { + self._timeout.unref(); + } + } + + self._fn(self._attempts); + }, timeout); + + if (this._options.unref) { + this._timer.unref(); + } + + return true; +}; + +RetryOperation.prototype.attempt = function(fn, timeoutOps) { + this._fn = fn; + + if (timeoutOps) { + if (timeoutOps.timeout) { + this._operationTimeout = timeoutOps.timeout; + } + if (timeoutOps.cb) { + this._operationTimeoutCb = timeoutOps.cb; + } + } + + var self = this; + if (this._operationTimeoutCb) { + this._timeout = setTimeout(function() { + self._operationTimeoutCb(); + }, self._operationTimeout); + } + + this._operationStart = new Date().getTime(); + + this._fn(this._attempts); +}; + +RetryOperation.prototype.try = function(fn) { + console.log('Using RetryOperation.try() is deprecated'); + this.attempt(fn); +}; + +RetryOperation.prototype.start = function(fn) { + console.log('Using RetryOperation.start() is deprecated'); + this.attempt(fn); +}; + +RetryOperation.prototype.start = RetryOperation.prototype.try; + +RetryOperation.prototype.errors = function() { + return this._errors; +}; + +RetryOperation.prototype.attempts = function() { + return this._attempts; +}; + +RetryOperation.prototype.mainError = function() { + if (this._errors.length === 0) { + return null; + } + + var counts = {}; + var mainError = null; + var mainErrorCount = 0; + + for (var i = 0; i < this._errors.length; i++) { + var error = this._errors[i]; + var message = error.message; + var count = (counts[message] || 0) + 1; + + counts[message] = count; + + if (count >= mainErrorCount) { + mainError = error; + mainErrorCount = count; + } + } + + return mainError; +}; diff --git a/tests/fixtures/npm/node_modules/retry/package.json b/tests/fixtures/npm/node_modules/retry/package.json new file mode 100644 index 0000000000..48f35e8cff --- /dev/null +++ b/tests/fixtures/npm/node_modules/retry/package.json @@ -0,0 +1,36 @@ +{ + "author": "Tim Koschützki (http://debuggable.com/)", + "name": "retry", + "description": "Abstraction for exponential and custom retry strategies for failed operations.", + "license": "MIT", + "version": "0.13.1", + "homepage": "https://github.com/tim-kos/node-retry", + "repository": { + "type": "git", + "url": "git://github.com/tim-kos/node-retry.git" + }, + "files": [ + "lib", + "example" + ], + "directories": { + "lib": "./lib" + }, + "main": "index.js", + "engines": { + "node": ">= 4" + }, + "dependencies": {}, + "devDependencies": { + "fake": "0.2.0", + "istanbul": "^0.4.5", + "tape": "^4.8.0" + }, + "scripts": { + "test": "./node_modules/.bin/istanbul cover ./node_modules/tape/bin/tape ./test/integration/*.js", + "release:major": "env SEMANTIC=major npm run release", + "release:minor": "env SEMANTIC=minor npm run release", + "release:patch": "env SEMANTIC=patch npm run release", + "release": "npm version ${SEMANTIC:-patch} -m \"Release %s\" && git push && git push --tags && npm publish" + } +} diff --git a/tests/harness/npm-cases.ts b/tests/harness/npm-cases.ts index 6dc8e81470..6270f7b5c5 100644 --- a/tests/harness/npm-cases.ts +++ b/tests/harness/npm-cases.ts @@ -19,14 +19,14 @@ export function npmCases(fixturesRoot: string): NpmCase[] { return [ ...globSync(join(fixturesRoot, "npm/cases/*/main.ts")) .sort() - // 2465-2469, 2556-2567, 2571 and 2580-2596 are the --npm-static bundler-emitted-CJS + // 2465-2469, 2556-2567, 2571 and 2580-2598 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-7]|2571|258[0-9]|259[0-6])-[^/]+\/main\.ts$/.test(entry), + !/\/(246[5-9]|255[6-9]|256[0-7]|2571|258[0-9]|259[0-8])-[^/]+\/main\.ts$/.test(entry), ) .map((entry) => ({ name: entry.split("/").at(-2)!, diff --git a/tests/harness/npm-static.test.ts b/tests/harness/npm-static.test.ts index 1c12d184c8..7e8a173e36 100644 --- a/tests/harness/npm-static.test.ts +++ b/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-2567, 2571, 2580-2596) + // the bundler-emitted-CJS mini packages (cases 2465-2469, 2556-2567, 2571, 2580-2598) ...globSync(join(fixturesRoot, "npm/node_modules/gt*/**/*.{js,cjs,json}")).sort(), ...globSync(join(fixturesRoot, "npm/store/gt*/**/*.{js,cjs,json}")).sort(), ]; @@ -1673,6 +1673,17 @@ describe(`npm-static pilots${sanitize ? " (sanitized)" : ""}`, () => { ]); }, 180_000); + // 2597/2598: exported functions that reach sibling exports through + // `this`, in a generic package and in retry 0.13.1. + test("exports through this 2597-exports-this byte-match Node", async () => { + await expectNodeParity(join(fixturesRoot, "npm/cases/2597-exports-this/main.ts"), [ + "gtexportsthis", + ]); + }, 180_000); + test("retry 0.13.1 2598-retry-timeouts byte-matches Node", async () => { + await expectNodeParity(join(fixturesRoot, "npm/cases/2598-retry-timeouts/main.ts"), ["retry"]); + }, 180_000); + // 2562: a file with unverifiable interop that no compiled code can load // (the program imports a leaf data module and only a type from the // main entry) leaves its package static. From 8ac501ba04b77f98882f78cab54699b879f1f821 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Tue, 20 Oct 2026 19:21:25 -0500 Subject: [PATCH 15/22] Refuse namespace reads of inherited names and of names an entry star re-exports A member read of a name a module does not export answered undefined. Two such names have values on the namespace object: - Object.prototype's members (constructor, toString, __proto__ and the rest), since __toCommonJS builds an ordinary object. - Names a star re-export into the bundle's entry adds: the entry's export table lists only its own exports. Both keep the package refused, as other reads the rewrite cannot follow do. --- .../frontend/npm-static-bundled-esm.test.ts | 52 +++++++++++++++++++ .../src/frontend/npm-static-bundled-esm.ts | 21 +++++++- 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/packages/compiler/src/frontend/npm-static-bundled-esm.test.ts b/packages/compiler/src/frontend/npm-static-bundled-esm.test.ts index 6e758e0d51..fe2cc9f7d8 100644 --- a/packages/compiler/src/frontend/npm-static-bundled-esm.test.ts +++ b/packages/compiler/src/frontend/npm-static-bundled-esm.test.ts @@ -413,3 +413,55 @@ init_index(); console.log(late()); `); }); + +const reExportingEntry = (use: string): string => `"use strict"; +${helpers} +var __reExport = (target, mod, secondTarget) => (__copyProps(target, mod, "default"), secondTarget && __copyProps(secondTarget, mod, "default")); +var other_exports = {}; +__export(other_exports, { EXTRA: () => EXTRA }); +var EXTRA; +var init_other = __esm({ "src/other.ts"() { EXTRA = "extra"; } }); +var index_exports = {}; +__export(index_exports, { KIND: () => KIND, late: () => late }); +module.exports = __toCommonJS(index_exports); +__reExport(index_exports, (init_other(), __toCommonJS(other_exports)), module.exports); +var KIND; +var init_index = __esm({ "src/index.ts"() { KIND = "lambda"; } }); +function late() { + const index = (init_index(), __toCommonJS(index_exports)); + return ${use}; +} +init_index(); +console.log(late()); +`; + +test("an entry that star re-exports a module still answers its own names", () => { + expectSameBehavior(reExportingEntry("index.KIND")); +}); + +test("a read through the entry namespace of a name a star re-export adds keeps the package refused", () => { + expect(lower(reExportingEntry("index.KIND + ' ' + index.EXTRA"))).toEqual({ + degrade: expect.stringContaining( + "reads the namespace of a bundled ES module ('index_exports')", + ) as string, + }); +}); + +test.each([ + ["constructor", "lazy.constructor === Object"], + ["toString", "typeof lazy.toString"], + ["__proto__", "lazy.__proto__ === Object.prototype"], + ["hasOwnProperty", "typeof lazy.hasOwnProperty"], +])("a read of the inherited %s through a namespace keeps the package refused", (_, use) => { + expect( + lower(`"use strict"; +${helpers}${lazyModule} +const lazy = (init_lazy(), __toCommonJS(lazy_exports)); +console.log(${use}); +`), + ).toEqual({ + degrade: expect.stringContaining( + "reads the namespace of a bundled ES module ('lazy_exports')", + ) as string, + }); +}); diff --git a/packages/compiler/src/frontend/npm-static-bundled-esm.ts b/packages/compiler/src/frontend/npm-static-bundled-esm.ts index 80fc23ec97..d509d1945d 100644 --- a/packages/compiler/src/frontend/npm-static-bundled-esm.ts +++ b/packages/compiler/src/frontend/npm-static-bundled-esm.ts @@ -546,6 +546,21 @@ export function lowerBundledEsmInits( ) tables.set(name, { stmt: null, entries: [] }); } + // A star re-export into the entry (`__reExport(main, ns, module.exports)`) + // adds names its export table does not list, so the entry namespace's + // missing names are unknown. + const entryReExports = + mainTarget !== null && + (refsByName.get(mainTarget) ?? []).some((x) => { + const p = x.parent; + return ( + p !== undefined && + ts.isCallExpression(p) && + ts.isIdentifier(p.expression) && + helperBase(p.expression.text) === "__reExport" && + p.arguments.nodes[0] === x + ); + }); const lowered = new Set(); /** Some binding between the use site and the top level rebinds a name * the table's getters read. */ @@ -761,7 +776,10 @@ export function lowerBundledEsmInits( } const values = new Map(table.entries); // A name the module does not export reads undefined on the namespace - // object (`__esModule` reads true); plain reads only, never calls. + // object (`__esModule` reads true); plain reads only, never calls. The + // namespace is an ordinary object, so a name Object.prototype answers + // (constructor, toString, __proto__) is no missing name, and neither + // is any name of an entry that star re-exports other modules. const missing = new Set(); const readNames = new Set(); const memberRead = (access: ts.Node | undefined, of: ts.Node): boolean => { @@ -777,6 +795,7 @@ export function lowerBundledEsmInits( const parent = access.parent; if (parent !== undefined && ts.isCallExpression(parent) && parent.expression === access) return false; + if (name in Object.prototype || (isMain && entryReExports)) return false; missing.add(name); return true; } From d5044b99590b15071e8f284d7eac7031bf927fd1 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Tue, 20 Oct 2026 17:41:39 -0700 Subject: [PATCH 16/22] Count a module's own member export of its function as no write of its members `module.exports = spawn; module.exports.spawn = spawn` (cross-spawn 7) stores the function as one of its own member exports. That use of `spawn` counted as handing the value to code that can write its members, so every member was stored on the function as a runtime property, and the typed `module.exports.sync = spawnSync` failed at load again (SC1101; npm-static case 2596). A top-level member export store writes no member of the function. Co-Authored-By: Claude Opus 5.5 --- .../compiler/src/frontend/lowering/cjs-exports-object.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/compiler/src/frontend/lowering/cjs-exports-object.ts b/packages/compiler/src/frontend/lowering/cjs-exports-object.ts index ae2bcd96cf..61efcb74f2 100644 --- a/packages/compiler/src/frontend/lowering/cjs-exports-object.ts +++ b/packages/compiler/src/frontend/lowering/cjs-exports-object.ts @@ -662,6 +662,15 @@ function wholeExportWrites(lowerer: Lowerer, dep: ts.SourceFile): ReadonlySet Date: Tue, 20 Oct 2026 17:54:18 -0700 Subject: [PATCH 17/22] Format the exports-this stage Co-Authored-By: Claude Opus 5.5 --- .../src/frontend/npm-static-exports-this.test.ts | 8 ++++++-- .../src/frontend/npm-static-exports-this.ts | 14 ++++++++++---- packages/compiler/src/frontend/npm-static.ts | 3 +-- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/packages/compiler/src/frontend/npm-static-exports-this.test.ts b/packages/compiler/src/frontend/npm-static-exports-this.test.ts index 462ee6a197..8db75b7bbb 100644 --- a/packages/compiler/src/frontend/npm-static-exports-this.test.ts +++ b/packages/compiler/src/frontend/npm-static-exports-this.test.ts @@ -55,10 +55,14 @@ test("other uses of `this`, replaced exports and detached functions keep their t expect(rewrite(`exports.f = function () { return this.missing; };\n`)).toBeNull(); // The exports object replaced. expect( - rewrite(`exports.f = function () { return this.g(); };\nexports.g = function () { return 1; };\nmodule.exports = { f: exports.f };\n`), + rewrite( + `exports.f = function () { return this.g(); };\nexports.g = function () { return 1; };\nmodule.exports = { f: exports.f };\n`, + ), ).toBeNull(); // The function handed elsewhere inside the module. expect( - rewrite(`exports.f = function () { return this.g(); };\nexports.g = function () { return 1; };\nsetTimeout(exports.f, 0);\n`), + rewrite( + `exports.f = function () { return this.g(); };\nexports.g = function () { return 1; };\nsetTimeout(exports.f, 0);\n`, + ), ).toBeNull(); }); diff --git a/packages/compiler/src/frontend/npm-static-exports-this.ts b/packages/compiler/src/frontend/npm-static-exports-this.ts index dc74d39661..c275ef53da 100644 --- a/packages/compiler/src/frontend/npm-static-exports-this.ts +++ b/packages/compiler/src/frontend/npm-static-exports-this.ts @@ -55,7 +55,11 @@ export function rewriteExportsThis(sf: ts.SourceFile): string | null { } ts.walkPreorder(sf, (n) => { if (replaced) return "stop"; - if (ts.isBinaryExpression(n) && n.operatorToken.kind >= ts.SyntaxKind.FirstAssignment && n.operatorToken.kind <= ts.SyntaxKind.LastAssignment) { + if ( + ts.isBinaryExpression(n) && + n.operatorToken.kind >= ts.SyntaxKind.FirstAssignment && + n.operatorToken.kind <= ts.SyntaxKind.LastAssignment + ) { const left = unparen(n.left); if (ts.isIdentifier(left) && left.text === "exports") replaced = true; if ( @@ -76,7 +80,7 @@ export function rewriteExportsThis(sf: ts.SourceFile): string | null { let ok = true; const walk = (n: ts.Node): void => { if (!ok) return; - if (n !== fn && (ts.isFunctionLike(n) && !ts.isArrowFunction(n))) return; + if (n !== fn && ts.isFunctionLike(n) && !ts.isArrowFunction(n)) return; if (ts.isClassDeclaration(n) || ts.isClassExpression(n)) return; if (n.kind === ts.SyntaxKind.ThisKeyword) { const parent = n.parent; @@ -101,7 +105,8 @@ export function rewriteExportsThis(sf: ts.SourceFile): string | null { let detached = false; ts.walkPreorder(sf, (n) => { if (detached) return "stop"; - if (!ts.isPropertyAccessExpression(n) || !ts.isIdentifier(n.name) || n.name.text !== name) return undefined; + if (!ts.isPropertyAccessExpression(n) || !ts.isIdentifier(n.name) || n.name.text !== name) + return undefined; if (exportsMember(n) !== name) return undefined; const parent = n.parent; const isStore = @@ -114,7 +119,8 @@ export function rewriteExportsThis(sf: ts.SourceFile): string | null { return undefined; }); if (detached) continue; - for (const use of uses) edits.push({ start: use.getStart(sf), end: use.getEnd(), text: "exports" }); + for (const use of uses) + edits.push({ start: use.getStart(sf), end: use.getEnd(), text: "exports" }); } if (edits.length === 0) return null; return applyEdits(sf.text, 0, sf.text.length, edits); diff --git a/packages/compiler/src/frontend/npm-static.ts b/packages/compiler/src/frontend/npm-static.ts index 5975f3679b..dfbf23bcd9 100644 --- a/packages/compiler/src/frontend/npm-static.ts +++ b/packages/compiler/src/frontend/npm-static.ts @@ -456,8 +456,7 @@ export function npmStaticFsShadow(services: FrontendServices): NpmStaticFsShadow const aliased = original === null ? null : services.builtinAliasRequires(original, path); if (aliased !== null) original = aliased; // Exported functions that reach sibling exports through `this`. - const exportsThis = - original === null ? null : services.exportsThis(original, path); + const exportsThis = original === null ? null : services.exportsThis(original, path); if (exportsThis !== null) original = exportsThis; // tsc's interop helpers around builtin modules. const tscHelpers = original === null ? null : services.tscBuiltinImports(original, path); From 6b6df7e4cc130a2a761873bf6fecea7a71452703 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Tue, 20 Oct 2026 20:39:36 -0500 Subject: [PATCH 18/22] Look inside classes for early calls, count only member writes of function exports, and leave written interop wrappers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - A class runs its heritage, decorators, computed member names, static field initializers and static blocks where it is defined, so a call there can call a function export before the variables it reads are assigned. The re-entry check now skips only method bodies and instance field initializers. - Passing a function whole export to other code (a re-export, a callback argument) no longer marks all of its members as runtime properties. Named writes, computed-key writes and the keys builtins such as Object.assign and Object.defineProperty write still do, so typed members such as `module.exports.sync = spawnSync` stay static exports. - A tsc interop binding of a builtin whose `.default` or namespace members are written keeps its helper: the write changes the helper's object, and respelling `x.default = …` as `x = …` assigned the const binding. --- .../frontend/lowering/cjs-exports-object.ts | 54 +++++++++++-- .../src/frontend/lowering/var-hoisting.ts | 75 +++++++++++-------- .../frontend/npm-static-tsc-helpers.test.ts | 11 +++ .../src/frontend/npm-static-tsc-helpers.ts | 40 +++++++++- .../compiler/test/cjs-settled-exports.test.ts | 34 +++++++++ 5 files changed, 175 insertions(+), 39 deletions(-) diff --git a/packages/compiler/src/frontend/lowering/cjs-exports-object.ts b/packages/compiler/src/frontend/lowering/cjs-exports-object.ts index 61efcb74f2..1f2b0dbc5c 100644 --- a/packages/compiler/src/frontend/lowering/cjs-exports-object.ts +++ b/packages/compiler/src/frontend/lowering/cjs-exports-object.ts @@ -576,10 +576,10 @@ export function cjsWholeExportSlotOwner( /** The member names program files may write on `dep`'s whole export value * through a require of it: assignments, updates and deletes of `fmt.name`, - * `fmt["name"]` or `require("./lib").name`. "any" for a computed key, and - * for any use of the value other than a member access, a call or `typeof`, - * which hands it to code that can write it (`Object.assign(fmt, …)`, - * `mutate(fmt)`, a reassignment of the binding). */ + * `fmt["name"]` or `require("./lib").name`, and the keys builtins such as + * `Object.assign(fmt, { name })` and `Object.defineProperty(fmt, "name", …)` + * write. "any" for a computed key. Writes by code the value is passed to + * (`mutate(fmt)`) are not seen. */ function wholeExportWrites(lowerer: Lowerer, dep: ts.SourceFile): ReadonlySet | "any" { const cached = lowerer.cjsWholeExportWrites.get(dep); if (cached !== undefined) return cached; @@ -596,8 +596,50 @@ function wholeExportWrites(lowerer: Lowerer, dep: ts.SourceFile): ReadonlySet { + if (callee === "Object.assign") { + for (const source of rest) { + if (!ts.isObjectLiteralExpression(source)) { + any = true; + continue; + } + for (const prop of source.properties.nodes) { + const key = + ts.isPropertyAssignment(prop) || + ts.isShorthandPropertyAssignment(prop) || + ts.isMethodDeclaration(prop) + ? prop.name + : undefined; + if (key !== undefined && (ts.isIdentifier(key) || ts.isStringLiteralLike(key))) + names.add(key.text); + else any = true; + } + } + } else if ( + callee === "Object.defineProperty" || + callee === "Reflect.set" || + callee === "Reflect.defineProperty" || + callee === "Reflect.deleteProperty" + ) { + const key = rest[0]; + if (key !== undefined && ts.isStringLiteralLike(key)) names.add(key.text); + else any = true; + } else if ( + callee === "Object.defineProperties" || + callee === "Object.setPrototypeOf" || + callee === "Reflect.setPrototypeOf" ) { any = true; } diff --git a/packages/compiler/src/frontend/lowering/var-hoisting.ts b/packages/compiler/src/frontend/lowering/var-hoisting.ts index 8e9e52f4cb..5f37f81169 100644 --- a/packages/compiler/src/frontend/lowering/var-hoisting.ts +++ b/packages/compiler/src/frontend/lowering/var-hoisting.ts @@ -393,43 +393,54 @@ function primitiveArgument(e: ts.Expression): boolean { /** True when evaluating `stmt` cannot re-enter the module: no call into * user code, no construction, no require, no read of `module`/`exports` - * beyond stores into them. Function and class bodies do not run here. */ + * beyond stores into them. Function bodies, class methods and instance + * field initializers do not run here. */ function cannotReenter(stmt: ts.Statement): boolean { if (ts.isFunctionDeclaration(stmt) || ts.isEmptyStatement(stmt)) return true; let quiet = true; - ts.walkPreorder(stmt, (n) => { - if (!quiet) return "stop"; - if ( - ts.isFunctionLike(n) || - ts.isClassDeclaration(n) || - ts.isClassExpression(n) || - ts.isClassStaticBlockDeclaration(n) - ) - return "skip"; - if (ts.isCallExpression(n)) { - if (!builtinCall(n)) quiet = false; - } else if (ts.isNewExpression(n) || ts.isTaggedTemplateExpression(n)) { - quiet = false; - } else if (ts.isIdentifier(n) && (n.text === "module" || n.text === "exports")) { - // Only as the root of a store target. - let target: ts.Node = n; - while ( - target.parent !== undefined && - ts.isPropertyAccessExpression(target.parent) && - target.parent.expression === target - ) - target = target.parent; - const assign = target.parent; + const visit = (root: ts.Node): void => { + ts.walkPreorder(root, (n) => { + if (!quiet) return "stop"; + // A class runs its heritage, decorators, computed member names, + // static field initializers and static blocks where it is defined; + // methods and instance field initializers run later. if ( - assign === undefined || - !ts.isBinaryExpression(assign) || - assign.operatorToken.kind !== ts.SyntaxKind.EqualsToken || - assign.left !== target - ) + ts.isFunctionLike(n) || + (ts.isPropertyDeclaration(n) && + !ts.getModifiers(n)?.some((modifier) => modifier.kind === ts.SyntaxKind.StaticKeyword)) + ) { + for (const modifier of ts.getModifiers(n) ?? []) + if (ts.isDecorator(modifier)) visit(modifier); + const name = (n as { name?: ts.Node }).name; + if (name !== undefined && ts.isComputedPropertyName(name)) visit(name); + return "skip"; + } + if (ts.isCallExpression(n)) { + if (!builtinCall(n)) quiet = false; + } else if (ts.isNewExpression(n) || ts.isTaggedTemplateExpression(n)) { quiet = false; - } - return undefined; - }); + } else if (ts.isIdentifier(n) && (n.text === "module" || n.text === "exports")) { + // Only as the root of a store target. + let target: ts.Node = n; + while ( + target.parent !== undefined && + ts.isPropertyAccessExpression(target.parent) && + target.parent.expression === target + ) + target = target.parent; + const assign = target.parent; + if ( + assign === undefined || + !ts.isBinaryExpression(assign) || + assign.operatorToken.kind !== ts.SyntaxKind.EqualsToken || + assign.left !== target + ) + quiet = false; + } + return undefined; + }); + }; + visit(stmt); return quiet; } diff --git a/packages/compiler/src/frontend/npm-static-tsc-helpers.test.ts b/packages/compiler/src/frontend/npm-static-tsc-helpers.test.ts index 735c9d298b..b14290ba86 100644 --- a/packages/compiler/src/frontend/npm-static-tsc-helpers.test.ts +++ b/packages/compiler/src/frontend/npm-static-tsc-helpers.test.ts @@ -64,6 +64,17 @@ test("other references, program modules and default reads of a namespace keep th expect( rewrite(`${helpers}const m = __importDefault(require("./m"));\nm.default();\n`), ).toBeNull(); + // Writes through the wrapper change the helper's object, not the module. + expect( + rewrite( + `${helpers}const events_1 = __importDefault(require("events"));\nevents_1.default = null;\nconsole.log(events_1.default);\n`, + ), + ).toBeNull(); + expect( + rewrite( + `${helpers}const p = __importStar(require("path"));\np.sep = "!";\n[p.extra] = [1];\nconsole.log(p.sep);\n`, + ), + ).toBeNull(); // `ns.default` reads the namespace copy's own default. expect( rewrite(`${helpers}const p = __importStar(require("path"));\nconsole.log(p.default === p);\n`), diff --git a/packages/compiler/src/frontend/npm-static-tsc-helpers.ts b/packages/compiler/src/frontend/npm-static-tsc-helpers.ts index fcb73be5b6..67b2785de2 100644 --- a/packages/compiler/src/frontend/npm-static-tsc-helpers.ts +++ b/packages/compiler/src/frontend/npm-static-tsc-helpers.ts @@ -57,11 +57,14 @@ function interopReferences( if (!ok) return "stop"; if (!ts.isIdentifier(n) || n.text !== name || n === decl.name) return undefined; const parent = n.parent; + // A write through the wrapper (`x.default = …`, `ns.member = …`) + // changes the helper's object, not the module. if ( parent === undefined || !ts.isPropertyAccessExpression(parent) || parent.expression !== n || - parent.questionDotToken !== undefined + parent.questionDotToken !== undefined || + isWriteTarget(parent) ) { ok = false; return "stop"; @@ -78,6 +81,41 @@ function interopReferences( return ok ? defaults : null; } +/** Whether an access is assigned (directly or as a destructuring or + * for-in/of target), updated or deleted. */ +function isWriteTarget(access: ts.Expression): boolean { + let node: ts.Node = access; + while (node.parent !== undefined && ts.isParenthesizedExpression(node.parent)) node = node.parent; + const parent = node.parent; + if (parent === undefined) return false; + if (ts.isPrefixUnaryExpression(parent) || ts.isPostfixUnaryExpression(parent)) + return ( + parent.operator === ts.SyntaxKind.PlusPlusToken || + parent.operator === ts.SyntaxKind.MinusMinusToken + ); + if (ts.isDeleteExpression(parent)) return true; + let target: ts.Node = node; + for (;;) { + const up = target.parent; + if (up === undefined) return false; + if (ts.isBinaryExpression(up)) + return ( + up.left === target && + up.operatorToken.kind >= ts.SyntaxKind.FirstAssignment && + up.operatorToken.kind <= ts.SyntaxKind.LastAssignment + ); + if ( + ((ts.isPropertyAssignment(up) && up.initializer === target) || + ts.isSpreadElement(up) || + ts.isSpreadAssignment(up)) && + up.parent !== undefined + ) + target = up.parent; + else if (ts.isArrayLiteralExpression(up) || ts.isObjectLiteralExpression(up)) target = up; + else return (ts.isForInStatement(up) || ts.isForOfStatement(up)) && up.initializer === target; + } +} + /** The module text with its builtin interop bindings unwrapped, or null. */ export function unwrapTscBuiltinImports(sf: ts.SourceFile): string | null { if (ts.isExternalModule(sf) && !sf.fileName.endsWith(".cjs")) return null; diff --git a/packages/compiler/test/cjs-settled-exports.test.ts b/packages/compiler/test/cjs-settled-exports.test.ts index f3c58bb69c..2e1a4c44dc 100644 --- a/packages/compiler/test/cjs-settled-exports.test.ts +++ b/packages/compiler/test/cjs-settled-exports.test.ts @@ -130,3 +130,37 @@ function Version() { ), ).toEqual(["version.cjs: a computed read from a runtime-optional global"]); }); + +test("a class static initializer above a function export's table keeps the table optional", () => { + // Static field initializers and static blocks run where the class is + // defined, so either can call the exported function before `var t` is + // assigned. + for (const member of ["static early = module.exports();", "static { module.exports(); }"]) { + expect( + fences( + { + "main.cjs": `"use strict"; +const version = require("./version.cjs"); +console.log(version()); +`, + "version.cjs": `"use strict"; +exports = module.exports = Version; +class Init { + ${member} +} +var t = {}; +var R = 0; +function tok(n) { + t[n] = R++; +} +tok("A"); +function Version() { + return typeof t; +} +`, + }, + "runtime-optional", + ), + ).toEqual(["version.cjs: a computed read from a runtime-optional global"]); + } +}); From 691e21877cbec9a08f707b3b8551234c04f3ee92 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Tue, 20 Oct 2026 18:42:23 -0700 Subject: [PATCH 19/22] Record order baselines for the exports-this cases Co-Authored-By: Claude Opus 5.5 --- .../npm/cases/2597-exports-this/main.ts.json | 16 ++++++++++++++++ .../npm/cases/2598-retry-timeouts/main.ts.json | 16 ++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2597-exports-this/main.ts.json create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2598-retry-timeouts/main.ts.json diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2597-exports-this/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2597-exports-this/main.ts.json new file mode 100644 index 0000000000..182827e6de --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2597-exports-this/main.ts.json @@ -0,0 +1,16 @@ +{ + "order": [ + "/tests/fixtures/npm/cases/2597-exports-this/main.ts" + ], + "diags": [ + { + "code": "SC0001", + "message": "Could not find a declaration file for module 'gtexportsthis'. '/tests/fixtures/npm/node_modules/gtexportsthis/index.js' implicitly has an 'any' type.\n Try `npm i --save-dev @types/gtexportsthis` if it exists or add a new declaration (.d.ts) file containing `declare module 'gtexportsthis';`", + "loc": { + "file": "/tests/fixtures/npm/cases/2597-exports-this/main.ts", + "start": 203, + "end": 218 + } + } + ] +} diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2598-retry-timeouts/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2598-retry-timeouts/main.ts.json new file mode 100644 index 0000000000..96882d1d33 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2598-retry-timeouts/main.ts.json @@ -0,0 +1,16 @@ +{ + "order": [ + "/tests/fixtures/npm/cases/2598-retry-timeouts/main.ts" + ], + "diags": [ + { + "code": "SC0001", + "message": "Could not find a declaration file for module 'retry'. '/tests/fixtures/npm/node_modules/retry/index.js' implicitly has an 'any' type.\n Try `npm i --save-dev @types/retry` if it exists or add a new declaration (.d.ts) file containing `declare module 'retry';`", + "loc": { + "file": "/tests/fixtures/npm/cases/2598-retry-timeouts/main.ts", + "start": 230, + "end": 237 + } + } + ] +} From 6fd1804fc508cf54a68d3b65e400944e4d5611c2 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Tue, 20 Oct 2026 18:46:47 -0700 Subject: [PATCH 20/22] Accept destructured requires above a function export A module that binds `const {a, b} = require(...)` above `module.exports = f; module.exports.g = ...` (execa 5's index.js shape) skipped the function-export rewrite, because only identifier require bindings counted as inert. The members then never reached the inferred export type, and `execa.command` read as missing. A plain object pattern (identifier elements, no defaults, rest, computed keys or nesting) runs no user code, so it counts as inert too. Co-Authored-By: Claude Opus 5.5 --- .../npm-static-cjs-exports-alias.test.ts | 20 ++++++++++++++++++ .../frontend/npm-static-cjs-exports-alias.ts | 19 ++++++++++++++++- .../npm/cases/2599-execa-shape/main.ts.json | 16 ++++++++++++++ .../npm/cases/2599-execa-shape/main.ts | 8 +++++++ .../npm/node_modules/gtexecashape/index.js | 21 +++++++++++++++++++ .../node_modules/gtexecashape/lib/command.js | 4 ++++ .../node_modules/gtexecashape/package.json | 1 + tests/harness/npm-cases.ts | 4 ++-- tests/harness/npm-static.test.ts | 9 +++++++- 9 files changed, 98 insertions(+), 4 deletions(-) create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2599-execa-shape/main.ts.json create mode 100644 tests/fixtures/npm/cases/2599-execa-shape/main.ts create mode 100644 tests/fixtures/npm/node_modules/gtexecashape/index.js create mode 100644 tests/fixtures/npm/node_modules/gtexecashape/lib/command.js create mode 100644 tests/fixtures/npm/node_modules/gtexecashape/package.json diff --git a/packages/compiler/src/frontend/npm-static-cjs-exports-alias.test.ts b/packages/compiler/src/frontend/npm-static-cjs-exports-alias.test.ts index 2309918483..0ff5dc1f4b 100644 --- a/packages/compiler/src/frontend/npm-static-cjs-exports-alias.test.ts +++ b/packages/compiler/src/frontend/npm-static-cjs-exports-alias.test.ts @@ -99,7 +99,27 @@ ${report}`); expect(viaConst).toContain('Version.tag = "t"'); }); +test("destructured require bindings above a function export are inert", () => { + // execa 5: `const {a, b} = require(...)` lines sit above the export. + const rewritten = expectSameBehavior(`'use strict'; +const {basename, extname} = require('path'); +const Version = (v) => ({ v, base: basename(v) }); +module.exports = Version; +module.exports.ext = (v) => extname(v); +console.log(module.exports.ext("a.txt"), Version("/x/y").base); +${report}`); + expect(rewritten).toContain("Version.ext = "); +}); + test.each([ + [ + "a destructuring default", + `const {basename = 1} = require('path');\nconst Version = () => 1;\nmodule.exports = Version;\nmodule.exports.x = 1;\n`, + ], + [ + "a destructuring rest", + `const {...rest} = require('path');\nconst Version = () => 1;\nmodule.exports = Version;\nmodule.exports.x = 1;\n`, + ], ["a let binding", `let Version = () => 1;\nmodule.exports = Version;\nmodule.exports.x = 1;\n`], [ "a reassigned var", diff --git a/packages/compiler/src/frontend/npm-static-cjs-exports-alias.ts b/packages/compiler/src/frontend/npm-static-cjs-exports-alias.ts index b081a7e8d7..8e5b77e7c0 100644 --- a/packages/compiler/src/frontend/npm-static-cjs-exports-alias.ts +++ b/packages/compiler/src/frontend/npm-static-cjs-exports-alias.ts @@ -62,6 +62,21 @@ function isRequireValue(expr: ts.Expression): boolean { return false; } +/** `{ a, b: c }`: a destructuring of plain member reads (no defaults, no + * computed keys, no nested patterns, no rest), which runs no user code. */ +function plainObjectPattern(name: ts.BindingName): boolean { + return ( + ts.isObjectBindingPattern(name) && + name.elements.nodes.every( + (el) => + el.dotDotDotToken === undefined && + el.initializer === undefined && + ts.isIdentifier(el.name) && + (el.propertyName === undefined || ts.isIdentifier(el.propertyName)), + ) + ); +} + /** True for a statement whose evaluation runs no code that could observe * the export object: declarations and member writes (`F.prototype.m = * function …`) without calls, and directives. */ @@ -79,7 +94,9 @@ function inertOrRequire(stmt: ts.Statement, requires: boolean): boolean { ts.isVariableStatement(stmt) && stmt.declarationList.declarations.nodes.every( (d) => - ts.isIdentifier(d.name) && d.initializer !== undefined && isRequireValue(d.initializer), + (ts.isIdentifier(d.name) || plainObjectPattern(d.name)) && + d.initializer !== undefined && + isRequireValue(d.initializer), ) ) return true; diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2599-execa-shape/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2599-execa-shape/main.ts.json new file mode 100644 index 0000000000..520db17c19 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/2599-execa-shape/main.ts.json @@ -0,0 +1,16 @@ +{ + "order": [ + "/tests/fixtures/npm/cases/2599-execa-shape/main.ts" + ], + "diags": [ + { + "code": "SC0001", + "message": "Could not find a declaration file for module 'gtexecashape'. '/tests/fixtures/npm/node_modules/gtexecashape/index.js' implicitly has an 'any' type.\n Try `npm i --save-dev @types/gtexecashape` if it exists or add a new declaration (.d.ts) file containing `declare module 'gtexecashape';`", + "loc": { + "file": "/tests/fixtures/npm/cases/2599-execa-shape/main.ts", + "start": 244, + "end": 258 + } + } + ] +} diff --git a/tests/fixtures/npm/cases/2599-execa-shape/main.ts b/tests/fixtures/npm/cases/2599-execa-shape/main.ts new file mode 100644 index 0000000000..c71ff40e4f --- /dev/null +++ b/tests/fixtures/npm/cases/2599-execa-shape/main.ts @@ -0,0 +1,8 @@ +// execa 5's export shape: `const execa = (file, args, options) => …` exported +// whole behind identifier and destructuring requires, then members attached +// to it (`module.exports.command = …`). A default import reads them. +import execa from "gtexecashape"; + +console.log(execa("/usr/bin/node", ["-v"])); +console.log(execa.command("git status --short", { maxBuffer: 10 })); +console.log(execa.sync("ls", [])); diff --git a/tests/fixtures/npm/node_modules/gtexecashape/index.js b/tests/fixtures/npm/node_modules/gtexecashape/index.js new file mode 100644 index 0000000000..bd16db1912 --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtexecashape/index.js @@ -0,0 +1,21 @@ +'use strict'; +const path = require('path'); +const {joinCommand, parseCommand} = require('./lib/command'); + +const DEFAULT_MAX_BUFFER = 1000 * 1000 * 100; + +const execa = (file, args, options) => { + const opts = options || {}; + return `run ${joinCommand(path.basename(file), args || [])} buffer=${opts.maxBuffer || DEFAULT_MAX_BUFFER}`; +}; + +module.exports = execa; + +module.exports.sync = (file, args, options) => { + return `sync ${execa(file, args, options)}`; +}; + +module.exports.command = (command, options) => { + const [file, ...args] = parseCommand(command); + return execa(file, args, options); +}; diff --git a/tests/fixtures/npm/node_modules/gtexecashape/lib/command.js b/tests/fixtures/npm/node_modules/gtexecashape/lib/command.js new file mode 100644 index 0000000000..f7db8fa21c --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtexecashape/lib/command.js @@ -0,0 +1,4 @@ +'use strict'; +const parseCommand = (command) => command.trim().split(/ +/g); +const joinCommand = (file, args) => [file, ...args].join(' '); +module.exports = { parseCommand, joinCommand }; diff --git a/tests/fixtures/npm/node_modules/gtexecashape/package.json b/tests/fixtures/npm/node_modules/gtexecashape/package.json new file mode 100644 index 0000000000..92fc987f1c --- /dev/null +++ b/tests/fixtures/npm/node_modules/gtexecashape/package.json @@ -0,0 +1 @@ +{"name":"gtexecashape","version":"5.1.1","description":"execa 5`s index.js shape: identifier and destructuring requires, then `const execa = (file, args, options) => …; module.exports = execa; module.exports.command = …`","main":"index.js"} diff --git a/tests/harness/npm-cases.ts b/tests/harness/npm-cases.ts index 6dc8e81470..abfb60bbb5 100644 --- a/tests/harness/npm-cases.ts +++ b/tests/harness/npm-cases.ts @@ -19,14 +19,14 @@ export function npmCases(fixturesRoot: string): NpmCase[] { return [ ...globSync(join(fixturesRoot, "npm/cases/*/main.ts")) .sort() - // 2465-2469, 2556-2567, 2571 and 2580-2596 are the --npm-static bundler-emitted-CJS + // 2465-2469, 2556-2567, 2571 and 2580-2599 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-7]|2571|258[0-9]|259[0-6])-[^/]+\/main\.ts$/.test(entry), + !/\/(246[5-9]|255[6-9]|256[0-7]|2571|258[0-9]|259[0-9])-[^/]+\/main\.ts$/.test(entry), ) .map((entry) => ({ name: entry.split("/").at(-2)!, diff --git a/tests/harness/npm-static.test.ts b/tests/harness/npm-static.test.ts index 1c12d184c8..ccfb4cfa90 100644 --- a/tests/harness/npm-static.test.ts +++ b/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-2567, 2571, 2580-2596) + // the bundler-emitted-CJS mini packages (cases 2465-2469, 2556-2567, 2571, 2580-2599) ...globSync(join(fixturesRoot, "npm/node_modules/gt*/**/*.{js,cjs,json}")).sort(), ...globSync(join(fixturesRoot, "npm/store/gt*/**/*.{js,cjs,json}")).sort(), ]; @@ -1673,6 +1673,13 @@ describe(`npm-static pilots${sanitize ? " (sanitized)" : ""}`, () => { ]); }, 180_000); + // 2599: execa 5's export shape behind destructuring requires. + test("execa's export shape 2599-execa-shape byte-matches Node", async () => { + await expectNodeParity(join(fixturesRoot, "npm/cases/2599-execa-shape/main.ts"), [ + "gtexecashape", + ]); + }, 180_000); + // 2562: a file with unverifiable interop that no compiled code can load // (the program imports a leaf data module and only a type from the // main entry) leaves its package static. From 9ddbd94fee9eb0e167862fc0af41cf5a8e0cce3f Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Tue, 20 Oct 2026 21:18:19 -0500 Subject: [PATCH 21/22] Keep `this` in exported functions that another export detaches or that shadow exports - A sibling read as a value through `this.f` inside the module (`var g = this.b; g()`) now counts as a detached use, as `exports.f` already did, so `b` keeps its `this`. - An exported function that declares its own `exports` binding (a parameter, a `var`, an arrow parameter or its own name) keeps its text, and so does a module that declares `exports` at module scope: the respelled reads would name that binding instead of the exports object. --- .../frontend/npm-static-exports-this.test.ts | 48 ++++++++++++++++ .../src/frontend/npm-static-exports-this.ts | 57 +++++++++++++++++-- 2 files changed, 99 insertions(+), 6 deletions(-) diff --git a/packages/compiler/src/frontend/npm-static-exports-this.test.ts b/packages/compiler/src/frontend/npm-static-exports-this.test.ts index 8db75b7bbb..95d61a3cfa 100644 --- a/packages/compiler/src/frontend/npm-static-exports-this.test.ts +++ b/packages/compiler/src/frontend/npm-static-exports-this.test.ts @@ -66,3 +66,51 @@ test("other uses of `this`, replaced exports and detached functions keep their t ), ).toBeNull(); }); + +test("a function another export detaches through `this` keeps its text", () => { + // `a` reads `this.b` as a value and calls it without a receiver, so `b` + // runs with the global `this`, where `c` is missing. + const out = expectSameBehavior( + `exports.a = function () { + var g = this.b; + try { return g(); } catch (e) { return e.constructor.name; } +}; +exports.b = function () { return this.c(); }; +exports.c = function () { return "c"; }; +`, + `const lib = require("LIB"); console.log(lib.a(), lib.b());`, + ); + expect(out).toContain("var g = exports.b;"); + expect(out).toContain("return this.c();"); +}); + +test("a function that declares its own `exports` binding keeps its text", () => { + // A parameter, a var, an arrow parameter, the function name and a catch + // binding each shadow the module's exports object. + for (const fn of [ + `function (exports) { return this.g(); }`, + `function () { var exports = { g: () => "local" }; return this.g(); }`, + `function () { return [1].map((exports) => this.g())[0]; }`, + `function exports() { return this.g(); }`, + `function () { try { throw 0; } catch (exports) { return this.g(); } }`, + ]) { + expect( + rewrite(`exports.f = ${fn};\nexports.g = function () { return "g"; };\n`), + fn, + ).toBeNull(); + } + // A module-scope binding named exports is not the exports object. + expect( + rewrite( + `var exports = module.exports;\nexports.f = function () { return this.g(); };\nexports.g = function () { return "g"; };\n`, + ), + ).toBeNull(); + // The shadowed shapes run as Node runs them. + expectSameBehavior( + `exports.f = function (exports) { return this.g(); }; +exports.g = function () { return "g"; }; +exports.h = function () { return this.g(); }; +`, + `const lib = require("LIB"); console.log(lib.f({ g: () => "shadow" }), lib.h());`, + ); +}); diff --git a/packages/compiler/src/frontend/npm-static-exports-this.ts b/packages/compiler/src/frontend/npm-static-exports-this.ts index c275ef53da..fc547fbb44 100644 --- a/packages/compiler/src/frontend/npm-static-exports-this.ts +++ b/packages/compiler/src/frontend/npm-static-exports-this.ts @@ -5,10 +5,11 @@ * a static table with no runtime object to pass. When every `this` in such * a function names another export the module writes (`this.X` where the * module has `exports.X = …`), the exports object is never replaced, and - * the module itself calls the function only through `exports.f(…)`, `this.X` - * reads `exports.X`: the same member Node reads for those calls. Only a - * caller that detaches the function or rebinds its `this` would observe - * the difference. */ + * the module itself calls the function only through `exports.f(…)` or + * `this.f(…)`, `this.X` reads `exports.X`: the same member Node reads for + * those calls. A function that declares its own `exports` binding keeps + * its `this`. Only a caller outside the module that detaches the function + * or rebinds its `this` would observe the difference. */ import * as ts from "./ts7/syntax.js"; import { applyEdits, unparen, type Edit } from "./npm-static-bundled-chunks.js"; @@ -17,6 +18,33 @@ export function mayHoldExportsThis(source: string): boolean { return source.includes("this.") && source.includes("exports."); } +/** Whether `id` names the binding a declaration introduces. */ +function isDeclaredName(id: ts.Identifier): boolean { + const parent = id.parent; + if (parent === undefined) return false; + if (ts.isBindingElement(parent)) return parent.name === id; + return ( + (ts.isVariableDeclaration(parent) || + ts.isParameterDeclaration(parent) || + ts.isFunctionDeclaration(parent) || + ts.isFunctionExpression(parent) || + ts.isClassDeclaration(parent) || + ts.isClassExpression(parent)) && + parent.name === id + ); +} + +/** Whether `root` declares a binding named `exports` (at any depth). */ +function declaresExports(root: ts.Node): boolean { + let found = false; + ts.walkPreorder(root, (n) => { + if (found) return "stop"; + if (ts.isIdentifier(n) && n.text === "exports" && isDeclaredName(n)) found = true; + return undefined; + }); + return found; +} + function exportsMember(e: ts.Expression): string | null { const u = unparen(e); if (!ts.isPropertyAccessExpression(u) || !ts.isIdentifier(u.name)) return null; @@ -73,6 +101,19 @@ export function rewriteExportsThis(sf: ts.SourceFile): string | null { return undefined; }); if (replaced) return null; + // A module-scope `exports` binding is not the module's exports object. + if ( + sf.statements.nodes.some( + (stmt) => + (ts.isVariableStatement(stmt) && + stmt.declarationList.declarations.nodes.some( + (d) => ts.isIdentifier(d.name) && d.name.text === "exports", + )) || + ((ts.isFunctionDeclaration(stmt) || ts.isClassDeclaration(stmt)) && + stmt.name?.text === "exports"), + ) + ) + return null; const edits: Edit[] = []; for (const [name, fn] of functions) { // `this` uses directly in the body (arrow functions share it). @@ -101,13 +142,17 @@ export function rewriteExportsThis(sf: ts.SourceFile): string | null { }; fn.forEachChild(walk); if (!ok || uses.length === 0) continue; - // Inside the module the function is only called through the exports. + // `exports` in the respelled reads must name the module's object. + if (declaresExports(fn)) continue; + // Inside the module the function is only called through the exports, + // or through `this` in an exported function (respelled `exports`). let detached = false; ts.walkPreorder(sf, (n) => { if (detached) return "stop"; if (!ts.isPropertyAccessExpression(n) || !ts.isIdentifier(n.name) || n.name.text !== name) return undefined; - if (exportsMember(n) !== name) return undefined; + if (exportsMember(n) !== name && unparen(n.expression).kind !== ts.SyntaxKind.ThisKeyword) + return undefined; const parent = n.parent; const isStore = parent !== undefined && From ac058614d6bbd5a05b1f4da212615705d932ee59 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Wed, 21 Oct 2026 01:47:30 -0500 Subject: [PATCH 22/22] Count a function export handed to other code as a write of every member A use of a function whole export other than a member access, a call or `typeof` hands it to code that can write any member (`mutate(fmt)`), so every member becomes a runtime property of the function again. A module that re-exports the value whole (`module.exports = require("./lib")`) contributes its own requirers' writes instead. A member whose value has no dynamic form, such as a typed function, stays the export's static value, so `module.exports.sync = spawnSync` keeps loading (npm case 2596). --- .../frontend/lowering/cjs-exports-object.ts | 51 ++++++++++++++++--- .../6485-cjs-whole-export-members/writer.cjs | 7 ++- 2 files changed, 49 insertions(+), 9 deletions(-) diff --git a/packages/compiler/src/frontend/lowering/cjs-exports-object.ts b/packages/compiler/src/frontend/lowering/cjs-exports-object.ts index 1ab258ea82..8083eeca10 100644 --- a/packages/compiler/src/frontend/lowering/cjs-exports-object.ts +++ b/packages/compiler/src/frontend/lowering/cjs-exports-object.ts @@ -35,7 +35,7 @@ import { import { cjsWholeExportValueOf, inlineProgramRequireOf } from "./cjs-inline-require.js"; import { isCjsModuleGlobal } from "./lower-node-module.js"; import { bindingNeverReassigned } from "./binding-analysis.js"; -import { DYN, type IrExpr, type IrGlobal } from "../../ir/ir.js"; +import { DYN, canConvertToDyn, type IrExpr, type IrGlobal } from "../../ir/ir.js"; /** True for the CommonJS wrapper's `exports` binding (not shadowed by any * source declaration) in a CommonJS JS file. */ @@ -554,8 +554,7 @@ function requiredModuleOfMemberAccess( const dep = inlineProgramRequireOf(lowerer, declaration.initializer)?.dep ?? null; if (dep === null || !wholeExportIsFunction(lowerer, dep)) return null; // A member program code may write on the value is a runtime slot. - const writes = wholeExportWrites(lowerer, dep); - return writes === "any" || writes.has(access.name.text) ? null : dep; + return wholeExportMemberIsSlot(lowerer, dep, access.name.text) ? null : dep; } /** The function value of `sf`'s whole export (`module.exports = format`) @@ -567,22 +566,43 @@ export function cjsWholeExportSlotOwner( sf: ts.SourceFile, name: string, ): ts.Identifier | null { - if (!wholeExportIsFunction(lowerer, sf)) return null; - const writes = wholeExportWrites(lowerer, sf); - if (writes !== "any" && !writes.has(name)) return null; + if (!wholeExportMemberIsSlot(lowerer, sf, name)) return null; const whole = cjsWholeExportValueOf(sf); return whole !== null && ts.isIdentifier(whole.rhs) ? whole.rhs : null; } +/** True when member `name` of `sf`'s function whole export is a runtime + * property of the function: program code may write it, and its value has + * a dynamic form. A member whose value has none (a typed function such as + * cross-spawn's `spawnSync`) stays the export's static value. */ +function wholeExportMemberIsSlot(lowerer: Lowerer, sf: ts.SourceFile, name: string): boolean { + if (!wholeExportIsFunction(lowerer, sf)) return false; + const writes = wholeExportWrites(lowerer, sf); + if (writes !== "any" && !writes.has(name)) return false; + const symbol = lowerer.cjsModuleExportSymbol(sf, name); + const g = symbol ? lowerer.globalsBySymbol.get(symbol) : undefined; + return ( + g !== undefined && + canConvertToDyn( + g.type, + (id) => lowerer.shapes.get(id), + (id) => lowerer.unions.get(id), + ) + ); +} + /** The member names program files may write on `dep`'s whole export value * through a require of it: assignments, updates and deletes of `fmt.name`, * `fmt["name"]` or `require("./lib").name`, and the keys builtins such as * `Object.assign(fmt, { name })` and `Object.defineProperty(fmt, "name", …)` - * write. "any" for a computed key. Writes by code the value is passed to - * (`mutate(fmt)`) are not seen. */ + * write, and those of a module that re-exports the value whole. "any" for + * a computed key and for any other use of the value, which hands it to code + * that can write any member (`mutate(fmt)`). */ function wholeExportWrites(lowerer: Lowerer, dep: ts.SourceFile): ReadonlySet | "any" { const cached = lowerer.cjsWholeExportWrites.get(dep); if (cached !== undefined) return cached; + // A re-export cycle reaching this module again counts every member. + lowerer.cjsWholeExportWrites.set(dep, "any"); const names = new Set(); let any = false; const use = (value: ts.Node): void => { @@ -605,6 +625,21 @@ function wholeExportWrites(lowerer: Lowerer, dep: ts.SourceFile): ReadonlySet