Stop rendering sidebar cookie state into cached pages (#64007) · github/docs@c455b1a · GitHub
Skip to content

Commit c455b1a

Browse files
heiskrCopilot
heiskr
and
Copilot
authored
Stop rendering sidebar cookie state into cached pages (#64007)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: bf5940de-658e-4a1a-bffa-3018fb78b138
1 parent bc5837d commit c455b1a

9 files changed

Lines changed: 204 additions & 66 deletions

File tree

‎src/fixtures/tests/playwright-rendering.spec.ts‎

Lines changed: 33 additions & 0 deletions

‎src/frame/components/DefaultLayout.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ export const DefaultLayout = (props: Props) => {
221221
>
222222
Skip to main content
223223
</a>
224-
<SidebarCollapseProvider initialCollapsed={mainContext.sidebarCollapsed}>
224+
<SidebarCollapseProvider>
225225
<Header isNarrowMenuOpen={isNarrowMenuOpen} onNarrowMenuToggle={setIsNarrowMenuOpen} />
226226
<div inert={isNarrowMenuOpen} aria-hidden={isNarrowMenuOpen || undefined}>
227227
<ClientSideLanguageRedirect />

‎src/frame/components/context/MainContext.tsx‎

Lines changed: 0 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import type { Response } from 'express'
55
import type { BreadcrumbT } from '@/frame/components/page-header/Breadcrumbs'
66
import type { FeatureFlags } from '@/frame/components/hooks/useFeatureFlags'
77
import type { ExtendedRequest, Permalink, SidebarLink } from '@/types'
8-
import { SIDEBAR_EXPANDED_COOKIE_NAME, SIDEBAR_COLLAPSED_COOKIE_NAME } from '@/frame/lib/constants'
98

109
export type ProductT = {
1110
external: boolean
@@ -126,14 +125,6 @@ export type MainContextT = {
126125
} | null
127126
relativePath?: string | null
128127
sidebarTree?: ProductTreeNode | null
129-
// Per-category expand/collapse overrides for the doc-tree sidebar, read from the
130-
// sidebar_expanded cookie during SSR so the tree renders in its persisted state
131-
// on first paint (no post-mount flash). Keyed by locale-prefixed href.
132-
sidebarExpanded?: Record<string, boolean> | null
133-
// Whether the desktop doc-tree rail is collapsed, read from the sidebar_collapsed
134-
// cookie during SSR so the rail renders in its persisted state on first paint
135-
// (no flash of the open sidebar before it collapses post-mount).
136-
sidebarCollapsed?: boolean
137128
status: number
138129
xHost?: string
139130
}
@@ -172,18 +163,6 @@ export function addUINamespaces(req: ExtendedRequest, ui: UIStrings, namespaces:
172163
}
173164
}
174165

175-
// Guard cookie parsing so a malformed or absent sidebar_expanded value degrades to no overrides.
176-
function parseSidebarExpandedCookie(req: ExtendedRequest): Record<string, boolean> {
177-
const raw = req.cookies?.[SIDEBAR_EXPANDED_COOKIE_NAME]
178-
if (!raw) return {}
179-
try {
180-
const parsed = JSON.parse(raw)
181-
return parsed && typeof parsed === 'object' ? (parsed as Record<string, boolean>) : {}
182-
} catch {
183-
return {}
184-
}
185-
}
186-
187166
// Translations add unused ms.* properties such as ms.openlocfilehash and
188167
// ms.sourcegitcommit, so remove them before building UI namespaces.
189168
export const getMainContext = async (
@@ -296,10 +275,6 @@ export const getMainContext = async (
296275
relativePath: context.page?.relativePath || null,
297276
// Sidebar and REST pages need the minimal product tree.
298277
sidebarTree: (includeSidebarTree && context.sidebarTree) || null,
299-
sidebarExpanded: includeSidebarTree ? parseSidebarExpandedCookie(req) : null,
300-
sidebarCollapsed: includeSidebarTree
301-
? req.cookies?.[SIDEBAR_COLLAPSED_COOKIE_NAME] === 'true'
302-
: false,
303278
status: res.statusCode,
304279
xHost: req.get('x-host') || '',
305280
}

‎src/frame/components/hooks/useIsHydrating.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { useSyncExternalStore } from 'react'
2+
3+
const subscribe = () => () => {}
4+
const getClientSnapshot = () => false
5+
const getServerSnapshot = () => true
6+
7+
// True on the server and while React hydrates server HTML, false for client renders after
8+
// that. Hydrated components flip to false together in one re-render right after hydration
9+
// commits, so components mounted by client-side navigation start at false.
10+
export function useIsHydrating(): boolean {
11+
return useSyncExternalStore(subscribe, getClientSnapshot, getServerSnapshot)
12+
}

‎src/frame/components/sidebar/SidebarCollapseContext.tsx‎

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,16 @@ import type { ReactNode } from 'react'
33
import { useRouter } from 'next/router'
44

55
import Cookies from '@/frame/components/lib/cookies'
6+
import { useIsHydrating } from '@/frame/components/hooks/useIsHydrating'
67
import { SIDEBAR_COLLAPSED_COOKIE_NAME } from '@/frame/lib/constants'
78

89
// Keep two sidebar state channels separate. collapsed persists in a cookie and
910
// syncs the desktop toggle with the rail layout, mirroring
1011
// src/landings/components/useSidebarExpandState.tsx. mobileNavOpen never persists;
11-
// the mobile trigger reads it for inline nav expansion. getMainContext reads the
12-
// cookie server-side and passes initialCollapsed, so server markup and hydration
13-
// match. Without an initial value, the client falls back to the SSR-safe cookie helper.
12+
// the mobile trigger reads it for inline nav expansion. Page HTML is publicly cached
13+
// and does not vary on Cookie, so the server and hydration render the rail open. The
14+
// cookie applies in the re-render right after hydration, and client-side mounts read it
15+
// on their first render.
1416

1517
function readCollapsed(): boolean {
1618
try {
@@ -41,30 +43,21 @@ type SidebarCollapseContextValue = {
4143

4244
const SidebarCollapseContext = createContext<SidebarCollapseContextValue | null>(null)
4345

44-
export function SidebarCollapseProvider({
45-
children,
46-
initialCollapsed,
47-
}: {
48-
children: ReactNode
49-
initialCollapsed?: boolean
50-
}) {
46+
export function SidebarCollapseProvider({ children }: { children: ReactNode }) {
5147
const { asPath } = useRouter()
52-
// Prefer the server-read cookie value so server markup and hydration match.
53-
const [collapsed, setCollapsedState] = useState(() => initialCollapsed ?? readCollapsed())
48+
const hydrating = useIsHydrating()
49+
const saved = useMemo(() => !hydrating && readCollapsed(), [hydrating])
50+
// Stays null until the reader toggles, so the saved state shows after hydration.
51+
const [chosen, setChosen] = useState<boolean | null>(null)
52+
const collapsed = chosen ?? saved
5453
const [mobileNavOpen, setMobileNavOpen] = useState(false)
5554

5655
const setCollapsed = useCallback((next: boolean) => {
57-
setCollapsedState(next)
56+
setChosen(next)
5857
persistCollapsed(next)
5958
}, [])
6059

61-
const toggleCollapsed = useCallback(() => {
62-
setCollapsedState((prev) => {
63-
const next = !prev
64-
persistCollapsed(next)
65-
return next
66-
})
67-
}, [])
60+
const toggleCollapsed = useCallback(() => setCollapsed(!collapsed), [collapsed, setCollapsed])
6861

6962
const toggleMobileNav = useCallback(() => setMobileNavOpen((prev) => !prev), [])
7063
const closeMobileNav = useCallback(() => setMobileNavOpen(false), [])

‎src/frame/tests/main-context.test.ts‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
import { describe, expect, test } from 'vitest'
2+
import type { Response } from 'express'
3+
4+
import { getMainContext } from '@/frame/components/context/MainContext'
5+
import type { ExtendedRequest } from '@/types'
6+
7+
const UI_NAMESPACES = [
8+
'alerts',
9+
'header',
10+
'search',
11+
'old_search',
12+
'survey',
13+
'toc',
14+
'meta',
15+
'scroll_button',
16+
'pages',
17+
'picker',
18+
'footer',
19+
'contribution_cta',
20+
'support',
21+
'rest',
22+
'cookbook_landing',
23+
'code_tabs',
24+
]
25+
26+
function createRequest(cookies: Record<string, string>): ExtendedRequest {
27+
return {
28+
cookies,
29+
protocol: 'https',
30+
hostname: 'docs.github.com',
31+
originalUrl: '/en/get-started/foo/bar',
32+
get: () => undefined,
33+
context: {
34+
site: { data: { ui: Object.fromEntries(UI_NAMESPACES.map((ns) => [ns, {}])) } },
35+
page: { documentType: 'article', title: 'Bar', permalinks: [] },
36+
sidebarTree: { title: 'Get started', href: '/en/get-started', childPages: [] },
37+
enterpriseServerReleases: { releasesWithOldestDeprecationDate: [], releaseCandidate: null },
38+
enterpriseServerVersions: [],
39+
allVersions: {},
40+
currentVersion: 'free-pro-team@latest',
41+
currentPathWithoutLanguage: '/get-started/foo/bar',
42+
nonEnterpriseDefaultVersion: 'free-pro-team@latest',
43+
},
44+
} as unknown as ExtendedRequest
45+
}
46+
47+
const res = { statusCode: 200 } as Response
48+
49+
describe('getMainContext', () => {
50+
// Page HTML and _next/data are publicly CDN-cached without varying on Cookie.
51+
test('does not depend on per-user sidebar cookies', async () => {
52+
const withCookies = await getMainContext(
53+
createRequest({
54+
sidebar_expanded: JSON.stringify({ '/en/get-started': false }),
55+
sidebar_collapsed: 'true',
56+
}),
57+
res,
58+
)
59+
const withoutCookies = await getMainContext(createRequest({}), res)
60+
61+
expect(withCookies).not.toHaveProperty('sidebarExpanded')
62+
expect(withCookies).not.toHaveProperty('sidebarCollapsed')
63+
expect(withCookies).toEqual(withoutCookies)
64+
})
65+
})

‎src/frame/tests/sidebar-state-ssr.test.ts‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { createElement } from 'react'
2+
import { renderToString } from 'react-dom/server'
3+
import { describe, expect, test, vi } from 'vitest'
4+
5+
import {
6+
SidebarCollapseProvider,
7+
useSidebarCollapsed,
8+
} from '@/frame/components/sidebar/SidebarCollapseContext'
9+
import {
10+
SidebarExpandStateProvider,
11+
useSidebarExpandState,
12+
} from '@/landings/components/useSidebarExpandState'
13+
import { SIDEBAR_COLLAPSED_COOKIE_NAME, SIDEBAR_EXPANDED_COOKIE_NAME } from '@/frame/lib/constants'
14+
15+
// Serve saved state so the tests prove server markup ignores cookies even when readable.
16+
vi.mock('@/frame/components/lib/cookies', () => ({
17+
default: {
18+
get: (name: string) =>
19+
({
20+
[SIDEBAR_COLLAPSED_COOKIE_NAME]: 'true',
21+
[SIDEBAR_EXPANDED_COOKIE_NAME]: JSON.stringify({ '/en/saved': true }),
22+
})[name],
23+
set: () => undefined,
24+
},
25+
}))
26+
27+
vi.mock('next/router', () => ({
28+
useRouter: () => ({ asPath: '/en/saved' }),
29+
}))
30+
31+
function ExpandedProbe() {
32+
const [expanded] = useSidebarExpandState('/en/saved', false)
33+
return createElement('span', null, `expanded:${expanded}`)
34+
}
35+
36+
function CollapsedProbe() {
37+
const { collapsed } = useSidebarCollapsed()
38+
return createElement('span', null, `collapsed:${collapsed}`)
39+
}
40+
41+
describe('sidebar state server rendering', () => {
42+
test('expand state renders the default tree despite a saved cookie', () => {
43+
const html = renderToString(
44+
createElement(SidebarExpandStateProvider, null, createElement(ExpandedProbe)),
45+
)
46+
expect(html).toContain('expanded:false')
47+
})
48+
49+
test('collapse state renders the rail open despite a saved cookie', () => {
50+
const html = renderToString(
51+
createElement(SidebarCollapseProvider, null, createElement(CollapsedProbe)),
52+
)
53+
expect(html).toContain('collapsed:false')
54+
})
55+
})

‎src/landings/components/SidebarProduct.tsx‎

Lines changed: 6 additions & 3 deletions

0 commit comments

Comments
 (0)