From 66d2010388cb572fc5617c322555e2e8120f7eb5 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Tue, 20 Oct 2026 03:08:09 -0500 Subject: [PATCH 1/2] Remove lazy loaders together and keep their receiver rule Lazy loaders that share a var statement each removed their declarators on their own, and declarators of one statement removed separately leave overlapping edits, of which applyEdits keeps the first. A later loader in the statement (one using __toESM, say) stayed behind and the interop pass refused the package. Every removed declarator of the pass (the interop requires, the loaders and the unused export tables) now goes into one set, removed once. A call through a loader (`load().fn()`) respelled the callee as the export's live read, which drops the namespace receiver. Loader calls now take the rule namespace calls already use: the callee must be a top-level function that never reads `this`; other calls keep the package refused. --- .../frontend/npm-static-bundled-esm.test.ts | 32 +++++++++++++++ .../src/frontend/npm-static-bundled-esm.ts | 39 +++++++++++-------- 2 files changed, 55 insertions(+), 16 deletions(-) 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 1e2ba24a3..98decbb41 100644 --- a/packages/compiler/src/frontend/npm-static-bundled-esm.test.ts +++ b/packages/compiler/src/frontend/npm-static-bundled-esm.test.ts @@ -152,6 +152,38 @@ test("minified initializers sharing a var statement lower the same way", () => { ); }); +test("memoized lazy loaders sharing a var statement lower together", () => { + const lowered = expectSameBehavior(`${helpers}var a_exports = {}; +__export(a_exports, { v: () => v }); +var v, init_a = __esm({ "a.ts"() { console.log("a init"); v = 1; } }); +var b_exports = {}; +__export(b_exports, { default: () => w }); +var w, init_b = __esm({ "b.ts"() { console.log("b init"); w = 2; } }); +var nsA, loadA = () => nsA ??= (init_a(), __toCommonJS(a_exports)), nsB, loadB = () => nsB ??= __toESM((init_b(), __toCommonJS(b_exports))); +console.log(loadA().v, loadB().default, loadA().v); +`); + expect(lowered).not.toContain("loadA"); + expect(lowered).not.toContain("loadB"); +}); + +test("calls through a memoized lazy loader keep the receiver rule", () => { + const source = (use: string): string => `${helpers}var w_exports = {}; +__export(w_exports, { who: () => who, plain: () => plain }); +function who() { return this.__esModule; } +function plain(x) { return "plain " + x; } +var init_w = __esm({ "w.ts"() {} }); +var ns, load = () => ns ??= (init_w(), __toCommonJS(w_exports)); +console.log(${use}); +`; + expect(lower(source("load().who()"))).toEqual({ + degrade: expect.stringContaining( + "calls only of exported functions that do not read 'this'", + ) as string, + }); + const lowered = expectSameBehavior(source("load().plain(1), (load().plain)(2)")); + expect(lowered).not.toContain("load()"); +}); + test.each([ ["is asynchronous", `var init_a = __esm({ async "a.ts"() { await 0; } });\ninit_a();`], [ diff --git a/packages/compiler/src/frontend/npm-static-bundled-esm.ts b/packages/compiler/src/frontend/npm-static-bundled-esm.ts index 6e9c4f315..9c77128e1 100644 --- a/packages/compiler/src/frontend/npm-static-bundled-esm.ts +++ b/packages/compiler/src/frontend/npm-static-bundled-esm.ts @@ -449,7 +449,6 @@ export function lowerBundledEsmInits( } } } - edits.push(...removeDeclarators(sf, deadDeclarators)); /* Namespaces of lazily initialized modules read through members. */ let mainTarget: string | null = null; @@ -500,6 +499,17 @@ export function lowerBundledEsmInits( }; const getterText = (value: ts.Expression): string => applyEdits(source, value.getStart(sf), value.getEnd(), edits); + /** A call through the namespace (`ns.fn()`, `L().fn()`) passes the object + * the read came from as `this`; the lowered object differs from Node's + * module object (`__esModule`, property attributes), so it must stay + * unobservable: the callee is a top-level function that never reads + * `this`. */ + const receiverHidden = (values: ReadonlyMap, name: string): boolean => { + const value = values.get(name); + if (value === undefined || !ts.isIdentifier(value) || !stable(value.text)) return false; + const decl = top.get(value.text)?.node; + return decl !== undefined && ts.isFunctionDeclaration(decl) && !readsThis(decl); + }; /* Memoized lazy loaders of those namespaces, `L = () => M ??= NS` (a * bundle plugin's lazy re-exports): every `L().name` becomes the @@ -564,7 +574,7 @@ export function lowerBundledEsmInits( const table = tables.get(target); if (table === undefined) continue; const loaderWhy = { - degrade: `its memoized lazy loader '${name}' returns the namespace of a bundled ES module as a value, a module object the static rewrite cannot represent (only member reads of its result compile)`, + degrade: `its memoized lazy loader '${name}' returns the namespace of a bundled ES module as a value, a module object the static rewrite cannot represent (only member reads of its result compile, and calls only of exported functions that do not read 'this')`, }; const memo = memoize.left.text; const memoDecl = top.get(memo)?.node; @@ -612,6 +622,10 @@ export function lowerBundledEsmInits( const member = (access as ts.PropertyAccessExpression).name.text; const value = entries.get(member); if (value === undefined || shadowed(access, table.entries)) return loaderWhy; + // The rewrite reads the export without the namespace object, so a + // call through it (`L().fn()`) loses its receiver. + if (calledWithReceiver(access as ts.Expression) && !receiverHidden(entries, member)) + return loaderWhy; const init = initCall === null ? "" : `${initCall.getText(sf)}, `; edits.push({ start: access.getStart(sf), @@ -619,7 +633,7 @@ export function lowerBundledEsmInits( text: `(${init}${getterText(value)})`, }); } - edits.push(...removeDeclarators(sf, new Set([d, memoDecl]))); + deadDeclarators.add(d).add(memoDecl); loaderSpans.push({ start: d.getStart(sf), end: d.getEnd() }); lowered.add(target); } @@ -664,24 +678,13 @@ export function lowerBundledEsmInits( else break; } const values = new Map(table.entries); - // A call through the namespace (`ns.fn()`) passes the object the read - // came from as `this`; the lowered object differs from Node's module - // object (`__esModule`, property attributes), so it must stay - // unobservable: the callee is a top-level function that never reads - // `this`. - const receiverHidden = (name: string): boolean => { - const value = values.get(name); - if (value === undefined || !ts.isIdentifier(value) || !stable(value.text)) return false; - const decl = top.get(value.text)?.node; - return decl !== undefined && ts.isFunctionDeclaration(decl) && !readsThis(decl); - }; 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(access.name.text)); + (!calledWithReceiver(access) || receiverHidden(values, access.name.text)); 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). @@ -792,8 +795,12 @@ export function lowerBundledEsmInits( ) return { degrade: ESM_SHAPE_DEGRADE }; edits.push({ start: table.stmt.getStart(sf), end: table.stmt.getEnd(), text: "" }); - edits.push(...removeDeclarators(sf, new Set([decl]))); + deadDeclarators.add(decl); } + // Every removed declarator in one pass: declarators of one statement + // removed separately would leave overlapping edits, and applyEdits keeps + // only the first of them. + edits.push(...removeDeclarators(sf, deadDeclarators)); /* The run-once closures. */ for (const init of inits.values()) { From 603c6720f9f7dd8e155bbf3e3bbfd62d848da059 Mon Sep 17 00:00:00 2001 From: Chris Tate Date: Tue, 20 Oct 2026 03:13:40 -0500 Subject: [PATCH 2/2] 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") {