From 050c81e4d2e4a3de54895fbe1ccfa055f46bc033 Mon Sep 17 00:00:00 2001 From: "Alexis H. Munsayac" Date: Tue, 29 Sep 2026 15:03:38 +0800 Subject: [PATCH] perf(directives): skip needless Babel work in compiler transforms - Skip the server function transform for modules that do not contain the directive. - Append the lazy module id as a string instead of parsing with Babel. Babel now runs only when the module calls `lazy(`. - Register each hoisted server function declaration instead of crawling the whole program scope per function. - Remove unused declarations with a worklist in one pass instead of repeated traversals and crawls. - Merge the two directive placement checks into one traversal. - Keep unused `for...in` and `for...of` loop variables, which used to crash the compile. Co-Authored-By: Claude Opus 5.5 --- .changeset/faster-compiler-transforms.md | 10 ++ packages/start/src/config/lazy.spec.ts | 43 ++++++ packages/start/src/config/lazy.ts | 42 ++---- packages/start/src/directives/compile.spec.ts | 59 ++++++++ packages/start/src/directives/index.ts | 5 + packages/start/src/directives/plugin.ts | 17 +-- .../src/directives/remove-unused-variables.ts | 137 ++++++++++++------ packages/start/src/directives/validate.ts | 35 ++--- 8 files changed, 238 insertions(+), 110 deletions(-) create mode 100644 .changeset/faster-compiler-transforms.md create mode 100644 packages/start/src/config/lazy.spec.ts diff --git a/.changeset/faster-compiler-transforms.md b/.changeset/faster-compiler-transforms.md new file mode 100644 index 000000000..8ad0e1a05 --- /dev/null +++ b/.changeset/faster-compiler-transforms.md @@ -0,0 +1,10 @@ +--- +"@solidjs/start": patch +--- + +Speed up the server function and lazy transforms. + +- Modules without `"use server"` are no longer parsed by the server function transform. +- The server build no longer parses a module with Babel only to add its lazy id. +- Modules with many server functions compile about 4 times faster. +- A module with a server function no longer fails to compile when it has an unused `for...of` or `for...in` loop variable, or a chain of unused declarations. diff --git a/packages/start/src/config/lazy.spec.ts b/packages/start/src/config/lazy.spec.ts new file mode 100644 index 000000000..39adb2fad --- /dev/null +++ b/packages/start/src/config/lazy.spec.ts @@ -0,0 +1,43 @@ +import { describe, expect, it } from "vitest"; + +import lazy from "./lazy.ts"; + +async function transform(code: string, id = `${process.cwd()}/src/routes/page.tsx`) { + const plugin = lazy() as any; + const context = { environment: { name: "ssr" } }; + return plugin.transform.call(context, code, id); +} + +describe("lazy", () => { + it("appends the module id without moving existing code", async () => { + const code = `import { A } from "./a.ts";\nexport default function Page() { return A; }`; + const result = await transform(code); + + expect(result.code.startsWith(code)).toBe(true); + expect(result.code).toContain(`export const id$$ = "src/routes/page.tsx";`); + expect(result.map).toBeNull(); + }); + + it("imports lazy from the server runtime", async () => { + const result = await transform( + `import { lazy } from "solid-js";\nconst Page = lazy(() => import("./page.tsx"));\nexport default Page;`, + ); + + expect(result.code).toContain(`import { lazy } from "@solidjs/start/server";`); + expect(result.code).not.toMatch(/import \{ lazy \} from "solid-js"/); + expect(result.code).toContain(`export const id$$ = "src/routes/page.tsx";`); + expect(result.map).toBeTruthy(); + }); + + it("skips a module without a default export or lazy", async () => { + expect(await transform(`import { A } from "./a.ts";\nexport const B = A;`)).toBeUndefined(); + }); + + it("skips the client environment", async () => { + const plugin = lazy() as any; + const context = { environment: { name: "client" } }; + expect( + await plugin.transform.call(context, `import A from "./a.ts";\nexport default A;`, "/a.tsx"), + ).toBeUndefined(); + }); +}); diff --git a/packages/start/src/config/lazy.ts b/packages/start/src/config/lazy.ts index 3c041474f..72fbfc3d2 100644 --- a/packages/start/src/config/lazy.ts +++ b/packages/start/src/config/lazy.ts @@ -6,22 +6,6 @@ import { basename, relative, sep } from "node:path/posix"; import type { PluginOption } from "vite"; import { VITE_ENVIRONMENTS } from "./constants.ts"; -const idTransform = (id: string): PluginItem => { - return { - visitor: { - Program(path) { - path.node.body.unshift( - t.exportNamedDeclaration( - t.variableDeclaration("const", [ - t.variableDeclarator(t.identifier("id$$"), t.stringLiteral(id)), - ]), - ), - ); - }, - }, - }; -}; - const importTransform = (): PluginItem => { return { visitor: { @@ -96,24 +80,21 @@ const lazy = (): PluginOption => { if (src.indexOf("import") === -1) return; if (id.includes("entry-server")) return; - const plugins: PluginItem[] = []; - - const hasDefaultExport = src.indexOf("export default") !== -1; - if (hasDefaultExport) { - const localId = relative(cwd, id); - const chunkName = sharedChunkNames[id]; - plugins.push(idTransform(chunkName ?? localId)); + // Appended rather than prepended, so no existing code moves and the + // source map of the input still applies to the output. + let idExport = ""; + if (src.indexOf("export default") !== -1) { + const chunkName = sharedChunkNames[id] ?? relative(cwd, id); + idExport = `\nexport const id$$ = ${JSON.stringify(chunkName)};\n`; } - const hasLazy = src.indexOf("lazy(") !== -1; - if (hasLazy) plugins.push(importTransform()); - - if (!plugins.length) { - return; + if (src.indexOf("lazy(") === -1) { + if (!idExport) return; + return { code: src + idExport, map: null }; } const transformed = await babel.transformAsync(src, { - plugins, + plugins: [importTransform()], parserOpts: { plugins: ["jsx", "typescript"], }, @@ -127,8 +108,7 @@ const lazy = (): PluginOption => { if (!transformed?.code) return; - const { code, map } = transformed; - return { code, map }; + return { code: transformed.code + idExport, map: transformed.map }; }, }; }; diff --git a/packages/start/src/directives/compile.spec.ts b/packages/start/src/directives/compile.spec.ts index 3a8d51217..1ce040790 100644 --- a/packages/start/src/directives/compile.spec.ts +++ b/packages/start/src/directives/compile.spec.ts @@ -38,6 +38,65 @@ describe("compile", () => { expect(result.code).not.toContain("./server-module.ts"); }); + it("removes declarations that only unused declarations read", async () => { + const result = await compile( + "/src/server-action.ts", + ` + import { db } from "./db.ts"; + + const first = db; + const second = first; + const third = second; + + export const serverAction = async () => { + "use server"; + return third; + }; + `, + clientOptions, + ); + + expect(result.code).not.toContain("./db.ts"); + expect(result.code).not.toMatch(/first|second|third/); + }); + + it("removes every declaration of an unused var", async () => { + const result = await compile( + "/src/server-action.ts", + ` + var value = 1; + var value = 2; + + export const serverAction = async () => { + "use server"; + return 1; + }; + `, + clientOptions, + ); + + expect(result.code).not.toContain("value"); + }); + + it("keeps an unused loop variable", async () => { + const result = await compile( + "/src/server-action.ts", + ` + for (const item of []) {} + for (const key in {}) {} + + export const serverAction = async () => { + "use server"; + return 1; + }; + `, + clientOptions, + ); + + expect(result.code).toContain("for (const item of [])"); + expect(result.code).toContain("for (const key in {})"); + }); + it("preserves live value specifiers from a mixed import", async () => { const result = await compile( "/src/server-action.ts", diff --git a/packages/start/src/directives/index.ts b/packages/start/src/directives/index.ts index 9e438b61f..62137e271 100644 --- a/packages/start/src/directives/index.ts +++ b/packages/start/src/directives/index.ts @@ -236,6 +236,11 @@ export function serverFunctionsPlugin(options: ServerFunctionsOptions): Plugin[] if (!filter(id)) { return null; } + // Most modules have no directive. Skipping them avoids a Babel parse + // and print whose output would be thrown away. + if (!code.includes(DIRECTIVE)) { + return null; + } const result = await compile(id!, code, { ...(mode === "server" ? serverOptions : clientOptions), diff --git a/packages/start/src/directives/plugin.ts b/packages/start/src/directives/plugin.ts index 1e512bd33..5592620c5 100644 --- a/packages/start/src/directives/plugin.ts +++ b/packages/start/src/directives/plugin.ts @@ -11,11 +11,7 @@ import { isPathValid, unwrapPath } from "./paths.ts"; import { removeUnusedVariables } from "./remove-unused-variables.ts"; import type { ImportDefinition } from "./types.ts"; import xxHash32 from "./xxhash32.ts"; -import { - assertHoistable, - assertNoMethodDirectives, - collectMisplacedDirectives, -} from "./validate.ts"; +import { assertHoistable, validateDirectivePlacement } from "./validate.ts"; export interface StateContext { env: "production" | "development"; @@ -129,9 +125,12 @@ function transformFunction( const sourceID = generateUniqueName(path, "serverFn"); - rootStatement.insertBefore( + const [source] = rootStatement.insertBefore( t.variableDeclaration("const", [t.variableDeclarator(sourceID, sourceReference)]), ); + // Registering only the new declaration keeps the scope current without a + // full crawl per function. References are recounted once at the end. + path.scope.getProgramParent().registerDeclaration(source!); // Clone the source function to replace the server function path.replaceWith( @@ -145,8 +144,6 @@ function transformFunction( ]), ); } - - path.scope.crawl(); } function traceBinding(path: babel.NodePath, name: string): Binding | undefined { @@ -412,8 +409,7 @@ export function directivesPlugin(): babel.PluginObj { name: "solid-start:directives", visitor: { Program(program, ctx) { - assertNoMethodDirectives(program, ctx.opts.directive); - ctx.opts.warnings.push(...collectMisplacedDirectives(program, ctx.opts.directive)); + ctx.opts.warnings.push(...validateDirectivePlacement(program, ctx.opts.directive)); const isModuleLevel = isDirectiveValid(ctx.opts, program.node.directives); if (isModuleLevel) { @@ -438,7 +434,6 @@ export function directivesPlugin(): babel.PluginObj { transformFunction(ctx.opts, path, false); }, }); - program.scope.crawl(); if (ctx.opts.count > 0) { ctx.opts.valid = true; diff --git a/packages/start/src/directives/remove-unused-variables.ts b/packages/start/src/directives/remove-unused-variables.ts index 9af4346fb..fa055f3c3 100644 --- a/packages/start/src/directives/remove-unused-variables.ts +++ b/packages/start/src/directives/remove-unused-variables.ts @@ -1,4 +1,5 @@ import type * as babel from "@babel/core"; +import type { Binding } from "@babel/traverse"; import * as t from "@babel/types"; import { isPathValid } from "./paths.ts"; @@ -11,6 +12,10 @@ function isInvalidForRemoval(path: babel.NodePath) { // This one is for destructured variables let target = path; if (isPathValid(path, t.isVariableDeclarator)) { + // The loop variable of `for...in` and `for...of` cannot be left out. + if (path.parentPath.key === "left" && path.parentPath.parentPath?.isFor()) { + return true; + } target = path.get("id"); } return isPathValid(target, t.isObjectPattern) || isPathValid(target, t.isArrayPattern); @@ -32,55 +37,93 @@ function countValidImport(node: t.ImportDeclaration): number { return count; } +function isRemovableKind(binding: Binding): boolean { + switch (binding.kind) { + case "const": + case "let": + case "var": + case "hoisted": + case "module": + return true; + case "local": + case "param": + case "unknown": + return false; + } +} + +/** + * Removing a node drops the references it held. Bindings that lose their last + * reference this way become unused too, so they are returned to be removed next. + */ +function dereferenceRemoved(path: babel.NodePath, unused: Binding[]): void { + function dereference(child: babel.NodePath): void { + const binding = child.scope.getBinding(child.node.name); + if (!binding || binding.references === 0 || !binding.referencePaths.includes(child)) { + return; + } + binding.dereference(); + if (binding.references === 0) { + unused.push(binding); + } + } + if (path.isIdentifier() || path.isJSXIdentifier()) { + dereference(path); + } + path.traverse({ + ReferencedIdentifier: dereference, + }); +} + +function removeBinding(binding: Binding, unused: Binding[]): void { + if (binding.path.removed || binding.references !== 0 || !isRemovableKind(binding)) { + return; + } + const parent = binding.path.parentPath; + let target: babel.NodePath; + if (isPathValid(parent, t.isImportDeclaration)) { + target = countValidImport(parent.node) <= 1 ? parent : binding.path; + } else if (isInvalidForRemoval(binding.path)) { + return; + } else { + target = binding.path; + } + // A repeated `var` declaration is tracked as a reassignment of the first one, + // so it has to go along with it. + const targets = [target]; + for (const violation of binding.constantViolations) { + if (isPathValid(violation, t.isVariableDeclarator) && t.isIdentifier(violation.node.id)) { + targets.push(violation); + } + } + for (const current of targets) { + if (!current.removed) { + dereferenceRemoved(current, unused); + current.remove(); + } + } +} + +/** + * Removes declarations nothing reads. Removing one can leave another unused, + * so each removal queues the bindings it was the last reader of, which avoids + * walking the whole program again until nothing changes. + */ export function removeUnusedVariables(program: babel.NodePath) { - // TODO(Alexis): - // This implementation is simple but slow - // We repeat removing unused variables from each pass - // until no potential unused variables are left. - // There might be a simpler implementation. - let dirty = true; + program.scope.crawl(); - while (dirty) { - dirty = false; - program.traverse({ - BindingIdentifier(path) { - const binding = path.scope.getBinding(path.node.name); + const unused: Binding[] = []; + program.traverse({ + BindingIdentifier(path) { + const binding = path.scope.getBinding(path.node.name); + if (binding && binding.identifier === path.node && binding.references === 0) { + unused.push(binding); + } + }, + }); - if (binding) { - switch (binding.kind) { - case "const": - case "let": - case "var": - case "hoisted": - case "module": - if (binding.references === 0 && !binding.path.removed) { - const parent = binding.path.parentPath; - if (isPathValid(parent, t.isImportDeclaration)) { - if (countValidImport(parent.node) <= 1) { - parent.remove(); - } else { - binding.path.remove(); - } - dirty = true; - } else if (!isInvalidForRemoval(binding.path)) { - binding.path.remove(); - dirty = true; - } - } - break; - case "local": - case "param": - case "unknown": - break; - } - } - }, - VariableDeclaration(path) { - if (path.node.declarations.length === 0) { - path.remove(); - } - }, - }); - program.scope.crawl(); + // Oldest first, so declarations are removed in source order. + for (let i = 0; i < unused.length; i++) { + removeBinding(unused[i]!, unused); } } diff --git a/packages/start/src/directives/validate.ts b/packages/start/src/directives/validate.ts index aff3a5219..b77137de0 100644 --- a/packages/start/src/directives/validate.ts +++ b/packages/start/src/directives/validate.ts @@ -170,15 +170,22 @@ export function assertHoistable(path: babel.NodePath, directi } /** + * Checks where the directive is written, in one walk over the module. + * * A directive only applies to a function body. The transform ignores one in a * method, which ships the method body and every module it imports to the - * browser. + * browser, so that fails the build. + * + * A directive string that is not in a directive prologue does nothing. It is + * almost always meant to be one, so it is returned as a warning. Otherwise the + * module compiles as if it had no server functions. */ -export function assertNoMethodDirectives( +export function validateDirectivePlacement( program: babel.NodePath, directive: string, -): void { - function check( +): string[] { + const warnings: string[] = []; + function checkMethod( child: babel.NodePath, ): void { for (const current of child.node.body.directives) { @@ -191,23 +198,9 @@ export function assertNoMethodDirectives( } } program.traverse({ - ObjectMethod: check, - ClassMethod: check, - ClassPrivateMethod: check, - }); -} - -/** - * A directive string that is not in a directive prologue does nothing. It is - * almost always meant to be one, so report it. Otherwise the module compiles as - * if it had no server functions. - */ -export function collectMisplacedDirectives( - program: babel.NodePath, - directive: string, -): string[] { - const warnings: string[] = []; - program.traverse({ + ObjectMethod: checkMethod, + ClassMethod: checkMethod, + ClassPrivateMethod: checkMethod, ExpressionStatement(child) { const expression = child.node.expression; if (!t.isStringLiteral(expression) || expression.value !== directive) {