diff --git a/.changeset/fix-2100-exports-disappear.md b/.changeset/fix-2100-exports-disappear.md new file mode 100644 index 000000000..c47e32ea9 --- /dev/null +++ b/.changeset/fix-2100-exports-disappear.md @@ -0,0 +1,5 @@ +--- +"@solidjs/start": patch +--- + +fix: keep non-picked route exports that picked exports reference instead of deleting their bindings, so API handlers can call helpers exported from the same file (#2100); also fixes picked exports declared via `export { ... }` specifiers being dropped (#1659) diff --git a/packages/start/src/config/fs-routes/tree-shake.spec.ts b/packages/start/src/config/fs-routes/tree-shake.spec.ts new file mode 100644 index 000000000..47e94c0bd --- /dev/null +++ b/packages/start/src/config/fs-routes/tree-shake.spec.ts @@ -0,0 +1,289 @@ +import { describe, expect, it } from "vitest"; + +import { treeShake } from "./tree-shake.ts"; + +async function shake(code: string, pick: string[]) { + const plugin = treeShake() as any; + const id = `/routes/route.ts?${pick.map(p => `pick=${p}`).join("&")}`; + const result = await plugin.transform(code, id); + return result?.code as string | undefined; +} + +describe("treeShake", () => { + it("keeps only the picked export", async () => { + const code = ` + export const GET = () => "get"; + export const POST = () => "post"; + `; + + const output = await shake(code, ["GET"]); + + expect(output).toContain("export const GET"); + expect(output).not.toContain("POST"); + }); + + // https://github.com/solidjs/solid-start/issues/2100 + it("keeps a non-picked export that a picked export references", async () => { + const code = ` + export const hello = () => "hello"; + export const GET = async () => { + return hello(); + }; + `; + + const output = await shake(code, ["GET"]); + + expect(output).toContain(`const hello = () => "hello"`); + expect(output).not.toContain("export const hello"); + expect(output).toContain("export const GET"); + }); + + it("keeps a non-picked exported function that a picked export references", async () => { + const code = ` + export function helper() { + return "helper"; + } + export const GET = () => helper(); + `; + + const output = await shake(code, ["GET"]); + + expect(output).toContain("function helper()"); + expect(output).not.toContain("export function helper"); + expect(output).toContain("export const GET"); + }); + + it("removes a non-picked export that nothing references", async () => { + const code = ` + export const secret = globalThis.createSecret(); + export const GET = () => "ok"; + `; + + const output = await shake(code, ["GET"]); + + expect(output).not.toContain("secret"); + expect(output).toContain("export const GET"); + }); + + // https://github.com/solidjs/solid-start/issues/1659 + it("keeps picked exports declared via specifiers, including aliases", async () => { + const code = ` + const handler = () => "ok"; + export { handler as GET, handler as POST }; + `; + + const output = await shake(code, ["GET"]); + + expect(output).toContain("handler as GET"); + expect(output).not.toContain("POST"); + }); + + it("splits mixed variable declarations while preserving evaluation order", async () => { + const code = ` + export const first = 1, GET = () => first; + `; + + const output = await shake(code, ["GET"]); + + expect(output).toMatch(/const first = 1;\s*export const GET/); + expect(output).not.toContain("export const first"); + }); + + it("keeps a named default export that a picked export references", async () => { + const code = ` + export default function Page() { + return "page"; + } + export const GET = () => Page(); + `; + + const output = await shake(code, ["GET"]); + + expect(output).toContain("function Page()"); + expect(output).not.toContain("export default"); + expect(output).toContain("export const GET"); + }); + + it("removes an unreferenced default export, including anonymous ones", async () => { + const named = await shake( + ` + export default function Page() { return "page"; } + export const GET = () => "ok"; + `, + ["GET"], + ); + expect(named).not.toContain("Page"); + + const anonymous = await shake( + ` + export default function () { return "page"; } + export const GET = () => "ok"; + `, + ["GET"], + ); + expect(anonymous).not.toContain("export default"); + expect(anonymous).toContain("export const GET"); + }); + + it("removes imports only used by removed exports", async () => { + const code = ` + import { db } from "./db.ts"; + export const POST = () => db.write(); + export const GET = () => "ok"; + `; + + const output = await shake(code, ["GET"]); + + expect(output).not.toContain("db"); + expect(output).toContain("export const GET"); + }); + + it("removes unreachable export cycles and their imports", async () => { + const code = ` + import { register } from "./registry.ts"; + export const first = register(() => second); + export const second = register(() => first); + export const GET = () => "ok"; + `; + + const output = await shake(code, ["GET"]); + + expect(output).not.toContain("register"); + expect(output).not.toContain("first"); + expect(output).not.toContain("second"); + expect(output).toContain("export const GET"); + }); + + it("keeps an export cycle reachable from a picked export", async () => { + const code = ` + export const first = () => second(); + export const second = () => first(); + export const GET = () => first(); + `; + + const output = await shake(code, ["GET"]); + + expect(output).toContain("const first"); + expect(output).toContain("const second"); + expect(output).not.toContain("export const first"); + expect(output).not.toContain("export const second"); + }); + + it("removes an unreachable cycle through a non-exported binding", async () => { + const code = ` + import { register } from "./registry.ts"; + export const first = register(() => helper()); + const helper = () => first; + export const GET = () => "ok"; + `; + + const output = await shake(code, ["GET"]); + + expect(output).not.toContain("register"); + expect(output).not.toContain("first"); + expect(output).not.toContain("helper"); + expect(output).toContain("export const GET"); + }); + + it("ignores type-only references when determining reachability", async () => { + const code = ` + import { initializeSecret } from "./secret.ts"; + export const secret = initializeSecret(); + type Secret = typeof secret; + export const GET = (_value: Secret) => "ok"; + `; + + const output = await shake(code, ["GET"]); + + expect(output).not.toContain("initializeSecret"); + expect(output).not.toContain("const secret"); + expect(output).toContain("export const GET"); + }); + + it("keeps runtime references wrapped in TypeScript syntax", async () => { + const code = ` + type Handler = () => string; + export const helper = () => "ok"; + export const GET = () => helper satisfies Handler; + `; + + const output = await shake(code, ["GET"]); + + expect(output).toContain("const helper"); + expect(output).not.toContain("export const helper"); + }); + + it("removes an unreferenced named default class", async () => { + const code = ` + export default class Page { + static { globalThis.initializePage(); } + } + export const GET = () => "ok"; + `; + + const output = await shake(code, ["GET"]); + + expect(output).not.toContain("Page"); + expect(output).not.toContain("initializePage"); + expect(output).toContain("export const GET"); + }); + + it("keeps a class reachable from a picked export", async () => { + const code = ` + export class Helper { + static value = "ok"; + } + export const GET = () => Helper.value; + `; + + const output = await shake(code, ["GET"]); + + expect(output).toContain("class Helper"); + expect(output).not.toContain("export class Helper"); + expect(output).toContain("export const GET"); + }); + + it("removes an unreachable cycle through a non-exported class", async () => { + const code = ` + import { register } from "./registry.ts"; + export const first = register(() => Helper); + class Helper { + value() { return first; } + } + export const GET = () => "ok"; + `; + + const output = await shake(code, ["GET"]); + + expect(output).not.toContain("register"); + expect(output).not.toContain("first"); + expect(output).not.toContain("Helper"); + expect(output).toContain("export const GET"); + }); + + it("preserves declare when splitting mixed variable declarations", async () => { + const code = ` + export declare const helper: string, GET: () => string; + `; + + const output = await shake(code, ["GET"]); + + expect(output).not.toContain("helper"); + expect(output).toContain("export declare const GET"); + }); + + it("removes an unpicked var initializer when the binding is redeclared", async () => { + const code = ` + import { initializeSecret } from "./secret.ts"; + var secret = initializeSecret(); + export var secret; + var secret; + export const GET = () => "ok"; + `; + + const output = await shake(code, ["GET"]); + + expect(output).not.toContain("initializeSecret"); + expect(output).toContain("export const GET"); + }); +}); diff --git a/packages/start/src/config/fs-routes/tree-shake.ts b/packages/start/src/config/fs-routes/tree-shake.ts index ad91affe1..db63a3329 100644 --- a/packages/start/src/config/fs-routes/tree-shake.ts +++ b/packages/start/src/config/fs-routes/tree-shake.ts @@ -5,12 +5,14 @@ import type * as Babel from "@babel/core"; import type { NodePath, PluginObj, PluginPass } from "@babel/core"; import type { Binding } from "@babel/traverse"; +import type { Identifier } from "@babel/types"; import { basename } from "pathe"; import type { Plugin } from "vite"; type State = Omit & { opts: { pick: string[] }; refs: Set; + candidates: Set>; done: boolean; }; @@ -33,13 +35,28 @@ function treeShakeTransform({ types: t }: typeof Babel): PluginObj { return path.node.id && path.node.id.type === "Identifier" ? path.get("id") : null; } - function isIdentifierReferenced(ident: any) { + function isTypeOnlyReference(path: NodePath) { + return path.getAncestry().some(parent => { + if (parent.isTSType()) return true; + if (parent.isExportNamedDeclaration()) return parent.node.exportKind === "type"; + if (parent.isExportSpecifier()) return parent.node.exportKind === "type"; + return false; + }); + } + + function runtimeReferences(binding: Binding) { + return binding.referencePaths.filter(ref => !isTypeOnlyReference(ref)); + } + + function isIdentifierReferenced(ident: any, includeTypes = false) { const b: Binding | undefined = ident.scope.getBinding(ident.node.name); - if (b?.referenced) { + if (b) { + const references = b.constantViolations.concat( + includeTypes ? b.referencePaths : runtimeReferences(b), + ); + if (!references.length) return false; if (b.path.type === "FunctionDeclaration") { - return !b.constantViolations - .concat(b.referencePaths) - .every(ref => ref.findParent(p => p === b.path)); + return !references.every(ref => ref.findParent(p => p === b.path)); } return true; } @@ -47,13 +64,13 @@ function treeShakeTransform({ types: t }: typeof Babel): PluginObj { } function markFunction(path: any, state: any) { const ident = getIdentifier(path); - if (ident && ident.node && isIdentifierReferenced(ident)) { + if (ident && ident.node && isIdentifierReferenced(ident, true)) { state.refs.add(ident); } } function markImport(path: any, state: any) { const local = path.get("local"); - if (isIdentifierReferenced(local)) { + if (isIdentifierReferenced(local, true)) { state.refs.add(local); } } @@ -63,13 +80,14 @@ function treeShakeTransform({ types: t }: typeof Babel): PluginObj { Program: { enter(path, state) { state.refs = new Set(); + state.candidates = new Set(); state.done = false; path.traverse( { VariableDeclarator(variablePath, variableState: any) { if (variablePath.node.id.type === "Identifier") { const local = variablePath.get("id"); - if (isIdentifierReferenced(local)) { + if (isIdentifierReferenced(local, true)) { variableState.refs.add(local); } } else if (variablePath.node.id.type === "ObjectPattern") { @@ -85,7 +103,7 @@ function treeShakeTransform({ types: t }: typeof Babel): PluginObj { throw new Error("invariant"); })(), ); - if (isIdentifierReferenced(local)) { + if (isIdentifierReferenced(local, true)) { variableState.refs.add(local); } }); @@ -101,31 +119,40 @@ function treeShakeTransform({ types: t }: typeof Babel): PluginObj { } else { return; } - if (isIdentifierReferenced(local)) { + if (isIdentifierReferenced(local, true)) { variableState.refs.add(local); } }); } }, ExportDefaultDeclaration(exportNamedPath) { - // if opts.keep is true, we don't remove the routeData export if (state.opts.pick && !state.opts.pick.includes("default")) { - exportNamedPath.remove(); + const decl = exportNamedPath.node.declaration; + // A named function/class declaration creates a module-scope + // binding that picked exports may reference, so only strip + // the `export default`; the sweep below removes the + // declaration again if nothing references it. + if ((t.isFunctionDeclaration(decl) || t.isClassDeclaration(decl)) && decl.id) { + const [newPath] = exportNamedPath.replaceWith(decl); + const id = (newPath as NodePath).get("id") as NodePath; + state.refs.add(id); + state.candidates.add(id); + } else { + exportNamedPath.remove(); + } } }, ExportNamedDeclaration(exportNamedPath) { - // if opts.keep is false, we don't remove the routeData export + // No pick list means nothing gets pruned. if (!state.opts.pick) { return; } const specifiers = exportNamedPath.get("specifiers"); if (specifiers.length) { specifiers.forEach(s => { - if ( - t.isIdentifier(s.node.exported) - ? s.node.exported.name - : state.opts.pick.includes(s.node.exported.value) - ) { + const exported = s.node.exported; + const exportedName = t.isIdentifier(exported) ? exported.name : exported.value; + if (!state.opts.pick.includes(exportedName)) { s.remove(); } }); @@ -139,24 +166,51 @@ function treeShakeTransform({ types: t }: typeof Babel): PluginObj { return; } switch (decl.node.type) { - case "FunctionDeclaration": { + case "FunctionDeclaration": + case "ClassDeclaration": { const name = decl.node.id?.name; if (name && state.opts.pick && !state.opts.pick.includes(name)) { - exportNamedPath.remove(); + // Keep the declaration in module scope since picked + // exports may reference it; the sweep below removes it + // again if unreferenced. + const [newPath] = exportNamedPath.replaceWith(decl.node); + const id = (newPath as NodePath).get("id") as NodePath; + state.refs.add(id); + state.candidates.add(id); } break; } case "VariableDeclaration": { - const inner = decl.get("declarations") as Array>; - inner.forEach(d => { - if (d.node.id.type !== "Identifier") { - return; - } - const name = d.node.id.name; - if (state.opts.pick && !state.opts.pick.includes(name)) { - d.remove(); - } + const declNode = decl.node; + // Destructuring declarators are conservatively treated as + // picked (matching the previous behavior of leaving them + // untouched). + const isPicked = (d: (typeof declNode.declarations)[number]) => + d.id.type !== "Identifier" || state.opts.pick.includes(d.id.name); + if (declNode.declarations.every(isPicked)) { + break; + } + // Split into one declaration per declarator (preserving + // evaluation order) and export only the picked ones. + // Unpicked bindings are kept since picked exports may + // reference them; the sweep below removes them again if + // unreferenced. + const statements = declNode.declarations.map(d => { + const single = t.variableDeclaration(declNode.kind, [d]); + single.declare = declNode.declare; + return isPicked(d) ? t.exportNamedDeclaration(single, []) : single; }); + const newPaths = exportNamedPath.replaceWithMultiple(statements); + for (const p of newPaths) { + if (!p.isVariableDeclaration()) continue; + for (const d of p.get("declarations")) { + if (d.node.id.type === "Identifier") { + const id = d.get("id") as NodePath; + state.refs.add(id); + state.candidates.add(id); + } + } + } break; } default: { @@ -167,6 +221,8 @@ function treeShakeTransform({ types: t }: typeof Babel): PluginObj { FunctionDeclaration: markFunction, FunctionExpression: markFunction, ArrowFunctionExpression: markFunction, + ClassDeclaration: markFunction, + ClassExpression: markFunction, ImportSpecifier: markImport, ImportDefaultSpecifier: markImport, ImportNamespaceSpecifier: markImport, @@ -183,11 +239,104 @@ function treeShakeTransform({ types: t }: typeof Babel): PluginObj { state, ); + // References alone cannot distinguish live bindings from dead cycles. + // Build a graph of localized exports and retain only those reachable + // from runtime code outside that graph. + path.scope.crawl(); + const tracked = new Map>(); + const candidateOwners = new Map>(); + const addOwner = ( + owners: Map>, + binding: Binding, + owner: NodePath, + ) => { + const paths = owners.get(binding) ?? new Set(); + paths.add(owner); + owners.set(binding, paths); + }; + + for (const identifier of state.refs) { + if (identifier.node?.type !== "Identifier" || !identifier.parentPath) continue; + const binding = identifier.scope.getBinding(identifier.node.name); + if (binding) addOwner(tracked, binding, identifier.parentPath); + } + for (const identifier of state.candidates) { + if (!identifier.parentPath) continue; + const binding = identifier.scope.getBinding(identifier.node.name); + if (!binding) continue; + addOwner(tracked, binding, identifier.parentPath); + addOwner(candidateOwners, binding, identifier.parentPath); + } + + const dependencies = new Map>(); + const live = new Set(); + for (const [binding, owners] of tracked) { + dependencies.set(binding, new Set()); + if ( + [...owners].some(owner => + owner.findParent( + parent => + parent.isExportNamedDeclaration() || parent.isExportDefaultDeclaration(), + ), + ) + ) { + live.add(binding); + } + } + + const ownerBindings = new Map(); + for (const [binding, owners] of tracked) { + for (const owner of owners) { + ownerBindings.set(owner, binding); + } + } + const owningBinding = (reference: NodePath) => { + const owner = reference.find(parent => ownerBindings.has(parent)); + return owner && ownerBindings.get(owner); + }; + + for (const target of tracked.keys()) { + const references = target.constantViolations.concat(runtimeReferences(target)); + for (const reference of references) { + const source = owningBinding(reference); + if (source) dependencies.get(source)?.add(target); + else live.add(target); + } + } + + const pending = [...live]; + while (pending.length) { + const source = pending.pop()!; + for (const dependency of dependencies.get(source) ?? []) { + if (live.has(dependency)) continue; + live.add(dependency); + pending.push(dependency); + } + } + + const deadCandidates = new Set( + [...candidateOwners.keys()].filter(candidate => !live.has(candidate)), + ); + path.traverse({ + VariableDeclarator(variablePath) { + if (variablePath.node.id.type !== "Identifier") return; + const binding = variablePath.scope.getBinding(variablePath.node.id.name); + if (binding && deadCandidates.has(binding)) variablePath.remove(); + }, + }); + for (const [candidate, owners] of candidateOwners) { + if (!deadCandidates.has(candidate)) continue; + for (const owner of owners) { + if (!owner.removed && !owner.isVariableDeclarator()) owner.remove(); + } + } + const refs = state.refs; + const refNodes = new Set([...refs].map(ref => ref.node)); let count = 0; const sweepFunction = (sweepPath: any) => { const ident = getIdentifier(sweepPath); - if (ident && ident.node && refs.has(ident) && !isIdentifierReferenced(ident)) { + if (ident && ident.node && refNodes.has(ident.node) && !isIdentifierReferenced(ident)) { ++count; if ( t.isAssignmentExpression(sweepPath.parentPath) || @@ -201,7 +350,7 @@ function treeShakeTransform({ types: t }: typeof Babel): PluginObj { }; function sweepImport(sweepPath: any) { const local = sweepPath.get("local"); - if (refs.has(local) && !isIdentifierReferenced(local)) { + if (refNodes.has(local.node) && !isIdentifierReferenced(local)) { ++count; sweepPath.remove(); if (sweepPath.parent.specifiers.length === 0) { @@ -216,7 +365,7 @@ function treeShakeTransform({ types: t }: typeof Babel): PluginObj { VariableDeclarator(variablePath) { if (variablePath.node.id.type === "Identifier") { const local = variablePath.get("id"); - if (refs.has(local) && !isIdentifierReferenced(local)) { + if (refNodes.has(local.node) && !isIdentifierReferenced(local)) { ++count; variablePath.remove(); } @@ -234,7 +383,7 @@ function treeShakeTransform({ types: t }: typeof Babel): PluginObj { throw new Error("invariant"); })(), ); - if (refs.has(local) && !isIdentifierReferenced(local)) { + if (refNodes.has(local.node) && !isIdentifierReferenced(local)) { ++count; p.remove(); } @@ -255,7 +404,7 @@ function treeShakeTransform({ types: t }: typeof Babel): PluginObj { } else { return; } - if (refs.has(local) && !isIdentifierReferenced(local)) { + if (refNodes.has(local.node) && !isIdentifierReferenced(local)) { ++count; e.remove(); } @@ -268,6 +417,8 @@ function treeShakeTransform({ types: t }: typeof Babel): PluginObj { FunctionDeclaration: sweepFunction, FunctionExpression: sweepFunction, ArrowFunctionExpression: sweepFunction, + ClassDeclaration: sweepFunction, + ClassExpression: sweepFunction, ImportSpecifier: sweepImport, ImportDefaultSpecifier: sweepImport, ImportNamespaceSpecifier: sweepImport,