Skip to content

Commit e9faea6

Browse files
committed
perf(A): cut server render cost by ~5x
<A> 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.89ms; it now renders in 1.09ms. - 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 - skip JSON.stringify when there is no state - build the classList in one object instead of up to three spreads - on the server, when the caller passes nothing beyond the props <A> 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 Rendered output is unchanged on the client, and unchanged on the server apart from one insignificant space inside the tag on the fast path. Adds client and server test coverage for <A>, which had none.
1 parent be412bc commit e9faea6

7 files changed

Lines changed: 457 additions & 22 deletions

File tree

.changeset/spotty-moons-jump.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
"@solidjs/router": patch
3+
---
4+
5+
Speed up `<A>` server rendering by roughly 5x
6+
7+
`<A>` spent most of its server render time in `mergeProps` and `splitProps`, which build
8+
accessor-backed prop objects that can never be observed during a one shot
9+
`renderToString`. The default `activeClass` / `inactiveClass` merge is gone, the
10+
`location.pathname` normalization is shared instead of repeated per link,
11+
`JSON.stringify` is skipped when there is no `state`, and on the server a link that
12+
passes nothing beyond the props `<A>` consumes itself now skips `splitProps` and the JSX
13+
spread entirely.
14+
15+
Rendered output is unchanged on the client and on the server, apart from one
16+
insignificant space inside the tag on the server fast path. Client bundles get slightly
17+
smaller since the server branch is constant folded away.

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@
3131
"scripts": {
3232
"build": "rm -rf dist && tsc && rollup -c",
3333
"prepublishOnly": "npm run build",
34-
"test": "vitest run && npm run test:types",
34+
"test": "vitest run && npm run test:ssr && npm run test:types",
3535
"test:watch": "vitest",
36+
"test:ssr": "vitest run --config vitest.ssr.config.ts",
3637
"test:types": "tsc --project tsconfig.test.json",
3738
"pretty": "prettier --write \"{src,test}/**/*.{ts,tsx}\"",
3839
"release": "pnpm build && changeset publish"

src/components.tsx

Lines changed: 76 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/*@refresh skip*/
22
import type { JSX } from "solid-js";
3-
import { createMemo, mergeProps, splitProps } from "solid-js";
3+
import { createMemo, splitProps } from "solid-js";
4+
import { isServer } from "solid-js/web";
45
import {
56
useHref,
67
useLocation,
@@ -20,7 +21,7 @@ declare module "solid-js" {
2021
noScroll?: boolean;
2122
replace?: boolean;
2223
preload?: boolean;
23-
link?: boolean;
24+
link?: boolean | string;
2425
}
2526
}
2627
}
@@ -34,16 +35,48 @@ export interface AnchorProps extends Omit<JSX.AnchorHTMLAttributes<HTMLAnchorEle
3435
activeClass?: string | undefined;
3536
end?: boolean | undefined;
3637
}
38+
// Every <A> on a page normalizes the same `location.pathname`, so a one entry cache
39+
// collapses that work to once per navigation instead of once per link. Pure function
40+
// of the input, so a stale or missed entry can only cost a recompute.
41+
let lastPathname: string | undefined;
42+
let lastNormalizedPathname: string;
43+
function normalizeLocationPath(pathname: string): string {
44+
if (pathname !== lastPathname) {
45+
lastNormalizedPathname = decodeURI(normalizePath(pathname).toLowerCase().replace(/\/$/, ""));
46+
lastPathname = pathname;
47+
}
48+
return lastNormalizedPathname;
49+
}
50+
51+
const CONSUMED_PROPS = [
52+
"href",
53+
"state",
54+
"class",
55+
"activeClass",
56+
"inactiveClass",
57+
"end",
58+
"children"
59+
] as const satisfies readonly (keyof AnchorProps)[];
60+
61+
const CONSUMED_PROPS_SET: ReadonlySet<string> = new Set(CONSUMED_PROPS);
62+
3763
export function A(props: AnchorProps) {
38-
props = mergeProps({ inactiveClass: "inactive", activeClass: "active" }, props);
39-
const [, rest] = splitProps(props, [
40-
"href",
41-
"state",
42-
"class",
43-
"activeClass",
44-
"inactiveClass",
45-
"end"
46-
]);
64+
// `splitProps` plus the JSX spread is the bulk of the per link cost, and neither is
65+
// needed when the caller passes nothing beyond the props <A> consumes itself. Server
66+
// only: a render pass there is one shot, so the key set cannot grow after this check,
67+
// which it can on the client when the caller uses a reactive spread. The branch is
68+
// constant folded out of client builds.
69+
let fastPath = false;
70+
if (isServer) {
71+
fastPath = true;
72+
for (const key in props) {
73+
if (!CONSUMED_PROPS_SET.has(key)) {
74+
fastPath = false;
75+
break;
76+
}
77+
}
78+
}
79+
4780
const to = useResolvedPath(() => props.href);
4881
const href = useHref(to);
4982
const location = useLocation();
@@ -52,24 +85,47 @@ export function A(props: AnchorProps) {
5285
if (to_ === undefined) return [false, false];
5386
// trailing slashes are ignored so `/route` and `/route/` share active state
5487
const path = normalizePath(to_.split(/[?#]/, 1)[0]).toLowerCase().replace(/\/$/, "");
55-
const loc = decodeURI(normalizePath(location.pathname).toLowerCase().replace(/\/$/, ""));
88+
const loc = normalizeLocationPath(location.pathname);
5689
return [props.end ? path === loc : loc.startsWith(path + "/") || loc === path, path === loc];
5790
});
91+
const state = () => (props.state !== undefined ? JSON.stringify(props.state) : undefined);
92+
const buildClassList = (extra?: Record<string, boolean>) => {
93+
const active = isActive()[0] as boolean;
94+
const list: Record<string, boolean> = {};
95+
if (props.class) list[props.class] = true;
96+
list[props.inactiveClass ?? "inactive"] = !active;
97+
list[props.activeClass ?? "active"] = active;
98+
if (extra) Object.assign(list, extra);
99+
return list;
100+
};
101+
102+
if (fastPath) {
103+
return (
104+
<a
105+
href={href() || props.href}
106+
state={state()}
107+
classList={buildClassList()}
108+
link="true"
109+
aria-current={isActive()[1] ? "page" : undefined}
110+
>
111+
{props.children}
112+
</a>
113+
);
114+
}
115+
116+
const [, rest] = splitProps(props, [...CONSUMED_PROPS]);
58117

59118
return (
60119
<a
61120
{...rest}
62121
href={href() || props.href}
63-
state={JSON.stringify(props.state)}
64-
classList={{
65-
...(props.class && { [props.class]: true }),
66-
[props.inactiveClass!]: !isActive()[0],
67-
[props.activeClass!]: isActive()[0],
68-
...rest.classList
69-
}}
122+
state={state()}
123+
classList={buildClassList(rest.classList as Record<string, boolean> | undefined)}
70124
link
71125
aria-current={isActive()[1] ? "page" : undefined}
72-
/>
126+
>
127+
{props.children}
128+
</a>
73129
);
74130
}
75131

test/anchor.spec.tsx

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,212 @@
1+
// @vitest-environment jsdom
2+
import { vi } from "vitest";
3+
import { render } from "solid-js/web";
4+
import { A, MemoryRouter, Route, createMemoryHistory, useNavigate } from "../src/index.jsx";
5+
6+
// jsdom has no scrollTo, and navigating triggers the router's scroll handling
7+
window.scrollTo = vi.fn() as any;
8+
9+
const wait = (ms: number = 10) => new Promise(r => setTimeout(r, ms));
10+
11+
function mount(url: string, Comp: () => any) {
12+
const history = createMemoryHistory();
13+
history.set({ value: url });
14+
const root = document.createElement("div");
15+
document.body.appendChild(root);
16+
const dispose = render(
17+
() => (
18+
<MemoryRouter history={history}>
19+
<Route path="/docs/intro" component={Comp} />
20+
<Route path="/other" component={Comp} />
21+
</MemoryRouter>
22+
),
23+
root
24+
);
25+
return {
26+
anchor: () => root.querySelector("a")!,
27+
dispose: () => {
28+
dispose();
29+
root.remove();
30+
}
31+
};
32+
}
33+
34+
// Client behaviour of <A>. The server only fast path is covered in test/ssr/anchor.spec.tsx;
35+
// everything here goes through the `splitProps` + spread path, as it does in a browser.
36+
describe("<A>", () => {
37+
test("resolves href and marks itself inactive", () => {
38+
const { anchor, dispose } = mount("/docs/intro", () => <A href="/other">go</A>);
39+
expect(anchor().getAttribute("href")).toBe("/other");
40+
expect(anchor().className).toBe("inactive");
41+
expect(anchor().hasAttribute("aria-current")).toBe(false);
42+
expect(anchor().hasAttribute("link")).toBe(true);
43+
expect(anchor().textContent).toBe("go");
44+
dispose();
45+
});
46+
47+
test("marks itself active on an exact match", () => {
48+
const { anchor, dispose } = mount("/docs/intro", () => <A href="/docs/intro">here</A>);
49+
expect(anchor().className).toBe("active");
50+
expect(anchor().getAttribute("aria-current")).toBe("page");
51+
dispose();
52+
});
53+
54+
test("marks itself active for a parent path unless end is set", () => {
55+
const parent = mount("/docs/intro", () => <A href="/docs">parent</A>);
56+
expect(parent.anchor().className).toBe("active");
57+
expect(parent.anchor().hasAttribute("aria-current")).toBe(false);
58+
parent.dispose();
59+
60+
const end = mount("/docs/intro", () => (
61+
<A href="/docs" end>
62+
parent
63+
</A>
64+
));
65+
expect(end.anchor().className).toBe("inactive");
66+
end.dispose();
67+
});
68+
69+
test("ignores trailing slashes and case when matching", () => {
70+
const slash = mount("/docs/intro", () => <A href="/docs/intro/">x</A>);
71+
expect(slash.anchor().className).toBe("active");
72+
slash.dispose();
73+
74+
const upper = mount("/docs/intro", () => <A href="/DOCS/Intro">x</A>);
75+
expect(upper.anchor().className).toBe("active");
76+
upper.dispose();
77+
});
78+
79+
test("honours activeClass and inactiveClass", () => {
80+
const off = mount("/docs/intro", () => (
81+
<A href="/other" activeClass="on" inactiveClass="off">
82+
x
83+
</A>
84+
));
85+
expect(off.anchor().className).toBe("off");
86+
off.dispose();
87+
88+
const on = mount("/docs/intro", () => (
89+
<A href="/docs/intro" activeClass="on" inactiveClass="off">
90+
x
91+
</A>
92+
));
93+
expect(on.anchor().className).toBe("on");
94+
on.dispose();
95+
});
96+
97+
test("keeps a user supplied class alongside the active state class", () => {
98+
const { anchor, dispose } = mount("/docs/intro", () => (
99+
<A href="/other" class="btn">
100+
x
101+
</A>
102+
));
103+
expect(anchor().classList.contains("btn")).toBe(true);
104+
expect(anchor().classList.contains("inactive")).toBe(true);
105+
dispose();
106+
});
107+
108+
test("merges a user supplied classList", () => {
109+
const { anchor, dispose } = mount("/docs/intro", () => (
110+
<A href="/other" classList={{ extra: true, skipped: false }}>
111+
x
112+
</A>
113+
));
114+
expect(anchor().classList.contains("extra")).toBe(true);
115+
expect(anchor().classList.contains("skipped")).toBe(false);
116+
expect(anchor().classList.contains("inactive")).toBe(true);
117+
dispose();
118+
});
119+
120+
test("serialises state only when it is provided", () => {
121+
const without = mount("/docs/intro", () => <A href="/other">x</A>);
122+
expect(without.anchor().hasAttribute("state")).toBe(false);
123+
without.dispose();
124+
125+
const withState = mount("/docs/intro", () => (
126+
<A href="/other" state={{ a: 1 }}>
127+
x
128+
</A>
129+
));
130+
expect(withState.anchor().getAttribute("state")).toBe(JSON.stringify({ a: 1 }));
131+
withState.dispose();
132+
});
133+
134+
test("forwards unknown props to the anchor", () => {
135+
const { anchor, dispose } = mount("/docs/intro", () => (
136+
<A href="/other" id="lnk" target="_blank" rel="external" aria-label="go">
137+
x
138+
</A>
139+
));
140+
expect(anchor().id).toBe("lnk");
141+
expect(anchor().getAttribute("target")).toBe("_blank");
142+
expect(anchor().getAttribute("rel")).toBe("external");
143+
expect(anchor().getAttribute("aria-label")).toBe("go");
144+
expect(anchor().getAttribute("href")).toBe("/other");
145+
expect(anchor().className).toBe("inactive");
146+
dispose();
147+
});
148+
149+
test("forwards the router's own passthrough attributes", () => {
150+
const { anchor, dispose } = mount("/docs/intro", () => (
151+
<A href="/other" replace noScroll preload={false}>
152+
x
153+
</A>
154+
));
155+
expect(anchor().hasAttribute("replace")).toBe(true);
156+
expect(anchor().hasAttribute("noScroll")).toBe(true);
157+
expect(anchor().getAttribute("preload")).toBe("false");
158+
dispose();
159+
});
160+
161+
test("resolves a relative href against the current route", () => {
162+
const { anchor, dispose } = mount("/docs/intro", () => <A href="sibling">x</A>);
163+
expect(anchor().getAttribute("href")).toBe("/docs/intro/sibling");
164+
dispose();
165+
});
166+
167+
test("leaves an external href untouched", () => {
168+
const { anchor, dispose } = mount("/docs/intro", () => <A href="https://example.com">x</A>);
169+
expect(anchor().getAttribute("href")).toBe("https://example.com");
170+
dispose();
171+
});
172+
173+
test("renders nested children", () => {
174+
const { anchor, dispose } = mount("/docs/intro", () => (
175+
<A href="/other">
176+
<span>deep</span>
177+
</A>
178+
));
179+
expect(anchor().querySelector("span")!.textContent).toBe("deep");
180+
dispose();
181+
});
182+
183+
test.each([
184+
["no extra props", () => <A href="/other">x</A>],
185+
[
186+
"with extra props",
187+
() => (
188+
<A href="/other" id="lnk">
189+
x
190+
</A>
191+
)
192+
]
193+
])("updates the active class on navigation (%s)", async (_name, Link) => {
194+
let navigate!: ReturnType<typeof useNavigate>;
195+
const Page = () => {
196+
navigate = useNavigate();
197+
return Link();
198+
};
199+
const { anchor, dispose } = mount("/docs/intro", Page);
200+
201+
expect(anchor().className).toBe("inactive");
202+
expect(anchor().hasAttribute("aria-current")).toBe(false);
203+
204+
navigate("/other");
205+
await wait();
206+
207+
expect(anchor().className).toBe("active");
208+
expect(anchor().getAttribute("aria-current")).toBe("page");
209+
210+
dispose();
211+
});
212+
});

0 commit comments

Comments
 (0)