diff --git a/.gitignore b/.gitignore index b6fd60a996..e24a397a7c 100644 --- a/.gitignore +++ b/.gitignore @@ -35,6 +35,7 @@ __test_project__/ # Dependencies node_modules/ +node_modules # NPM config # Local npm configs may contain credentials. Track only the root policy file. diff --git a/docs/guides/data-fetching.md b/docs/guides/data-fetching.md index 078c084462..193d2eb48e 100644 --- a/docs/guides/data-fetching.md +++ b/docs/guides/data-fetching.md @@ -45,6 +45,29 @@ entirely, including their top-level side effects. Put client initialization in a separate client-referenced module or a bare side-effect import that is not only used by a server data hook. +Declare each data hook in the page module itself. Re-exporting one from another +file fails the browser build, with or without an alias: + +```tsx +// Not supported. Both forms stop the build. +export { getServerData } from "../server/loaders.ts"; +export { loadDashboard as getServerData } from "../server/loaders.ts"; +``` + +A re-export binds no local declaration, so Veryfront has nothing to strip and +the loader module, its imports, and anything it closes over would reach the +browser. Declare the hook in the page and import the helpers it needs: + +```tsx +// app/dashboard/page.tsx +import type { DataContext } from "veryfront"; +import { loadDashboard } from "../server/loaders.ts"; + +export async function getServerData(ctx: DataContext) { + return { props: await loadDashboard(ctx) }; +} +``` + The `props` you return are passed to the page component. To read the same props data from a layout or nested component without prop-drilling, use `usePageContext().data` (see diff --git a/src/transforms/pipeline/index.ts b/src/transforms/pipeline/index.ts index d76222c8c5..5a07757de8 100644 --- a/src/transforms/pipeline/index.ts +++ b/src/transforms/pipeline/index.ts @@ -68,9 +68,9 @@ const SSR_PIPELINE: TransformPlugin[] = [ const BROWSER_PIPELINE: TransformPlugin[] = [ parsePlugin, + browserServerExportsStripPlugin, // Drop server-only hooks + their now-unused imports compilePlugin, cssStripPlugin, // Strip CSS imports before they hit import resolution - browserServerExportsStripPlugin, // Drop server-only hooks + their now-unused imports browserNodeBuiltinImportsPlugin, // node:* named imports -> namespace + destructure resolveImportsPlugin, // Unified import resolution finalizePlugin, diff --git a/src/transforms/pipeline/stages/browser-server-exports-strip.test.ts b/src/transforms/pipeline/stages/browser-server-exports-strip.test.ts index 4912ead0d7..4eb37c105a 100644 --- a/src/transforms/pipeline/stages/browser-server-exports-strip.test.ts +++ b/src/transforms/pipeline/stages/browser-server-exports-strip.test.ts @@ -1,5 +1,6 @@ import "#veryfront/schemas/_test-setup.ts"; import "../../plugins/__tests__/code-parser-setup.ts"; +import "../../mdx/compiler/__tests__/content-processor-setup.ts"; import { stop as stopEsbuild } from "#veryfront/platform/compat/esbuild.ts"; import { assertEquals, assertRejects, assertStringIncludes } from "#veryfront/testing/assert.ts"; import { afterAll, describe, it } from "#veryfront/testing/bdd.ts"; @@ -10,7 +11,6 @@ import { moduleReferenceWalkers, stripServerOnlyExports, } from "./browser-server-exports-strip.ts"; -import { COMPILE_SOURCE_MAP_DIRECTIVE_METADATA, compilePlugin } from "./compile.ts"; import { runPipeline } from "../index.ts"; import { type TransformContext, TransformStage } from "../types.ts"; @@ -28,6 +28,38 @@ describe("browser-server-exports-strip", () => { await stopEsbuild(); }); + /** + * Both walkers' answers restricted to `names`, so a fixture asserts which + * module-level bindings each one attributes to runtime code without + * listing every local it also sees. + */ + async function referencesAmong( + code: string, + names: string[], + filePath = "page.tsx", + ): Promise<{ referenced: string[]; free: string[] }> { + const parser = tryResolve("CodeParser"); + if (!parser) throw new Error("no CodeParser extension is registered"); + const ast = await parser.parse({ code, filePath }); + const { referenced, free } = moduleReferenceWalkers(ast); + const pick = (found: Set) => names.filter((name) => found.has(name)); + return { referenced: pick(referenced), free: pick(free) }; + } + + /** + * The defect this classification fixes is the two walkers disagreeing, so + * every fixture asserts the same expectation against both. + */ + async function assertWalkers( + code: string, + names: string[], + expected: string[], + ): Promise { + const { referenced, free } = await referencesAmong(code, names); + assertEquals(referenced, expected, "referencedIdentifiers"); + assertEquals(free, expected, "freeReferencedIdentifiers"); + } + describe("emptying server-only hooks", () => { it("empties an exported async function declaration body", async () => { const code = [ @@ -132,6 +164,35 @@ describe("browser-server-exports-strip", () => { assertNotIncludes(result, "../lib/uses-crypto.js"); }); + it("empties a hook exported under an escaped string-literal alias", async () => { + const code = [ + `import { hashOf } from "../lib/uses-crypto.js";`, + `const API_KEY = "sk-live-example";`, + `function loadIt() { return hashOf(API_KEY); }`, + `export { loadIt as "get\\u0053erverData" };`, + ].join("\n"); + + const result = await stripServerOnlyExports(code); + + assertNotIncludes(result, "hashOf"); + assertNotIncludes(result, "sk-live-example"); + assertNotIncludes(result, "../lib/uses-crypto.js"); + }); + + it("empties a hook exported under a direct string-literal alias", async () => { + const code = [ + `import { hashOf } from "../lib/uses-crypto.js";`, + `function loadIt() { return hashOf("secret"); }`, + `export { loadIt as "getServerData" };`, + ].join("\n"); + + const result = await stripServerOnlyExports(code); + + assertNotIncludes(result, "hashOf"); + assertNotIncludes(result, "secret"); + assertNotIncludes(result, "../lib/uses-crypto.js"); + }); + it("empties a hook declared before a separate export clause", async () => { const code = [ `function getStaticData() { return readSecret(); }`, @@ -237,6 +298,49 @@ describe("browser-server-exports-strip", () => { assertStringIncludes((error as Error).message, "getServerData"); }); + it("fails the build when a hook aliases an import-equals binding", async () => { + // Reachable only since this pass moved ahead of compilePlugin. Running + // after compile, esbuild had lowered the alias to a `const` the variable + // path stubbed; running before it, the declaration is a + // TSImportEqualsDeclaration that `emptyServerOnlyHooks` cannot stub, so + // without this the source passed through untouched and shipped + // `require("./server")` -- and the server graph -- to the browser. + const code = `import Loader = require("./server");\nexport { Loader as getServerData };`; + + const error = await assertRejects(() => stripServerOnlyExports(code, "pages/x.tsx")); + + assertStringIncludes((error as Error).message, "Loader"); + assertStringIncludes((error as Error).message, "pages/x.tsx"); + }); + + it("fails the build when a hook is itself an exported import-equals", async () => { + const code = `export import getServerData = require("./server");`; + + const error = await assertRejects(() => stripServerOnlyExports(code, "pages/x.tsx")); + + assertStringIncludes((error as Error).message, "getServerData"); + }); + + it("still strips when an unrelated import-equals shares the module", async () => { + // The rejection is scoped to import-equals bindings a hook resolves to. + // An unrelated one must not fail a build the pass can handle. + const code = + `import Unrelated = require("./other");\nexport async function getServerData() { return { a: 1 }; }\nexport default function Page() { return Unrelated; }`; + + const stripped = await stripServerOnlyExports(code, "pages/x.tsx"); + + assertNotIncludes(stripped, "return { a: 1 }"); + assertStringIncludes(stripped, "Unrelated"); + }); + + it("fails the build when an escaped string-literal hook is namespace re-exported", async () => { + const code = `export * as "get\\u0053erverData" from "./loader.ts";`; + + const error = await assertRejects(() => stripServerOnlyExports(code, "pages/x.tsx")); + + assertStringIncludes((error as Error).message, "getServerData"); + }); + it("fails the build when a hook is exported from a destructuring pattern", async () => { const code = [ `import { loaders } from "./loaders.ts";`, @@ -286,7 +390,11 @@ describe("browser-server-exports-strip", () => { assertEquals(occurrences(result, "loadJob"), 0); }); - it("keeps an unrelated unused project import as a side-effect import", async () => { + // The pass owns nothing in this import, so it does not touch it. Reducing + // it to `import "./client-metrics.ts"` would convert an import the compiler + // can erase into one it must preserve. `compile` runs after this stage and + // erases it, which is what keeps the browser artifact free of the module. + it("leaves an unrelated unused project import exactly as authored", async () => { const code = [ `import { initClientMetrics } from "./client-metrics.ts";`, `export async function getServerData() { return { props: {} }; }`, @@ -295,8 +403,7 @@ describe("browser-server-exports-strip", () => { const result = await stripServerOnlyExports(code); - assertStringIncludes(result, `import "./client-metrics.ts"`); - assertEquals(occurrences(result, "initClientMetrics"), 0); + assertStringIncludes(result, `import { initClientMetrics } from "./client-metrics.ts"`); }); it("keeps an unrelated import when a hook-local binding shadows its name", async () => { @@ -311,8 +418,9 @@ describe("browser-server-exports-strip", () => { const result = await stripServerOnlyExports(code); - assertStringIncludes(result, `import "./client-metrics.ts"`); - assertEquals(occurrences(result, "initClientMetrics"), 0); + // The shadowing local does not make the import hook-owned, so the import + // is not deleted. Nothing the pass owns is in it, so it stays as authored. + assertStringIncludes(result, `import { initClientMetrics } from "./client-metrics.ts"`); }); it("removes a hook-only import even when a nested hook scope shadows its name", async () => { @@ -351,8 +459,7 @@ describe("browser-server-exports-strip", () => { const result = await stripServerOnlyExports(code); - assertStringIncludes(result, `import "./client-metrics.ts"`); - assertEquals(occurrences(result, "initClientMetrics"), 0); + assertStringIncludes(result, `import { initClientMetrics } from "./client-metrics.ts"`); assertEquals(occurrences(result, "makeData"), 0); }); @@ -418,8 +525,7 @@ describe("browser-server-exports-strip", () => { const result = await stripServerOnlyExports(code); - assertStringIncludes(result, `import "./client-metrics.ts"`); - assertEquals(occurrences(result, "initClientMetrics"), 0); + assertStringIncludes(result, `import { initClientMetrics } from "./client-metrics.ts"`); }); it("removes a hook-only import read after a for-loop block binding of the same name", async () => { @@ -470,8 +576,7 @@ describe("browser-server-exports-strip", () => { const result = await stripServerOnlyExports(code); - assertStringIncludes(result, `import "./client-init.ts"`); - assertEquals(occurrences(result, "ctx"), 0); + assertStringIncludes(result, `import { ctx } from "./client-init.ts"`); }); it("tracks a hook dependency inside TypeScript expression wrappers", async () => { @@ -789,6 +894,25 @@ describe("browser-server-exports-strip", () => { assertEquals(occurrences(result, "rest"), 0); }); + it("keeps partially hook-owned destructuring and pinned JSX factory names", async () => { + const code = [ + `/** @jsxRuntime classic */`, + `import { loadSecret, makeFactory } from "../lib/shared.ts";`, + `const { token, client } = loadSecret();`, + `const React = makeFactory();`, + `export async function getServerData() { return { props: { token, React } }; }`, + `export default function Page() { return
; }`, + ].join("\n"); + + const result = await stripServerOnlyExports(code, "page.tsx"); + + assertStringIncludes(result, "loadSecret"); + assertStringIncludes(result, "token"); + assertStringIncludes(result, "client"); + assertStringIncludes(result, "makeFactory"); + assertStringIncludes(result, "const React"); + }); + it("conservatively keeps a pattern with a default value", async () => { const code = [ `import { getEnv } from "veryfront";`, @@ -904,7 +1028,13 @@ describe("browser-server-exports-strip", () => { assertStringIncludes(result, "a, b"); }); - it("keeps side effects for ordinary imports with mixed hook-only bindings", async () => { + // Mixed import: the pass owns `loadSecret`, and once the hook is emptied + // nothing reads `initClient` either. The whole statement goes. Trimming it + // to `import { initClient } from "./client-setup.ts"` instead is only + // erasable under the `ts` and `tsx` loaders; under `js`, `jsx` and `mdx` + // the compile stage must keep the module for its side effects and rewrites + // the remainder into the bare import this stage may never ship. + it("deletes a mixed import whose surviving specifier nothing reads", async () => { const code = [ `import { initClient, loadSecret } from "./client-setup.ts";`, `export async function getServerData() { return { props: { token: loadSecret() } }; }`, @@ -913,9 +1043,25 @@ describe("browser-server-exports-strip", () => { const result = await stripServerOnlyExports(code); - assertStringIncludes(result, `import "./client-setup.ts"`); - assertEquals(occurrences(result, "initClient"), 0); + assertNotIncludes(result, "client-setup.ts"); assertEquals(occurrences(result, "loadSecret"), 0); + assertEquals(occurrences(result, "initClient"), 0); + }); + + it("keeps every specifier of a mixed import the browser still reads", async () => { + // The boundary of the rule above. `initClient` has a surviving reader, so + // the statement stays exactly as authored: the pass never edits an import + // the browser still needs. + const code = [ + `import { initClient, loadSecret } from "./client-setup.ts";`, + `export async function getServerData() { return { props: { token: loadSecret() } }; }`, + `export default function Page() { return initClient(); }`, + ].join("\n"); + + const result = await stripServerOnlyExports(code); + + assertStringIncludes(result, `import { initClient, loadSecret } from "./client-setup.ts"`); + assertStringIncludes(result, "return initClient()"); }); it("keeps a bare side-effect import untouched", async () => { @@ -1154,82 +1300,46 @@ describe("browser-server-exports-strip", () => { assertStringIncludes(result, "TestD as default"); }); - it("does not retain a pre-strip inline source map", async () => { + it("emits a compile map built from already-stripped input", async () => { + // The strip runs before compile, so the compiler never sees the hook and + // its map cannot carry the server-only source. Nothing has to stash or + // restore the directive, and the map survives to the browser normally. const source = [ `import { getEnv } from "veryfront";`, - `const KEY = getEnv("SERVER_VALUE");`, - `const CLIENT_MARKER = "//# sourceMappingURL=data:text/plain;base64,AAAA";`, + `const KEY = getEnv("SERVER_ONLY_HOOK_SOURCE");`, `export async function getServerData() { return { props: { key: KEY } }; }`, - `export default function Page() { return CLIENT_MARKER; }`, + `export default function Page() { return null; }`, ].join("\n"); - const compileContext = { - ...ctx(source, "browser"), - dev: true, - jsxImportSource: "react", - }; - const compiled = await compilePlugin.transform(compileContext); - const directive = compileContext.metadata.get(COMPILE_SOURCE_MAP_DIRECTIVE_METADATA); - assertEquals(typeof directive, "string"); - const match = String(directive).match( - /\/\/[#@]\s*sourceMappingURL=data:application\/json;base64,([A-Za-z0-9+/]+={0,2})/, - ); - assertEquals(match === null, false); - const sourceMap = JSON.parse(atob(match?.[1] ?? "")) as { - sourcesContent?: string[]; - }; - assertEquals( - sourceMap.sourcesContent?.some((content) => content.includes("SERVER_VALUE")), - true, - ); - const result = await browserServerExportsStripPlugin.transform({ - ...compileContext, - code: compiled, - }); + const result = await runPipeline( + source, + "/project/pages/test.tsx", + "/project", + { projectId: "source-map-post-reorder", dev: true, ssr: false }, + ); - assertNotIncludes(result, "SERVER_VALUE"); - assertNotIncludes(result, "sourceMappingURL=data:application/json;base64,"); - assertStringIncludes(result, "sourceMappingURL=data:text/plain;base64,AAAA"); - }); + assertNotIncludes(result.code, "SERVER_ONLY_HOOK_SOURCE"); + assertStringIncludes(result.code, "sourceMappingURL=data:application/json;base64,"); - it("removes a pre-strip inline map before an appended MDX export", async () => { - const source = [ - `import { getEnv } from "veryfront";`, - `const KEY = getEnv("SERVER_VALUE");`, - `const CLIENT_MARKER = "//# sourceMappingURL=data:text/plain;base64,AAAA";`, - `const MDXLayout = () => CLIENT_MARKER;`, - `export async function getServerData() { return { props: { key: KEY } }; }`, - ].join("\n"); - const compileContext = { - ...ctx(source, "browser"), - filePath: "pages/test.mdx", - dev: true, - jsxImportSource: "react", - }; - const compiled = await compilePlugin.transform(compileContext); + const encoded = result.code.match( + /sourceMappingURL=data:application\/json;base64,([A-Za-z0-9+/]+={0,2})/, + )?.[1]; + assertEquals(typeof encoded, "string"); + const map = JSON.parse(atob(String(encoded))) as { sourcesContent?: string[] }; assertEquals( - String(compileContext.metadata.get(COMPILE_SOURCE_MAP_DIRECTIVE_METADATA)).includes( - "sourceMappingURL=data:application/json;base64,", - ), - true, + map.sourcesContent?.some((content) => content.includes("SERVER_ONLY_HOOK_SOURCE")), + false, ); - assertNotIncludes(compiled, "sourceMappingURL=data:application/json;base64,"); - assertStringIncludes(compiled, "export { MDXLayout };"); - - const result = await browserServerExportsStripPlugin.transform({ - ...compileContext, - code: compiled, - }); - - assertNotIncludes(result, "SERVER_VALUE"); - assertNotIncludes(result, "sourceMappingURL=data:application/json;base64,"); - assertStringIncludes(result, "sourceMappingURL=data:text/plain;base64,AAAA"); - assertStringIncludes(result, "export { MDXLayout };"); }); - it("removes the compile map after an intermediate plugin appends code", async () => { + it("runs before compile even when a custom plugin re-sorts the pipeline", async () => { + // Array position decides order until a custom plugin registers, at which + // point the pipeline is re-sorted by `stage`. Both must place this pass + // before compile, so this case pins the stage number and the case above + // pins the array position. const source = [ - `const KEY = "SERVER_ONLY_HOOK_SOURCE";`, + `import { getEnv } from "veryfront";`, + `const KEY = getEnv("SERVER_ONLY_HOOK_SOURCE");`, `export async function getServerData() { return { props: { key: KEY } }; }`, `export default function Page() { return null; }`, ].join("\n"); @@ -1238,57 +1348,40 @@ describe("browser-server-exports-strip", () => { source, "/project/pages/test.tsx", "/project", - { projectId: "source-map-intermediate-plugin", dev: true, ssr: false }, + { projectId: "source-map-post-reorder-sorted", dev: true, ssr: false }, { plugins: [{ - name: "append-after-compile", - stage: TransformStage.COMPILE + 0.5, - transform: (ctx) => `${ctx.code}\nexport const APPENDED = true;`, + name: "inert-custom-plugin", + stage: TransformStage.FINALIZE + 0.5, + transform: (ctx) => ctx.code, }], }, ); assertNotIncludes(result.code, "SERVER_ONLY_HOOK_SOURCE"); - assertNotIncludes(result.code, "sourceMappingURL=data:application/json;base64,"); - assertStringIncludes(result.code, "export const APPENDED = true;"); + const encoded = result.code.match( + /sourceMappingURL=data:application\/json;base64,([A-Za-z0-9+/]+={0,2})/, + )?.[1]; + assertEquals(typeof encoded, "string"); + const map = JSON.parse(atob(String(encoded))) as { sourcesContent?: string[] }; + assertEquals( + map.sourcesContent?.some((content) => content.includes("SERVER_ONLY_HOOK_SOURCE")), + false, + ); }); - it("does not confuse a copied map string with the compile map comment", async () => { - const source = [ - `const KEY = "SERVER_ONLY_HOOK_SOURCE";`, - `export async function getServerData() { return { props: { key: KEY } }; }`, - `export default function Page() { return null; }`, - ].join("\n"); - + it("keeps the compile map when no server hook exists", async () => { const result = await runPipeline( - source, + `export default function Page() { return null; }`, "/project/pages/test.tsx", "/project", - { projectId: "source-map-duplicate-text", dev: true, ssr: false }, - { - plugins: [{ - name: "copy-map-before-appending", - stage: TransformStage.COMPILE + 0.5, - transform: (ctx) => { - const directive = ctx.code.match( - /\/\/[#@]\s*sourceMappingURL=data:application\/json;base64,[A-Za-z0-9+/]+={0,2}/, - )?.[0]; - const copied = directive - ? `export const COPIED_COMPILE_MAP = ${JSON.stringify(directive)};\n` - : ""; - return `${copied}${ctx.code}\nexport const APPENDED = true;`; - }, - }], - }, + { projectId: "source-map-no-server-hook", dev: true, ssr: false }, ); - assertNotIncludes(result.code, "SERVER_ONLY_HOOK_SOURCE"); - assertNotIncludes(result.code, "sourceMappingURL=data:application/json;base64,"); - assertNotIncludes(result.code, "COPIED_COMPILE_MAP"); - assertStringIncludes(result.code, "export const APPENDED = true;"); + assertStringIncludes(result.code, "sourceMappingURL=data:application/json;base64,"); }); - it("does not restore a compile map after an intermediate plugin removes the hook", async () => { + it("leaves a post-compile plugin's appended code alone", async () => { const source = [ `const KEY = "SERVER_ONLY_HOOK_SOURCE";`, `export async function getServerData() { return { props: { key: KEY } }; }`, @@ -1299,30 +1392,18 @@ describe("browser-server-exports-strip", () => { source, "/project/pages/test.tsx", "/project", - { projectId: "source-map-intermediate-removal", dev: true, ssr: false }, + { projectId: "source-map-intermediate-plugin", dev: true, ssr: false }, { plugins: [{ - name: "remove-server-hook", + name: "append-after-compile", stage: TransformStage.COMPILE + 0.5, - transform: () => `export default function Page() { return null; }`, + transform: (ctx) => `${ctx.code}\nexport const APPENDED = true;`, }], }, ); assertNotIncludes(result.code, "SERVER_ONLY_HOOK_SOURCE"); - assertNotIncludes(result.code, "sourceMappingURL=data:application/json;base64,"); - assertStringIncludes(result.code, "export default function Page()"); - }); - - it("keeps the compile map when no server hook or intermediate change exists", async () => { - const result = await runPipeline( - `export default function Page() { return null; }`, - "/project/pages/test.tsx", - "/project", - { projectId: "source-map-no-server-hook", dev: true, ssr: false }, - ); - - assertStringIncludes(result.code, "sourceMappingURL=data:application/json;base64,"); + assertStringIncludes(result.code, "export const APPENDED = true;"); }); it("removes an external source map reference after stripping", async () => { @@ -1338,45 +1419,871 @@ describe("browser-server-exports-strip", () => { assertNotIncludes(result, "sourceMappingURL=page.js.map"); }); - it("does not run for the ssr target", () => { - assertEquals(browserServerExportsStripPlugin.condition?.(ctx("", "ssr")), false); - assertEquals(browserServerExportsStripPlugin.condition?.(ctx("", "browser")), true); + it("keeps a server import, a secret and a hook-only helper out of the artifact", async () => { + // The security property this stage exists for, asserted through the real + // browser pipeline rather than through `stripServerOnlyExports` alone. + const source = [ + `import { hashOf } from "./lib/uses-crypto.ts";`, + `import { getEnv } from "veryfront";`, + `const API_KEY = getEnv("SERVER_ONLY_HOOK_SOURCE");`, + `function loadUser(id) { return hashOf(API_KEY + id); }`, + `export async function getServerData(ctx) {`, + ` return { props: { user: loadUser(ctx.params.id) } };`, + `}`, + `export default function Page({ user }) { return user; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.tsx", "/project", { + projectId: "pre-compile-security-property", + ssr: false, + }); + + assertNotIncludes(result.code, "uses-crypto"); + assertNotIncludes(result.code, "SERVER_ONLY_HOOK_SOURCE"); + assertStringIncludes(result.code, "server-only"); }); - }); - describe("TypeScript reference classification", () => { + it("does not pin a hook-only binding read from a type position", async () => { + // Pre-compile input carries TypeScript the compiled input never had. A + // `typeof` in an erased parameter annotation must not count as a browser + // read and keep the secret alive. + const source = [ + `import { getEnv } from "veryfront";`, + `const KEY = getEnv("SERVER_ONLY_HOOK_SOURCE");`, + `export async function getServerData() { return { props: { k: KEY } }; }`, + `export default function Page(p: { k: typeof KEY }) { return p.k; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.tsx", "/project", { + projectId: "pre-compile-typeof-parameter", + ssr: false, + }); + + assertNotIncludes(result.code, "SERVER_ONLY_HOOK_SOURCE"); + }); + + it("does not drag a server module in through a `ReturnType` alias", async () => { + const source = [ + `import { schema } from "./lib/server-schema.ts";`, + `import { getEnv } from "veryfront";`, + `const SECRET = getEnv("SERVER_ONLY_HOOK_SOURCE");`, + `type Data = ReturnType;`, + `export async function getServerData() {`, + ` return { props: { d: schema.parse(SECRET) as Data } };`, + `}`, + `export default function Page() { return null; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.tsx", "/project", { + projectId: "pre-compile-returntype-alias", + ssr: false, + }); + + assertNotIncludes(result.code, "SERVER_ONLY_HOOK_SOURCE"); + assertNotIncludes(result.code, "server-schema"); + }); + + it("deletes a mixed value and type import the hook owned", async () => { + // `Cfg` is erased before the module runs, so counting it would leave a + // bare `import "./lib/server-only-lib.js"` that executes in the browser. + const source = [ + `import { hashOf, type Cfg } from "./lib/server-only-lib.ts";`, + `export async function getServerData() {`, + ` const c: Cfg = { a: 1 };`, + ` return { props: { h: hashOf(c) } };`, + `}`, + `export default function Page() { return null; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.tsx", "/project", { + projectId: "pre-compile-mixed-specifier-import", + ssr: false, + }); + + assertNotIncludes(result.code, "server-only-lib"); + }); + + it("does not demote an unrelated unused import to a side-effect import", async () => { + // The pre-compile leak this ordering used to open. The pass owns nothing + // in this import, and the old reduction to `import "./lib/server-only-lib.js"` + // turned an import the compiler can erase into one it must preserve, so + // the module and its transitive graph executed in the browser. Left as + // authored, `compile` erases it and the artifact matches `main`. + const source = [ + `import { unusedThing } from "./lib/server-only-lib.ts";`, + `export async function getServerData() { return { props: {} }; }`, + `export default function Page() { return null; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.tsx", "/project", { + projectId: "pre-compile-unused-import-demotion", + ssr: false, + }); + + assertNotIncludes(result.code, "server-only-lib"); + assertEquals(occurrences(result.code, "unusedThing"), 0); + }); + + it("does not demote an unrelated unused import in development either", async () => { + // `compilePlugin` sets `treeShaking: !ctx.dev`, so the case above only + // proves the tree-shaken path. What erases this import is not tree + // shaking, it is TypeScript unused-import elision, which the `ts` and + // `tsx` loaders perform in both modes. Pinned so a future change to + // either setting cannot quietly make development ship the module. + const source = [ + `import { unusedThing } from "./lib/server-only-lib.ts";`, + `export async function getServerData() { return { props: {} }; }`, + `export default function Page() { return null; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.tsx", "/project", { + projectId: "pre-compile-unused-import-demotion-dev", + dev: true, + ssr: false, + }); + + assertNotIncludes(result.code, "server-only-lib"); + assertEquals(occurrences(result.code, "unusedThing"), 0); + }); + + it("keeps a genuine side-effect import in the artifact", async () => { + // Paired with the case above. An import authored as a side-effect import + // is preserved by design: it is not an unused named import, and nothing + // about the ordering change makes it droppable. + const source = [ + `import "./lib/analytics.ts";`, + `import { getEnv } from "veryfront";`, + `const KEY = getEnv("SERVER_ONLY_HOOK_SOURCE");`, + `export async function getServerData() { return { props: { k: KEY } }; }`, + `export default function Page() { return null; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.tsx", "/project", { + projectId: "pre-compile-genuine-side-effect-import", + ssr: false, + }); + + assertStringIncludes(result.code, "lib/analytics"); + assertNotIncludes(result.code, "SERVER_ONLY_HOOK_SOURCE"); + }); + + // The dialect axis. This stage emits the same text whatever the extension + // is, so what reaches the browser is decided by the loader the compile + // stage picks (`getLoaderFromPath` in + // `src/transforms/esm/transform-utils.ts`). Under `ts` and `tsx` esbuild + // may elide an unused import, because a specifier can name a type. Under + // `js`, `jsx`, `md` and `mdx` it must keep the module for its side effects, + // so it rewrites whatever this pass leaves behind into a bare + // `import "./x.js"`. A guard written on a single `.tsx` page therefore + // proves nothing about the other four. + const SERVER_LIB = "./lib/server-only-lib.js"; + /** - * Both walkers' answers restricted to `names`, so a fixture asserts which - * module-level bindings each one attributes to runtime code without - * listing every local it also sees. + * Whether the artifact still imports the server-only module, under the + * authored specifier or under the base64 filesystem URL the MDX path + * rewrites it to. */ - async function referencesAmong( - code: string, - names: string[], - filePath = "page.tsx", - ): Promise<{ referenced: string[]; free: string[] }> { - const parser = tryResolve("CodeParser"); - if (!parser) throw new Error("no CodeParser extension is registered"); - const ast = await parser.parse({ code, filePath }); - const { referenced, free } = moduleReferenceWalkers(ast); - const pick = (found: Set) => names.filter((name) => found.has(name)); - return { referenced: pick(referenced), free: pick(free) }; + function importsServerLib(code: string): boolean { + if (code.includes("server-only-lib")) return true; + for (const match of code.matchAll(/\/_veryfront\/fs\/([A-Za-z0-9_-]+)\.js/g)) { + const base64 = (match[1] ?? "").replaceAll("-", "+").replaceAll("_", "/"); + const decoded = atob(base64 + "=".repeat((4 - (base64.length % 4)) % 4)); + if (decoded.includes("server-only-lib")) return true; + } + return false; } - /** - * The defect this classification fixes is the two walkers disagreeing, so - * every fixture asserts the same expectation against both. - */ - async function assertWalkers( - code: string, - names: string[], - expected: string[], - ): Promise { - const { referenced, free } = await referencesAmong(code, names); - assertEquals(referenced, expected, "referencedIdentifiers"); - assertEquals(free, expected, "freeReferencedIdentifiers"); + function assertNoServerLibImport(code: string, cell: string): void { + assertEquals( + importsServerLib(code), + false, + `${cell} still reaches the server-only module:\n${code}`, + ); + } + + const pageOf = (ext: string): string => + ext === "tsx" || ext === "jsx" + ? `export default function Page() { return ok; }` + : `export default function Page() { return null; }`; + + const importShapes = [ + { + name: "mixed specifiers", + statement: `import { hashOf, unusedSibling } from "${SERVER_LIB}";`, + leftover: "unusedSibling", + }, + { + name: "a default-import sibling", + statement: `import defaultLib, { hashOf } from "${SERVER_LIB}";`, + leftover: "defaultLib", + }, + ] as const; + + for (const ext of ["ts", "tsx", "js", "jsx"] as const) { + for (const shape of importShapes) { + for (const dev of [false, true] as const) { + const mode = dev ? "dev" : "prod"; + const cell = `${ext}-${mode}-${shape.name}`; + it(`deletes an import with ${shape.name} on a .${ext} page in ${mode}`, async () => { + const source = [ + shape.statement, + `export async function getServerData() { return { props: { h: hashOf(1) } }; }`, + pageOf(ext), + ].join("\n"); + + const result = await runPipeline(source, `/project/pages/probe.${ext}`, "/project", { + projectId: `pre-compile-dialect-${cell}`, + ssr: false, + dev, + }); + + assertNoServerLibImport(result.code, cell); + assertEquals(occurrences(result.code, shape.leftover), 0); + assertStringIncludes(result.code, "getServerData"); + }); + } + } + } + + for (const shape of importShapes) { + for (const dev of [false, true] as const) { + const mode = dev ? "dev" : "prod"; + const cell = `mdx-${mode}-${shape.name}`; + it(`deletes an import with ${shape.name} on a .mdx page in ${mode}`, async () => { + // MDX compiles to JSX, so it takes the `jsx` loader and the same + // side-effect rule. The specifier is rewritten to a base64 filesystem + // URL on the way out, which is why the assertion decodes it. + const source = [ + shape.statement, + ``, + `export async function getServerData() { return { props: { h: hashOf(1) } }; }`, + ``, + `# heading`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/probe.mdx", "/project", { + projectId: `pre-compile-dialect-${cell}`, + ssr: false, + dev, + }); + + assertNoServerLibImport(result.code, cell); + assertEquals(occurrences(result.code, shape.leftover), 0); + assertStringIncludes(result.code, "heading"); + }); + } + } + + // `import x = require("./x.js")` and `import x = ns.member` are node shapes + // this pass never met while it ran after compile: esbuild had already + // lowered them to `const x = require("./x.js")`. Pre-compile they reach the + // browser untouched, where the server-only module stays in the graph and + // `require` is not defined, so the page dies on a ReferenceError instead of + // rendering. + const importEqualsShapes = [ + { + name: "an import-equals require", + statements: [`import srv = require("${SERVER_LIB}");`], + }, + { + name: "an import-equals namespace alias", + statements: [ + `import * as lib from "${SERVER_LIB}";`, + `import srv = lib.helpers;`, + ], + }, + ] as const; + + for (const ext of ["ts", "tsx"] as const) { + for (const shape of importEqualsShapes) { + for (const dev of [false, true] as const) { + const mode = dev ? "dev" : "prod"; + const cell = `${ext}-${mode}-${shape.name}`; + it(`deletes ${shape.name} on a .${ext} page in ${mode}`, async () => { + const source = [ + ...shape.statements, + `export async function getServerData() { return { props: { h: srv.hashOf(1) } }; }`, + pageOf(ext), + ].join("\n"); + + const result = await runPipeline(source, `/project/pages/probe.${ext}`, "/project", { + projectId: `pre-compile-import-equals-${cell}`, + ssr: false, + dev, + }); + + assertNoServerLibImport(result.code, cell); + assertNotIncludes(result.code, "require("); + assertStringIncludes(result.code, "getServerData"); + }); + } + } } + it("keeps a jsx pragma that sits above a removed import", async () => { + // Babel attaches the file's opening comments to its first statement, so + // removing that import would take the pragma with it and silently switch + // the JSX factory back to the configured default. + const source = [ + `/** @jsxImportSource preact */`, + `import { hashOf } from "./lib/server-only-lib.ts";`, + `export async function getServerData() { return { props: { h: hashOf(1) } }; }`, + `export default function Page() { return
; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.tsx", "/project", { + projectId: "pre-compile-jsx-pragma", + ssr: false, + }); + + assertNotIncludes(result.code, "server-only-lib"); + assertStringIncludes(result.code, "preact/jsx-runtime"); + }); + + it("keeps a legal banner between two removed trailing declarations", async () => { + // Nothing survives after the banner, so it has no statement left to lead. + const source = [ + `import { getEnv } from "veryfront";`, + `export async function getServerData() { return { props: { s: SECRET, r: RAW } }; }`, + `const RAW = getEnv("SERVER_ONLY_HOOK_SOURCE");`, + `/*! package license */`, + `const SECRET = RAW + "x";`, + ].join("\n"); + + const result = await stripServerOnlyExports(source, "pages/test.tsx"); + + assertNotIncludes(result, "SERVER_ONLY_HOOK_SOURCE"); + assertStringIncludes(result, "/*! package license */"); + }); + + // The JSX axis. Post-compile this pass only ever met lowered factory calls, + // so an intrinsic tag, an attribute name and a member property all reached + // the walkers as plain strings. Pre-compile they arrive as JSXIdentifiers + // and were counted as runtime reads, in both directions: an import that + // shares a tag name survived into the browser artifact, and an unrelated + // declaration that shares one was deleted. Asserted through `runPipeline`, + // because what the browser receives is the artifact, not the stage output. + it("does not keep a hook-only import that shares an intrinsic tag name", async () => { + const source = [ + `import { table } from "./lib/server-only-lib.ts";`, + `export async function getServerData() { return { props: { rows: table() } }; }`, + `export default function Page() { return
x
; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.jsx", "/project", { + projectId: "pre-compile-jsx-intrinsic-collision", + ssr: false, + }); + + assertNotIncludes(result.code, "server-only-lib"); + assertStringIncludes(result.code, `"table"`); + }); + + it("does not keep a hook-only import that shares a jsx attribute name", async () => { + const source = [ + `import { placeholder, title, href } from "./lib/server-only-lib.ts";`, + `export async function getServerData() {`, + ` return { props: { p: placeholder(title(href())) } };`, + `}`, + `export default function Page() {`, + ` return x;`, + `}`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.jsx", "/project", { + projectId: "pre-compile-jsx-attribute-collision", + ssr: false, + }); + + assertNotIncludes(result.code, "server-only-lib"); + assertStringIncludes(result.code, "placeholder"); + }); + + it("does not keep a hook-only import that shares a member element property", async () => { + const source = [ + `import { Item } from "./lib/server-only-lib.ts";`, + `import { UI } from "./lib/ui.ts";`, + `export async function getServerData() { return { props: { p: Item() } }; }`, + `export default function Page() { return x; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.jsx", "/project", { + projectId: "pre-compile-jsx-member-collision", + ssr: false, + }); + + assertNotIncludes(result.code, "server-only-lib"); + // The leftmost object is a real read, so its import has to stay. + assertStringIncludes(result.code, "lib/ui"); + }); + + it("keeps an unrelated declaration that shares a tag name used in the hook", async () => { + // The deletion direction. `
` inside the hook is string text, + // so `section` never enters the hook's dependency closure and the + // module-scope declaration keeps its browser side effect, along with the + // import it is the last user of. + const source = [ + `import { boot } from "./lib/analytics.ts";`, + `const section = boot();`, + `export async function getServerData() { return { props: { s:
} }; }`, + `export default function Page() { return null; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.jsx", "/project", { + projectId: "pre-compile-jsx-unrelated-declaration", + ssr: false, + }); + + assertStringIncludes(result.code, "lib/analytics"); + assertStringIncludes(result.code, "boot"); + }); + + it("keeps an unrelated declaration that shares an attribute name", async () => { + const source = [ + `import { boot } from "./lib/analytics.ts";`, + `const placeholder = boot();`, + `export async function getServerData() {`, + ` return { props: { s: } };`, + `}`, + `export default function Page() { return null; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.jsx", "/project", { + projectId: "pre-compile-jsx-unrelated-attribute-declaration", + ssr: false, + }); + + assertStringIncludes(result.code, "lib/analytics"); + assertStringIncludes(result.code, "boot"); + }); + + it("keeps the component import a capitalised element name reads", async () => { + const source = [ + `import { Card } from "./lib/card.ts";`, + `import { getEnv } from "veryfront";`, + `const KEY = getEnv("SERVER_ONLY_HOOK_SOURCE");`, + `export async function getServerData() { return { props: { k: KEY } }; }`, + `export default function Page() { return ; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.jsx", "/project", { + projectId: "pre-compile-jsx-component-import", + ssr: false, + }); + + assertStringIncludes(result.code, "lib/card"); + assertNotIncludes(result.code, "SERVER_ONLY_HOOK_SOURCE"); + }); + + it("keeps the imports a member object and an expression container read", async () => { + const source = [ + `import { UI } from "./lib/ui.ts";`, + `import { cls, items } from "./lib/client.ts";`, + `export async function getServerData() { return { props: {} }; }`, + `export default function Page() {`, + ` return {items.map((i) => i)};`, + `}`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.jsx", "/project", { + projectId: "pre-compile-jsx-positive-reads", + ssr: false, + }); + + assertStringIncludes(result.code, "lib/ui"); + assertStringIncludes(result.code, "lib/client"); + }); + + // The classic JSX pragmas name a factory the compile stage calls. That + // stage now runs after this one, so nothing in the module reads the name + // yet and the import that provides it looked dead. + it("keeps the factory import a classic `@jsx` pragma names", async () => { + const source = [ + `/** @jsxRuntime classic */`, + `/** @jsx h */`, + `import { h } from "veryfront";`, + `export async function getServerData() { return { props: {} }; }`, + `export default function Page() { return
; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.jsx", "/project", { + projectId: "pre-compile-classic-jsx-pragma", + ssr: false, + }); + + // The artifact is minified, so bind the emitted call to the surviving + // import rather than to a spelling: `h` is renamed, and the point is that + // whatever the call names is imported and not a free variable. + const call = /(?[A-Za-z_$][\w$]*)\("div",\s*null\)/.exec(result.code); + assertEquals(call !== null, true, `no classic factory call in:\n${result.code}`); + const local = call?.groups?.local ?? ""; + assertEquals( + new RegExp(`\\bh as ${local}\\b`).test(result.code), + true, + `the factory call is not bound by an import in:\n${result.code}`, + ); + }); + + it("keeps the fragment import a classic `@jsxFrag` pragma names", async () => { + const source = [ + `/** @jsxRuntime classic */`, + `/** @jsx h */`, + `/** @jsxFrag Frag */`, + `import { h, Frag } from "veryfront";`, + `export async function getServerData() { return { props: {} }; }`, + `export default function Page() { return <>
; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.jsx", "/project", { + projectId: "pre-compile-classic-jsxfrag-pragma", + ssr: false, + }); + + assertStringIncludes(result.code, "_veryfront/index.client.js"); + assertEquals(occurrences(result.code, "Frag") > 0, true, result.code); + }); + + it("keeps the default classic factory when the module names none", async () => { + // `@jsxRuntime classic` alone makes esbuild emit `React.createElement`, + // so the classic runtime pins `React` whatever the module named. + // The hook owns the only other read of `React`, so without the default + // pin the import is deleted and the emitted call has no binding. + const source = [ + `/** @jsxRuntime classic */`, + `import { React } from "./lib/react-shim.ts";`, + `export async function getServerData() { return { props: { v: React.version } }; }`, + `export default function Page() { return
; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.jsx", "/project", { + projectId: "pre-compile-classic-default-factory", + ssr: false, + }); + + assertStringIncludes(result.code, "lib/react-shim"); + const call = /(?[A-Za-z_$][\w$]*)\.createElement\("div",\s*null\)/.exec(result.code); + assertEquals(call !== null, true, `no default classic call in:\n${result.code}`); + assertEquals( + new RegExp(`\\bReact as ${call?.groups?.local ?? ""}\\b`).test(result.code), + true, + `the default factory is not bound by an import in:\n${result.code}`, + ); + }); + + it("does not pin a `@jsx` pragma without the classic runtime", async () => { + // esbuild ignores `@jsx` under the automatic runtime, so the name it + // holds is not called by anything and must not keep a hook-only import. + const source = [ + `/** @jsx table */`, + `import { table } from "./lib/server-only-lib.ts";`, + `export async function getServerData() { return { props: { t: table() } }; }`, + `export default function Page() { return
; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.jsx", "/project", { + projectId: "pre-compile-automatic-runtime-pragma", + ssr: false, + }); + + assertNotIncludes(result.code, "server-only-lib"); + }); + + // Defect 4A. `isIntrinsicJsxName` implements Babel's `isCompatTag` rule, + // which is only valid for a bare JSXIdentifier that IS the element name. + // Running it on the OBJECT of a member element name classified `motion`, + // `styled`, `ui` and `dialog` as string tag text, so the pass deleted the + // binding they read and the page died on a ReferenceError. Three trigger + // paths reach it, and the second needs no hook relationship at all. + it("keeps a lowercase member object a stripped hook also read", async () => { + const source = [ + `import { motion } from "./lib/motion.ts";`, + `import { getEnv } from "veryfront";`, + `const KEY = getEnv("SERVER_ONLY_HOOK_SOURCE");`, + `export async function getServerData() {`, + ` return { props: { k: KEY, variants: motion.presets } };`, + `}`, + `export default function Page() { return x; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.jsx", "/project", { + projectId: "pre-compile-jsx-member-object-hook-owned", + ssr: false, + }); + + assertStringIncludes(result.code, "lib/motion"); + // The hook's own dependency still goes. + assertNotIncludes(result.code, "SERVER_ONLY_HOOK_SOURCE"); + }); + + // The droppable-source branch deletes a `node:` or `veryfront` import the + // moment nothing reads it, whatever the hooks owned. A member object the + // walkers read as tag text is read by nothing, so the import went and the + // element it names lost its binding. `node:` is prefix-matched, so the + // specifier here only has to carry the prefix. + for ( + const source of ["veryfront/ui", "node:widgets"] as const + ) { + it(`keeps a lowercase member object imported from ${source}`, async () => { + const code = [ + `import { dialog } from "${source}";`, + `export async function getServerData() { return { props: {} }; }`, + `export default function Page() { return x; }`, + ].join("\n"); + + const result = await runPipeline(code, "/project/pages/test.jsx", "/project", { + projectId: `pre-compile-jsx-member-object-${source.replace(/\W+/g, "-")}`, + ssr: false, + }); + + assertStringIncludes(result.code, "dialog"); + assertStringIncludes(result.code, ".Root"); + }); + } + + it("keeps a hook-owned module-scope declaration a lowercase member object reads", async () => { + const source = [ + `import { makeStyled } from "./lib/styled.ts";`, + `const styled = makeStyled();`, + `export async function getServerData() { return { props: { s: styled.id } }; }`, + `export default function Page() { return x; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.jsx", "/project", { + projectId: "pre-compile-jsx-member-object-module-scope", + ssr: false, + }); + + assertStringIncludes(result.code, "lib/styled"); + assertStringIncludes(result.code, "makeStyled"); + }); + + // Defect 4B, the leak direction. `export { token as clientToken } from + // "./client-utils.js"` reads no local binding: `token` names an export of + // the SOURCE module. Neither walker had a case for it, so the generic + // descent counted both halves of the specifier as free reads and the + // hook-owned `import { token } from "./server-only-lib.js"` stayed alive. + for (const ext of ["jsx", "mdx"] as const) { + it(`deletes a hook-owned import a re-export clause only looks like it reads on .${ext}`, async () => { + const source = ext === "mdx" + ? [ + `import { token } from "${SERVER_LIB}";`, + ``, + `export { token as clientToken } from "./lib/client-utils.js";`, + ``, + `export async function getServerData() { return { props: { t: token() } }; }`, + ``, + `# heading`, + ].join("\n") + : [ + `import { token } from "${SERVER_LIB}";`, + `export { token as clientToken } from "./lib/client-utils.js";`, + `export async function getServerData() { return { props: { t: token() } }; }`, + `export default function Page() { return ok; }`, + ].join("\n"); + + const result = await runPipeline(source, `/project/pages/reexport.${ext}`, "/project", { + projectId: `pre-compile-reexport-source-${ext}`, + ssr: false, + }); + + assertNoServerLibImport(result.code, `reexport-${ext}`); + // The re-export itself is untouched: it is the module's contract. + assertStringIncludes(result.code, "clientToken"); + }); + } + + it("still reads the local half of an export clause with no source", async () => { + // The other side of 4B. Without a `source`, `token` really is this + // module's binding, so the import that provides it has to stay. + const source = [ + `import { token } from "./lib/client-utils.js";`, + `export { token as clientToken };`, + `export async function getServerData() { return { props: {} }; }`, + `export default function Page() { return ok; }`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/reexport-local.jsx", "/project", { + projectId: "pre-compile-reexport-local", + ssr: false, + }); + + assertStringIncludes(result.code, "client-utils"); + assertStringIncludes(result.code, "clientToken"); + }); + + it("keeps a component import that shares its name with an intrinsic tag", async () => { + // The positive half of the element-name rule, through the artifact: + // `` reads the import, `
` beside it is string text, and + // the hook-only `rows` import still goes. + const source = [ + `import { Table } from "./lib/table.ts";`, + `import { rows } from "${SERVER_LIB}";`, + `export async function getServerData() { return { props: { r: rows() } }; }`, + `export default function Page() {`, + ` return <>
;`, + `}`, + ].join("\n"); + + const result = await runPipeline(source, "/project/pages/test.jsx", "/project", { + projectId: "pre-compile-jsx-component-beside-intrinsic", + ssr: false, + }); + + assertStringIncludes(result.code, "lib/table"); + assertNoServerLibImport(result.code, "component-beside-intrinsic"); + }); + + it("does not run for the ssr target", () => { + assertEquals(browserServerExportsStripPlugin.condition?.(ctx("", "ssr")), false); + assertEquals(browserServerExportsStripPlugin.condition?.(ctx("", "browser")), true); + }); + }); + + // Running before compile, this pass sees JSX that esbuild used to lower into + // factory calls before either walker ever ran. A `JSXIdentifier` is only + // sometimes a binding read, and the walkers counted every one of them. + describe("JSX reference classification", () => { + const page = (body: string) => + [ + `import { table, placeholder, Item, UI, Card, cls, motion, Table, Fragment, Bar }` + + ` from "./mod.js";`, + `export default function Page() { return ${body}; }`, + ].join("\n"); + + const NAMES = ["table", "placeholder", "Item", "UI", "Card", "cls"]; + /** The same fixture names plus the ones only the newer cases use. */ + const ALL = [...NAMES, "motion", "Table", "Fragment", "Bar"]; + + it("does not read an intrinsic element name", async () => { + // `` is the string "table" to the factory, never the import. + await assertWalkers(page(`
x
`), NAMES, []); + }); + + it("does not read a custom element name", async () => { + // A dash cannot appear in an identifier, so `` is string text. + await assertWalkers(page(``), NAMES, []); + }); + + it("does not read an attribute name", async () => { + await assertWalkers(page(``), NAMES, ["Card"]); + }); + + it("does not read either half of a namespaced name", async () => { + // esbuild emits `("svg:circle", { "xlink:href": "x" })`: string text on + // both sides of the colon, for an element name and an attribute alike. + await assertWalkers(page(``), NAMES, []); + }); + + it("reads only the leftmost object of a member element name", async () => { + await assertWalkers(page(`x`), NAMES, ["UI"]); + }); + + it("reads only the leftmost object of a nested member element name", async () => { + await assertWalkers(page(``), NAMES, ["UI"]); + }); + + it("reads a capitalised element name", async () => { + await assertWalkers(page(``), NAMES, ["Card"]); + }); + + it("reads an expression container and a spread", async () => { + await assertWalkers( + page(`{placeholder}`), + NAMES, + ["table", "placeholder", "Card", "cls"], + ); + }); + + // Defect 4A at the walker level. Babel's `isCompatTag` rule answers for a + // bare element name only. The object of a member element name is a binding + // read whatever its case, which is the whole reason ``, + // `` and `` render at all. + it("reads a lowercase member element object", async () => { + await assertWalkers(page(`x`), ALL, ["motion"]); + }); + + it("reads a lowercase member element object nested two deep", async () => { + await assertWalkers(page(``), ALL, ["motion"]); + }); + + it("reads a capitalised element name beside an intrinsic one", async () => { + await assertWalkers(page(`
`), ALL, ["Table"]); + }); + + it("reads neither spelling of a fragment", async () => { + await assertWalkers(page(`<>
`), ALL, []); + await assertWalkers(page(`
`), ALL, ["Fragment"]); + }); + + it("reads an attribute value but not the attribute name", async () => { + await assertWalkers(page(``), ALL, ["Card", "Bar"]); + }); + }); + + // The positions that hold a fixed name rather than a reference. Each of them + // is one cell of the shared classification, and each one that a walker got + // wrong would pin a server-only import into the browser artifact. + describe("fixed-name positions are not reads", () => { + const moduleOf = (body: string) => + [`import { token, meta, order, target } from "./mod.js";`, body].join("\n"); + + const NAMES = ["token", "meta", "order", "target"]; + + it("does not read an object literal key", async () => { + await assertWalkers(moduleOf(`const o = { token: 1, [order]: 2 };`), NAMES, ["order"]); + }); + + it("does not read a class private name", async () => { + await assertWalkers( + moduleOf(`class Box { #token = 1; #meta() { return this.#token; } }`), + NAMES, + [], + ); + }); + + it("does not read a statement label", async () => { + await assertWalkers( + moduleOf(`function loop() { token: for (;;) { break token; } }`), + NAMES, + [], + ); + }); + + it("does not read either half of a meta property", async () => { + await assertWalkers( + moduleOf(`const u = import.meta.url; function F() { return new.target; }`), + NAMES, + [], + ); + }); + + // A method name and a class member name are property keys, exactly like an + // object literal key. Counting one as a read pins the import it collides + // with, which is the leak direction. + it("does not read a fixed method or class member key", async () => { + await assertWalkers( + moduleOf( + `const o = { token() { return 1; } };` + + ` class Box { meta = 1; order() {} accessor target = 2; }`, + ), + NAMES, + [], + ); + }); + + it("reads a computed method or class member key", async () => { + await assertWalkers( + moduleOf( + `const o = { [token]() { return 1; } };` + + ` class Box { [meta] = 1; [order]() {} accessor [target] = 2; }`, + ), + NAMES, + ["token", "meta", "order", "target"], + ); + }); + }); + + describe("TypeScript reference classification", () => { describe("erased type positions do not reference a binding", () => { it("ignores `typeof` in a parameter type annotation", async () => { await assertWalkers( @@ -1469,11 +2376,14 @@ export const v = live();`, // esbuild both emit a `__decorate` call for it, so the decorator is a // real read. Erasing it deleted the import the call needs and produced a // ReferenceError at browser module evaluation. + // The hook reads `audit` too, so the import is hook-owned and the pass + // deletes it outright unless the decorator counts as a runtime read. + // That makes the paired case below discriminate. const code = [ 'import { audit } from "./audit.ts";', 'import { getEnv } from "veryfront";', 'const KEY = getEnv("SECRET");', - "export async function getServerData() { return { props: { k: KEY } }; }", + "export async function getServerData() { return { props: { k: KEY, a: audit } }; }", "class Model {", " @audit declare id: string;", "}", @@ -1483,8 +2393,7 @@ export const v = live();`, const result = await stripServerOnlyExports(code, "/project/app/page.tsx"); // The NAMED binding must survive: the emitted `__decorate` call reads - // `audit` by name. Asserting only on the specifier would pass even when - // the import is demoted to a bare side-effect import, which is the bug. + // `audit` by name. assertStringIncludes(result, "{ audit }"); assertNotIncludes(result, 'getEnv("SECRET")'); }); @@ -1494,7 +2403,7 @@ export const v = live();`, 'import { audit } from "./audit.ts";', 'import { getEnv } from "veryfront";', 'const KEY = getEnv("SECRET");', - "export async function getServerData() { return { props: { k: KEY } }; }", + "export async function getServerData() { return { props: { k: KEY, a: audit } }; }", "class Model {", " declare id: string;", "}", @@ -1503,10 +2412,11 @@ export const v = live();`, const result = await stripServerOnlyExports(code, "/project/app/page.tsx"); - // The binding goes; the module specifier is demoted to a side-effect - // import by dropUnusedImportBindings, which is pre-existing behaviour - // and not what this case is about. - assertNotIncludes(result, "{ audit }"); + // Nothing reads `audit` once the hook is emptied, and the hook owned it, + // so the whole import goes. Contrast the decorated case above, where the + // `__decorate` call keeps the named binding alive. + assertNotIncludes(result, "./audit.ts"); + assertEquals(occurrences(result, "audit"), 0); assertNotIncludes(result, 'getEnv("SECRET")'); }); @@ -1556,11 +2466,13 @@ export enum Level { Low = compute(SEED) }`, it("does not hoist a block-scoped enum into the enclosing function scope", async () => { // An enum nested in a block is block scoped: TypeScript emits `let` there. // Hoisting it into the function scope made the outer `consume(Alias)` read - // look shadowed, so import liveness reduced the named import to a bare - // side-effect import and left `client` with an unresolved binding. + // look shadowed, so import liveness treated the whole import as hook-owned + // and deleted it, leaving `client` with an unresolved binding. The hook + // reads `Alias` as well, so only the `consume(Alias)` read keeps the + // import alive and the assertion below discriminates. const code = [ 'import { Alias, secret } from "./lib.ts";', - "export async function getServerData() { return { props: { s: secret } }; }", + "export async function getServerData() { return { props: { s: secret, a: Alias } }; }", "export function client() {", " consume(Alias);", " if (false) { enum Alias { X } }", @@ -1574,12 +2486,14 @@ export enum Level { Low = compute(SEED) }`, assertStringIncludes(result, "import { Alias"); }); - it("still reduces an import a same-scope enum genuinely shadows", async () => { + it("still deletes an import a same-scope enum genuinely shadows", async () => { // Paired with the case above: asserting only that the import survives - // would also pass if the pass stopped reducing imports altogether. + // would also pass if the pass stopped classifying shadows at all. Here + // the enum really does shadow `Alias`, so both bindings are hook-owned + // and unread, and the whole import goes. const code = [ 'import { Alias, secret } from "./lib.ts";', - "export async function getServerData() { return { props: { s: secret } }; }", + "export async function getServerData() { return { props: { s: secret, a: Alias } }; }", "export function client() {", " enum Alias { X }", " consume(Alias);", @@ -1590,7 +2504,7 @@ export enum Level { Low = compute(SEED) }`, const result = await stripServerOnlyExports(code, "/project/app/page.tsx"); - assertNotIncludes(result, "import { Alias"); + assertNotIncludes(result, "./lib.ts"); }); it("still strips a module binding an enum initialiser genuinely reads", async () => { @@ -1783,11 +2697,12 @@ export function hook() { const result = await stripServerOnlyExports(code, "page.tsx"); - assertNotIncludes(result, "{ secretOnly, Low }"); - // A mixed project import keeps its pre-existing side-effect contract. - // The TypeScript fix removes the false live binding; it does not prove - // that evaluating the imported module is safe to delete. - assertStringIncludes(result, `import "../lib/server-only.ts"`); + // The hook owned `secretOnly` and `Level.Low` is an enum member access, + // not a read of the imported `Low`, so no binding of this import has a + // reader left and the statement goes whole. Counting `Level.Low` as a + // read would keep the statement and `secretOnly` with it. + assertNotIncludes(result, "server-only.ts"); + assertEquals(occurrences(result, "secretOnly"), 0); assertStringIncludes(result, "Level.Low"); }); @@ -1866,11 +2781,28 @@ export function hook() { const result = await stripServerOnlyExports(code, "page.tsx"); - assertNotIncludes(result, "{ secretOnly, Alias }"); - assertStringIncludes(result, `import "../lib/server-only.ts"`); + // `Alias` inside `M` is the namespace-local alias, not the import, so + // the import has no reader left and the hook-owned statement goes + // whole. Reading it as a use of the import would keep `secretOnly`. + assertNotIncludes(result, "server-only.ts"); + assertEquals(occurrences(result, "secretOnly"), 0); assertStringIncludes(result, "import Alias = ClientNS"); }); + it("keeps an exported import-equals the hook was the only reader of", async () => { + // `export import` is part of the module's contract, so it is never a + // pruning candidate even when the stripped hook was its only reader. + const code = [ + `export import srv = require("../lib/server-only.ts");`, + `export function getServerData() { return { props: { s: srv.load() } }; }`, + `export default function Page() { return null; }`, + ].join("\n"); + + const result = await stripServerOnlyExports(code, "page.tsx"); + + assertStringIncludes(result, `export import srv = require("../lib/server-only.ts")`); + }); + it("drops a hook-only binding that matches a qualified-name property", async () => { const code = [ `import { getEnv } from "veryfront";`, @@ -1900,8 +2832,9 @@ export function hook() { const result = await stripServerOnlyExports(code, "page.tsx"); - assertNotIncludes(result, "{ secretOnly, Alias }"); - assertStringIncludes(result, `import "../lib/server-only.ts"`); + // Same boundary with a namespace-local enum in place of the alias. + assertNotIncludes(result, "server-only.ts"); + assertEquals(occurrences(result, "secretOnly"), 0); assertStringIncludes(result, "enum Alias"); }); diff --git a/src/transforms/pipeline/stages/browser-server-exports-strip.ts b/src/transforms/pipeline/stages/browser-server-exports-strip.ts index 473e73d485..cf5fec7fe7 100644 --- a/src/transforms/pipeline/stages/browser-server-exports-strip.ts +++ b/src/transforms/pipeline/stages/browser-server-exports-strip.ts @@ -25,14 +25,16 @@ * * - Only an exported declaration is emptied. A private helper called * `getServerData` is ordinary client code. - * - An import whose bindings all fall out of use because they were in the - * stripped hook's dependency closure is deleted. Reducing it to a side-effect - * import keeps the imported module in the browser graph, including any - * transitive server-only modules it reaches. Node built-ins and Veryfront - * framework imports are also deleted when unused, because their browser - * side-effect imports are known unsafe or unnecessary. Other already-unused - * imports are still reduced to side-effect imports for compatibility with the - * older conservative behavior. + * - An import the stripped hooks owned a binding of is deleted whole once no + * binding it declares is read any more, `import srv = require("./x.js")` + * included. Keeping any part of it, as a bare side-effect import or as the + * specifiers the hooks did not own, keeps the imported module in the browser + * graph along with every transitive server-only module it reaches. Node + * built-ins and Veryfront framework imports are also deleted when unused, + * because their browser side-effect imports are known unsafe or unnecessary. + * An import the hooks own nothing of is left exactly as authored, and the + * compile stage runs after this one and erases it when it is genuinely + * unused. See the ordering note in `stripServerOnlyExports`. * * Hooks are matched on the name they are *exported* under, not the name they * are declared with, because that is what the runtime looks up: the data @@ -60,17 +62,24 @@ import type { ASTNode, CodeParser } from "#veryfront/extensions/parser/index.ts" import type { TransformContext, TransformPlugin } from "../types.ts"; import { TransformStage } from "../types.ts"; import { - COMPILE_SOURCE_MAP_DIRECTIVE_METADATA, - COMPILE_SOURCE_MAP_INPUT_METADATA, -} from "./compile.ts"; + isEcmaScriptIdentifier, + isErasedNode, + isNode, + isReferenceChildKey, + type Node, + referenceChildren, + referenceClassOf, +} from "./reference-classification.ts"; /** Exports that only ever execute on the server. */ -const SERVER_ONLY_EXPORTS = ["getServerData", "getStaticData", "getStaticPaths"]; - -// The compile stage runs before this pass and embeds its input in a development -// sourcemap. Once a hook is stripped, any prior map is stale and may contain or -// point at a verbatim copy of the server-only source. Match only an actual -// trailing directive so source-map-like text inside authored strings survives. +/** The hook names this pass empties. Exported so tests read the real policy. */ +export const SERVER_ONLY_EXPORTS = ["getServerData", "getStaticData", "getStaticPaths"]; + +// This pass runs on pre-compile source, so no compiler map exists yet. A map +// directive can still be present in checked-in build output. Once a hook is +// stripped that map is stale and may point at a verbatim copy of the +// server-only source, so drop it. Match only an actual trailing directive so +// source-map-like text inside authored strings survives. const SOURCE_MAP_SUFFIX = /(^|\r?\n)[\t ]*\/\/[#@][\t ]*sourceMappingURL=[^"'`\s]+[\t ]*(?:\r?\n)?$/; @@ -78,22 +87,16 @@ function dropSourceMapSuffix(code: string): string { return code.replace(SOURCE_MAP_SUFFIX, "$1"); } -function appendSourceMapDirective(code: string, directive: string): string { - const separator = code.length > 0 && !code.endsWith("\n") ? "\n" : ""; - return `${code}${separator}${directive}\n`; -} - /** Source the stub nodes are lifted from, so no node shape is hand-built. */ const STUB_SOURCE = `function __vfStub() { throw new Error("server-only"); } const __vfStubInit = function () { throw new Error("server-only"); };`; -type Node = Record & { type: string }; - -function isNode(value: unknown): value is Node { - return typeof value === "object" && value !== null && - typeof (value as { type?: unknown }).type === "string"; -} - +/** + * Every child node, whatever position it sits in. Used for `var` hoisting, + * which has to find a declaration wherever it is written. Reference counting + * uses `referenceChildren` instead, which skips the positions that hold a fixed + * name rather than a reference. + */ function children(node: Node): Node[] { const found: Node[] = []; @@ -111,94 +114,51 @@ function children(node: Node): Node[] { } /** - * Walk every node in the tree. Returning `false` from `visit` skips the - * subtree, which is how import statements stay out of the reference count. + * Walk every node that can hold a runtime reference. Returning `false` from + * `visit` skips the subtree, which is how import statements stay out of the + * reference count. Positions that hold a fixed name are skipped by + * `referenceChildren` itself, so both walkers agree on them by construction. */ function walk(node: Node, visit: (node: Node) => boolean | void): void { if (visit(node) === false) return; - for (const child of children(node)) walk(child, visit); + for (const child of referenceChildren(node)) walk(child, visit); +} + +function nodeName(value: unknown): string | null { + if (!isNode(value)) return null; + const name = value.name; + return typeof name === "string" ? name : null; +} + +function exportName(value: unknown): string | null { + return isNode(value) ? (stringLiteralText(value) ?? nodeName(value)) : null; } /** - * TypeScript nodes that survive type erasure and emit runtime code. + * Whether `code` could name a server-only export. This only decides whether to + * parse, so it stays cheap, but it has to be sound in one direction: a false + * negative ships the hook, the secret it closes over and its server imports to + * the browser, and no later stage gets a second chance. * - * Everything else the TypeScript grammar adds is erased before the module - * runs, so an identifier read inside it is a type reference and must not keep a - * binding alive. Getting the split wrong is unsafe in both directions: treating - * a runtime node as erased deletes live code, and treating an erased node as - * runtime pins a server-only import into the browser artifact. + * Soundness comes from the grammar rather than from a list of spellings. An + * exported name is an IdentifierName or a StringLiteral, and in both the only + * way to write a character as anything other than itself is an escape sequence, + * which always starts with a backslash. A module whose text holds no backslash + * therefore spells every name it exports verbatim, and the substring test above + * settles it. Any backslash at all and this parses instead, which lets + * `exportName` read the name the way the runtime will. * - * The list is closed and enumerable, which is the point: it is a decidable - * question, unlike proving what a module does to an intrinsic. A TypeScript - * node type this pass does not know is erased by default. Any new TypeScript - * node type that emits runtime code must be added to this allowlist. + * The `export` test is sound for the same reason: a reserved word written with + * an escape is a syntax error, so `export` cannot hide either. * - * The split is invisible while this stage runs after the compile stage, which - * erases every TypeScript node before this pass sees the module. It exists so - * the stage stays correct when it runs on authored source. + * Do not narrow this to the escape forms someone has thought of. Reading raw + * source for a name the parser normalizes is what let + * `export { loadIt as get\u0053erverData }` through. */ -const RUNTIME_TS_NODE_TYPES = new Set([ - // Value expressions wrapping a value expression plus an erased type operand. - "TSAsExpression", - "TSSatisfiesExpression", - "TSNonNullExpression", - "TSTypeAssertion", - "TSInstantiationExpression", - // `enum E { A = compute() }` emits an object and evaluates each initialiser. - "TSEnumDeclaration", - "TSEnumBody", - "TSEnumMember", - // `namespace N { … }` with a body emits an IIFE over a runtime object. - "TSModuleDeclaration", - "TSModuleBlock", - // `constructor(private dep = fallback())` emits an assignment in the body. - "TSParameterProperty", - // `import L = require("./l.ts")` and `import A = N.Sub` both emit a binding. - "TSImportEqualsDeclaration", - "TSExternalModuleReference", - "TSQualifiedName", - // `export = handler` emits an assignment to the module export. - "TSExportAssignment", -]); - -/** - * Whether the compiler erases `node` and everything under it, so no identifier - * inside it is a runtime read. - * - * Both reference walkers ask this, and they must ask the same question. A - * walker that counts a type-position read as a runtime reference keeps the - * server-only import that binding came from; a walker that skips a runtime - * TypeScript node reports live code as dead. - */ -/** Whether a node carries decorators, which emit a runtime call even when the - * declaration they annotate is ambient. */ -function nodeHasDecorators(node: Node): boolean { - const decorators = node.decorators; - return Array.isArray(decorators) && decorators.length > 0; -} - -function isErasedTypeNode(node: Node): boolean { - // `declare const`, `declare class`, `declare namespace`, `declare enum` and - // `declare prop: T` are all ambient: they emit nothing. - // - // Decorators are the exception. Both tsc and esbuild emit a runtime - // `__decorate` call for `@audit declare id: string`, so the decorator - // expression is a real read even though the property it annotates is not. - // Erasing it here deletes the import the decorator needs and the emitted - // call then throws a ReferenceError at module evaluation. - if (node.declare === true) return !nodeHasDecorators(node); - // `import { type Cfg }`, `export { type Cfg }`, `export type { Cfg }`. - if (node.importKind === "type" || node.exportKind === "type") return true; - if (!node.type.startsWith("TS")) return false; - if (!RUNTIME_TS_NODE_TYPES.has(node.type)) return true; - // An ambient `declare module "x";` has no body to run. - return node.type === "TSModuleDeclaration" && !isNode(node.body); -} - -function nodeName(value: unknown): string | null { - if (!isNode(value)) return null; - const name = value.name; - return typeof name === "string" ? name : null; +function mayNameServerOnlyExport(code: string): boolean { + if (!/\bexport\b/.test(code)) return false; + if (SERVER_ONLY_EXPORTS.some((name) => code.includes(name))) return true; + return code.includes("\\"); } function bodyOf(ast: ASTNode): Node[] { @@ -333,13 +293,14 @@ function exportedHookBindings(body: Node[]): { locals: Set; unhandled: s for (const specifier of Array.isArray(statement.specifiers) ? statement.specifiers : []) { if (!isNode(specifier)) continue; if (specifier.exportKind === "type") continue; - if (!isHook(nodeName(specifier.exported))) continue; + const exported = exportName(specifier.exported); + if (!isHook(exported)) continue; // `export { x as getServerData } from "./loader"` never binds `x` here, // so there is no body to empty and the module it points at is still // pulled into the graph. if (isNode(statement.source)) { - unhandled.push(`export { … as ${nodeName(specifier.exported)} } from …`); + unhandled.push(`export { … as ${exported} } from …`); continue; } @@ -374,6 +335,25 @@ function exportedHookBindings(body: Node[]): { locals: Set; unhandled: s } } + // `import Loader = require("./server")` binds a hook's local name without + // being a function or variable declaration. `emptyServerOnlyHooks` has + // nothing to stub, so it reports no change and the caller returns the source + // untouched -- shipping the server module and its whole graph to the browser, + // which is the exact failure this pass exists to prevent. Rejecting is the + // same fail-closed answer the shapes above get, and it is louder than a + // silent pass-through. + // + // Reachable only since this pass moved ahead of `compilePlugin`: running + // after compile, esbuild had already lowered the alias to a `const` that the + // variable-declaration path stubbed. + for (const statement of body) { + if (statement.type !== "TSImportEqualsDeclaration") continue; + const name = nodeName(statement.id); + if (name && (locals.has(name) || (statement.isExport === true && isHook(name)))) { + unhandled.push(`import ${name} = require(…)`); + } + } + return { locals, unhandled }; } @@ -428,8 +408,10 @@ function emptyServerOnlyHooks( /** * Identifiers the module reads, ignoring import statements and the positions * where an identifier is a fixed name rather than a reference (`a.hashOf`, - * `{ hashOf: 1 }`). This flat walk is used for module-declaration liveness; - * import liveness uses the scope-aware walker below. + * `{ hashOf: 1 }`). Which positions those are is + * `reference-classification.ts`, shared with the scope-aware walker below so + * the two cannot drift. This flat walk is used for module-declaration + * liveness; import liveness uses the scope-aware walker. * * `excluded` holds identifier nodes that are binding *positions* rather than * references (the `id` a declaration introduces), so a declaration is not @@ -437,27 +419,13 @@ function emptyServerOnlyHooks( */ function referencedIdentifiers(body: Node[], excluded?: WeakSet): Set { const referenced = new Set(); - // Filled in as each parent is visited, which always happens before its - // children. + // An enum member initialiser can name a sibling member without qualifying it. + // That is the one fixed-name position the shared classification cannot + // express as a child key, because it depends on the names the enclosing enum + // declares. Filled in as each parent is visited, which always happens before + // its children. const fixedNames = new WeakSet(); - const markFixedName = (node: Node): void => { - const property = node.type === "MemberExpression" || node.type === "OptionalMemberExpression" - ? node.property - : node.type === "ObjectProperty" || node.type === "ObjectMethod" || - node.type === "ClassMethod" || node.type === "ClassProperty" || - node.type === "ClassAccessorProperty" - ? node.key - : node.type === "TSEnumDeclaration" || node.type === "TSEnumMember" - ? node.id - : node.type === "TSQualifiedName" - ? node.right - : undefined; - - if (node.computed === true) return; - if (isNode(property)) fixedNames.add(property); - }; - const markEnumLocalReferences = (node: Node): void => { if (node.type !== "TSEnumDeclaration") return; const container = isNode(node.body) ? node.body : node; @@ -490,12 +458,13 @@ function referencedIdentifiers(body: Node[], excluded?: WeakSet): Set { node.type !== "TSEnumDeclaration" && node.type !== "TSModuleDeclaration" && node.type !== "TSImportEqualsDeclaration" ) return false; - if (!isErasedTypeNode(node)) bindPatternNames(scope, node.id); + if (!isErasedNode(node)) bindPatternNames(scope, node.id); return true; }; @@ -653,8 +643,12 @@ function freeReferencedIdentifiers(root: Node): Set { } }; + // The generic fallback. It descends only through the child keys the shared + // classification allows, so a fixed-name position is skipped here exactly as + // it is in the flat walker. Rounds three and four of review both fell through + // this fallback while it descended into everything. const visitChildren = (node: Node, scopes: LexicalScope[]): void => { - for (const child of children(node)) visit(child, scopes); + for (const child of referenceChildren(node)) visit(child, scopes); }; const visitDecorators = (node: Node, scopes: LexicalScope[]): void => { @@ -760,12 +754,6 @@ function freeReferencedIdentifiers(root: Node): Set { } }; - const visitObjectMember = (node: Node, scopes: LexicalScope[]): void => { - visitDecorators(node, scopes); - if (node.computed === true && isNode(node.key)) visit(node.key, scopes); - if (isNode(node.value)) visit(node.value, scopes); - }; - const visitFor = (node: Node, scopes: LexicalScope[]): void => { const loopScope: LexicalScope = { kind: "block", names: new Set() }; const scoped = [loopScope, ...scopes]; @@ -801,14 +789,20 @@ function freeReferencedIdentifiers(root: Node): Set { // Same classification `referencedIdentifiers` uses. A value-emitting // TypeScript node such as an enum or a namespace body falls through to the // generic walk below, and its erased type operand is skipped there in turn. - if (isErasedTypeNode(node)) return; + if (isErasedNode(node)) return; - if (node.type === "Identifier" || node.type === "JSXIdentifier") { + if (referenceClassOf(node) === "read") { const name = nodeName(node); if (name && !isLexicallyBound(name, scopes)) free.add(name); return; } + // Every JSX position that holds a name rather than a reference is a child + // key the shared classification already skipped, so an intrinsic tag name, + // a namespaced name, an attribute name and the property of `` + // never reach the branch above. Everything else in a JSX tree is ordinary + // expression syntax and reaches the generic walk below. + if ( node.type === "Program" || node.type === "BlockStatement" || node.type === "TSModuleBlock" @@ -935,23 +929,25 @@ function freeReferencedIdentifiers(root: Node): Set { return; } - if (node.type === "MemberExpression" || node.type === "OptionalMemberExpression") { - if (isNode(node.object)) visit(node.object, scopes); - if (node.computed === true && isNode(node.property)) visit(node.property, scopes); - return; - } - + // A property holds no scope of its own, so the classification alone decides + // which of its children are reads: a computed key, the value, and any + // decorator. if ( node.type === "ObjectProperty" || node.type === "ClassProperty" || - node.type === "ClassAccessorProperty" + node.type === "ClassAccessorProperty" || node.type === "ClassPrivateProperty" ) { - visitObjectMember(node, scopes); + visitChildren(node, scopes); return; } - if (node.type === "ObjectMethod" || node.type === "ClassMethod") { + if ( + node.type === "ObjectMethod" || node.type === "ClassMethod" || + node.type === "ClassPrivateMethod" + ) { visitDecorators(node, scopes); - if (node.computed === true && isNode(node.key)) visit(node.key, scopes); + // `{ [key]: v }` reads `key`; `{ key: v }` does not. The classification + // owns that distinction for both walkers. + if (isReferenceChildKey(node, "key") && isNode(node.key)) visit(node.key, scopes); visitFunction(node, scopes); return; } @@ -1015,9 +1011,8 @@ function hookReferencedIdentifiers(body: Node[], targets: Set): Set; @@ -1085,11 +1080,14 @@ function isNameDescriptor(node: Node | undefined, valueParam: string): boolean { } /** - * Bindings for esbuild's `keepNames` helper. Release modules are compiled - * before the browser transform, so their declarations are followed by calls - * like `__name(loadPage, "loadPage")`. Recognise the helper by its exact - * `Object.defineProperty(target, "name", …)` semantics rather than by its - * minified binding name. + * Bindings for esbuild's `keepNames` helper: a declaration in compiled output + * is followed by a call like `__name(loadPage, "loadPage")`. Recognise the + * helper by its exact `Object.defineProperty(target, "name", ...)` semantics + * rather than by its minified binding name. + * + * Neither caller feeds this pass compiled input any more: the browser pipeline + * now runs it before the compile stage, and the code splitter reads raw project + * files. It is kept for a module that already carries checked-in build output. */ function compilerNameHelperBindings(body: Node[]): Set { const initializers = new Map(); @@ -1164,6 +1162,82 @@ function compilerNameRegistrations(body: Node[]): CompilerNameRegistration[] { return registrations; } +/** + * Factory bindings the classic JSX pragmas make the compiled module call. + * + * The pipeline compiles with `jsx: "automatic"`, but a module can opt itself + * out with `@jsxRuntime classic`, and esbuild then emits calls to the factory + * `@jsx` names (`h("div", null)`) and the fragment `@jsxFrag` names. That + * lowering happens in the compile stage, which now runs AFTER this one, so at + * this point nothing in the module reads those names and the import that + * provides them looks dead: `dropUnusedImportBindings` deletes it and the page + * dies on `h is not defined`. + * + * Pinning the pragma targets is preferred over the blunter alternative of + * skipping import pruning for any classic-pragma module. Skipping would leave + * every import of such a module untouched, a genuinely server-only one + * included, which reopens the leak this whole pass exists to close. Pinning + * keeps exactly the bindings the emitted factory calls reference. + * + * Two boundaries, both checked against esbuild's own behaviour: + * + * - The pragmas only take effect under the classic runtime. `@jsx h` on its own + * is ignored and the automatic runtime is used, so pinning `h` there would + * keep an import nothing calls, and a `@jsx` word inside an unrelated comment + * could pin a server-only binding. + * - `@jsxRuntime classic` with no `@jsx` falls back to esbuild's default + * `React.createElement` / `React.Fragment`, so the classic runtime always + * pins at least `React`. That is the fail-closed half: a classic module never + * loses its factory import, whatever it did or did not name. + * + * `@jsxImportSource` names a module for the automatic runtime, and esbuild + * writes that import itself, so it pins no binding here. + */ +const JSX_PRAGMA_RUNTIME = /@jsxRuntime\s+(\S+)/; +const JSX_PRAGMA_FACTORY = /@jsx\s+(\S+)/; +const JSX_PRAGMA_FRAGMENT = /@jsxFrag\s+(\S+)/; +/** esbuild's classic factory when the module names none. */ +const DEFAULT_CLASSIC_FACTORY_ROOT = "React"; + +/** + * The binding a pragma expression reads: `React.createElement` reads `React`. + * + * The identifier test is the Unicode one, shared with + * {@link isIntrinsicJsxName}. esbuild parses `@jsx Ħ.créate` and emits + * `Ħ.créate("div", null)`, so an ASCII-only test drops the pin and + * `dropUnusedImportBindings` then deletes the factory import the emitted call + * needs. A pragma esbuild cannot parse as an expression is not pinned here + * either: esbuild ignores it and falls back to `React.createElement`, which + * `DEFAULT_CLASSIC_FACTORY_ROOT` already pins. + */ +export function pragmaRootBinding(expression: string | undefined): string | null { + const root = expression?.split(/[.([]/)[0]?.trim(); + return root && isEcmaScriptIdentifier(root) ? root : null; +} + +export function jsxPragmaBindings(ast: ASTNode): Set { + const pinned = new Set(); + const comments = (ast as { comments?: unknown }).comments; + if (!Array.isArray(comments)) return pinned; + + const texts: string[] = []; + for (const comment of comments) { + const text = isNode(comment) ? comment.value : undefined; + if (typeof text === "string" && text.includes("@jsx")) texts.push(text); + } + if (!texts.some((text) => JSX_PRAGMA_RUNTIME.exec(text)?.[1] === "classic")) return pinned; + + for (const text of texts) { + for (const pattern of [JSX_PRAGMA_FACTORY, JSX_PRAGMA_FRAGMENT]) { + const root = pragmaRootBinding(pattern.exec(text)?.[1]); + if (root) pinned.add(root); + } + } + pinned.add(DEFAULT_CLASSIC_FACTORY_ROOT); + + return pinned; +} + /** * Drop the top-level declarations the emptied server-only hooks closed over. * @@ -1178,7 +1252,11 @@ function compilerNameRegistrations(body: Node[]): CompilerNameRegistration[] { * fixpoint: removing one binding can leave a helper it was the last user of * newly dead *within the closure*. */ -function dropUnusedModuleScopeBindings(body: Node[], hookClosure: Set): Node[] { +function dropUnusedModuleScopeBindings( + body: Node[], + hookClosure: Set, + pinned: Set, +): Node[] { let current = body; for (;;) { @@ -1202,7 +1280,7 @@ function dropUnusedModuleScopeBindings(body: Node[], hookClosure: Set): const removedDecls: ModuleScopeDecl[] = []; for (const decl of decls) { const inClosure = decl.names.every((name) => hookClosure.has(name)); - const unused = decl.names.every((name) => !referenced.has(name)); + const unused = decl.names.every((name) => !referenced.has(name) && !pinned.has(name)); if (!inClosure || !unused) continue; removedDecls.push(decl); @@ -1266,7 +1344,7 @@ function importedBindings(statement: Node): string[] { // `import { hashOf, type Cfg }`: `Cfg` is erased before the module runs, so // it is not a binding that has to be kept alive. Counting it would stop // `hashOf` alone from proving the import hook-only, and the statement would - // be reduced to a side-effect import instead of deleted. + // survive as `import { type Cfg }` instead of being deleted. if (specifier.importKind === "type") continue; const name = nodeName(specifier.local); if (name) bindings.push(name); @@ -1276,14 +1354,19 @@ function importedBindings(statement: Node): string[] { } /** - * Drop imports nothing references any more when their bindings are in the - * stripped hook's dependency closure, or when their source is known unsafe or - * unnecessary as a browser side-effect import. Keeping a hook-only import as a - * bare side-effect import would keep its transitive graph in the browser - * artifact, which is exactly what this stage strips. Other unused imports keep - * the legacy conservative side-effect rewrite. + * Drop imports nothing references any more when the stripped hooks owned any of + * their bindings, or when their source is known unsafe or unnecessary as a + * browser side-effect import. Keeping a hook-only import in any form would keep + * its transitive graph in the browser artifact, which is exactly what this + * stage strips. An import whose bindings the hooks never owned keeps every one + * of those bindings exactly as authored, and a bare `import "./x.ts"` written + * by hand is never touched. */ -function dropUnusedImportBindings(body: Node[], hookClosure: Set): Node[] { +function dropUnusedImportBindings( + body: Node[], + hookClosure: Set, + pinned: Set, +): Node[] { // Import liveness must be scope-aware. A local enum member, namespace, // parameter property, or ordinary nested binding can share a spelling with // an import without reading that imported binding. @@ -1296,28 +1379,87 @@ function dropUnusedImportBindings(body: Node[], hookClosure: Set): Node[ const bindings = importedBindings(statement); // Already a side-effect import: nothing to drop. if (bindings.length === 0) return true; - if (bindings.some((binding) => referenced.has(binding))) return true; + // A binding still read by surviving code keeps the statement, and keeps it + // with every specifier it was authored with. + if (bindings.some((binding) => referenced.has(binding) || pinned.has(binding))) return true; const source = isNode(statement.source) ? statement.source.value : undefined; const isKnownDroppableSource = typeof source === "string" && (source.startsWith("node:") || source === "veryfront" || source.startsWith("veryfront/")); - // Two different reasons to delete rather than reduce, and one to reduce. - // A node: or veryfront source is unsafe or pointless as a browser - // side-effect import whatever used it. A project-relative source is deleted - // only when the stripped hook owned every binding, because that module is - // reached solely through server-only code and a bare side-effect import - // would keep its whole transitive graph in the browser artifact. An import - // the hook never touched was already unused before this pass ran, so it - // keeps the legacy reduction and its side effects with it. - if (isKnownDroppableSource || bindings.every((binding) => hookClosure.has(binding))) { + // Two reasons to delete the whole statement. A node: or veryfront source is + // unsafe or pointless as a browser side-effect import whatever used it. Any + // other source goes once the stripped hooks owned a binding of it and no + // binding it declares is read any more: that module is reached solely + // through server-only code, so nothing of the statement may survive. + // + // Trimming it to the specifiers the hooks did not own is not enough, and + // the loader decides whether that is visible. This stage emits the same + // text whatever the extension is; the compile stage that runs after it + // erases an unused import only under the `ts` and `tsx` loaders, where a + // specifier may name a type. Under `js`, `jsx`, `md` and `mdx` + // (`getLoaderFromPath` in `src/transforms/esm/transform-utils.ts`) esbuild + // must preserve the module for its side effects, so it rewrites the + // remainder into exactly the bare `import "./x.js"` this stage must never + // ship. Deleting the statement is also what the pass did when it ran after + // compile, so the artifact matches the ordering this one replaced. + if (isKnownDroppableSource || bindings.some((binding) => hookClosure.has(binding))) { return false; } - statement.specifiers = []; + // Anything else is an import this pass does not own a single binding of, + // so it is left exactly as authored, unused specifiers included. See the + // ordering note at the call site: rewriting the statement here would turn + // an import the compiler can erase into one it must preserve, which is how + // a server-only module leaks into the browser. return true; }); } +/** + * Move the leading comments of removed statements onto the next surviving one. + * + * Babel attaches a file's opening comments to its first statement, so removing + * that statement takes them with it. Before the reorder this pass ran on + * compiled output, where esbuild had already consumed the pragmas and hoisted + * what it kept. On authored source a `@jsxImportSource` pragma above a + * hook-only import would silently switch the JSX factory, and legal banners and + * lint suppressions would disappear the same way. + */ +function retainLeadingComments(before: Node[], after: Node[]): Node[] { + const kept = new Set(after); + let orphaned: unknown[] = []; + let lastKept: Node | undefined; + + for (const statement of before) { + if (!kept.has(statement)) { + const comments = statement.leadingComments; + if (Array.isArray(comments)) orphaned = [...orphaned, ...comments]; + continue; + } + lastKept = statement; + if (orphaned.length === 0) continue; + const existing = Array.isArray(statement.leadingComments) ? statement.leadingComments : []; + statement.leadingComments = [ + ...orphaned, + ...existing.filter((comment) => !orphaned.includes(comment)), + ]; + orphaned = []; + } + + // Comments after the last surviving statement have nowhere to lead, so they + // trail it instead. Without this a legal banner between two removed tail + // declarations is dropped. + if (orphaned.length > 0 && lastKept !== undefined) { + const existing = Array.isArray(lastKept.trailingComments) ? lastKept.trailingComments : []; + lastKept.trailingComments = [ + ...existing.filter((comment) => !orphaned.includes(comment)), + ...orphaned, + ]; + } + + return after; +} + function setBody(ast: ASTNode, body: Node[]): void { const program = (ast as { program?: unknown }).program; const target: Node = isNode(program) ? program : ast; @@ -1354,8 +1496,8 @@ export async function stripServerOnlyExports( code: string, filePath?: string, ): Promise { - // Cheap pre-check: no mention of a hook means no parse. - if (!SERVER_ONLY_EXPORTS.some((name) => code.includes(name))) return code; + // Cheap pre-check: no plausible server-only export shape means no parse. + if (!mayNameServerOnlyExport(code)) return code; const parser = tryResolve("CodeParser"); if (!parser) { @@ -1395,8 +1537,41 @@ export async function stripServerOnlyExports( // Drop the module-scope state the emptied hooks were the last user of, then // the imports that leaves unused. Order matters: pruning `const API_KEY = // getEnv(...)` is what makes the `veryfront` import droppable. - const pruned = dropUnusedModuleScopeBindings(body, hookClosure); - setBody(ast, dropUnusedImportBindings(pruned, hookClosure)); + // + // PIPELINE ORDERING DEPENDENCY. `dropUnusedImportBindings` leaves an import + // it does not own exactly as authored, including a named import nothing + // references any more. That is only safe because + // `browserServerExportsStripPlugin` runs at `TransformStage.PARSE + 0.5`, so + // `compilePlugin` (`TransformStage.COMPILE`) runs AFTER this pass and esbuild + // erases the import it now finds unused. Move this pass back after compile + // and nothing erases those imports: the module ships to the browser with its + // whole transitive graph. Any change to that ordering must either keep a + // stage that erases unused imports after this one or restore the old + // reduction to a bare side-effect import here. + // + // The erasure is TypeScript unused-import elision under the `ts` and `tsx` + // loaders, not tree shaking, so `treeShaking: !ctx.dev` in `compile.ts` does + // not gate it and development output matches production. It is also why an + // import the hooks own a binding of is deleted outright whatever the loader: + // under `js`, `jsx`, `md` and `mdx` nothing elides, and see the comment in + // `dropUnusedImportBindings` for what esbuild does with the remainder there. + // + // The other caller, `createSplitterPlugin` in + // `src/build/bundler/code-splitter/esbuild-plugin.ts`, meets the same + // requirement a different way: it strips inside an esbuild `onLoad` hook, so + // esbuild bundles and tree-shakes the stripped module afterwards. + // Bindings the compile stage will call on this module's behalf, which no + // statement reads yet. See `jsxPragmaBindings`. + const pinned = jsxPragmaBindings(ast); + + const pruned = retainLeadingComments( + body, + dropUnusedModuleScopeBindings(body, hookClosure, pinned), + ); + setBody( + ast, + retainLeadingComments(pruned, dropUnusedImportBindings(pruned, hookClosure, pinned)), + ); const generated = await parser.generate(ast); return dropSourceMapSuffix(generated.code); @@ -1404,18 +1579,15 @@ export async function stripServerOnlyExports( export const browserServerExportsStripPlugin: TransformPlugin = { name: "browser-server-exports-strip", - // After esbuild compile and CSS strip, before any import resolution, so the - // dropped bindings are never rewritten or pre-fetched. - stage: TransformStage.COMPILE + 0.6, + // After the MDX parse, before the esbuild compile, so the hook body never + // reaches the compiler: no keepNames helper is emitted for a declaration this + // pass removes, and the compile sourcemap is built from stripped input. The + // array position in BROWSER_PIPELINE and this stage number must agree, + // because a registered custom plugin re-sorts the pipeline by stage. + // `stripServerOnlyExports` also depends on compile running after this stage + // to erase the imports it leaves unused. Read the ordering note there before + // changing this number. + stage: TransformStage.PARSE + 0.5, condition: (ctx: TransformContext) => ctx.target === "browser", - transform: async (ctx: TransformContext) => { - const directive = ctx.metadata.get(COMPILE_SOURCE_MAP_DIRECTIVE_METADATA); - const compileInput = ctx.metadata.get(COMPILE_SOURCE_MAP_INPUT_METADATA); - ctx.metadata.delete(COMPILE_SOURCE_MAP_DIRECTIVE_METADATA); - ctx.metadata.delete(COMPILE_SOURCE_MAP_INPUT_METADATA); - const result = await stripServerOnlyExports(ctx.code, ctx.filePath); - return result === ctx.code && compileInput === ctx.code && typeof directive === "string" - ? appendSourceMapDirective(result, directive) - : result; - }, + transform: (ctx: TransformContext) => stripServerOnlyExports(ctx.code, ctx.filePath), }; diff --git a/src/transforms/pipeline/stages/compile.ts b/src/transforms/pipeline/stages/compile.ts index ef36c61ab2..5a3c840eed 100644 --- a/src/transforms/pipeline/stages/compile.ts +++ b/src/transforms/pipeline/stages/compile.ts @@ -15,18 +15,6 @@ const ReflectApply = Reflect.apply; const ReflectGetOwnPropertyDescriptor = Reflect.getOwnPropertyDescriptor; const SOURCE_MAP_SUFFIX = /(^|\r?\n)[\t ]*\/\/[#@][\t ]*sourceMappingURL=[^"'`\s]+[\t ]*(?:\r?\n)?$/; -export const COMPILE_SOURCE_MAP_DIRECTIVE_METADATA = "compileSourceMapDirective"; -export const COMPILE_SOURCE_MAP_INPUT_METADATA = "compileSourceMapInput"; - -function trailingSourceMapDirective(code: string): string | undefined { - const match = SOURCE_MAP_SUFFIX.exec(code); - return match?.[0].slice((match[1] ?? "").length).trimEnd(); -} - -function dropTrailingSourceMapDirective(code: string): string { - return code.replace(SOURCE_MAP_SUFFIX, "$1"); -} - function appendBeforeSourceMap(code: string, addition: string): string { const match = SOURCE_MAP_SUFFIX.exec(code); if (match?.index === undefined) return code + addition; @@ -108,27 +96,10 @@ export const compilePlugin: TransformPlugin = { /\bconst\s+MDXLayout\b/.test(code) && !/export\s+\{[^}]*MDXLayout/.test(code) ) { - // Keep esbuild's directive last. Browser server-hook stripping removes - // a stale compile map after rewriting, and a framework export appended - // after the directive would otherwise hide that suffix. + // Keep esbuild's directive last. code = appendBeforeSourceMap(code, "\nexport { MDXLayout };\n"); } - const sourceMapDirective = trailingSourceMapDirective(code); - if (ctx.target === "browser" && sourceMapDirective) { - // Keep the compiler map out of intermediate browser plugins. The - // server-export strip stage restores it only when the map-free compile - // output is unchanged and no hook is rewritten. This makes the real - // comment unambiguous even if a plugin copies its text into a string, - // appends code, or removes the hook itself. - ctx.metadata.set(COMPILE_SOURCE_MAP_DIRECTIVE_METADATA, sourceMapDirective); - code = dropTrailingSourceMapDirective(code); - ctx.metadata.set(COMPILE_SOURCE_MAP_INPUT_METADATA, code); - } else { - ctx.metadata.delete(COMPILE_SOURCE_MAP_DIRECTIVE_METADATA); - ctx.metadata.delete(COMPILE_SOURCE_MAP_INPUT_METADATA); - } - return code; } catch (err) { const errorMsg = err instanceof Error ? err.message : String(err); diff --git a/src/transforms/pipeline/stages/compiler-predicates.test.ts b/src/transforms/pipeline/stages/compiler-predicates.test.ts new file mode 100644 index 0000000000..a198a32181 --- /dev/null +++ b/src/transforms/pipeline/stages/compiler-predicates.test.ts @@ -0,0 +1,1030 @@ +/** + * Differential tests for the predicates this stage uses to stand in for the + * compiler. + * + * `reference-classification.test.ts` pins the node-type TABLE: it fails when + * `@babel/types` gains a type nobody classified. It cannot fail when a + * PREDICATE that approximates a compiler rule drifts from the compiler, because + * the table says nothing about what the rule is supposed to be. + * + * Every test here compares a predicate against the thing it approximates, + * measured on the real artifact, rather than against a second hand-written + * expectation of the same rule. A hand-written expectation restates the + * predicate and passes whether or not either one is right. Asking esbuild what + * it emitted fails when the two disagree, whoever moved. + * + * The predicates covered, and the rule each one stands in for: + * + * | predicate | rule it approximates | + * |---|---| + * | `isIntrinsicJsxName` | esbuild's tag-text test for a JSX element name | + * | `pragmaRootBinding`, `jsxPragmaBindings` | esbuild's classic JSX pragma parsing | + * | `referenceClassOf`'s `declare` short-circuit | ambient declarations emit nothing | + * | its `nodeHasDecorators` exception | a decorator on an ambient member still emits | + * | its `importKind` / `exportKind` short-circuits | type-only elision | + * | `RUNTIME_TS_NODE_TYPES` / the erased TypeScript list | TypeScript type erasure | + * | `importedBindings`' type-specifier skip | type-only elision, same family | + * | `compilerNameHelperBindings` | esbuild's `keepNames` helper shape | + * | `SOURCE_MAP_SUFFIX` in the strip | esbuild's sourcemap comment | + * | `mayNameServerOnlyExport` | the export names esbuild reads off the module | + * + * Three rules in this stage are deliberately not here, each for a different + * reason. + * + * - `freeReferencedIdentifiers` and `patternBoundNames` approximate ECMAScript + * SCOPING. No compiler reports a scope resolution in its output: esbuild + * resolves scopes internally and the artifact never names the binding a + * reference resolved to, so there is no answer to compare against. Renaming + * under `minify` is the closest observable proxy and it is not the same + * question, because esbuild is free to rename or not. That pair is covered by + * behavioural fixtures in `browser-server-exports-strip.test.ts` instead. + * - `retainLeadingComments` approximates the PARSER's comment attachment, not + * the compiler's. Its differential partner would be Babel, which is also the + * tool that produced the input, so a comparison would be circular. It is + * covered by `keeps a jsx pragma that sits above a removed import`. + * - `SERVER_ONLY_EXPORTS` and `isKnownDroppableSource` are framework policy. + * Nothing outside this repository owns the rule, so there is nothing to be + * differential against. The SET of hook names is policy; deciding whether a + * module exports one of them is not, and that half has its own oracle below. + * Keep the two apart: conflating them is how a raw-text gate that could not + * see an escaped export name went unnoticed. + * + * @module transforms/pipeline/stages/compiler-predicates.test + */ + +import "#veryfront/schemas/_test-setup.ts"; +import "../../plugins/__tests__/code-parser-setup.ts"; +import { stop as stopEsbuild, transform } from "#veryfront/platform/compat/esbuild.ts"; +import { assertEquals, assertStringIncludes } from "#veryfront/testing/assert.ts"; +import { afterAll, describe, it } from "#veryfront/testing/bdd.ts"; +import { tryResolve } from "#veryfront/extensions/contracts.ts"; +import type { ASTNode, CodeParser } from "#veryfront/extensions/parser/index.ts"; +import { ESBUILD_SUPPORTED_FEATURES } from "../../esm/transform-utils.ts"; +import { isEcmaScriptIdentifier, isIntrinsicJsxName } from "./reference-classification.ts"; +import { + jsxPragmaBindings, + moduleReferenceWalkers, + pragmaRootBinding, + SERVER_ONLY_EXPORTS, + stripServerOnlyExports, +} from "./browser-server-exports-strip.ts"; +import { runPipeline } from "../index.ts"; + +type Node = Record & { type: string }; + +function isNode(value: unknown): value is Node { + return typeof value === "object" && value !== null && + typeof (value as { type?: unknown }).type === "string"; +} + +function parser(): CodeParser { + const found = tryResolve("CodeParser"); + if (!found) throw new Error("no CodeParser extension is registered"); + return found; +} + +function walk(node: Node, visit: (node: Node) => void): void { + visit(node); + for (const value of Object.values(node)) { + if (Array.isArray(value)) { + for (const entry of value) if (isNode(entry)) walk(entry, visit); + continue; + } + if (isNode(value)) walk(value, visit); + } +} + +/** + * The compile stage's own esbuild settings, minus minification and source maps. + * + * Kept equal to `compile.ts` in every field that decides what survives into the + * artifact, so a verdict measured here is the verdict the pipeline gets. + * `treeShaking` is off because the question is what the COMPILER erases, not + * what the bundler later drops as unreachable. + */ +const COMPILE_OPTIONS = { + format: "esm", + target: "es2022", + supported: ESBUILD_SUPPORTED_FEATURES, + jsx: "automatic", + jsxImportSource: "react", + minify: false, + sourcemap: false, + treeShaking: false, + keepNames: true, +} as const; + +/* ------------------------------------------------------------------ * + * Oracle 1: does the artifact treat a JSX element name as string text? * + * ------------------------------------------------------------------ */ + +type ElementNameVerdict = "tag-text" | "binding-read" | "rejected"; + +/** + * What esbuild emitted for ``, read off the artifact. + * + * The classic runtime is used with a factory this fixture owns, so the emitted + * call is unambiguous: `__vfFactory("div", null)` passed string text and + * `__vfFactory(Card, null)` passed a binding. The first argument is read from + * the parsed artifact rather than matched as text, because esbuild escapes + * every non-ASCII code point in both positions (`"caf\xE9"` and `Café`) + * and a text match cannot tell those apart. + */ +async function esbuildElementNameVerdict(name: string): Promise { + let code: string; + try { + const result = await transform(`<${name} />;`, { + ...COMPILE_OPTIONS, + loader: "jsx", + jsx: "transform", + jsxFactory: "__vfFactory", + }); + code = result.code; + } catch { + return "rejected"; + } + + const ast = await parser().parse({ code, filePath: "artifact.js" }); + let verdict: ElementNameVerdict | null = null; + walk(ast as unknown as Node, (node) => { + if (verdict !== null || node.type !== "CallExpression") return; + const args = Array.isArray(node.arguments) ? node.arguments.filter(isNode) : []; + const first = args[0]; + if (!first) return; + verdict = first.type === "StringLiteral" ? "tag-text" : "binding-read"; + }); + if (verdict === null) throw new Error(`no factory call in the artifact for <${name} />`); + return verdict; +} + +/** Whether the pipeline's parser accepts `name` as a bare JSX element name. */ +async function parserAcceptsElementName(name: string): Promise { + try { + await parser().parse({ code: `<${name} />;`, filePath: "page.tsx" }); + return true; + } catch { + return false; + } +} + +/** + * The corpus. Every entry is a name esbuild and the parser both accept, so the + * differential assertion is meaningful for all of them. The two families where + * the two tools disagree get their own tests below. + */ +const ELEMENT_NAMES: readonly string[] = [ + // ASCII lowercase: intrinsic tags, including the spellings that are not + // identifiers at all. + "div", + "table", + "a", + "x1", + "aB", + "x-", + "my-widget", + "data-x", + // ASCII uppercase: component references. + "Card", + "A", + "X1", + "AB", + // `_` and `$` are identifier starts by convention, not by Unicode property. + "_", + "$", + "_Card", + "$Card", + "_1", + "$1", + // Latin-1 accents. `café` is a valid identifier AND tag text, because the + // ASCII-lowercase clause fires first; `Café` is a component. + "café", + "Café", + "él", + "Él", + // A combining mark in an ID_Continue position, precomposed and decomposed. + "é", + "É", + "X́", + // Greek and Cyrillic: a lowercase letter that is not ASCII lowercase, so + // esbuild reads a binding however the reader would pronounce the case. + "Ωmega", + "ωmega", + "Привет", + "привет", + // CJK: no case at all. + "日本語", + "コンポーネント", + // ID_Start code points outside the letter categories. + "ℕat", + "Ⅻ", + "℘", + // ZWJ and ZWNJ are permitted in ID_Continue positions and nowhere else. + "A‍b", + "A‌b", +]; + +/** Names neither tool accepts, so nothing in the pipeline can ever see them. */ +const UNPARSEABLE_ELEMENT_NAMES: readonly string[] = [ + "\u{1F389}", + "emoji\u{1F389}", + "́x", + "-x", +]; + +describe("compiler predicates, measured differentially", () => { + afterAll(async () => { + await stopEsbuild(); + }); + + describe("isIntrinsicJsxName against esbuild", () => { + for (const name of ELEMENT_NAMES) { + it(`agrees with the artifact for <${name} />`, async () => { + const verdict = await esbuildElementNameVerdict(name); + assertEquals( + verdict === "rejected", + false, + `esbuild rejected <${name} />, so this name belongs in another corpus`, + ); + assertEquals( + isIntrinsicJsxName(name), + verdict === "tag-text", + `esbuild emitted a ${verdict} for <${name} />`, + ); + }); + } + + for (const name of UNPARSEABLE_ELEMENT_NAMES) { + it(`is never asked about <${name} />, which neither tool parses`, async () => { + assertEquals(await esbuildElementNameVerdict(name), "rejected"); + assertEquals(await parserAcceptsElementName(name), false); + }); + } + + // The predicate has to be able to disagree with the artifact, or the loop + // above proves nothing. This is the skew the ASCII-only spelling had. + it("disagrees with the artifact when the identifier test is ASCII-only", async () => { + const asciiOnly = (name: string) => + /^[a-z]/.test(name) || !/^[A-Za-z_$][A-Za-z0-9_$]*$/.test(name); + + const skewed: string[] = []; + for (const name of ELEMENT_NAMES) { + const verdict = await esbuildElementNameVerdict(name); + if (asciiOnly(name) !== (verdict === "tag-text")) skewed.push(name); + } + + assertEquals(skewed.length > 0, true, "the ASCII-only predicate must fail this corpus"); + }); + + // The two tools do not have the same Unicode tables. Both directions are + // recorded, because both are safe and neither is obvious. + it("retains the binding for a code point newer than esbuild's tables", async () => { + // U+1C8A, added in Unicode 14. The parser accepts it, esbuild does not, + // so the module fails to compile with a diagnostic. The predicate must + // still answer "binding read": the over-retaining direction leaves a + // loud build error, and the other one would delete an import first. + const name = "ᲊ"; + + assertEquals(await parserAcceptsElementName(name), true); + assertEquals(await esbuildElementNameVerdict(name), "rejected"); + assertEquals(isIntrinsicJsxName(name), false); + }); + + it("never sees an astral element name, which the parser rejects first", async () => { + // U+1D49E. esbuild reads a binding here, but the parser refuses the name, + // so the strip stage fails the build before the predicate is consulted. + const name = "\u{1D49E}ard"; + + assertEquals(await esbuildElementNameVerdict(name), "binding-read"); + assertEquals(await parserAcceptsElementName(name), false); + }); + }); + + describe("jsxPragmaBindings against esbuild", () => { + /** + * The bindings esbuild's artifact needs from the module, read off the + * artifact. + * + * The fixture holds intrinsic tags and a fragment only, so every call in + * the output is a JSX factory call. A factory root is free when the + * artifact does not import it: under the automatic runtime esbuild writes + * its own `jsx` import, and under the classic runtime it writes nothing and + * the module has to provide the name. + */ + async function esbuildFactoryRoots(pragma: string): Promise { + const source = `${pragma}\nexport const el =
;\n` + + `export const frag = <>;`; + const { code } = await transform(source, { ...COMPILE_OPTIONS, loader: "tsx" }); + const ast = await parser().parse({ code, filePath: "artifact.js" }); + + const imported = new Set(); + const roots = new Set(); + walk(ast as unknown as Node, (node) => { + if (node.type === "ImportDeclaration") { + for (const specifier of Array.isArray(node.specifiers) ? node.specifiers : []) { + if (!isNode(specifier) || !isNode(specifier.local)) continue; + const local = specifier.local.name; + if (typeof local === "string") imported.add(local); + } + return; + } + if (node.type !== "CallExpression") return; + const args = Array.isArray(node.arguments) ? node.arguments.filter(isNode) : []; + // The callee is the factory. The first argument is the element, which + // is a string for an intrinsic tag and the fragment binding for `<>`. + for (const candidate of [node.callee, args[0]]) { + const root = rootIdentifier(candidate); + if (root) roots.add(root); + } + }); + + return [...roots].filter((name) => !imported.has(name)).sort(); + } + + /** `React.createElement` and `React` both have the root `React`. */ + function rootIdentifier(value: unknown): string | null { + let node = value; + while (isNode(node) && node.type === "MemberExpression") node = node.object; + if (!isNode(node) || node.type !== "Identifier") return null; + return typeof node.name === "string" ? node.name : null; + } + + /** What the strip pass pins for the same module. */ + async function pinnedBindings(pragma: string): Promise { + const source = `${pragma}\nexport const el =
;\n` + + `export const frag = <>;`; + const ast = await parser().parse({ code: source, filePath: "page.tsx" }); + return [...jsxPragmaBindings(ast as ASTNode)].sort(); + } + + const PRAGMAS: readonly { label: string; text: string }[] = [ + { label: "no pragma", text: "" }, + { label: "an automatic-runtime import source", text: "/** @jsxImportSource preact */" }, + { label: "a factory without the classic runtime", text: "/** @jsx h */" }, + { label: "the classic runtime with no factory", text: "/** @jsxRuntime classic */" }, + { + label: "an ASCII factory and fragment", + text: "/** @jsxRuntime classic @jsx h @jsxFrag Fr */", + }, + { + label: "the runtime named after the factory", + text: "/** @jsx h @jsxRuntime classic */", + }, + { + label: "the pragmas split across two line comments", + text: "// @jsxRuntime classic\n// @jsx h", + }, + { + label: "a Latin-1 member factory", + text: "/** @jsxRuntime classic @jsx \u0126.cr\u00E9ate @jsxFrag \u0126.Fr\u00E1gment */", + }, + { + label: "a Cyrillic member factory", + text: "/** @jsxRuntime classic @jsx \u043A\u0440\u0435\u043E.\u044D\u043B\u0435\u043C */", + }, + { + label: "a CJK member factory", + text: "/** @jsxRuntime classic @jsx \u65E5\u672C.\u8981\u7D20 */", + }, + { + label: "a factory that is not an expression at all", + text: "/** @jsxRuntime classic @jsx \u{1F389} */", + }, + ]; + + for (const { label, text } of PRAGMAS) { + it(`pins what the artifact calls, given ${label}`, async () => { + const roots = await esbuildFactoryRoots(text); + const pinned = await pinnedBindings(text); + + // Everything the artifact calls must survive the strip, or the page + // dies on the factory. Nothing beyond that may be pinned except the + // documented classic-runtime default, which is the fail-closed half. + for (const root of roots) { + assertEquals( + pinned.includes(root), + true, + `the artifact calls ${root}, which is not pinned. pinned: ${pinned.join(", ")}`, + ); + } + for (const name of pinned) { + assertEquals( + roots.includes(name) || name === "React", + true, + `${name} is pinned but the artifact never calls it`, + ); + } + }); + } + + // The skew check for this predicate: with the ASCII-only identifier test, + // the non-ASCII factories stop being pinned and the corpus fails. + it("stops pinning a non-ASCII factory when the identifier test is ASCII-only", () => { + const ascii = (expression: string): string | null => { + const root = expression.split(/[.([]/)[0]?.trim(); + return root && /^[A-Za-z_$][A-Za-z0-9_$]*$/.test(root) ? root : null; + }; + + assertEquals(pragmaRootBinding("\u0126.cr\u00E9ate"), "\u0126"); + assertEquals(ascii("\u0126.cr\u00E9ate"), null); + }); + }); + + describe("TypeScript erasure against esbuild", () => { + /** + * Whether the artifact still needs `Probe`. + * + * Every fixture imports `Probe` and uses it in exactly one position. Under + * the `tsx` loader esbuild deletes an import whose bindings only appear in + * erased positions, so the presence of the import in the artifact is the + * compiler's own answer to "is this a value read?". Tree shaking is off, so + * the only thing that can remove it is type erasure. + */ + async function esbuildKeepsProbe(source: string): Promise { + const { code } = await transform(source, { ...COMPILE_OPTIONS, loader: "tsx" }); + return code.includes(PROBE_MODULE); + } + + /** What each walker attributes to runtime code for the same module. */ + async function walkerReadsProbe(source: string): Promise<{ flat: boolean; scoped: boolean }> { + const ast = await parser().parse({ code: source, filePath: "page.tsx" }); + const { referenced, free } = moduleReferenceWalkers(ast as ASTNode); + return { flat: referenced.has(PROBE), scoped: free.has(PROBE) }; + } + + const PROBE = "Probe"; + const PROBE_MODULE = "./probe.ts"; + const IMPORT = `import { ${PROBE} } from "${PROBE_MODULE}";`; + + /** + * One fixture per predicate the classification applies to a TypeScript + * construct. The expected answer is not written down: esbuild is asked. + */ + const TS_FIXTURES: readonly { label: string; source: string }[] = [ + { label: "a plain value read", source: `${IMPORT}\nexport const a = Probe;` }, + { label: "a JSX element name", source: `${IMPORT}\nexport const el = ;` }, + { label: "a parameter type", source: `${IMPORT}\nexport function f(p: Probe) { return p; }` }, + { label: "a `typeof` query", source: `${IMPORT}\nexport type A = typeof Probe;` }, + { label: "a variable type", source: `${IMPORT}\nexport let a: Probe;` }, + // The importKind and exportKind short-circuits. + { + label: "an inline type import specifier", + source: `import { type ${PROBE} } from "${PROBE_MODULE}";\nexport type A = Probe;`, + }, + { + label: "a type-only import statement", + source: `import type { ${PROBE} } from "${PROBE_MODULE}";\nexport type A = Probe;`, + }, + { label: "a value re-export", source: `${IMPORT}\nexport { Probe };` }, + { label: "an inline type export specifier", source: `${IMPORT}\nexport { type Probe };` }, + { label: "a type-only export statement", source: `${IMPORT}\nexport type { Probe };` }, + // The `declare` short-circuit. + { + label: "an ambient constant", + source: `${IMPORT}\ndeclare const cfg: typeof Probe;\nexport const a = 1;`, + }, + { + label: "an ambient class heritage clause", + source: `${IMPORT}\ndeclare class C extends Probe {}\nexport const a = 1;`, + }, + { + label: "an ambient enum", + source: `${IMPORT}\ndeclare enum E { A = 1 }\nexport const a = 1;`, + }, + { + label: "an ambient namespace", + source: `${IMPORT}\ndeclare namespace N { const v: typeof Probe; }\nexport const a = 1;`, + }, + { + label: "an ambient module declaration", + source: `${IMPORT}\ndeclare module "m" { const v: typeof Probe; }\nexport const a = 1;`, + }, + // The runtime TypeScript nodes. + { + label: "a real class heritage clause", + source: `${IMPORT}\nexport class C extends Probe {}`, + }, + { + label: "an implements clause", + source: `${IMPORT}\nexport class C implements Probe { m() { return 1; } }`, + }, + { + label: "an interface heritage clause", + source: `${IMPORT}\nexport interface I extends Probe {}`, + }, + { label: "an enum initialiser", source: `${IMPORT}\nexport enum E { A = Probe.v }` }, + { + label: "a namespace body", + source: `${IMPORT}\nexport namespace N { export const v = Probe; }`, + }, + { label: "an `as` expression", source: `${IMPORT}\nexport const a = Probe as unknown;` }, + { + label: "a `satisfies` expression", + source: `${IMPORT}\nexport const a = Probe satisfies unknown;`, + }, + { label: "a non-null assertion", source: `${IMPORT}\nexport const a = Probe!;` }, + { + label: "a parameter property initialiser", + source: `${IMPORT}\nexport class C { constructor(private p = Probe) {} }`, + }, + { + label: "an import-equals alias", + source: `${IMPORT}\nimport Alias = Probe;\nexport const a = Alias;`, + }, + { + label: "an abstract method return type", + source: `${IMPORT}\nexport abstract class C { abstract m(): Probe; }`, + }, + { + label: "an overload signature", + source: + `${IMPORT}\nexport function f(p: Probe): void;\nexport function f(p: unknown) { void p; }`, + }, + { label: "an index signature", source: `${IMPORT}\nexport type A = { [k: string]: Probe };` }, + { + label: "a mapped type", + source: `${IMPORT}\nexport type A = { [K in keyof T]: Probe };`, + }, + { + label: "a conditional type", + source: `${IMPORT}\nexport type A = T extends Probe ? 1 : 0;`, + }, + ]; + + for (const { label, source } of TS_FIXTURES) { + it(`agrees with the artifact about ${label}`, async () => { + const kept = await esbuildKeepsProbe(source); + const { flat, scoped } = await walkerReadsProbe(source); + + assertEquals(flat, kept, "referencedIdentifiers disagreed with the artifact"); + assertEquals(scoped, kept, "freeReferencedIdentifiers disagreed with the artifact"); + }); + } + + // The decorator exception to the `declare` short-circuit. esbuild emits a + // `__decorateClass` call for a decorated ambient member, so the decorator + // is a real read even though the member it annotates emits nothing. + it("agrees with the artifact about a decorator on an ambient member", async () => { + const source = `${IMPORT}\nexport class C { @Probe declare id: string; }`; + const { code } = await transform(source, { + ...COMPILE_OPTIONS, + loader: "tsx", + tsconfigRaw: { compilerOptions: { experimentalDecorators: true } }, + }); + + // Stated so the fixture cannot silently stop exercising the exception. + assertStringIncludes(code, "__decorateClass"); + + const kept = code.includes(PROBE_MODULE); + assertEquals(kept, true); + const { flat, scoped } = await walkerReadsProbe(source); + assertEquals(flat, kept); + assertEquals(scoped, kept); + }); + }); + + describe("compiler-emitted shapes against esbuild's own output", () => { + const HOOK_MODULE = [ + `import { hashOf } from "./lib/uses-crypto.ts";`, + `function loadUser(id) { return hashOf(id); }`, + `export async function getServerData(ctx) {`, + ` return { props: { u: loadUser(ctx.id) } };`, + `}`, + `export default function Page() { return null; }`, + ].join("\n"); + + // `compilerNameHelperBindings` recognises esbuild's `keepNames` helper by + // its `Object.defineProperty(target, "name", ...)` shape rather than by its + // binding name, because minification renames it. The fixture is the real + // compiler's minified output, so the shape cannot drift from the compiler + // without this failing. + it("recognises the keepNames helper in real minified output", async () => { + const { code } = await transform(HOOK_MODULE, { + ...COMPILE_OPTIONS, + loader: "tsx", + minify: true, + }); + + // The fixture has to contain the helper, or the test proves nothing. + assertStringIncludes(code, `"name"`); + assertStringIncludes(code, "configurable"); + + const stripped = await stripServerOnlyExports(code, "page.js"); + + assertEquals(stripped.includes("uses-crypto"), false, stripped); + assertEquals(/\bhashOf\b/.test(stripped), false, stripped); + }); + + // `SOURCE_MAP_SUFFIX` approximates the sourcemap comment esbuild writes. + // The pass rewrites the module, so a stale map has to go with it. + it("drops the sourcemap comment esbuild actually writes", async () => { + const { code } = await transform(HOOK_MODULE, { + ...COMPILE_OPTIONS, + loader: "tsx", + sourcemap: "inline", + }); + + assertStringIncludes(code, "sourceMappingURL="); + + const stripped = await stripServerOnlyExports(code, "page.js"); + + assertEquals(stripped.includes("sourceMappingURL="), false); + }); + }); + + /* ------------------------------------------------------------------- * + * Oracle 5: which names does the artifact actually export? * + * ------------------------------------------------------------------- */ + + describe("the exports this pass acts on against the exports the artifact has", () => { + /** + * The closing case for this stage's one remaining raw-text decision. + * + * `SERVER_ONLY_EXPORTS` is the SET of hook names, and that set is framework + * policy with no differential partner. Deciding whether a module exports + * one of them is a different question, and it is a parser rule the compiler + * answers exactly. Every other fixture here reads a predicate's verdict off + * the artifact. This one asks the artifact which names the module exports, + * then asks whether the pass emptied the same set. + * + * A fixture that asserts one spelling only pins that spelling. Set equality + * against the compiler fails for any spelling, including ones nobody has + * thought of, which is the whole reason the gate must not read raw text. + */ + + const HOOK_NAMES: readonly string[] = SERVER_ONLY_EXPORTS; + + function programBody(ast: ASTNode): Node[] { + const program = (ast as { program?: unknown }).program; + const source: Node = isNode(program) ? program : (ast as unknown as Node); + const body = source.body; + return Array.isArray(body) ? body.filter(isNode) : []; + } + + function nodesIn(value: unknown): Node[] { + return Array.isArray(value) ? value.filter(isNode) : []; + } + + /** An export or binding name with any escape already normalized by the parser. */ + function nameOf(value: unknown): string | null { + if (!isNode(value)) return null; + if (value.type === "StringLiteral" && typeof value.value === "string") return value.value; + return typeof value.name === "string" ? value.name : null; + } + + /** Every exported name in `body`, paired with the local binding it reads. */ + function exportedNames(body: Node[]): Array<{ exported: string; local: string | null }> { + const names: Array<{ exported: string; local: string | null }> = []; + + for (const statement of body) { + if (statement.type !== "ExportNamedDeclaration") continue; + if (statement.exportKind === "type") continue; + + for (const specifier of nodesIn(statement.specifiers)) { + const exported = nameOf(specifier.exported); + if (exported) names.push({ exported, local: nameOf(specifier.local) }); + } + + const declaration = statement.declaration; + if (!isNode(declaration)) continue; + + const direct = nameOf(declaration.id); + if (direct) names.push({ exported: direct, local: direct }); + + for (const declarator of nodesIn(declaration.declarations)) { + const name = nameOf(declarator.id); + if (name) names.push({ exported: name, local: name }); + } + } + + return names; + } + + /** The hook names in `names`, deduplicated and ordered so sets compare. */ + function hooksIn(names: string[]): string[] { + return [...new Set(names.filter((name) => HOOK_NAMES.includes(name)))].sort(); + } + + /** Whether `node` holds the `throw new Error("server-only")` stub. */ + function isStub(node: Node): boolean { + let found = false; + walk(node, (child) => { + if (child.type === "StringLiteral" && child.value === "server-only") found = true; + }); + return found; + } + + /** Every top-level binding in `statement`, paired with the value bound. */ + function boundValues(statement: Node): Array<[string, Node]> { + const declaration = + statement.type === "ExportNamedDeclaration" && isNode(statement.declaration) + ? statement.declaration + : statement; + const bound: Array<[string, Node]> = []; + + const direct = nameOf(declaration.id); + if (direct) bound.push([direct, declaration]); + + for (const declarator of nodesIn(declaration.declarations)) { + const name = nameOf(declarator.id); + if (name && isNode(declarator.init)) bound.push([name, declarator.init]); + } + + return bound; + } + + /** + * The hook-named exports the compiled artifact has. This is the oracle: it + * is esbuild's own answer, so no spelling in the authored source can hide a + * name from it. + */ + async function artifactHookExports(source: string, loader: "tsx" | "js"): Promise { + const { code } = await transform(source, { ...COMPILE_OPTIONS, loader }); + const ast = await parser().parse({ code, filePath: "artifact.js" }); + return hooksIn(exportedNames(programBody(ast)).map((entry) => entry.exported)); + } + + /** The hook-named exports this pass emptied, read off the pass's output. */ + async function strippedHookExports(source: string, filePath: string): Promise { + const output = await stripServerOnlyExports(source, filePath); + const ast = await parser().parse({ code: output, filePath }); + const body = programBody(ast); + + const stubbed = new Set(); + for (const statement of body) { + for (const [name, value] of boundValues(statement)) { + if (isStub(value)) stubbed.add(name); + } + } + + return hooksIn( + exportedNames(body) + .filter((entry) => entry.local !== null && stubbed.has(entry.local)) + .map((entry) => entry.exported), + ); + } + + const SECRET = `const KEY = getEnv("SERVER_ONLY_HOOK_SOURCE");`; + const PREAMBLE = [`import { getEnv } from "veryfront";`, SECRET].join("\n"); + const LOADER = `async function loadIt() { return { props: { k: KEY } }; }`; + const PAGE = `export default function Page() { return null; }`; + + /** + * One authored module per way the language lets you spell an export name. + * + * The escaped spellings are written with a literal backslash so the module + * text carries the escape, which is the whole point: the raw text never + * contains the substring `getServerData`. + */ + const SPELLINGS: Array< + { label: string; file: string; loader: "tsx" | "js"; hidden: boolean; source: string } + > = [ + { + label: "a plain declaration", + file: "plain-declaration.tsx", + loader: "tsx", + hidden: false, + source: [ + PREAMBLE, + `export async function getServerData() { return { props: { k: KEY } }; }`, + PAGE, + ].join("\n"), + }, + { + label: "a plain export clause", + file: "plain-clause.tsx", + loader: "tsx", + hidden: false, + source: [PREAMBLE, LOADER, `export { loadIt as getServerData };`, PAGE].join("\n"), + }, + { + label: "a string-literal export name", + file: "string-name.tsx", + loader: "tsx", + hidden: false, + source: [PREAMBLE, LOADER, `export { loadIt as "getServerData" };`, PAGE].join("\n"), + }, + { + label: "an escaped identifier in an export clause", + file: "escaped-clause.tsx", + loader: "tsx", + hidden: true, + source: [PREAMBLE, LOADER, `export { loadIt as get\\u0053erverData };`, PAGE].join("\n"), + }, + { + label: "an escaped identifier on the declaration", + file: "escaped-declaration.tsx", + loader: "tsx", + hidden: true, + source: [ + PREAMBLE, + `export async function get\\u0053erverData() { return { props: { k: KEY } }; }`, + PAGE, + ].join("\n"), + }, + { + label: "an escaped identifier in an export clause, on a .js module", + file: "escaped-clause.js", + loader: "js", + hidden: true, + source: [PREAMBLE, LOADER, `export { loadIt as get\\u0053erverData };`, PAGE].join("\n"), + }, + { + label: "an escaped string-literal name with no space after as", + file: "escaped-string-name.tsx", + loader: "tsx", + hidden: true, + source: [PREAMBLE, LOADER, `export{loadIt as"get\\u0053erverData"};`, PAGE].join("\n"), + }, + { + label: "a string-literal name split by a line continuation", + file: "continued-string-name.tsx", + loader: "tsx", + hidden: true, + source: [PREAMBLE, LOADER, `export{loadIt as"get\\`, `ServerData"};`, PAGE].join("\n"), + }, + { + label: "a second hook alongside the first", + file: "two-hooks.tsx", + loader: "tsx", + hidden: true, + source: [ + PREAMBLE, + LOADER, + `export { loadIt as get\\u0053erverData };`, + `export async function getStaticPaths() { return { paths: [KEY] }; }`, + PAGE, + ].join("\n"), + }, + { + label: "no hook export at all", + file: "no-hook.tsx", + loader: "tsx", + hidden: false, + source: [PREAMBLE, LOADER, `export { loadIt as loadSomething };`, PAGE].join("\n"), + }, + ]; + + for (const spelling of SPELLINGS) { + it(`acts on exactly the hook exports the artifact has, given ${spelling.label}`, async () => { + const expected = await artifactHookExports(spelling.source, spelling.loader); + const acted = await strippedHookExports(spelling.source, spelling.file); + + assertEquals(acted, expected, spelling.source); + }); + } + + // The fixtures above prove nothing unless the hidden spellings really are + // hidden. A `\u` escape that this test file resolved at its own parse time + // would spell the hook name in the raw source and pass for the wrong + // reason, which is exactly the failure the whole block exists to catch. + it("writes the hidden spellings so the raw text never spells the hook name", () => { + const hidden = SPELLINGS.filter((spelling) => spelling.hidden); + + assertEquals(hidden.length, 6); + for (const spelling of hidden) { + assertEquals(spelling.source.includes("getServerData"), false, spelling.file); + } + }); + }); + + describe("the defect, measured through the real browser pipeline", () => { + /** The module-scope names still present in the browser artifact. */ + async function artifact(source: string, projectId: string): Promise { + const result = await runPipeline(source, "/project/pages/test.tsx", "/project", { + projectId, + dev: true, + ssr: false, + }); + return result.code; + } + + // Defect 5. The hook owns the import, so the pass may delete it; the JSX + // element is the only surviving read, and the ASCII-only test could not see + // it. The artifact still called the binding, so the page died on load. + it("keeps a non-ASCII component import the artifact still calls", async () => { + const source = [ + `import { Caf\u00E9 } from "./ui/cafe.tsx";`, + `export async function getServerData() { return { props: { n: Caf\u00E9.name } }; }`, + `export default function Page() { return ; }`, + ].join("\n"); + + const code = await artifact(source, "predicate-jsx-non-ascii-import"); + + assertStringIncludes(code, "./ui/cafe.js"); + }); + + it("keeps a non-ASCII component declaration the artifact still calls", async () => { + const source = [ + `import { render } from "./lib/server-render.ts";`, + `const Caf\u00E9 = () => null;`, + `export async function getServerData() { return { props: { h: render(Caf\u00E9) } }; }`, + `export default function Page() { return ; }`, + ].join("\n"); + + const code = await artifact(source, "predicate-jsx-non-ascii-declaration"); + + // esbuild escapes a non-ASCII identifier in its output, so matching the + // authored spelling would never hit. The defect deleted the declaration + // and left the factory call, so both halves are asserted. + assertStringIncludes(code, "const Caf"); + assertStringIncludes(code, "jsx(Caf"); + }); + + // The sibling. A classic-pragma module whose factory root is not ASCII + // lost the import the emitted `Ħ.créate(...)` call needs. + it("keeps the factory import a non-ASCII classic pragma names", async () => { + const source = [ + `/** @jsxRuntime classic @jsx \u0126.cr\u00E9ate @jsxFrag \u0126.Fr\u00E1gment */`, + `import { \u0126 } from "./ui/factory.ts";`, + `import { load } from "./lib/server-load.ts";`, + `export async function getServerData() { return { props: { h: load(\u0126) } }; }`, + `export default function Page() { return
; }`, + ].join("\n"); + + const code = await artifact(source, "predicate-pragma-non-ascii-factory"); + + assertStringIncludes(code, "./ui/factory.js"); + }); + }); + + describe("the shared identifier test against the runtime and the compiler", () => { + /** + * Whether the runtime parses `name` as one identifier. + * + * A function declaration is the question, not a `var` statement: a `var` + * name can be followed by `,`, `=`, `;` or a line terminator and still + * parse, so those characters would look like valid identifier parts. Only + * a single BindingIdentifier fits between `function` and `(`. + */ + function runtimeAcceptsIdentifier(name: string): boolean { + try { + new Function(`function ${name}() {}`); + return true; + } catch { + return false; + } + } + + /** + * Every code point where the predicate and the host runtime's own lexer + * give different answers, as a start character and inside an identifier. + * + * `isEcmaScriptIdentifier` reads `\p{ID_Start}` and `\p{ID_Continue}`, + * which are the ENGINE's Unicode tables, and the engine's lexer is a + * separate table versioned independently of them. On V8 the two agree over + * this whole range and this set is empty. On JavaScriptCore it is not: it + * names about twenty code points added in recent Unicode versions. + */ + function tableDisagreements(): { label: string; name: string }[] { + const found: { label: string; name: string }[] = []; + for (let cp = 0; cp <= 0x2FFFF; cp++) { + if (cp >= 0xD800 && cp <= 0xDFFF) continue; + const point = `U+${cp.toString(16).toUpperCase()}`; + const ch = String.fromCodePoint(cp); + if (isEcmaScriptIdentifier(ch) !== runtimeAcceptsIdentifier(ch)) { + found.push({ label: `${point} as a start`, name: ch }); + } + const inside = `A${ch}_`; + if (isEcmaScriptIdentifier(inside) !== runtimeAcceptsIdentifier(inside)) { + found.push({ label: `${point} as a continuation`, name: inside }); + } + } + return found; + } + + // The predicate is asked about names the COMPILER will meet, so the + // compiler decides whether a table disagreement can matter. Where esbuild + // refuses the name, the module never builds and no artifact can depend on + // which side was right; where esbuild accepts it, the two answers reach an + // artifact and one of them deletes a live import. + // + // Asserting the runtimes agree outright is the wrong assertion and it fails + // on JavaScriptCore, whose regular-expression tables and lexer are on + // different Unicode versions. This suite runs on Deno, Node and Bun. + it("never disagrees with the runtime about a name the compiler accepts", async () => { + const reachable: string[] = []; + for (const { label, name } of tableDisagreements()) { + if (await esbuildElementNameVerdict(name) !== "rejected") reachable.push(label); + } + + assertEquals( + reachable, + [], + "the host runtime and the predicate disagree about a name esbuild compiles, " + + "so one of the two answers reaches a browser artifact", + ); + }); + + // Pinned separately from the sweep because the sweep would still pass if + // the answer were reached a different way. Both joiners are valid in an + // ID_Continue position and in no other, and the strip has to agree. + it("accepts both joiners inside an identifier and neither at the start", () => { + assertEquals(isEcmaScriptIdentifier("A\u200Db"), true); + assertEquals(isEcmaScriptIdentifier("A\u200Cb"), true); + assertEquals(isEcmaScriptIdentifier("\u200Db"), false); + assertEquals(isEcmaScriptIdentifier("\u200Cb"), false); + assertEquals(runtimeAcceptsIdentifier("A\u200Db"), true); + assertEquals(runtimeAcceptsIdentifier("\u200Db"), false); + }); + }); +}); diff --git a/src/transforms/pipeline/stages/reference-classification.test.ts b/src/transforms/pipeline/stages/reference-classification.test.ts new file mode 100644 index 0000000000..51621c22cc --- /dev/null +++ b/src/transforms/pipeline/stages/reference-classification.test.ts @@ -0,0 +1,418 @@ +import "#veryfront/schemas/_test-setup.ts"; +import "../../plugins/__tests__/code-parser-setup.ts"; +import * as babelTypes from "npm:@babel/types@7.29.0"; +import { assertEquals } from "#veryfront/testing/assert.ts"; +import { describe, it } from "#veryfront/testing/bdd.ts"; +import { tryResolve } from "#veryfront/extensions/contracts.ts"; +import type { CodeParser } from "#veryfront/extensions/parser/index.ts"; +import { + DEFAULT_REFERENCE_CLASS, + DEFAULT_TS_REFERENCE_CLASS, + isReferenceChildKey, + NODE_REFERENCE_CLASSES, + referenceChildren, + referenceClassOf, + unclassifiedNodeTypes, +} from "./reference-classification.ts"; + +/** + * The specifiers this test pins. + * + * `@babel/types` is the guard's oracle: it defines the node types the + * classification has to cover. `@babel/parser` is what actually EMITS the nodes + * the walkers meet, and the manifest pins the two separately, so they can drift + * apart. A parser-only bump is the dangerous half: a new `TS`-prefixed node + * type would reach `DEFAULT_TS_REFERENCE_CLASS`, be treated as erased, and its + * identifiers would stop keeping bindings alive. That is the over-DELETE + * direction, and the oracle would stay green because `@babel/types` never + * moved. + * + * So both pins are asserted. Bumping either one in the manifest must fail here, + * and whoever bumps it has to re-run the guard against the new package rather + * than discover the gap in a browser. + */ +const PINNED_BABEL_TYPES = "npm:@babel/types@7.29.0"; +const PINNED_BABEL_PARSER = "npm:@babel/parser@7.29.2"; + +/** Paths that cover every branch of the parser extension's plugin choice. */ +const PARSER_PATHS: readonly (string | undefined)[] = [ + "page.tsx", + "page.ts", + "page.jsx", + "page.js", + "page.mjs", + "page.cjs", + "page.md", + "page.mdx", + undefined, +]; + +/** TypeScript-only syntax. Flow has no `satisfies`. */ +const TYPESCRIPT_ONLY = "const a = 1 satisfies number;"; + +/** Flow-only syntax. TypeScript has no `opaque type` and no `%checks`. */ +const FLOW_ONLY: readonly string[] = [ + "opaque type A = number;", + "declare module.exports: number;", +]; + +function codeParser(): CodeParser { + const found = tryResolve("CodeParser"); + if (!found) throw new Error("no CodeParser extension is registered"); + return found; +} + +/** Whether the parser accepts `code` on the plugin set it picks for `filePath`. */ +async function parses(code: string, filePath: string | undefined): Promise { + try { + await codeParser().parse({ code, filePath }); + return true; + } catch { + return false; + } +} + +const EXTENSION_MANIFEST = new URL( + "../../../../extensions/ext-parser-babel/deno.json", + import.meta.url, +); + +/** + * Every node type the pinned `@babel/types` defines, minus the two families + * this pass can never meet. + * + * Flow nodes need the `flow` parser plugin, which `pickPlugins` in + * `extensions/ext-parser-babel/src/parser-only.ts` never enables: it always + * enables `typescript`, and the two plugins are mutually exclusive. The + * deprecated aliases (`NumberLiteral`, `RestProperty`, …) are builder-only + * spellings the parser never emits. + * + * Everything else is in scope, including node types no enabled plugin can + * produce today. Classifying those too is what lets this guard demand total + * coverage of the package instead of coverage of a hand-maintained subset. + */ +function parseableNodeTypes(): string[] { + const flow = new Set(babelTypes.FLIPPED_ALIAS_KEYS.Flow ?? []); + const deprecated = new Set(Object.keys(babelTypes.DEPRECATED_KEYS)); + return Object.keys(babelTypes.NODE_FIELDS) + .filter((type) => !flow.has(type) && !deprecated.has(type)) + .sort(); +} + +describe("reference classification", () => { + describe("guard", () => { + // The point of the exercise. Four review rounds each found a node type + // nobody had classified reaching a descend-into-everything fallback and + // having its identifiers counted as runtime reads. This converts the next + // one from a silent misclassification into a failing build: a Babel + // upgrade that adds a node type must break here, not leak a server-only + // module into the browser artifact. + it("classifies every node type the pinned @babel/types defines", () => { + const missing = unclassifiedNodeTypes(parseableNodeTypes()); + + assertEquals( + missing, + [], + `unclassified node types. Add each to reference-classification.ts as a ` + + `read, an erased node or a structural one, then extend the walkers' ` + + `tests: ${missing.join(", ")}`, + ); + }); + + // The guard is only worth as much as the package it reads, so a parser + // upgrade that leaves this pin behind has to fail too. + it("pins the same @babel/types the parser extension resolves", async () => { + const manifest = JSON.parse(await Deno.readTextFile(EXTENSION_MANIFEST)) as { + imports?: Record; + }; + + assertEquals(manifest.imports?.["@babel/types"], PINNED_BABEL_TYPES); + }); + + // The oracle and the node source are pinned separately in the same + // manifest, so a parser-only bump would leave this guard green while a new + // node type fell to the erased default. Pin the parser too. + it("pins the @babel/parser the nodes are emitted by", async () => { + const manifest = JSON.parse(await Deno.readTextFile(EXTENSION_MANIFEST)) as { + imports?: Record; + }; + + assertEquals(manifest.imports?.["@babel/parser"], PINNED_BABEL_PARSER); + }); + + // `parseableNodeTypes` removes the Flow family from the guard's scope. That + // is sound only while the parser never enables the `flow` plugin. The + // dependency is asserted rather than described: Babel refuses to enable + // `flow` and `typescript` together, so a path that parses TypeScript-only + // syntax and refuses Flow-only syntax cannot be emitting Flow nodes. + it("never enables the Flow plugin the guard's filter depends on", async () => { + const accepted: string[] = []; + const refused: string[] = []; + + for (const filePath of PARSER_PATHS) { + const label = filePath ?? "no path"; + if (!await parses(TYPESCRIPT_ONLY, filePath)) refused.push(label); + for (const code of FLOW_ONLY) { + if (await parses(code, filePath)) accepted.push(`${label}: ${code}`); + } + } + + assertEquals( + refused, + [], + "a path refused TypeScript-only syntax, so `typescript` is no longer always enabled", + ); + assertEquals( + accepted, + [], + "a path accepted Flow-only syntax, so the guard's Flow filter now hides " + + "node types nobody classified", + ); + }); + + // A filter that removes nothing would make the assertion above vacuous. + it("filters a non-empty Flow family out of the guard's scope", () => { + const flow = new Set(babelTypes.FLIPPED_ALIAS_KEYS.Flow ?? []); + const covered = new Set(parseableNodeTypes()); + + assertEquals(flow.size > 0, true, "FLIPPED_ALIAS_KEYS.Flow must not be empty"); + assertEquals( + [...flow].some((type) => covered.has(type)), + false, + "a Flow node type reached the guard's scope", + ); + }); + + // A guard that cannot fail proves nothing, so assert that it reports a + // node type the classification does not name. + it("reports a node type the classification does not cover", () => { + assertEquals( + unclassifiedNodeTypes(["Identifier", "VfUnclassifiedNode", "VfUnclassifiedNode"]), + ["VfUnclassifiedNode"], + ); + }); + }); + + describe("defaults for an unrecognised node type", () => { + // Stated so a reader does not have to infer it: an unknown node is + // descended into and its identifiers count, which over-retains rather than + // over-deletes. See the justification on DEFAULT_REFERENCE_CLASS. + it("treats an unrecognised node as structural", () => { + assertEquals(referenceClassOf({ type: "VfUnclassifiedNode" }), DEFAULT_REFERENCE_CLASS); + assertEquals(DEFAULT_REFERENCE_CLASS, "structural"); + }); + + it("treats an unrecognised TypeScript node as erased", () => { + assertEquals(referenceClassOf({ type: "TSVfUnclassifiedType" }), DEFAULT_TS_REFERENCE_CLASS); + assertEquals(DEFAULT_TS_REFERENCE_CLASS, "erased"); + }); + + it("descends into an unrecognised node's children", () => { + const child = { type: "Identifier", name: "loadUser" }; + assertEquals(referenceChildren({ type: "VfUnclassifiedNode", operand: child }), [child]); + }); + }); + + describe("classification", () => { + it("names exactly two read positions", () => { + const reads = [...NODE_REFERENCE_CLASSES] + .filter(([, value]) => value === "read") + .map(([type]) => type) + .sort(); + + assertEquals(reads, ["Identifier", "JSXIdentifier"]); + }); + + // Defect 4A. `isCompatTag` is only valid for a bare JSXIdentifier that IS + // the element name. The object of `` is always a binding read. + it("reads the object of a lowercase JSX member element name", () => { + const object = { type: "JSXIdentifier", name: "motion" }; + const property = { type: "JSXIdentifier", name: "div" }; + const member = { type: "JSXMemberExpression", object, property }; + + assertEquals(referenceChildren(member), [object]); + assertEquals( + referenceChildren({ type: "JSXOpeningElement", name: member, attributes: [] }), + [member], + ); + }); + + it("does not read a lowercase bare JSX element name", () => { + const name = { type: "JSXIdentifier", name: "table" }; + + assertEquals(referenceChildren({ type: "JSXOpeningElement", name, attributes: [] }), []); + assertEquals(referenceChildren({ type: "JSXClosingElement", name }), []); + }); + + // Defect 4B. With a source, both halves of every specifier name an export + // of the source module, not a local binding. + it("reads no specifier of a re-export that carries a source", () => { + const specifiers = [{ + type: "ExportSpecifier", + local: { type: "Identifier", name: "token" }, + exported: { type: "Identifier", name: "clientToken" }, + }]; + const source = { type: "StringLiteral", value: "./client-utils.js" }; + + assertEquals(referenceChildren({ type: "ExportNamedDeclaration", specifiers, source }), []); + assertEquals( + isReferenceChildKey({ type: "ExportNamedDeclaration", specifiers, source }, "specifiers"), + false, + ); + }); + + it("reads the local half of an export clause with no source", () => { + const local = { type: "Identifier", name: "token" }; + const specifier = { + type: "ExportSpecifier", + local, + exported: { type: "Identifier", name: "clientToken" }, + }; + + assertEquals( + referenceChildren({ type: "ExportNamedDeclaration", specifiers: [specifier] }), + [specifier], + ); + assertEquals(referenceChildren(specifier), [local]); + }); + + // `export v from "./m.js"` and `export * as ns from "./m.js"`. Both carry a + // source by construction, so the parent guard already stops the walkers, + // but the cell is pinned so the table stays correct on its own. + it("reads the exported name of neither default nor namespace re-export", () => { + assertEquals( + referenceChildren({ + type: "ExportDefaultSpecifier", + exported: { type: "Identifier", name: "token" }, + }), + [], + ); + assertEquals( + referenceChildren({ + type: "ExportNamespaceSpecifier", + exported: { type: "Identifier", name: "token" }, + }), + [], + ); + }); + + // These three positions hold no identifier the walkers could report, so a + // misclassification is invisible in an artifact. Asserted here instead, so + // the cell is still pinned. + it("reads nothing of an export-all declaration", () => { + assertEquals( + referenceChildren({ + type: "ExportAllDeclaration", + source: { type: "StringLiteral", value: "./client-utils.js" }, + attributes: [{ + type: "ImportAttribute", + key: { type: "Identifier", name: "type" }, + value: { type: "StringLiteral", value: "json" }, + }], + }), + [], + ); + }); + + it("reads neither half of an import attribute", () => { + assertEquals( + referenceChildren({ + type: "ImportAttribute", + key: { type: "Identifier", name: "type" }, + value: { type: "StringLiteral", value: "json" }, + }), + [], + ); + }); + + // Neither walker hands an import statement to `referenceChildren`: both + // skip the statement outright. The cell is pinned anyway so the table stays + // correct for a caller that does. + it("reads neither half of an import specifier", () => { + assertEquals( + referenceChildren({ + type: "ImportSpecifier", + local: { type: "Identifier", name: "token" }, + imported: { type: "Identifier", name: "token" }, + }), + [], + ); + assertEquals( + referenceChildren({ + type: "ImportDefaultSpecifier", + local: { type: "Identifier", name: "token" }, + }), + [], + ); + assertEquals( + referenceChildren({ + type: "ImportNamespaceSpecifier", + local: { type: "Identifier", name: "token" }, + }), + [], + ); + assertEquals( + referenceChildren({ + type: "ImportDeclaration", + specifiers: [{ + type: "ImportDefaultSpecifier", + local: { type: "Identifier", name: "token" }, + }], + source: { type: "StringLiteral", value: "./server-only.js" }, + }), + [], + ); + }); + + // A parsed file carries its comments and tokens beside the program, and a + // comment is node-shaped, so descending into them would walk text. + it("reads neither the comments nor the tokens of a parsed file", () => { + const program = { type: "Program", body: [] }; + + assertEquals( + referenceChildren({ + type: "File", + program, + comments: [{ type: "CommentLine", value: " token" }], + tokens: [{ type: "Identifier", value: "token" }], + }), + [program], + ); + }); + + it("reads nothing of a directive", () => { + assertEquals( + referenceChildren({ + type: "Directive", + value: { type: "DirectiveLiteral", value: "use strict" }, + }), + [], + ); + }); + + // `Placeholder.name` is an Identifier node, unlike the string-valued + // `V8IntrinsicIdentifier.name`. No enabled parser plugin produces either, + // but the classification covers the whole package, so the cell is pinned. + it("reads the name of neither placeholder form", () => { + assertEquals( + referenceChildren({ + type: "Placeholder", + name: { type: "Identifier", name: "token" }, + }), + [], + ); + assertEquals(referenceChildren({ type: "V8IntrinsicIdentifier", name: "token" }), []); + }); + + it("reads a computed member property but not a fixed one", () => { + const object = { type: "Identifier", name: "registry" }; + const property = { type: "Identifier", name: "hashOf" }; + + assertEquals(referenceChildren({ type: "MemberExpression", object, property }), [object]); + assertEquals( + referenceChildren({ type: "MemberExpression", object, property, computed: true }), + [object, property], + ); + }); + }); +}); diff --git a/src/transforms/pipeline/stages/reference-classification.ts b/src/transforms/pipeline/stages/reference-classification.ts new file mode 100644 index 0000000000..a1de22a53d --- /dev/null +++ b/src/transforms/pipeline/stages/reference-classification.ts @@ -0,0 +1,629 @@ +/** + * Node-type classification shared by the two reference walkers in + * `browser-server-exports-strip.ts`. + * + * The strip pass has to answer one question about every node it meets: does an + * identifier in this position keep a binding alive at runtime? Getting it wrong + * is unsafe in both directions. Counting a fixed name as a read pins a + * server-only import into the browser artifact, which is the leak the pass + * exists to close. Missing a real read deletes live code, and the page dies on + * a ReferenceError. + * + * Four review rounds on the strip-before-compile reorder each found the same + * shape of defect: a node type nobody had classified reached a generic + * descend-into-everything fallback, and its identifiers were counted as runtime + * reads. TypeScript got an explicit classification and has been stable since; + * everything else was ad hoc. This module closes that asymmetry by classifying + * every node type the pinned `@babel/types` defines, once, for both walkers. + * + * Three answers, one per node type: + * + * - `read`: the node itself names a binding that is read at runtime. + * - `erased`: neither the node nor anything under it emits runtime code. + * - `structural`: neither, but a child can be a read. Descend, subject to + * {@link isReferenceChildKey}. + * + * Positions, not just node types, decide the answer: `a.hashOf` and + * `{ hashOf: 1 }` hold the same `Identifier` node in a position that is a fixed + * name rather than a reference. {@link NON_REFERENCE_KEYS} records those + * positions per parent type, so both walkers skip the same subtrees. + * + * The classification is enforced, not documented: `unclassifiedNodeTypes` + * backs a guard test that fails when `@babel/types` gains a node type this + * module does not name. A Babel upgrade must break the build, not leak. + * + * @module transforms/pipeline/stages/reference-classification + */ + +/** The minimal AST shape both walkers work with. */ +export type Node = Record & { type: string }; + +export function isNode(value: unknown): value is Node { + return typeof value === "object" && value !== null && + typeof (value as { type?: unknown }).type === "string"; +} + +/** How a node type participates in reference counting. */ +export type ReferenceClass = + /** The node itself names a binding that is read at runtime. */ + | "read" + /** Neither the node nor anything under it emits runtime code. */ + | "erased" + /** Neither a read nor erased. Descend, subject to `isReferenceChildKey`. */ + | "structural"; + +/** + * The class for a node type this module does not name. + * + * The two directions are not symmetric. Defaulting to a read over-retains: the + * pass keeps a binding, so a server-only import can survive into the browser + * artifact. Defaulting to "not a read" over-deletes: the pass removes a binding + * live code still uses, and the page fails to load. + * + * `structural` is the over-retaining choice and it is the right one here. + * + * - A reference walker answering "is this name read?" is only sound when it + * over-approximates. Under-approximating reads is the unsound direction for + * any liveness analysis, and it is the direction every regression in this + * pass has come from. + * - Over-retention degrades to the behaviour before this pass existed: the + * module keeps a binding it may not need. It never emits an artifact that + * throws. The compile stage still elides a genuinely unused import under the + * `ts` and `tsx` loaders, and the bundler still tree-shakes, so the retained + * binding is not necessarily even shipped. + * - Over-deletion produces a ReferenceError at module evaluation in the + * browser, which no later stage can undo. + * - New ECMAScript syntax is value syntax holding real expressions. Treating an + * unrecognised one as erased is nearly always wrong. + * + * This is a fallback, not a policy: the guard test makes an unclassified node + * type a build failure, so in a checked build the default never fires. + */ +export const DEFAULT_REFERENCE_CLASS: ReferenceClass = "structural"; + +/** + * The class for an unrecognised `TS`-prefixed node type. + * + * The TypeScript grammar inverts the argument above. Almost everything it adds + * is type syntax that the compiler erases before the module runs, and the + * handful of value-emitting forms is enumerated in + * {@link RUNTIME_TS_NODE_TYPES}. Descending into an unrecognised type node + * would count `p: typeof KEY` as a use of `KEY` and pin the server-only import + * it came from, which is the leak this pass exists to close. + * + * This keeps the choice made when the TypeScript classification was added. + * Like the default above, the guard test means it never fires in a checked + * build. + */ +export const DEFAULT_TS_REFERENCE_CLASS: ReferenceClass = "erased"; + +/** + * TypeScript nodes that survive type erasure and emit runtime code. + * + * The list is closed and enumerable, which is the point: it is a decidable + * question, unlike proving what a module does to an intrinsic. Any new + * TypeScript node type that emits runtime code must be added here. + * + * The split is invisible while this pass runs after the compile stage, which + * erases every TypeScript node before the walkers see the module. It exists so + * the pass stays correct when it runs on authored source. + */ +export const RUNTIME_TS_NODE_TYPES: ReadonlySet = new Set([ + // Value expressions wrapping a value expression plus an erased type operand. + "TSAsExpression", + "TSSatisfiesExpression", + "TSNonNullExpression", + "TSTypeAssertion", + "TSInstantiationExpression", + // `enum E { A = compute() }` emits an object and evaluates each initialiser. + "TSEnumDeclaration", + "TSEnumBody", + "TSEnumMember", + // `namespace N { … }` with a body emits an IIFE over a runtime object. + "TSModuleDeclaration", + "TSModuleBlock", + // `constructor(private dep = fallback())` emits an assignment in the body. + "TSParameterProperty", + // `import L = require("./l.ts")` and `import A = N.Sub` both emit a binding. + "TSImportEqualsDeclaration", + "TSExternalModuleReference", + "TSQualifiedName", + // `export = handler` emits an assignment to the module export. + "TSExportAssignment", +]); + +/** + * TypeScript nodes the compiler erases whole. An identifier under one of these + * is a type reference and must not keep a binding alive. + */ +const ERASED_TS_NODE_TYPES: readonly string[] = [ + "TSAnyKeyword", + "TSArrayType", + "TSBigIntKeyword", + "TSBooleanKeyword", + "TSCallSignatureDeclaration", + "TSConditionalType", + "TSConstructSignatureDeclaration", + "TSConstructorType", + // An overload signature and an ambient method signature have no body to run. + "TSDeclareFunction", + "TSDeclareMethod", + // `class A extends B implements C`: the heritage clause in a type position. + "TSExpressionWithTypeArguments", + "TSFunctionType", + "TSImportType", + "TSIndexSignature", + "TSIndexedAccessType", + "TSInferType", + "TSInterfaceBody", + "TSInterfaceDeclaration", + "TSIntersectionType", + "TSIntrinsicKeyword", + "TSLiteralType", + "TSMappedType", + "TSMethodSignature", + "TSNamedTupleMember", + // `export as namespace Lib`: a declaration-file-only global alias. + "TSNamespaceExportDeclaration", + "TSNeverKeyword", + "TSNullKeyword", + "TSNumberKeyword", + "TSObjectKeyword", + "TSOptionalType", + "TSParenthesizedType", + "TSPropertySignature", + "TSRestType", + "TSStringKeyword", + "TSSymbolKeyword", + "TSTemplateLiteralType", + "TSThisType", + "TSTupleType", + "TSTypeAliasDeclaration", + "TSTypeAnnotation", + "TSTypeLiteral", + "TSTypeOperator", + "TSTypeParameter", + "TSTypeParameterDeclaration", + "TSTypeParameterInstantiation", + "TSTypePredicate", + "TSTypeQuery", + "TSTypeReference", + "TSUndefinedKeyword", + "TSUnionType", + "TSUnknownKeyword", + "TSVoidKeyword", +]; + +/** + * Node types that name a binding directly. Every other identifier position in + * the grammar reaches one of these two through a `structural` parent. + */ +const READ_NODE_TYPES: readonly string[] = [ + "Identifier", + // A JSX identifier is a read everywhere except an intrinsic element name and + // the fixed-name positions in `NON_REFERENCE_KEYS`. See + // `isIntrinsicJsxElementName`. + "JSXIdentifier", +]; + +/** + * Everything else the pinned `@babel/types` defines, minus the Flow nodes and + * the deprecated aliases the guard test excludes. + * + * A `structural` node is neither a read nor erased: the walkers descend into + * the child keys `isReferenceChildKey` allows. Types listed here that no + * enabled parser plugin can produce (`BindExpression`, `DoExpression`, + * `RecordExpression`, the pipeline nodes, `Placeholder`, + * `V8IntrinsicIdentifier`, …) are classified anyway, so the guard test can + * demand total coverage of the package rather than of a hand-maintained subset + * of it. + */ +const STRUCTURAL_NODE_TYPES: readonly string[] = [ + "ArgumentPlaceholder", + "ArrayExpression", + "ArrayPattern", + "ArrowFunctionExpression", + "AssignmentExpression", + "AssignmentPattern", + "AwaitExpression", + "BigIntLiteral", + "BinaryExpression", + "BindExpression", + "BlockStatement", + "BooleanLiteral", + "BreakStatement", + "CallExpression", + "CatchClause", + "ClassAccessorProperty", + "ClassBody", + "ClassDeclaration", + "ClassExpression", + "ClassMethod", + "ClassPrivateMethod", + "ClassPrivateProperty", + "ClassProperty", + "ConditionalExpression", + "ContinueStatement", + "DebuggerStatement", + "DecimalLiteral", + "Decorator", + "Directive", + "DirectiveLiteral", + "DoExpression", + "DoWhileStatement", + "EmptyStatement", + "ExportAllDeclaration", + "ExportDefaultDeclaration", + "ExportDefaultSpecifier", + "ExportNamedDeclaration", + "ExportNamespaceSpecifier", + "ExportSpecifier", + "ExpressionStatement", + "File", + "ForInStatement", + "ForOfStatement", + "ForStatement", + "FunctionDeclaration", + "FunctionExpression", + "IfStatement", + "Import", + "ImportAttribute", + "ImportDeclaration", + "ImportDefaultSpecifier", + "ImportExpression", + "ImportNamespaceSpecifier", + "ImportSpecifier", + "InterpreterDirective", + "JSXAttribute", + "JSXClosingElement", + "JSXClosingFragment", + "JSXElement", + "JSXEmptyExpression", + "JSXExpressionContainer", + "JSXFragment", + "JSXMemberExpression", + "JSXNamespacedName", + "JSXOpeningElement", + "JSXOpeningFragment", + "JSXSpreadAttribute", + "JSXSpreadChild", + "JSXText", + "LabeledStatement", + "LogicalExpression", + "MemberExpression", + "MetaProperty", + "ModuleExpression", + "NewExpression", + "Noop", + "NullLiteral", + "NumericLiteral", + "ObjectExpression", + "ObjectMethod", + "ObjectPattern", + "ObjectProperty", + "OptionalCallExpression", + "OptionalMemberExpression", + "ParenthesizedExpression", + "PipelineBareFunction", + "PipelinePrimaryTopicReference", + "PipelineTopicExpression", + "Placeholder", + "PrivateName", + "Program", + "RecordExpression", + "RegExpLiteral", + "RestElement", + "ReturnStatement", + "SequenceExpression", + "SpreadElement", + "StaticBlock", + "StringLiteral", + "Super", + "SwitchCase", + "SwitchStatement", + "TaggedTemplateExpression", + "TemplateElement", + "TemplateLiteral", + "ThisExpression", + "ThrowStatement", + "TopicReference", + "TryStatement", + "TupleExpression", + "UnaryExpression", + "UpdateExpression", + "V8IntrinsicIdentifier", + "VariableDeclaration", + "VariableDeclarator", + "VoidPattern", + "WhileStatement", + "WithStatement", + "YieldExpression", +]; + +function buildClasses(): Map { + const classes = new Map(); + for (const type of READ_NODE_TYPES) classes.set(type, "read"); + for (const type of ERASED_TS_NODE_TYPES) classes.set(type, "erased"); + for (const type of RUNTIME_TS_NODE_TYPES) classes.set(type, "structural"); + for (const type of STRUCTURAL_NODE_TYPES) classes.set(type, "structural"); + return classes; +} + +/** Every node type this module classifies, keyed by `node.type`. */ +export const NODE_REFERENCE_CLASSES: ReadonlyMap = buildClasses(); + +/** The node types the classification does not cover, in input order. */ +export function unclassifiedNodeTypes(types: Iterable): string[] { + const missing: string[] = []; + for (const type of types) { + if (!NODE_REFERENCE_CLASSES.has(type) && !missing.includes(type)) missing.push(type); + } + return missing; +} + +/** + * Child keys that hold a fixed name, string-like text or a binding position + * rather than a runtime reference. Neither walker descends into them. + * + * Two positions are deliberately absent. `TSModuleDeclaration.id` and + * `TSImportEqualsDeclaration.id` are binding positions, and the flat walker + * counts them on purpose: that is the over-retaining direction for + * module-declaration liveness, and the scope-aware walker binds them instead of + * reading them. Moving them here would make the flat walker delete a + * module-scope declaration that shares the name. + */ +const NON_REFERENCE_KEYS: ReadonlyMap = new Map< + string, + readonly string[] +>([ + // `break outer`, `continue outer`, `outer:`: a label is its own namespace and + // never resolves to a binding. + ["BreakStatement", ["label"]], + ["ContinueStatement", ["label"]], + ["LabeledStatement", ["label"]], + // `import.meta`, `new.target`: both halves are keywords. + ["MetaProperty", ["meta", "property"]], + // `#count`: a private name is class-local, never a module binding. + ["PrivateName", ["id"]], + ["ClassPrivateProperty", ["key"]], + ["ClassPrivateMethod", ["key"]], + // `"use strict"`, `#!/usr/bin/env deno`: string text. + ["Directive", ["value"]], + ["DirectiveLiteral", ["value"]], + ["InterpreterDirective", ["value"]], + ["V8IntrinsicIdentifier", ["name"]], + ["Placeholder", ["name"]], + // `a.hashOf` and `{ hashOf: 1 }` are fixed names; `a[hashOf]` is a read, so + // these keys come back when `computed` is true. See `COMPUTED_KEY_TYPES`. + ["MemberExpression", ["property"]], + ["OptionalMemberExpression", ["property"]], + ["ObjectProperty", ["key"]], + ["ObjectMethod", ["key"]], + ["ClassMethod", ["key"]], + ["ClassProperty", ["key"]], + ["ClassAccessorProperty", ["key"]], + // An import statement declares bindings, it does not read them. Both walkers + // skip import statements outright as well; this keeps the answer the same for + // any caller that hands one to `referenceChildren` directly. + ["ImportDeclaration", ["specifiers", "source", "attributes", "assertions"]], + ["ImportSpecifier", ["local", "imported"]], + ["ImportDefaultSpecifier", ["local"]], + ["ImportNamespaceSpecifier", ["local"]], + ["ImportAttribute", ["key", "value"]], + // `export { a as b }`: `a` is a local read, `b` is the name the module + // publishes under. With a `source` the local half is not local either, which + // `isReferenceChildKey` handles on the parent. + ["ExportSpecifier", ["exported"]], + ["ExportDefaultSpecifier", ["exported"]], + ["ExportNamespaceSpecifier", ["exported"]], + // `export * from "./m.js"` and `export * as ns from "./m.js"` name exports of + // the source module. Nothing here is a local read. + ["ExportAllDeclaration", ["source", "attributes", "assertions"]], + ["ExportNamedDeclaration", ["source", "attributes", "assertions"]], + // `` compiles to `("svg:circle", { "xlink:href": + // "x" })`: both halves of a namespaced name are string text, whether it names + // an element or an attribute. + ["JSXNamespacedName", ["namespace", "name"]], + // `placeholder=`, `title=`, `href=`: an attribute name is a property key. + ["JSXAttribute", ["name"]], + // `` and ``: the property is a fixed name. The + // object is always a binding read, whatever its case. See + // `isIntrinsicJsxElementName`. + ["JSXMemberExpression", ["property"]], + // An enum declaration names itself and its members; only the initialisers + // read anything. + ["TSEnumDeclaration", ["id"]], + ["TSEnumMember", ["id"]], + // `import Alias = NS.Sub`: only `NS` is a read, `Sub` is a fixed name. + ["TSQualifiedName", ["right"]], + // A parsed file carries its comments and tokens as siblings of the program. + ["File", ["comments", "tokens"]], +]); + +/** Parents whose fixed-name key becomes a real read when `computed` is true. */ +const COMPUTED_KEY_TYPES: ReadonlySet = new Set([ + "MemberExpression", + "OptionalMemberExpression", + "ObjectProperty", + "ObjectMethod", + "ClassMethod", + "ClassProperty", + "ClassAccessorProperty", +]); + +/** Keys that never hold AST the walkers should visit. */ +const IGNORED_KEYS: ReadonlySet = new Set([ + "loc", + "range", + "extra", + "leadingComments", + "trailingComments", + "innerComments", +]); + +/** + * Whether `name` is spelled as an ECMAScript identifier. + * + * The grammar is Unicode, not ASCII: `IdentifierStart` is `$`, `_` or any + * `ID_Start` code point, and `IdentifierPart` adds `ID_Continue`, ZWNJ + * (`U+200C`) and ZWJ (`U+200D`). `$` and `_` are spelled out because neither + * carries the Unicode property. + * + * The two joiner escapes are redundant on V8, whose `\p{ID_Continue}` already + * matches both, so removing them changes no answer here. They stay because the + * grammar lists them separately from the property, and the property is the + * engine's table rather than the specification's rule. The differential test + * asserts the ANSWER for both joiners against the runtime, which is the part + * that has to keep holding. + * + * An ASCII-only spelling of this test is not a smaller version of it, it is a + * different predicate: `Café`, `Ωmega` and `日本語` are identifiers that an + * ASCII test rejects. The two callers both use the answer to decide whether a + * name is a binding read, so rejecting a valid identifier deletes live code. + * {@link isIntrinsicJsxName} and the JSX pragma pin in + * `browser-server-exports-strip.ts` are differentially tested against esbuild + * for exactly that reason. + */ +export function isEcmaScriptIdentifier(name: string): boolean { + return /^[$_\p{ID_Start}][$\u200C\u200D\p{ID_Continue}]*$/u.test(name); +} + +/** + * Whether a JSX element name is intrinsic tag text rather than a binding read. + * + * `
`, `
` and `` become string arguments to the JSX + * factory, so they can never resolve to an import or a module-scope + * declaration. `` is a real reference and must keep its import alive. + * + * This mirrors esbuild's rule, which is the compiler the pipeline actually + * runs: a name is tag text when its first character is ASCII `a`-`z`, or when + * the whole name is not spelled as an identifier. Both halves matter and the + * order matters. `café` is a valid identifier and still tag text, because the + * first clause fires first; `Café` is not tag text, because it fails the first + * clause and passes the second. + * + * The first clause is deliberately ASCII: esbuild compares the first character + * against `a`-`z`, so `ωmega` and `привет` are binding reads even though their + * first character is a lowercase letter. Widening it to `\p{Ll}` would delete + * those imports. + */ +export function isIntrinsicJsxName(name: string | null): boolean { + if (!name) return false; + if (/^[a-z]/.test(name)) return true; + return !isEcmaScriptIdentifier(name); +} + +/** + * Whether the `name` of a JSX opening or closing element is string text. + * + * The rule applies to a bare `JSXIdentifier` that IS the element name, and to + * nothing else. ``, ``, `` and + * `` are `JSXMemberExpression` names, and their object is always a + * binding read however it is spelled: the lowercase-means-string rule would + * delete the import the element needs and the page would die on a + * ReferenceError. A `JSXNamespacedName` is string text on both halves, which + * `NON_REFERENCE_KEYS` already records, so it needs no answer here. + */ +export function isIntrinsicJsxElementName(name: unknown): boolean { + if (!isNode(name) || name.type !== "JSXIdentifier") return false; + const text = name.name; + return isIntrinsicJsxName(typeof text === "string" ? text : null); +} + +/** Whether a node carries decorators, which emit a runtime call even when the + * declaration they annotate is ambient. */ +export function nodeHasDecorators(node: Node): boolean { + const decorators = node.decorators; + return Array.isArray(decorators) && decorators.length > 0; +} + +/** + * The class of one node, taking the node's own modifiers into account. + * + * Both reference walkers ask this, and they must ask the same question. A + * walker that counts a type-position read as a runtime reference keeps the + * server-only import that binding came from; a walker that skips a runtime + * TypeScript node reports live code as dead. + */ +export function referenceClassOf(node: Node): ReferenceClass { + // `declare const`, `declare class`, `declare namespace`, `declare enum` and + // `declare prop: T` are all ambient: they emit nothing. + // + // Decorators are the exception. Both tsc and esbuild emit a runtime + // `__decorate` call for `@audit declare id: string`, so the decorator + // expression is a real read even though the property it annotates is not. + // Erasing it here deletes the import the decorator needs and the emitted + // call then throws a ReferenceError at module evaluation. + if (node.declare === true && !nodeHasDecorators(node)) return "erased"; + // `import { type Cfg }`, `export { type Cfg }`, `export type { Cfg }`. + if (node.importKind === "type" || node.exportKind === "type") return "erased"; + + const known = NODE_REFERENCE_CLASSES.get(node.type); + if (known === undefined) { + return node.type.startsWith("TS") ? DEFAULT_TS_REFERENCE_CLASS : DEFAULT_REFERENCE_CLASS; + } + // An ambient `declare module "x";` has no body to run. + if (node.type === "TSModuleDeclaration" && !isNode(node.body)) return "erased"; + return known; +} + +/** Whether the compiler erases `node` and everything under it. */ +export function isErasedNode(node: Node): boolean { + return referenceClassOf(node) === "erased"; +} + +/** + * Whether the value under `key` of `parent` can hold a runtime identifier read. + * + * `false` means the whole subtree is a fixed name, string-like text or a + * binding position, and neither walker descends into it. + */ +export function isReferenceChildKey(parent: Node, key: string): boolean { + if (IGNORED_KEYS.has(key)) return false; + + // `
` is string text, `` and the `motion` of `` + // are reads. Only the element-name position gets the intrinsic rule. + if ( + (parent.type === "JSXOpeningElement" || parent.type === "JSXClosingElement") && key === "name" + ) { + return !isIntrinsicJsxElementName(parent.name); + } + + // `export { token as clientToken } from "./client-utils.js"` reads no local + // binding: `token` names an export of the SOURCE module. Without a source the + // local half of each specifier is a real read of this module's scope. + if (parent.type === "ExportNamedDeclaration" && key === "specifiers") { + return !isNode(parent.source); + } + + if ( + parent.computed === true && COMPUTED_KEY_TYPES.has(parent.type) && + (key === "key" || key === "property") + ) { + return true; + } + + return !NON_REFERENCE_KEYS.get(parent.type)?.includes(key); +} + +/** + * The children of `node` that can hold a runtime identifier read. + * + * Both walkers descend through this, so a position classified as a fixed name + * is skipped identically by each of them. + */ +export function referenceChildren(node: Node): Node[] { + const found: Node[] = []; + + for (const [key, value] of Object.entries(node)) { + if (!isReferenceChildKey(node, key)) continue; + + if (Array.isArray(value)) { + for (const entry of value) if (isNode(entry)) found.push(entry); + continue; + } + if (isNode(value)) found.push(value); + } + + return found; +}