Skip to content

Commit 29f745a

Browse files
MohamedH1998MohamedH1998
committed
perf: avoid structuredClone in sidebar generation for prerendered builds
1 parent 4c5c1e3 commit 29f745a

6 files changed

Lines changed: 235 additions & 3 deletions

File tree

‎.changeset/harden-sidebar-fast-path.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@astrojs/starlight': patch
3+
---
4+
5+
Harden sidebar prerender fast-path behavior while preserving exported API isolation.
6+
7+
- Keep `getSidebar()` and `getSidebarFromConfig()` clone-based and isolated per call.
8+
- Ensure clone-path sidebars always normalize current-link state before marking the active page.
9+
- Add regression tests for mixed fast-path/clone-path usage and stronger `config.prerender` gate test cleanup.

‎packages/starlight/__tests__/basics/route-data.test.ts‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { expect, test, vi } from 'vitest';
22
import { getRouteDataTestContext } from '../test-utils';
33
import { generateRouteData } from '../../utils/routing/data';
44
import { routes } from '../../utils/routing';
5+
import { flattenSidebar } from '../../utils/navigation';
56

67
vi.mock('astro:content', async () =>
78
(await import('../test-utils')).mockedAstroContent({
@@ -86,3 +87,85 @@ test('uses explicit last updated date from frontmatter', () => {
8687
expect(data.lastUpdated).toBeInstanceOf(Date);
8788
expect(data.lastUpdated).toEqual(route.entry.data.lastUpdated);
8889
});
90+
91+
test('uses getSidebarForRender when prerender is enabled', async () => {
92+
const [{ default: config }, navigation] = await Promise.all([
93+
import('virtual:starlight/user-config'),
94+
import('../../utils/navigation'),
95+
]);
96+
const originalPrerender = config.prerender;
97+
const getSidebarForRenderSpy = vi.spyOn(navigation, 'getSidebarForRender');
98+
const getSidebarSpy = vi.spyOn(navigation, 'getSidebar');
99+
100+
try {
101+
config.prerender = true;
102+
const route = routes[0]!;
103+
generateRouteData({
104+
props: { ...route, headings: [] },
105+
context: getRouteDataTestContext({ pathname: '/' }),
106+
});
107+
108+
expect(getSidebarForRenderSpy).toHaveBeenCalledTimes(1);
109+
expect(getSidebarSpy).not.toHaveBeenCalled();
110+
} finally {
111+
config.prerender = originalPrerender;
112+
getSidebarForRenderSpy.mockRestore();
113+
getSidebarSpy.mockRestore();
114+
}
115+
});
116+
117+
test('keeps prerender sidebar snapshots isolated between route data calls', async () => {
118+
const { default: config } = await import('virtual:starlight/user-config');
119+
const originalPrerender = config.prerender;
120+
121+
try {
122+
config.prerender = true;
123+
const homeRoute = routes[0]!;
124+
const impactRoute = routes[3]!;
125+
const firstData = generateRouteData({
126+
props: { ...homeRoute, headings: [] },
127+
context: getRouteDataTestContext({ pathname: '/' }),
128+
});
129+
const secondData = generateRouteData({
130+
props: { ...impactRoute, headings: [] },
131+
context: getRouteDataTestContext({ pathname: '/environmental-impact/' }),
132+
});
133+
134+
// firstData.sidebar must not have been mutated by the second generateRouteData call
135+
const firstCurrent = flattenSidebar(firstData.sidebar).filter((entry) => entry.isCurrent);
136+
const secondCurrent = flattenSidebar(secondData.sidebar).filter((entry) => entry.isCurrent);
137+
138+
expect(firstCurrent).toHaveLength(1);
139+
expect(firstCurrent[0]?.href).toBe('/');
140+
expect(secondCurrent).toHaveLength(1);
141+
expect(secondCurrent[0]?.href).toBe('/environmental-impact/');
142+
} finally {
143+
config.prerender = originalPrerender;
144+
}
145+
});
146+
147+
test('uses getSidebar when prerender is disabled', async () => {
148+
const [{ default: config }, navigation] = await Promise.all([
149+
import('virtual:starlight/user-config'),
150+
import('../../utils/navigation'),
151+
]);
152+
const originalPrerender = config.prerender;
153+
const getSidebarForRenderSpy = vi.spyOn(navigation, 'getSidebarForRender');
154+
const getSidebarSpy = vi.spyOn(navigation, 'getSidebar');
155+
156+
try {
157+
config.prerender = false;
158+
const route = routes[0]!;
159+
generateRouteData({
160+
props: { ...route, headings: [] },
161+
context: getRouteDataTestContext({ pathname: '/' }),
162+
});
163+
164+
expect(getSidebarSpy).toHaveBeenCalledTimes(1);
165+
expect(getSidebarForRenderSpy).not.toHaveBeenCalled();
166+
} finally {
167+
config.prerender = originalPrerender;
168+
getSidebarForRenderSpy.mockRestore();
169+
getSidebarSpy.mockRestore();
170+
}
171+
});

‎packages/starlight/__tests__/i18n-sidebar/i18n-sidebar.test.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,4 +303,40 @@ describe('getSidebar', () => {
303303

304304
getLocaleRoutes.mockRestore();
305305
});
306+
307+
test('reuses the same sidebar tree in getSidebarForRender and resets previous current entries', async () => {
308+
vi.resetModules();
309+
const navigation = await import('../../utils/navigation');
310+
311+
const first = navigation.getSidebarForRender('/', undefined);
312+
const second = navigation.getSidebarForRender('/environmental-impact/', undefined);
313+
314+
expect(first).toBe(second);
315+
316+
const links = navigation.flattenSidebar(second);
317+
const currentLinks = links.filter((entry) => entry.isCurrent);
318+
expect(currentLinks).toHaveLength(1);
319+
expect(currentLinks[0]?.href).toBe('/environmental-impact/');
320+
expect(links.find((entry) => entry.href === '/')?.isCurrent).toBe(false);
321+
});
322+
323+
test('tracks current sidebar entry independently per locale in getSidebarForRender', async () => {
324+
vi.resetModules();
325+
const navigation = await import('../../utils/navigation');
326+
327+
const enSidebar = navigation.getSidebarForRender('/getting-started/', undefined);
328+
const frSidebarFirst = navigation.getSidebarForRender('/fr/getting-started/', 'fr');
329+
const frSidebarSecond = navigation.getSidebarForRender('/fr/environmental-impact/', 'fr');
330+
331+
expect(frSidebarFirst).toBe(frSidebarSecond);
332+
expect(enSidebar).not.toBe(frSidebarSecond);
333+
334+
const enCurrent = navigation.flattenSidebar(enSidebar).filter((entry) => entry.isCurrent);
335+
const frCurrent = navigation.flattenSidebar(frSidebarSecond).filter((entry) => entry.isCurrent);
336+
337+
expect(enCurrent).toHaveLength(1);
338+
expect(enCurrent[0]?.href).toBe('/getting-started/');
339+
expect(frCurrent).toHaveLength(1);
340+
expect(frCurrent[0]?.href).toBe('/fr/environmental-impact/');
341+
});
306342
});

‎packages/starlight/__tests__/sidebar/navigation.test.ts‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { describe, expect, test, vi } from 'vitest';
2-
import { getSidebar } from '../../utils/navigation';
2+
import config from 'virtual:starlight/user-config';
3+
import { getSidebar, getSidebarFromConfig } from '../../utils/navigation';
34
import type { SidebarEntry } from '../../utils/routing/types';
45

56
vi.mock('astro:content', async () =>
@@ -194,4 +195,52 @@ describe('getSidebar', () => {
194195
expect(includesOnlyLinksWithTrailingSlash(internalLinksGroup)).toBe(true);
195196
expect(includesOnlyLinksWithTrailingSlash(autoGeneratedLinksGroup)).toBe(true);
196197
});
198+
199+
test('returns isolated sidebars on repeated getSidebar calls', () => {
200+
const first = getSidebar('/', undefined);
201+
const second = getSidebar('/', undefined);
202+
203+
expect(first).not.toBe(second);
204+
expect(first[0]).not.toBe(second[0]);
205+
206+
const firstHome = first[0];
207+
const secondHome = second[0];
208+
if (firstHome?.type !== 'link' || secondHome?.type !== 'link') {
209+
throw new Error('Expected first sidebar entry to be a link');
210+
}
211+
212+
firstHome.isCurrent = false;
213+
expect(secondHome.isCurrent).toBe(true);
214+
});
215+
216+
test('returns isolated sidebars from getSidebarFromConfig calls', () => {
217+
const first = getSidebarFromConfig(config.sidebar, '/', undefined);
218+
const second = getSidebarFromConfig(config.sidebar, '/', undefined);
219+
220+
expect(first).not.toBe(second);
221+
expect(first[0]).not.toBe(second[0]);
222+
223+
const firstHome = first[0];
224+
const secondHome = second[0];
225+
if (firstHome?.type !== 'link' || secondHome?.type !== 'link') {
226+
throw new Error('Expected first sidebar entry to be a link');
227+
}
228+
229+
firstHome.isCurrent = false;
230+
expect(secondHome.isCurrent).toBe(true);
231+
});
232+
233+
test('clone path remains correct after fast path mutates cached sidebar', async () => {
234+
vi.resetModules();
235+
const navigation = await import('../../utils/navigation');
236+
237+
navigation.getSidebarForRender('/', undefined);
238+
const clonedSidebar = navigation.getSidebar('/resources/plugins/', undefined);
239+
const currentLinks = navigation
240+
.flattenSidebar(clonedSidebar)
241+
.filter((entry) => entry.isCurrent);
242+
243+
expect(currentLinks).toHaveLength(1);
244+
expect(currentLinks[0]?.href).toBe('/resources/plugins/');
245+
});
197246
});

