From 9d3365c99b91edce2df02423919673edfee5bc62 Mon Sep 17 00:00:00 2001 From: Malte Ubl Date: Sat, 24 Oct 2026 13:14:39 -0700 Subject: [PATCH] Compile namespace-style npm packages: guarded declarations and var exports Two refusals kept pre-module JavaScript packages from compiling: - `var ns = ns || {}` (and `?? {}`): the read in the variable's own initializer made the checker type the namespace `any`, and requirers then failed to reach the binding (SC1030). In a module, the read sees the hoisted `undefined` whenever nothing else writes the variable, so a new package-source normalization respells the declaration as its default (`var ns = ({})`) when no other declaration, assignment, update, function, class, parameter or local of that name exists in the file and no earlier top-level statement mentions it. - `module.exports = { ns }` with a `var` binding was refused as exporting a mutable binding by reference. A `var` (or `let`) whose initialized declaration precedes the export statement at top level and that nothing reassigns exports exactly the value Node copies, so it now aliases like a const. The namespace benchmark (results/design-shape-inference/nsbench) now compiles and matches Node; the new npm case legacy-namespaces pins a namespace library with guarded declarations, constructor functions with prototype methods, enums, a revealing module and a handler registry. Co-Authored-By: Claude Opus 5.5 --- .../src/frontend/lowering/lower-stmts.ts | 20 ++- .../npm-static-namespace-defaults.test.ts | 63 ++++++++ .../frontend/npm-static-namespace-defaults.ts | 138 ++++++++++++++++++ packages/compiler/src/frontend/npm-static.ts | 5 + packages/compiler/src/frontend/services.ts | 11 ++ .../npm/cases/legacy-namespaces/main.ts.json | 6 + .../npm/cases/legacy-namespaces/main.ts | 26 ++++ .../node_modules/legacy-ns/index.d.ts | 2 + .../node_modules/legacy-ns/index.js | 97 ++++++++++++ .../node_modules/legacy-ns/package.json | 1 + .../npm/cases/legacy-namespaces/package.json | 1 + 11 files changed, 368 insertions(+), 2 deletions(-) create mode 100644 packages/compiler/src/frontend/npm-static-namespace-defaults.test.ts create mode 100644 packages/compiler/src/frontend/npm-static-namespace-defaults.ts create mode 100644 packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/legacy-namespaces/main.ts.json create mode 100644 tests/fixtures/npm/cases/legacy-namespaces/main.ts create mode 100644 tests/fixtures/npm/cases/legacy-namespaces/node_modules/legacy-ns/index.d.ts create mode 100644 tests/fixtures/npm/cases/legacy-namespaces/node_modules/legacy-ns/index.js create mode 100644 tests/fixtures/npm/cases/legacy-namespaces/node_modules/legacy-ns/package.json create mode 100644 tests/fixtures/npm/cases/legacy-namespaces/package.json diff --git a/packages/compiler/src/frontend/lowering/lower-stmts.ts b/packages/compiler/src/frontend/lowering/lower-stmts.ts index c50676863..851cae4c9 100644 --- a/packages/compiler/src/frontend/lowering/lower-stmts.ts +++ b/packages/compiler/src/frontend/lowering/lower-stmts.ts @@ -7445,7 +7445,7 @@ function lowerCjsExportStatement( 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 && !(sym && settledBeforeExport(lowerer, sym, cjs.expr))) { lowerer.unsupported( "SC1090", nameNode, @@ -7605,7 +7605,7 @@ function lowerCjsSingleValueExport( } const g = lowerer.globalsBySymbol.get(sym); if (g) { - if (g.mutable) { + if (g.mutable && !settledBeforeExport(lowerer, sym, rhs)) { lowerer.unsupported( "SC1090", rhs, @@ -12707,3 +12707,19 @@ function isAssertionInitializer(node: ts.Expression): boolean { while (ts.isParenthesizedExpression(e)) e = e.expression; return ts.isAsExpression(e) || ts.isTypeAssertion(e); } + +/** A `var` (or `let`) whose initialized declaration precedes the export + * statement in the module's top-level statements and that nothing ever + * reassigns: exporting it by reference reads the value Node copies. The + * legacy `var ns = ns || {}; …; module.exports = { ns }` shape. */ +function settledBeforeExport(lowerer: Lowerer, sym: ts.Symbol, exportNode: ts.Node): boolean { + const decl = sym.valueDeclaration; + if (!decl || !ts.isVariableDeclaration(decl) || decl.initializer === undefined) return false; + const stmt = decl.parent?.parent; + if (!stmt || !ts.isVariableStatement(stmt) || !ts.isSourceFile(stmt.parent)) return false; + let exportStmt: ts.Node = exportNode; + while (exportStmt.parent !== undefined && !ts.isSourceFile(exportStmt.parent)) + exportStmt = exportStmt.parent; + if (exportStmt.parent !== stmt.parent || stmt.end > exportStmt.pos) return false; + return bindingNeverReassigned(lowerer, sym, decl); +} diff --git a/packages/compiler/src/frontend/npm-static-namespace-defaults.test.ts b/packages/compiler/src/frontend/npm-static-namespace-defaults.test.ts new file mode 100644 index 000000000..93a776f7e --- /dev/null +++ b/packages/compiler/src/frontend/npm-static-namespace-defaults.test.ts @@ -0,0 +1,63 @@ +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().namespaceDefaults(source, "/pkg/index.js"); +} + +/** Node prints the same for the original and the rewritten module. */ +function expectSameOutput(source: string): void { + const result = rewrite(source); + expect(typeof result).toBe("string"); + const dir = mkdtempSync(join(tmpdir(), "scriptc-namespace-defaults-")); + dirs.push(dir); + const outputs = [source, result!].map((text, i) => { + const file = join(dir, `m${i}.cjs`); + writeFileSync(file, text); + const r = spawnSync(process.execPath, [file], { encoding: "utf8" }); + expect(r.status).toBe(0); + return r.stdout; + }); + expect(outputs[1]).toBe(outputs[0]); +} + +test("a guarded namespace declaration becomes its default", () => { + const source = `var app = app || {}; +app.util = app.util || {}; +app.util.twice = function (x) { return 2 * x; }; +var cfg = cfg ?? { depth: 1 }; +console.log(app.util.twice(3), cfg.depth, Object.keys(app).join(",")); +`; + expect(rewrite(source)).toBe(`var app = {}; +app.util = app.util || {}; +app.util.twice = function (x) { return 2 * x; }; +var cfg = { depth: 1 }; +console.log(app.util.twice(3), cfg.depth, Object.keys(app).join(",")); +`); + expectSameOutput(source); +}); + +test("any other write, declaration or earlier mention keeps the text", () => { + // Redeclared: the second declaration reads the first one's object. + expect(rewrite("var ns = ns || {};\nvar ns = ns || {};\n")).toBeNull(); + // Assigned elsewhere, updated, or declared as a function. + expect(rewrite("var ns = ns || {};\nfunction reset() { ns = {}; }\n")).toBeNull(); + expect(rewrite("var n = n || 0;\nn++;\n")).toBeNull(); + // A local of the same name somewhere in the file. + expect(rewrite("var ns = ns || {};\nfunction f(ns) { return ns; }\n")).toBeNull(); + // Mentioned by an earlier statement. + expect(rewrite("console.log(typeof ns);\nvar ns = ns || {};\n")).toBeNull(); + // let and const read their own binding in its temporal dead zone. + expect(rewrite("let ns = ns || {};\n")).toBeNull(); + // Not the declared name on the left. + expect(rewrite("var ns = other || {};\n")).toBeNull(); +}); diff --git a/packages/compiler/src/frontend/npm-static-namespace-defaults.ts b/packages/compiler/src/frontend/npm-static-namespace-defaults.ts new file mode 100644 index 000000000..2e4bf046c --- /dev/null +++ b/packages/compiler/src/frontend/npm-static-namespace-defaults.ts @@ -0,0 +1,138 @@ +/* The guarded namespace declaration of pre-module JavaScript: + * + * var ns = ns || {}; + * + * In a module (CommonJS or ES), `var` is module-local and hoisted, so the + * read in its own initializer sees `undefined` unless something writes the + * variable first. When nothing else in the file ever writes it (no other + * declaration, assignment or update of that name anywhere, no function or + * class of that name) and no earlier top-level statement mentions it, the + * initializer is its right operand: `var ns = {};`. Inference then types + * the namespace from that literal and the members later statements add, + * instead of `any` (the self-reference makes the checker give up). `??` + * is the same. Anything else keeps the original text. */ +import * as ts from "./ts7/syntax.js"; +import { tokenStartOfNode } from "./ts7/node-functions.js"; +import { applyEdits, unparen, type Edit } from "./npm-static-bundled-chunks.js"; + +/** A cheap probe: some `var X = X ||` or `?? ` may be present. */ +export function mayHoldNamespaceDefault(source: string): boolean { + return /\bvar\s+([A-Za-z_$][\w$]*)\s*=\s*\(?\s*\1\s*(\|\||\?\?)/.test(source); +} + +/** The module text with each guarded namespace declaration respelled as a + * plain one, or null when there is none. */ +export function rewriteNamespaceDefaults(sf: ts.SourceFile): string | null { + const stmts = sf.statements.nodes; + const edits: Edit[] = []; + for (let i = 0; i < stmts.length; i++) { + const stmt = stmts[i]!; + if (!ts.isVariableStatement(stmt)) continue; + const list = stmt.declarationList; + if ((list.flags & (ts.NodeFlags.Let | ts.NodeFlags.Const)) !== 0) continue; + for (const decl of list.declarations.nodes) { + if (!ts.isIdentifier(decl.name) || decl.initializer === undefined) continue; + const name = decl.name.text; + const init = unparen(decl.initializer); + if ( + !ts.isBinaryExpression(init) || + (init.operatorToken.kind !== ts.SyntaxKind.BarBarToken && + init.operatorToken.kind !== ts.SyntaxKind.QuestionQuestionToken) + ) + continue; + const left = unparen(init.left); + if (!ts.isIdentifier(left) || left.text !== name) continue; + if (mentionedBefore(stmts, i, name) || writtenElsewhere(sf, name, decl)) continue; + const right = init.right; + edits.push({ + start: tokenStartOfNode(decl.initializer), + end: decl.initializer.end, + // A literal stays bare: inference types `var ns = {}` from the + // members later statements add (a parenthesized one it does not). + text: + ts.isObjectLiteralExpression(unparen(right)) || + ts.isArrayLiteralExpression(unparen(right)) + ? sf.text.slice(tokenStartOfNode(right), right.end) + : `(${sf.text.slice(tokenStartOfNode(right), right.end)})`, + }); + } + } + if (edits.length === 0) return null; + return applyEdits(sf.text, 0, sf.text.length, edits); +} + +/** Whether a top-level statement before index `at` mentions `name`. */ +function mentionedBefore(stmts: readonly ts.Statement[], at: number, name: string): boolean { + for (let i = 0; i < at; i++) { + let found = false; + ts.walkPreorder(stmts[i]!, (n) => { + if (found) return "stop"; + if (ts.isIdentifier(n) && n.text === name) found = true; + return undefined; + }); + if (found) return true; + } + return false; +} + +/** Whether anything other than `decl` declares or writes `name`. */ +function writtenElsewhere(sf: ts.SourceFile, name: string, decl: ts.VariableDeclaration): boolean { + let written = false; + ts.walkPreorder(sf, (n) => { + if (written) return "stop"; + if (n === decl.name) return undefined; + if (ts.isBinaryExpression(n) && isAssignmentOperator(n.operatorToken.kind)) { + if (isName(n.left, name)) written = true; + } else if ( + (ts.isPrefixUnaryExpression(n) || ts.isPostfixUnaryExpression(n)) && + (n.operator === ts.SyntaxKind.PlusPlusToken || + n.operator === ts.SyntaxKind.MinusMinusToken) && + isName(n.operand, name) + ) { + written = true; + } else if ( + (ts.isFunctionDeclaration(n) || ts.isClassDeclaration(n)) && + n.name !== undefined && + n.name.text === name + ) { + written = true; + } else if ( + (ts.isVariableDeclaration(n) || ts.isParameter(n) || ts.isBindingElement(n)) && + n !== decl && + n.name !== undefined && + bindsName(n.name, name) + ) { + // Another declaration of the name (a redeclaration, or a local that + // shadows it) — keep the original text rather than reason about it. + written = true; + } else if ( + (ts.isForInStatement(n) || ts.isForOfStatement(n)) && + !ts.isVariableDeclarationList(n.initializer) && + isName(n.initializer, name) + ) { + written = true; + } + return undefined; + }); + return written; +} + +function bindsName(binding: ts.Node, name: string): boolean { + if (ts.isIdentifier(binding)) return binding.text === name; + let found = false; + ts.walkPreorder(binding, (n) => { + if (found) return "stop"; + if (ts.isIdentifier(n) && n.text === name) found = true; + return undefined; + }); + return found; +} + +function isName(node: ts.Node, name: string): boolean { + const n = unparen(node as ts.Expression); + return ts.isIdentifier(n) && n.text === name; +} + +function isAssignmentOperator(kind: ts.SyntaxKind): boolean { + return kind >= ts.SyntaxKind.FirstAssignment && kind <= ts.SyntaxKind.LastAssignment; +} diff --git a/packages/compiler/src/frontend/npm-static.ts b/packages/compiler/src/frontend/npm-static.ts index b00e14aa0..f51c6bbe2 100644 --- a/packages/compiler/src/frontend/npm-static.ts +++ b/packages/compiler/src/frontend/npm-static.ts @@ -505,6 +505,10 @@ export function npmStaticFsShadow(services: FrontendServices): NpmStaticFsShadow // literals they build. const enums = original === null ? null : services.enumIifes(original, path); if (enums !== null) original = enums; + // Guarded namespace declarations (`var ns = ns || {}`) read the + // hoisted `undefined`: the declaration is its default. + const namespaces = original === null ? null : services.namespaceDefaults(original, path); + if (namespaces !== null) original = namespaces; // A module that names its export object through a variable reaches // its members through `exports`. const exportsVar = original === null ? null : services.cjsExportsVar(original, path); @@ -517,6 +521,7 @@ export function npmStaticFsShadow(services: FrontendServices): NpmStaticFsShadow aliased !== null || fnExports !== null || enums !== null || + namespaces !== null || exportsVar !== null; if (original !== null) { const namepaths = services.jsDocNamepaths(path, original); diff --git a/packages/compiler/src/frontend/services.ts b/packages/compiler/src/frontend/services.ts index 14b366bfa..11fa0de72 100644 --- a/packages/compiler/src/frontend/services.ts +++ b/packages/compiler/src/frontend/services.ts @@ -14,6 +14,10 @@ import { } from "./npm-static-builtin-aliases.js"; import { mayHoldUmdWrapper, unwrapUmdFactory } from "./npm-static-umd.js"; import { mayHoldEnumIife, rewriteEnumIifes } from "./npm-static-enum-iife.js"; +import { + mayHoldNamespaceDefault, + rewriteNamespaceDefaults, +} from "./npm-static-namespace-defaults.js"; import { mayHoldTscBuiltinImport, unwrapTscBuiltinImports } from "./npm-static-tsc-helpers.js"; import { builtinPackageAliasFor } from "./package-shims.js"; import * as syntax from "./npm-static-declaration-syntax.js"; @@ -132,6 +136,13 @@ export class FrontendServices { if (!mayHoldEnumIife(source)) return null; return rewriteEnumIifes(this.parse(path, source, "js")); } + /** A package module with its guarded namespace declarations + * (`var ns = ns || {}`) respelled as plain ones (null: none). */ + namespaceDefaults(source: string, path: string): string | null { + this.ensureOpen(); + if (!mayHoldNamespaceDefault(source)) return null; + return rewriteNamespaceDefaults(this.parse(path, source, "js")); + } /** A package module with its requires of builtin-aliased polyfill * packages respelled as the builtins (null: none). */ builtinAliasRequires(source: string, path: string): string | null { diff --git a/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/legacy-namespaces/main.ts.json b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/legacy-namespaces/main.ts.json new file mode 100644 index 000000000..7f50545d6 --- /dev/null +++ b/packages/compiler/test/ts7/baselines/order-parity/tests/fixtures/npm/cases/legacy-namespaces/main.ts.json @@ -0,0 +1,6 @@ +{ + "order": [ + "/tests/fixtures/npm/cases/legacy-namespaces/main.ts" + ], + "diags": [] +} diff --git a/tests/fixtures/npm/cases/legacy-namespaces/main.ts b/tests/fixtures/npm/cases/legacy-namespaces/main.ts new file mode 100644 index 000000000..2f9483ba9 --- /dev/null +++ b/tests/fixtures/npm/cases/legacy-namespaces/main.ts @@ -0,0 +1,26 @@ +// A namespace-style library in the pre-module idiom (guarded namespace +// declarations, constructor functions with prototype methods, enums as +// objects, a revealing module, a handler registry, exported through +// `module.exports = { app }`), used from TypeScript. +import { app } from "legacy-ns"; + +const v = new app.geom.Vec(3, 4); +console.log(v.length(), v.add(new app.geom.Vec(1, 1)).scale(2).x, v instanceof app.geom.Vec); +console.log(Object.keys(app).join(","), Object.keys(app.math).join(","), Object.keys(app.geom.Kind).join(",")); +console.log(app.math.clamp(12, 0, 10), app.math.lerp(0, 10, 0.25), app.text.pad(7, 3), app.text.label(1, 42)); + +const shapes = []; +for (let i = 0; i < 12; i++) { + const center = new app.geom.Vec(app.math.lerp(-5, 5, (i % 4) / 3), app.math.lerp(-5, 5, Math.floor(i / 4) / 2)); + shapes.push(new app.geom.Shape(i % 3, center, 1 + (i % 4))); +} +let area = 0; +let hits = 0; +for (let step = 0; step < 200; step++) { + const probe = new app.geom.Vec(app.math.clamp(Math.sin(step) * 7, -5, 5), app.math.clamp(Math.cos(step * 0.7) * 7, -5, 5)); + for (const s of shapes) { + area += app.registry.run("area", s, probe); + hits += app.registry.run("hits", s, probe); + } +} +console.log(Math.round(area), hits, shapes[4].contains(shapes[4].center), shapes[0].area()); diff --git a/tests/fixtures/npm/cases/legacy-namespaces/node_modules/legacy-ns/index.d.ts b/tests/fixtures/npm/cases/legacy-namespaces/node_modules/legacy-ns/index.d.ts new file mode 100644 index 000000000..16c9fa62a --- /dev/null +++ b/tests/fixtures/npm/cases/legacy-namespaces/node_modules/legacy-ns/index.d.ts @@ -0,0 +1,2 @@ +declare const app: any; +export { app }; diff --git a/tests/fixtures/npm/cases/legacy-namespaces/node_modules/legacy-ns/index.js b/tests/fixtures/npm/cases/legacy-namespaces/node_modules/legacy-ns/index.js new file mode 100644 index 000000000..2efa926f5 --- /dev/null +++ b/tests/fixtures/npm/cases/legacy-namespaces/node_modules/legacy-ns/index.js @@ -0,0 +1,97 @@ +// A namespace-style geometry and text library in the pre-module idiom: +// guarded namespace declarations, constructor functions with prototype +// methods, enums as objects, a revealing module, and a static registry. +var app = app || {}; +app.math = app.math || {}; +app.geom = app.geom || {}; + +app.math.EPSILON = 1e-9; + +app.math.clamp = function (value, lo, hi) { + return value < lo ? lo : value > hi ? hi : value; +}; + +app.math.lerp = function (a, b, t) { + return a + (b - a) * t; +}; + +app.math.hypot = function (x, y) { + return Math.sqrt(x * x + y * y); +}; + +app.geom.Kind = { POINT: 0, CIRCLE: 1, RECT: 2 }; + +/** @constructor */ +app.geom.Vec = function (x, y) { + this.x = x; + this.y = y; +}; + +app.geom.Vec.prototype.add = function (other) { + return new app.geom.Vec(this.x + other.x, this.y + other.y); +}; + +app.geom.Vec.prototype.scale = function (k) { + return new app.geom.Vec(this.x * k, this.y * k); +}; + +app.geom.Vec.prototype.length = function () { + return app.math.hypot(this.x, this.y); +}; + +/** @constructor */ +app.geom.Shape = function (kind, center, size) { + this.kind = kind; + this.center = center; + this.size = size; +}; + +app.geom.Shape.prototype.area = function () { + switch (this.kind) { + case app.geom.Kind.CIRCLE: + return Math.PI * this.size * this.size; + case app.geom.Kind.RECT: + return this.size * this.size; + default: + return 0; + } +}; + +app.geom.Shape.prototype.contains = function (p) { + var d = p.add(this.center.scale(-1)).length(); + return d <= this.size + app.math.EPSILON; +}; + +app.text = (function () { + var digits = "0123456789"; + function pad(n, width) { + var s = ""; + var v = n; + do { + s = digits.charAt(v % 10) + s; + v = Math.floor(v / 10); + } while (v > 0); + while (s.length < width) s = "0" + s; + return s; + } + function label(kind, n) { + return (kind === app.geom.Kind.CIRCLE ? "c" : kind === app.geom.Kind.RECT ? "r" : "p") + pad(n, 4); + } + return { pad: pad, label: label }; +})(); + +app.registry = { + handlers: { + area: function (shape) { + return shape.area(); + }, + hits: function (shape, probe) { + return shape.contains(probe) ? 1 : 0; + }, + }, + run: function (name, shape, probe) { + return app.registry.handlers[name](shape, probe); + }, +}; + +module.exports = { app }; diff --git a/tests/fixtures/npm/cases/legacy-namespaces/node_modules/legacy-ns/package.json b/tests/fixtures/npm/cases/legacy-namespaces/node_modules/legacy-ns/package.json new file mode 100644 index 000000000..cde18c613 --- /dev/null +++ b/tests/fixtures/npm/cases/legacy-namespaces/node_modules/legacy-ns/package.json @@ -0,0 +1 @@ +{ "name": "legacy-ns", "version": "1.0.0", "main": "index.js", "types": "index.d.ts" } diff --git a/tests/fixtures/npm/cases/legacy-namespaces/package.json b/tests/fixtures/npm/cases/legacy-namespaces/package.json new file mode 100644 index 000000000..140c08a1c --- /dev/null +++ b/tests/fixtures/npm/cases/legacy-namespaces/package.json @@ -0,0 +1 @@ +{ "name": "legacy-namespaces-case", "private": true, "type": "module" }