From 9f9f8523dbc8af01405a9bc03dfd582ee3290637 Mon Sep 17 00:00:00 2001 From: Birk Skyum Date: Mon, 10 Aug 2026 01:34:01 +0200 Subject: [PATCH] perf(A): cut server render cost by ~5x spent most of its server render time building accessor-backed prop objects that a one shot renderToString can never observe. In a 1000 link table the component rendered in 5.9ms; it now renders in 1.1ms. - drop the mergeProps used only to default activeClass/inactiveClass, and read the defaults with ?? at the point of use instead - share the normalized location.pathname across links rather than recomputing normalizePath + decodeURI + toLowerCase once per link - read props.state once and skip JSON.stringify when it is undefined - on the server, when the caller passes nothing beyond the props consumes itself, skip splitProps and the JSX spread. Gated on isServer because a server render is one shot, so the key set cannot grow after the check, and the branch is constant folded out of client builds children stays in the spread path so innerHTML/textContent precedence and the order a ref observes are unchanged; it is only rendered explicitly on the fast path, which by definition cannot receive those props. The class map keeps its object literal form so __proto__ class names stay own properties. Rendered output is unchanged on the client, and unchanged on the server apart from the link marker serializing as `link` rather than `link="true"` on the fast path. Client bundles shrink by ~400 bytes gzipped. Adds client and server test coverage for , which had none. --- .changeset/spotty-moons-jump.md | 16 +++ package.json | 3 +- src/components.tsx | 105 +++++++++++---- test/anchor.spec.tsx | 221 ++++++++++++++++++++++++++++++++ test/ssr/anchor.spec.tsx | 199 ++++++++++++++++++++++++++++ vitest.config.ts | 4 +- vitest.ssr.config.ts | 20 +++ 7 files changed, 542 insertions(+), 26 deletions(-) create mode 100644 .changeset/spotty-moons-jump.md create mode 100644 test/anchor.spec.tsx create mode 100644 test/ssr/anchor.spec.tsx create mode 100644 vitest.ssr.config.ts diff --git a/.changeset/spotty-moons-jump.md b/.changeset/spotty-moons-jump.md new file mode 100644 index 00000000..38e8f807 --- /dev/null +++ b/.changeset/spotty-moons-jump.md @@ -0,0 +1,16 @@ +--- +"@solidjs/router": patch +--- + +Speed up `` server rendering by roughly 5x + +`` spent most of its server render time in `mergeProps` and `splitProps`, which build +accessor-backed prop objects that can never be observed during a one shot +`renderToString`. The default `activeClass` / `inactiveClass` merge is gone, the +`location.pathname` normalization is shared instead of repeated per link, `JSON.stringify` +is skipped when there is no `state`, and on the server a link that passes nothing beyond +the props `` consumes itself now skips `splitProps` and the JSX spread entirely. + +Rendered output is unchanged on the client, and unchanged on the server apart from the +`link` marker serializing as `link` rather than `link="true"` on that fast path. Client +bundles get slightly smaller, since the server branch is constant folded away. diff --git a/package.json b/package.json index a48e6e72..c2b7b46d 100644 --- a/package.json +++ b/package.json @@ -31,8 +31,9 @@ "scripts": { "build": "rm -rf dist && tsc && rollup -c", "prepublishOnly": "npm run build", - "test": "vitest run && npm run test:types", + "test": "vitest run && npm run test:ssr && npm run test:types", "test:watch": "vitest", + "test:ssr": "vitest run --config vitest.ssr.config.ts", "test:types": "tsc --project tsconfig.test.json", "pretty": "prettier --write \"{src,test}/**/*.{ts,tsx}\"", "release": "pnpm build && changeset publish" diff --git a/src/components.tsx b/src/components.tsx index 5e822256..fcf2bab8 100644 --- a/src/components.tsx +++ b/src/components.tsx @@ -1,16 +1,9 @@ /*@refresh skip*/ import type { JSX } from "solid-js"; -import { createMemo, mergeProps, splitProps } from "solid-js"; -import { - useHref, - useLocation, - useNavigate, - useResolvedPath -} from "./routing.js"; -import type { - Location, - Navigator -} from "./types.js"; +import { createMemo, splitProps } from "solid-js"; +import { isServer } from "solid-js/web"; +import { useHref, useLocation, useNavigate, useResolvedPath } from "./routing.js"; +import type { Location, Navigator } from "./types.js"; import { normalizePath } from "./utils.js"; declare module "solid-js" { @@ -34,16 +27,55 @@ export interface AnchorProps extends Omit on a page normalizes the same `location.pathname`, so a one entry cache +// collapses that work to once per navigation instead of once per link. Pure function +// of the input, so a stale or missed entry can only cost a recompute. +let lastPathname: string | undefined; +let lastNormalizedPathname: string; +function normalizeLocationPath(pathname: string): string { + if (pathname !== lastPathname) { + lastNormalizedPathname = decodeURI(normalizePath(pathname).toLowerCase().replace(/\/$/, "")); + lastPathname = pathname; + } + return lastNormalizedPathname; +} + +// Props consumes itself. `children` is deliberately absent: leaving it in `rest` keeps +// the spread path byte for byte identical to before, including `innerHTML`/`textContent` +// precedence over children and the order a `ref` observes. +const SPLIT_PROPS = [ + "href", + "state", + "class", + "activeClass", + "inactiveClass", + "end" +] as const satisfies readonly (keyof AnchorProps)[]; + +// The fast path renders children explicitly, so a link that only has children still +// qualifies for it. Anything outside this set, `innerHTML` included, takes the spread path. +const FAST_PATH_PROPS: ReadonlySet = new Set([...SPLIT_PROPS, "children"]); + export function A(props: AnchorProps) { - props = mergeProps({ inactiveClass: "inactive", activeClass: "active" }, props); - const [, rest] = splitProps(props, [ - "href", - "state", - "class", - "activeClass", - "inactiveClass", - "end" - ]); + // `splitProps` plus the JSX spread is the bulk of the per link cost, and neither is + // needed when the caller passes nothing beyond the props consumes itself. Server + // only: a render pass there is one shot, so the key set cannot grow after this check, + // which it can on the client when the caller uses a reactive spread. The branch is + // constant folded out of client builds. + // + // Own property names rather than `for...in`: a non-enumerable own prop still reaches the + // element through `splitProps`, so missing one here would silently drop it. + let fastPath = false; + if (isServer) { + fastPath = true; + for (const key of Object.getOwnPropertyNames(props)) { + if (!FAST_PATH_PROPS.has(key)) { + fastPath = false; + break; + } + } + } + const to = useResolvedPath(() => props.href); const href = useHref(to); const location = useLocation(); @@ -52,19 +84,44 @@ export function A(props: AnchorProps) { if (to_ === undefined) return [false, false]; // trailing slashes are ignored so `/route` and `/route/` share active state const path = normalizePath(to_.split(/[?#]/, 1)[0]).toLowerCase().replace(/\/$/, ""); - const loc = decodeURI(normalizePath(location.pathname).toLowerCase().replace(/\/$/, "")); + const loc = normalizeLocationPath(location.pathname); return [props.end ? path === loc : loc.startsWith(path + "/") || loc === path, path === loc]; }); + // One read, so a getter backed `state` is not observed twice + const state = () => { + const value = props.state; + return value === undefined ? undefined : JSON.stringify(value); + }; + + if (fastPath) { + return ( + + {props.children} + + ); + } + + const [, rest] = splitProps(props, [...SPLIT_PROPS]); return ( any) { + const history = createMemoryHistory(); + history.set({ value: url }); + const root = document.createElement("div"); + document.body.appendChild(root); + const dispose = render( + () => ( + + + + + ), + root + ); + return { + anchor: () => root.querySelector("a")!, + dispose: () => { + dispose(); + root.remove(); + } + }; +} + +// Client behaviour of . The server only fast path is covered in test/ssr/anchor.spec.tsx; +// everything here goes through the `splitProps` + spread path, as it does in a browser. +describe("", () => { + test("resolves href and marks itself inactive", () => { + const { anchor, dispose } = mount("/docs/intro", () => go); + expect(anchor().getAttribute("href")).toBe("/other"); + expect(anchor().className).toBe("inactive"); + expect(anchor().hasAttribute("aria-current")).toBe(false); + expect(anchor().hasAttribute("link")).toBe(true); + expect(anchor().textContent).toBe("go"); + dispose(); + }); + + test("marks itself active on an exact match", () => { + const { anchor, dispose } = mount("/docs/intro", () => here); + expect(anchor().className).toBe("active"); + expect(anchor().getAttribute("aria-current")).toBe("page"); + dispose(); + }); + + test("marks itself active for a parent path unless end is set", () => { + const parent = mount("/docs/intro", () => parent); + expect(parent.anchor().className).toBe("active"); + expect(parent.anchor().hasAttribute("aria-current")).toBe(false); + parent.dispose(); + + const end = mount("/docs/intro", () => ( + + parent + + )); + expect(end.anchor().className).toBe("inactive"); + end.dispose(); + }); + + test("ignores trailing slashes and case when matching", () => { + const slash = mount("/docs/intro", () => x); + expect(slash.anchor().className).toBe("active"); + slash.dispose(); + + const upper = mount("/docs/intro", () => x); + expect(upper.anchor().className).toBe("active"); + upper.dispose(); + }); + + test("honours activeClass and inactiveClass", () => { + const off = mount("/docs/intro", () => ( + + x + + )); + expect(off.anchor().className).toBe("off"); + off.dispose(); + + const on = mount("/docs/intro", () => ( + + x + + )); + expect(on.anchor().className).toBe("on"); + on.dispose(); + }); + + test("keeps a user supplied class alongside the active state class", () => { + const { anchor, dispose } = mount("/docs/intro", () => ( + + x + + )); + expect(anchor().classList.contains("btn")).toBe(true); + expect(anchor().classList.contains("inactive")).toBe(true); + dispose(); + }); + + test("merges a user supplied classList", () => { + const { anchor, dispose } = mount("/docs/intro", () => ( + + x + + )); + expect(anchor().classList.contains("extra")).toBe(true); + expect(anchor().classList.contains("skipped")).toBe(false); + expect(anchor().classList.contains("inactive")).toBe(true); + dispose(); + }); + + test("serialises state only when it is provided", () => { + const without = mount("/docs/intro", () => x); + expect(without.anchor().hasAttribute("state")).toBe(false); + without.dispose(); + + const withState = mount("/docs/intro", () => ( + + x + + )); + expect(withState.anchor().getAttribute("state")).toBe(JSON.stringify({ a: 1 })); + withState.dispose(); + }); + + test("forwards unknown props to the anchor", () => { + const { anchor, dispose } = mount("/docs/intro", () => ( + + x + + )); + expect(anchor().id).toBe("lnk"); + expect(anchor().getAttribute("target")).toBe("_blank"); + expect(anchor().getAttribute("rel")).toBe("external"); + expect(anchor().getAttribute("aria-label")).toBe("go"); + expect(anchor().getAttribute("href")).toBe("/other"); + expect(anchor().className).toBe("inactive"); + dispose(); + }); + + test("forwards the router's own passthrough attributes", () => { + const { anchor, dispose } = mount("/docs/intro", () => ( + + x + + )); + expect(anchor().hasAttribute("replace")).toBe(true); + expect(anchor().hasAttribute("noScroll")).toBe(true); + expect(anchor().getAttribute("preload")).toBe("false"); + dispose(); + }); + + test("resolves a relative href against the current route", () => { + const { anchor, dispose } = mount("/docs/intro", () => x); + expect(anchor().getAttribute("href")).toBe("/docs/intro/sibling"); + dispose(); + }); + + test("leaves an external href untouched", () => { + const { anchor, dispose } = mount("/docs/intro", () => x); + expect(anchor().getAttribute("href")).toBe("https://example.com"); + dispose(); + }); + + test("renders nested children", () => { + const { anchor, dispose } = mount("/docs/intro", () => ( + + deep + + )); + expect(anchor().querySelector("span")!.textContent).toBe("deep"); + dispose(); + }); + + // upstream inserts children through the spread, so a `ref` runs after they are in place + test("a ref sees the children already inserted", () => { + let seen: string | undefined; + const { dispose } = mount("/docs/intro", () => ( + (seen = el.textContent ?? undefined)}> + child + + )); + expect(seen).toBe("child"); + dispose(); + }); + + test.each([ + ["no extra props", () => x], + [ + "with extra props", + () => ( + + x + + ) + ] + ])("updates the active class on navigation (%s)", async (_name, Link) => { + let navigate!: ReturnType; + const Page = () => { + navigate = useNavigate(); + return Link(); + }; + const { anchor, dispose } = mount("/docs/intro", Page); + + expect(anchor().className).toBe("inactive"); + expect(anchor().hasAttribute("aria-current")).toBe(false); + + navigate("/other"); + await vi.waitFor(() => expect(anchor().className).toBe("active")); + + expect(anchor().getAttribute("aria-current")).toBe("page"); + + dispose(); + }); +}); diff --git a/test/ssr/anchor.spec.tsx b/test/ssr/anchor.spec.tsx new file mode 100644 index 00000000..d52e86ef --- /dev/null +++ b/test/ssr/anchor.spec.tsx @@ -0,0 +1,199 @@ +import { createComponent } from "solid-js"; +import { renderToString } from "solid-js/web"; +import { A, Route, StaticRouter } from "../../src/index.jsx"; + +// On the server takes a fast path that skips `splitProps` and the JSX spread when the +// caller passes nothing beyond the props consumes itself. Anything else, `innerHTML` +// included, falls back to the spread. These cover both, and the cases where the two could +// drift apart. +function renderAt(Comp: () => any, url = "http://localhost/docs/intro"): string { + const html = renderToString(() => ( + + + + )); + const start = html.indexOf("", start) + 4) + .replace(/ data-hk=("?)[0-9]*\1/g, "") + // the marker serialises as `link` or `link="true"` depending on whether the element + // has a spread; the router only ever tests for its presence + .replace(/ link(="true")?/, " link") + .replace(/ +>/g, ">") + ); +} + +const classOf = (html: string) => /class="([^"]*)"/.exec(html)?.[1]; + +describe(" server rendering", () => { + test("renders an inactive link", () => { + expect(renderAt(() => go)).toBe( + `go` + ); + }); + + test("renders an active link with aria-current", () => { + expect(renderAt(() => here)).toBe( + `here` + ); + }); + + test("honours end, activeClass and inactiveClass", () => { + expect( + classOf( + renderAt(() => ( + + x + + )) + ) + ).toBe("inactive"); + expect(classOf(renderAt(() => x))).toContain("active"); + expect( + classOf( + renderAt(() => ( + + x + + )) + ) + ).toBe("off"); + }); + + test("folds a user class into the state class", () => { + expect( + classOf( + renderAt(() => ( + + x + + )) + ) + ).toBe("btn inactive"); + }); + + test("omits state unless it is provided", () => { + expect(renderAt(() => x)).not.toContain("state="); + expect( + renderAt(() => ( + + x + + )) + ).toContain(`state="{"a":1}"`); + }); + + test("reads state once, so a getter is not observed twice", () => { + let reads = 0; + const props = { + href: "/other", + get state() { + return { read: ++reads }; + }, + children: "x" + }; + expect(renderAt(() => createComponent(A, props))).toContain(`state="{"read":1}"`); + expect(reads).toBe(1); + }); + + test("resolves relative hrefs and leaves external ones alone", () => { + expect(renderAt(() => x)).toContain(`href="/docs/intro/sibling"`); + expect(renderAt(() => x)).toContain( + `href="https://example.com"` + ); + }); + + test("renders children, including nested elements", () => { + expect(renderAt(() => text)).toContain(">text"); + expect( + renderAt(() => ( + + deep + + )) + ).toContain("deep"); + expect(renderAt(() => )).toContain(">"); + }); + + test("forwards extra props via the spread path", () => { + const html = renderAt(() => ( + + x + + )); + expect(html).toContain(`id="lnk"`); + expect(html).toContain(`target="_blank"`); + expect(html).toContain(`rel="external"`); + expect(html).toContain(`href="/other"`); + expect(classOf(html)).toBe("inactive"); + }); + + test("merges a classList through the spread path", () => { + const html = renderAt(() => ( + + x + + )); + expect(classOf(html)).toBe("inactive extra"); + }); + + // `innerHTML` and `textContent` only work if children stay in the spread, so the fallback + // must not supply an explicit children expression. + test("honours innerHTML and textContent", () => { + expect(renderAt(() => )).toContain( + "inside" + ); + expect(renderAt(() => )).toContain(">inside"); + }); + + test("gives innerHTML precedence over children", () => { + expect( + renderAt(() => ( + + kids + + )) + ).toContain("ih"); + }); + + // Class names are keys of an object literal, so `__proto__` has to land as an own property + // rather than reassigning a prototype. + test.each([ + ["class", "/other"], + ["inactiveClass", "/other"], + // activeClass only lands on a link whose href matches the current location + ["activeClass", "/docs/intro"] + ] as const)("handles a __proto__ value for %s", (key, href) => { + const html = renderAt(() => createComponent(A, { href, [key]: "__proto__", children: "x" })); + expect(classOf(html)).toContain("__proto__"); + }); + + test("handles a __proto__ key in a user classList", () => { + const html = renderAt(() => ( + + x + + )); + expect(classOf(html)).toContain("__proto__"); + }); + + // A non-enumerable own prop still reaches the element via splitProps, so the fast path + // check must not miss it. + test("does not drop a non-enumerable own prop", () => { + const props: any = { href: "/other", children: "x" }; + Object.defineProperty(props, "id", { value: "hidden", enumerable: false }); + expect(renderAt(() => createComponent(A, props))).toContain(`id="hidden"`); + }); + + test("both paths agree on the state classes", () => { + const fast = renderAt(() => x); + const spread = renderAt(() => ( + + x + + )); + expect(classOf(fast)).toBe(classOf(spread)); + expect(fast.includes(`aria-current="page"`)).toBe(spread.includes(`aria-current="page"`)); + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index bf3d6e7d..b16615ba 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -1,4 +1,4 @@ -import { defineConfig, Plugin } from "vitest/config"; +import { configDefaults, defineConfig, Plugin } from "vitest/config"; import solidPlugin from "vite-plugin-solid"; export default defineConfig({ @@ -20,6 +20,8 @@ export default defineConfig({ test: { environment: "jsdom", globals: true, + // test/ssr needs the SSR JSX transform and the real `isServer`, see vitest.ssr.config.ts + exclude: [...configDefaults.exclude, "test/ssr/**"], testTransformMode: { web: ["/\.[jt]sx?$/"] }, setupFiles: ["./test/setup.ts"], mockReset: true diff --git a/vitest.ssr.config.ts b/vitest.ssr.config.ts new file mode 100644 index 00000000..048ac130 --- /dev/null +++ b/vitest.ssr.config.ts @@ -0,0 +1,20 @@ +import { defineConfig, Plugin } from "vitest/config"; +import solidPlugin from "vite-plugin-solid"; + +// Server rendering needs the SSR JSX transform and the real `isServer`, so it cannot +// share the DOM config or its setup file. Specs live in test/ssr. +export default defineConfig({ + plugins: [solidPlugin({ solid: { generate: "ssr", hydratable: true } }) as Plugin], + resolve: { + conditions: ["module", "node", "development|production"] + }, + build: { + target: "esnext" + }, + test: { + environment: "node", + globals: true, + include: ["test/ssr/**/*.spec.tsx"], + mockReset: true + } +});