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 + } +});