‎packages/starlight/utils/navigation.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,7 @@ function sidebarFromDir(
365365
* @see getSidebarFromIntermediateSidebar
366366
*/
367367
const intermediateSidebars = new Map<string | undefined, SidebarEntry[]>();
368+
const lastCurrentByLocale = new Map<string | undefined, SidebarLink>();
368369

369370
/** Get the sidebar for the current page using the global config. */
370371
export function getSidebar(pathname: string, locale: string | undefined): SidebarEntry[] {
@@ -376,6 +377,28 @@ export function getSidebar(pathname: string, locale: string | undefined): Sideba
376377
return getSidebarFromIntermediateSidebar(intermediateSidebar, pathname);
377378
}
378379

380+
/**
381+
* Internal fast path used during prerendered route generation.
382+
* Mutates the cached intermediate sidebar in place by resetting the previous current entry
383+
* and marking the current page.
384+
*/
385+
export function getSidebarForRender(pathname: string, locale: string | undefined): SidebarEntry[] {
386+
let intermediateSidebar = intermediateSidebars.get(locale);
387+
if (!intermediateSidebar) {
388+
intermediateSidebar = getIntermediateSidebarFromConfig(config.sidebar, pathname, locale);
389+
intermediateSidebars.set(locale, intermediateSidebar);
390+
}
391+
392+
const previousCurrent = lastCurrentByLocale.get(locale);
393+
if (previousCurrent) previousCurrent.isCurrent = false;
394+
395+
const currentEntry = findAndMarkCurrentEntry(intermediateSidebar, pathname);
396+
if (currentEntry) lastCurrentByLocale.set(locale, currentEntry);
397+
else lastCurrentByLocale.delete(locale);
398+
399+
return intermediateSidebar;
400+
}
401+
379402
/** Get the sidebar for the current page using the specified sidebar config. */
380403
export function getSidebarFromConfig(
381404
sidebarConfig: StarlightConfig['sidebar'],
@@ -407,10 +430,22 @@ function getSidebarFromIntermediateSidebar(
407430
pathname: string
408431
): SidebarEntry[] {
409432
const sidebar = structuredClone(intermediateSidebar);
433+
resetCurrentEntries(sidebar);
410434
setIntermediateSidebarCurrentEntry(sidebar, pathname);
411435
return sidebar;
412436
}
413437

438+
/** Clears all `isCurrent` states in a sidebar tree. */
439+
function resetCurrentEntries(entries: SidebarEntry[]): void {
440+
for (const entry of entries) {
441+
if (entry.type === 'link') {
442+
entry.isCurrent = false;
443+
} else {
444+
resetCurrentEntries(entry.entries);
445+
}
446+
}
447+
}
448+
414449
/** Marks the current page as such in an intermediate sidebar. */
415450
function setIntermediateSidebarCurrentEntry(
416451
intermediateSidebar: SidebarEntry[],
@@ -429,6 +464,23 @@ function setIntermediateSidebarCurrentEntry(
429464
return false;
430465
}
431466

467+
function findAndMarkCurrentEntry(
468+
entries: SidebarEntry[],
469+
pathname: string
470+
): SidebarLink | undefined {
471+
for (const entry of entries) {
472+
if (entry.type === 'link' && pathsMatch(encodeURI(entry.href), pathname)) {
473+
entry.isCurrent = true;
474+
return entry;
475+
}
476+
if (entry.type === 'group') {
477+
const currentEntry = findAndMarkCurrentEntry(entry.entries, pathname);
478+
if (currentEntry) return currentEntry;
479+
}
480+
}
481+
return undefined;
482+
}
483+
432484
/** Generates a deterministic string based on the content of the passed sidebar. */
433485
export function getSidebarHash(sidebar: SidebarEntry[]): string {
434486
let hash = 0;

‎packages/starlight/utils/routing/data.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import project from 'virtual:starlight/project-context';
33
import config from 'virtual:starlight/user-config';
44
import { generateToC } from '../generateToC';
55
import { getNewestCommitDate } from 'virtual:starlight/git-info';
6-
import { getPrevNextLinks, getSidebar } from '../navigation';
6+
import { getPrevNextLinks, getSidebar, getSidebarForRender } from '../navigation';
77
import { ensureTrailingSlash } from '../path';
88
import { getRouteBySlugParam, normalizeCollectionEntry } from '../routing';
99
import type { Route, StarlightDocsEntry, StarlightRouteData } from './types';
@@ -44,7 +44,10 @@ export function generateRouteData({
4444
context: RouteDataContext;
4545
}): StarlightRouteData {
4646
const { entry, locale, lang } = props;
47-
const sidebar = getSidebar(context.url.pathname, locale);
47+
const renderSidebar = config.prerender
48+
? getSidebarForRender(context.url.pathname, locale)
49+
: getSidebar(context.url.pathname, locale);
50+
const sidebar = config.prerender ? structuredClone(renderSidebar) : renderSidebar;
4851
const siteTitle = getSiteTitle(lang);
4952
return {
5053
...props,

0 commit comments

Comments
 (0)