From d1c92ea7d5bc10be323a032b163139ccaa3d1a04 Mon Sep 17 00:00:00 2001 From: Tudor Date: Thu, 30 Jul 2026 15:35:54 +0100 Subject: [PATCH] docs(perf): spec for detail-page server/client split Design for splitting SchoolDetailView (65KB) and SecondarySchoolDetailView (46KB) into server section components behind a small client shell, following the existing components/compare/ decomposition pattern. Key constraint: a server component imported by a client component becomes client, so sections are composed in page.tsx and passed through the shell as children. Moving /api/national-averages server-side is a prerequisite, since the England-comparison deltas feed most sections. Co-Authored-By: Claude Opus 5 --- .../2026-07-30-server-client-split-design.md | 232 ++++++++++++++++++ 1 file changed, 232 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-30-server-client-split-design.md diff --git a/docs/superpowers/specs/2026-07-30-server-client-split-design.md b/docs/superpowers/specs/2026-07-30-server-client-split-design.md new file mode 100644 index 0000000..67ba55d --- /dev/null +++ b/docs/superpowers/specs/2026-07-30-server-client-split-design.md @@ -0,0 +1,232 @@ +# Detail pages: server/client split + +**Date:** 2026-07-30 +**Status:** Approved, ready for implementation planning +**Branch:** `perf/server-client-split` + +## Problem + +Every page of the site ships ~172 KB gzipped of JavaScript before any +page-specific code loads. The 404 page costs that much; `/admissions`, a static +content page with no data, costs 177 KB. A well-tuned Next.js app baseline is +90-110 KB. + +The cause is that nearly the whole site is a client component. +`SchoolDetailView.tsx` is 65 KB of source marked `'use client'`; +`SecondarySchoolDetailView.tsx` is 46 KB. Both are overwhelmingly presentational +markup driven by props the server already fetched. Every stat tile, table row and +Ofsted badge is serialised into the RSC payload *and* shipped again as JavaScript +to hydrate. + +Measured on a production build of `cab7b4f` (Next 16.1.6, Turbopack): + +| Metric | Value | +| --- | --- | +| Baseline JS, every page | 172 KB gz across 8 chunks | +| `/admissions` (no data) | 177 KB gz | +| Chart.js chunk | 64 KB gz (correctly lazy) | +| Leaflet chunk | 41 KB gz (correctly lazy) | +| CSS | ~30 KB gz, route-split | + +**Scope of this document.** The table above characterises the whole problem; this +design addresses one part of it — the route-level client JS on school detail +pages, the site's highest-traffic and most SEO-critical route. The shared +baseline and the other routes are separate work, tracked elsewhere. + +## What makes this tractable + +Two properties of the existing code make an aggressive split low-risk. + +**The section bodies are almost entirely static.** Across ~750 lines of sections +in `SchoolDetailView` (lines 596-1350), the only interactivity is five charts +(already `dynamic({ ssr: false })` islands), fourteen `` +instances (already a small client leaf), and two buttons driving the admissions +year/trend toggle. Everything else renders props. + +**The scroll-spy is already decoupled.** `SchoolDetailView.tsx:283` locates +sections with `document.getElementById(id)`, and `navItems` (lines 258-267) is +computed purely from data. The navigation does not care which component renders +the sections, so server-rendered children work unchanged. + +There is also a precedent to follow: `components/compare/` is already decomposed +this way — one file per section plus a `sectionShared.tsx` of primitives. + +## Architecture + +A server component imported by a client component becomes a client component. So +the sections cannot be children of a client `SchoolDetailView`. They are composed +in `page.tsx` and passed *through* the client shell as `children` — the pattern +already used at `app/page.tsx:98-99`. + +The header and navigation sit above the sections, and the sections are plain +siblings, so a single `children` slot suffices. Named slots were considered and +rejected: more ceremony, no benefit when sections render in sequence. + +``` +app/school/[slug]/page.tsx server; fetches details + nationalAverages +│ computes isSecondary / isAllThrough / navItems +└── 'use client' — the only large client file + │ back link · header + details reveal · hero map · compare CTA + │ sticky section nav · scroll spy · jump sheet + └── children: | server + └── components/school/ all server, mirroring components/compare/ + OfstedSection · ResultsSection · AdmissionsSection + InclusionSection · HistorySection · SchoolLifeSection + LocalAreaSection · FinancesSection · WellbeingSection + sectionShared.tsx Section/Card/Row primitives +``` + +Phase branching stays out of the route file: two thin server components +(`PrimarySchoolSections`, `SecondarySchoolSections`) return the section sequence, +and `page.tsx` picks one. + +### Client leaves inside server sections + +Importing a client component into a server component is the legal direction, so +these stay client and are used from server sections unchanged: + +- `MetricTooltip` / `InfoPopover` — exist, unchanged +- `PerformanceChart`, `SatsChart`, `AdmissionsTrendChart` — exist, stay `dynamic({ ssr: false })` +- `AdmissionsViewToggle` — **new**, ~25 lines; receives server-rendered year-view + and trend-view as two props and swaps between them +- `DeltaChip`, `SpecialSchoolNote` — already server-compatible (no `'use client'`) + +### What stays in the client shell, and why + +The shell is genuinely interactive throughout: `router.back()`, the details +reveal, `IntersectionObserver` hero-CTA tracking, the nav overflow fade, the +Escape-to-close sheet, and the comparison context. The goal is not to shrink the +shell but to stop the other ~750 lines being attached to it. + +### Section sharing + +| Shared by both views | Primary only | Secondary only | +| --- | --- | --- | +| Ofsted, Admissions, History, Finances, and the shell | Results (KS2), Inclusion, School Life, Local Area | Wellbeing | + +All-through schools render both the KS2 and KS4 section sets. The shared library +makes this natural rather than duplicated. + +### Prerequisite: national averages move server-side + +`SchoolDetailView.tsx:173-179` fetches `/api/national-averages` in a `useEffect`, +and `primaryAvg` / `secondaryAvg` feed the England-comparison deltas across most +sections. Those sections cannot be server components until that data arrives as a +prop. `SecondarySchoolDetailView.tsx:91-93` has the same fetch. + +`page.tsx` will fetch it in parallel with the school details (the endpoint is +backend-cached for an hour) and pass `nationalAvg` down. This is a prerequisite, +not an optional extra. It independently removes a client round-trip per detail +page and puts the deltas in the initial HTML instead of popping in after +hydration. + +### Layout + +`Footer` is already a server component — no change needed. `Navigation` genuinely +requires client (`usePathname` plus the comparison count) and stays as it is. +This item is much smaller than the original review estimated. + +### Unused dependency + +`hooks/useSchools.ts`, `useFilters.ts`, `useMetrics.ts` and `useSchoolDetails.ts` +are imported by nothing. They are the only consumers of `swr`. All four files and +the `swr` dependency are deleted. + +## Testing + +`SchoolDetailView` will not exist as a single component afterwards, so +characterization tests must not import it directly. A single render helper +absorbs the change: + +``` +__tests__/support/renderSchoolDetail.tsx the only file that changes at the split + before: render() + after: render() +``` + +Every assertion imports that helper and never changes. Assertions passing +untouched across the refactor is the proof that behaviour is preserved. + +**Coverage:** Ofsted panel (rating, date, report link); KS2 results rows and +England deltas; KS4 Attainment 8, Progress 8 and EBacc; admissions Q&A and the +year/trend toggle; inclusion and wellbeing figures; the history table; finances; +and the conditional `navItems` set. + +**Fixtures:** four shapes — primary, secondary, all-through, and special school. +The special-school path drops the England comparison via `isSpecialSchool()` and +is the site of a known past regression (PR #70), so it gets explicit coverage. + +**Environment gaps to close in `jest.setup.js`:** jsdom provides neither +`IntersectionObserver` (used twice in each view) nor `window.scrollTo`. Chart.js +needs a canvas, so the chart components are mocked. `next/jest` already handles +CSS modules, and `next/navigation` is already globally mocked. Section components +are synchronous functions, so React Testing Library renders both the server and +client pieces in jsdom without special handling. + +**E2E:** `e2e/journeys.spec.ts` gains detail-page assertions. CLAUDE.md requires +e2e updates in the same PR as user-facing changes. + +## Commit sequence + +One PR, but the diff is large, so it is structured to stay reviewable and +bisectable: + +1. `test:` jest.setup additions (IntersectionObserver, scrollTo), chart mocks, fixtures +2. `test:` characterization tests against the current components — all green before anything moves +3. `refactor:` server-side `nationalAverages` in `page.tsx`; drop both `useEffect` fetches +4. `refactor:` extract `components/school/sectionShared.tsx` and the server sections (pure moves, no logic edits) +5. `refactor:` add `SchoolDetailShell` and `AdmissionsViewToggle`; rewire `page.tsx`; delete the two old views +6. `chore:` delete the four unused SWR hooks; drop `swr` from `package.json` +7. `test:` extend `e2e/journeys.spec.ts` with detail-page assertions + +Commits 4 and 5 move the bytes; commits 1 and 2 are the safety net that makes +them safe. Commit 6 lands late so it cannot confuse a bisect. + +## Verification + +Before the work is claimed complete: + +- `npm run typecheck`, `npm test` and `npm run build` all green +- characterization tests passing **unmodified** from commit 2 +- before/after gzipped JS per route recorded in the PR description, measured the + same way as the figures in the Problem section + +## Success criteria + +- `/school/[slug]` route-specific JS drops to roughly 40-60 KB gz, from a client + tree currently built out of a 65 KB and a 46 KB source file +- No visual or behavioural change on any of the four school shapes +- One fewer client API round-trip per detail page + +**This change does not reduce the 172 KB baseline.** That baseline is react-dom, +the Next.js runtime, and the layout's client components (`Navigation`, +`ComparisonProvider`, `ComparisonToast`) — none of which this design touches, as +`Navigation` legitimately needs `usePathname` and the comparison count. The +baseline is a separate piece of work; it is cited above only to establish why +route-level client JS matters. Reporting must state the two numbers separately so +the unchanged baseline is not read as a failure of this change. + +Budget enforcement in CI was considered and deliberately deferred: measurement is +recorded in the PR, but no new CI gate is added in this change. + +## Non-goals + +- No visual redesign or copy changes to any section +- No change to `Navigation`, `Footer`, or any other route +- No CI bundle-budget gate +- Bugs found mid-move are recorded, not fixed inline + +## Risks + +**DOM drift.** Moving ~750 lines of JSX risks subtle markup changes that CSS +modules depend on. Commits 4 and 5 are strict moves and the section CSS moves +with them; any styling change is out of scope. + +**All-through schools.** The trickiest case: `isAllThrough` gates KS2 content +back on for a secondary-phase school (`app/school/[slug]/page.tsx:150-154`). It +gets a dedicated fixture. + +**Large diff.** Accepted deliberately in favour of designing the shared section +library against both consumers at once. Mitigated by the commit sequence above. + +**`swr` removal** is safe — nothing imports those hooks.