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 }`.
This commit is contained in:
Chris Tate
2026-10-20 03:13:40 -05:00
parent eb729399ad
commit 603c6720f9
2 changed files with 97 additions and 4 deletions
@@ -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 }");
});
@@ -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") {