From 603c6720f9f7dd8e155bbf3e3bbfd62d848da059 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Tue, 20 Oct 2026 03:13:40 -0500 Subject: [PATCH] Keep function export rewrites from capturing names or keys The pass respells `exports.x` and `module.exports.x` through the exported function's name. A parameter, local, nested function or catch binding of that name between a respelled read and the top level would capture it, so the pass now applies only when the file binds the name once, at its top-level declaration. `{ exports }` after the rebinding respelled to `{ SemVer }`, which renames the property; the shorthand now becomes `{ exports: SemVer }`. --- .../npm-static-cjs-exports-alias.test.ts | 81 +++++++++++++++++++ .../frontend/npm-static-cjs-exports-alias.ts | 20 ++++- 2 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 packages/compiler/src/frontend/npm-static-cjs-exports-alias.test.ts 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 new file mode 100644 index 000000000..c3704ea29 --- /dev/null +++ b/packages/compiler/src/frontend/npm-static-cjs-exports-alias.test.ts @@ -0,0 +1,81 @@ +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().cjsFunctionExports(source, "/pkg/index.js"); +} + +function run(file: string) { + const result = spawnSync(process.execPath, [file], { encoding: "utf8" }); + return { status: result.status, stdout: result.stdout, stderr: result.stderr }; +} + +/** 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-cjs-function-exports-")); + dirs.push(dir); + const original = join(dir, "original.cjs"); + const rewritten = join(dir, "rewritten.cjs"); + writeFileSync(original, source); + writeFileSync(rewritten, result as string); + const oracle = run(original); + expect(oracle.status).toBe(0); + expect(run(rewritten)).toEqual(oracle); + return result as string; +} + +const report = ` +console.log(typeof module.exports, module.exports === Version, Object.keys(Version).join(",")); +`; + +test("members of a function export object read and write the function", () => { + const rewritten = expectSameBehavior(`exports = module.exports = Version; +exports.parse = parse; +module.exports.major = (v) => parse(v)[0]; +function Version(v) { return { v }; } +function parse(v) { return v.split(".").map(Number); } +console.log(exports.major("2.1.0"), module.exports.parse("1.2").length); +${report}`); + expect(rewritten).toContain("Version.parse = parse"); + expect(rewritten).not.toContain("exports = module.exports"); +}); + +test.each([ + ["a parameter", "function inner(Version) { return exports.parse; }"], + ["a local", "function inner() { const Version = {}; return exports.parse; }"], + ["a nested function", "function inner() { function Version() {} return exports.parse; }"], + [ + "a catch binding", + "function inner() { try { throw 0; } catch (Version) { return exports.parse; } }", + ], +])("a nested binding of the function's name (%s) keeps the module as it is", (_, inner) => { + expect( + rewrite(`exports = module.exports = Version; +exports.parse = parse; +function Version() {} +function parse() {} +${inner} +`), + ).toBeNull(); +}); + +test("an `exports` shorthand property keeps its key", () => { + const rewritten = expectSameBehavior(`exports = module.exports = Version; +exports.tag = "v"; +function Version() {} +const holder = { exports }; +console.log(Object.keys(holder).join(","), holder.exports === Version, holder.exports.tag); +${report}`); + expect(rewritten).toContain("{ exports: Version }"); +}); 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 c88c9949e..dfe45b862 100644 --- a/packages/compiler/src/frontend/npm-static-cjs-exports-alias.ts +++ b/packages/compiler/src/frontend/npm-static-cjs-exports-alias.ts @@ -15,12 +15,14 @@ * runs and nothing replaces the export object again: the statement is a * top-level statement preceded only by inert declarations, it is the * file's only write of `module.exports` and `exports`, the function is a - * top-level declaration nothing reassigns, and `exports`/`module` are the - * CommonJS globals (never declared in the file). Other shapes answer + * top-level declaration nothing reassigns and the file's only binding of + * its name, and `exports`/`module` are the CommonJS globals (never + * declared in the file). Other shapes answer * null and keep their existing treatment. */ import * as ts from "./ts7/syntax.js"; import { applyEdits, + countDeclarations, declaredNames, isWritten, references, @@ -105,7 +107,10 @@ export function normalizeCjsFunctionExports(sf: ts.SourceFile): string | null { const fnDecls = stmts.filter( (st) => ts.isFunctionDeclaration(st) && st.name?.text === fnName && st.body !== undefined, ); - if (fnDecls.length !== 1) return null; + // The respelled reads name the function from wherever they sit, so no + // other binding of its name (a parameter, a local, a nested function) + // may exist anywhere in the file. + if (fnDecls.length !== 1 || countDeclarations(sf, fnName) !== 1) return null; const edits: Edit[] = []; let unsafe = false; @@ -131,7 +136,14 @@ export function normalizeCjsFunctionExports(sf: ts.SourceFile): string | null { unsafe = true; continue; } - edits.push({ start: r.getStart(sf), end: r.getEnd(), text: fnName }); + // `{ exports }` keeps its key: the shorthand spells out the value. + const shorthand = + r.parent !== undefined && ts.isShorthandPropertyAssignment(r.parent) && r.parent.name === r; + edits.push({ + start: r.getStart(sf), + end: r.getEnd(), + text: shorthand ? `exports: ${fnName}` : fnName, + }); continue; } if (r.text === "module") {