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 ea1c22ce5..caea3c366 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 f38d4117d..53564d787 100644 --- a/packages/compiler/src/frontend/npm-static-bundled-esm.ts +++ b/packages/compiler/src/frontend/npm-static-bundled-esm.ts @@ -480,7 +480,6 @@ export function lowerBundledEsmInits( edits.push(...h.edits); for (const d of h.declarators) deadDeclarators.add(d); } - edits.push(...removeDeclarators(sf, deadDeclarators)); /* Namespaces of lazily initialized modules read through members. */ let mainTarget: string | null = null; @@ -531,6 +530,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 @@ -595,7 +605,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; @@ -643,6 +653,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), @@ -650,7 +664,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); } @@ -695,24 +709,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). @@ -823,8 +826,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()) { 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") {