perf(detail): render school detail sections on the server #84

Merged
tudor merged 13 commits from perf/server-client-split into main 2026-08-02 21:03:33 +00:00
Owner

Splits the school detail pages into React Server Components behind a small client shell, so static section markup stops shipping to the browser as JavaScript.

Implements docs/superpowers/specs/2026-07-30-server-client-split-design.md via docs/superpowers/plans/2026-08-01-server-client-split.md.

Results

Metric Before (cab7b4f) After
/school/[slug] client chunk 65 KB + 46 KB source, all client 8 KB gz (33 KB raw)
Total static JS, all chunks 380.6 KB gz 350.7 KB gz
Client API round-trips per detail page 1 (/api/national-averages) 0
Shared baseline (every page) 172 KB gz 172 KB gz — unchanged, see below

Section markup is verifiably absent from every client chunk. Grepping the built
bundles for section-only strings — "Got their first choice", "Ofsted reports",
"Most deprived", "Share of budget spent on teachers", "Regularly missing school",
"Places offered" — returns 0 hits, while shell strings ("Add to Compare",
"Show all details", "Back to top") are still present.

The 172 KB baseline is deliberately unchanged. It is react-dom, the Next.js
runtime, and the layout's client components (Navigation, ComparisonProvider,
ComparisonToast). Navigation legitimately needs usePathname and the
comparison count, so none of it is in scope here. Shrinking the baseline is
separate work; the number is listed so an unchanged baseline is not misread as a
miss.

How behaviour was kept identical

14 characterization tests were written against the old components first and
had to pass byte-identical afterwards. The only file permitted to change was
the render helper that constructs the tree. It is verified unchanged:

git diff --exit-code 7c43a1b -- nextjs-app/__tests__/components/schoolDetail.characterization.test.tsx

They cover Ofsted (both OEIF and Report Card layouts), KS2 results with the
England delta, KS4 Attainment 8 / Progress 8 / EBacc, all-through dual
rendering, special-school comparison suppression (the PR #70 regression), the
admissions toggle, and conditional section rendering.

Architecture

A server component imported by a client component becomes a client component, so
the sections cannot be children of a client view. page.tsx composes them and
passes them through the shell as children — the pattern already used at
app/page.tsx:98-99. The scroll-spy already located sections via
document.getElementById, so it works unchanged against server-rendered
children.

The only client components in components/school/ are AdmissionsViewToggle
(the year/trend island) and charts.tsx (a wrapper that exists because
next/dynamic with ssr: false is illegal in a Server Component).

Judgement calls worth reviewing

Sharing follows measured similarity, not assumption. The design assumed
Ofsted, Admissions, History and Finances were shared. Measured: Finances 91%,
Ofsted 80%, History 40%, Admissions 14%. Only the first two are shared. This
costs no bytes — the win comes from sections being server components, not from
sharing them.

Ofsted's 80% hid a real fork. On a school with no overall grade the primary
page shows a "Not rated" badge while the secondary shows a four-area OEIF panel,
and the disclaimer copy differs. Both are preserved exactly behind a variant
prop rather than reconciled. Merging them is a human decision, not a side effect
of a refactor.

CSS merge (commit 2f78678, isolated so a visual regression bisects there).
Three categories:

  • 7 incidental-drift properties unioned in (overflow-wrap, word-break,
    min-width, user-select) — fixes that landed on one page and were never
    back-ported. Also fixes latent long-school-name overflow on the page missing them.
  • 4 genuinely visual differences kept as variant classes. font-weight on
    .genderSplitBoys/Girls was reclassified from drift to visual mid-work;
    unioning it would have bolded the primary page.
  • 17 rules that exist only in the secondary stylesheet but target shared class
    names (.card, .sectionTitle, .metricCard). Applied flat they would have
    restyled the primary page, so they are scoped under .secondaryScope.

Audited mechanically: every rule in both original stylesheets is a subset of the
merged one (0 MISSING, 0 LOST), excluding the documented variants.

One deliberately gated improvement: the secondary Finances section has a
premises-cost card the primary never had. It is behind showPremises so this PR
changes nothing visible; enabling it for primary is a one-line follow-up.

Verification

  • npm run typecheck — passes
  • npm test — 155 passing, 18 suites (was 125 / 16)
  • npm run build — passes
  • Characterization tests byte-identical to the commit that introduced them
  • CSS audit: 0 MISSING, 0 LOST

E2E gains two journeys for what actually changed: the admissions toggle island
and the sticky nav resolving to server-rendered sections. Not run locally — they
target staging, which deploys from main.

Not done

Nothing from the plan was skipped. Out of scope by design: the shared baseline,
Navigation/Footer (Footer is already a server component), and any visual or
copy change.

🤖 Generated with Claude Code

Splits the school detail pages into React Server Components behind a small client shell, so static section markup stops shipping to the browser as JavaScript. Implements `docs/superpowers/specs/2026-07-30-server-client-split-design.md` via `docs/superpowers/plans/2026-08-01-server-client-split.md`. ## Results | Metric | Before (`cab7b4f`) | After | | --- | --- | --- | | `/school/[slug]` client chunk | 65 KB + 46 KB source, all client | **8 KB gz** (33 KB raw) | | Total static JS, all chunks | 380.6 KB gz | **350.7 KB gz** | | Client API round-trips per detail page | 1 (`/api/national-averages`) | **0** | | Shared baseline (every page) | 172 KB gz | 172 KB gz — *unchanged, see below* | Section markup is verifiably absent from every client chunk. Grepping the built bundles for section-only strings — "Got their first choice", "Ofsted reports", "Most deprived", "Share of budget spent on teachers", "Regularly missing school", "Places offered" — returns **0 hits**, while shell strings ("Add to Compare", "Show all details", "Back to top") are still present. **The 172 KB baseline is deliberately unchanged.** It is react-dom, the Next.js runtime, and the layout's client components (`Navigation`, `ComparisonProvider`, `ComparisonToast`). `Navigation` legitimately needs `usePathname` and the comparison count, so none of it is in scope here. Shrinking the baseline is separate work; the number is listed so an unchanged baseline is not misread as a miss. ## How behaviour was kept identical 14 characterization tests were written against the **old** components first and had to pass **byte-identical** afterwards. The only file permitted to change was the render helper that constructs the tree. It is verified unchanged: ``` git diff --exit-code 7c43a1b -- nextjs-app/__tests__/components/schoolDetail.characterization.test.tsx ``` They cover Ofsted (both OEIF and Report Card layouts), KS2 results with the England delta, KS4 Attainment 8 / Progress 8 / EBacc, all-through dual rendering, special-school comparison suppression (the PR #70 regression), the admissions toggle, and conditional section rendering. ## Architecture A server component imported by a client component becomes a client component, so the sections cannot be children of a client view. `page.tsx` composes them and passes them **through** the shell as `children` — the pattern already used at `app/page.tsx:98-99`. The scroll-spy already located sections via `document.getElementById`, so it works unchanged against server-rendered children. The only client components in `components/school/` are `AdmissionsViewToggle` (the year/trend island) and `charts.tsx` (a wrapper that exists because `next/dynamic` with `ssr: false` is illegal in a Server Component). ## Judgement calls worth reviewing **Sharing follows measured similarity, not assumption.** The design assumed Ofsted, Admissions, History and Finances were shared. Measured: Finances 91%, Ofsted 80%, History 40%, Admissions 14%. Only the first two are shared. This costs no bytes — the win comes from sections being *server* components, not from sharing them. **Ofsted's 80% hid a real fork.** On a school with no overall grade the primary page shows a "Not rated" badge while the secondary shows a four-area OEIF panel, and the disclaimer copy differs. Both are preserved exactly behind a `variant` prop rather than reconciled. Merging them is a human decision, not a side effect of a refactor. **CSS merge (commit `2f78678`, isolated so a visual regression bisects there).** Three categories: - 7 incidental-drift properties unioned in (`overflow-wrap`, `word-break`, `min-width`, `user-select`) — fixes that landed on one page and were never back-ported. Also fixes latent long-school-name overflow on the page missing them. - 4 genuinely visual differences kept as variant classes. `font-weight` on `.genderSplitBoys/Girls` was reclassified from drift to visual mid-work; unioning it would have bolded the primary page. - 17 rules that exist only in the secondary stylesheet but target shared class names (`.card`, `.sectionTitle`, `.metricCard`). Applied flat they would have restyled the primary page, so they are scoped under `.secondaryScope`. Audited mechanically: every rule in both original stylesheets is a subset of the merged one (0 MISSING, 0 LOST), excluding the documented variants. **One deliberately gated improvement:** the secondary Finances section has a premises-cost card the primary never had. It is behind `showPremises` so this PR changes nothing visible; enabling it for primary is a one-line follow-up. ## Verification - `npm run typecheck` — passes - `npm test` — 155 passing, 18 suites (was 125 / 16) - `npm run build` — passes - Characterization tests byte-identical to the commit that introduced them - CSS audit: 0 MISSING, 0 LOST E2E gains two journeys for what actually changed: the admissions toggle island and the sticky nav resolving to server-rendered sections. Not run locally — they target staging, which deploys from `main`. ## Not done Nothing from the plan was skipped. Out of scope by design: the shared baseline, `Navigation`/`Footer` (Footer is already a server component), and any visual or copy change. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
tudor added 12 commits 2026-08-02 20:42:09 +00:00
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 <noreply@anthropic.com>
Planning surfaced a gap: the two detail views' CSS modules share 79 class
names, 22 with differing rules, 16 of those used in section markup by both
views. Shared sections cannot use one stylesheet without visual change.

Resolution: union the 14 incidental-drift differences (defensive overflow
properties never back-ported between the views), and keep the 2 genuine
visual differences (.genderBar, .heroStatValue) as distinct classes behind a
variant prop. Adds an isolated CSS-merge commit so a visual regression
bisects to the merge rather than a JSX move.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nine tasks mapping to the spec's commit sequence, each ending in an
independently testable deliverable.

Also corrects the spec's section-sharing table: measured similarity shows
Admissions (14%) and History (40%) are not shareable between the two views,
only Ofsted (80%) and Finances (91%). This costs no bytes -- the win comes
from sections being server components, not from sharing them -- and cuts the
diff and regression risk.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
jsdom provides neither IntersectionObserver nor scrollTo; both detail views
use them. Adds four fixtures (primary, secondary, all-through, special)
covering the branches the characterization tests will pin.

SchoolResult carries 57 required nullable fields, so fixtures are built from
an all-null base row and override only what they exercise.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
14 characterization tests covering Ofsted (both OEIF and Report Card layouts),
KS2 results with the England delta, KS4 Attainment 8 / Progress 8 / EBacc,
all-through dual rendering, special-school comparison suppression (PR #70),
the admissions year/trend toggle, and conditional section rendering.

All rendering goes through renderSchoolDetail(), the single seam the
server/client split is allowed to change. The assertions must survive the
refactor unmodified.

Excludes __tests__/support from testMatch: it holds fixtures and helpers, not
suites, and the glob was failing them as empty test files.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both detail views fetched /api/national-averages in a useEffect, so the
England-comparison deltas popped in after hydration and every section that
uses them was pinned to the client. The page now fetches it in parallel with
the school details (backend-cached 1h, degrades to null) and passes it down.

Removes one client round-trip per detail page and unblocks the section
extraction.

Characterization tests pass unmodified; only the render helper changed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Pure data-shape logic moves out of the client components so page.tsx can
compute the section list on the server without importing them.

The secondary page is not a variant of the primary one -- different section
ids (gcse, wellbeing), different flags, and History gated on more than one
year rather than at least one -- so it gets its own computeSecondaryFlags and
buildSecondaryNavItems rather than bending a shared function.

Adds 16 unit tests covering the all-through and special-school branches,
previously reachable only through a full component render.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Builds components/school/schoolSections.module.css from the primary module,
plus 35 secondary-only rules, so shared section components have one stylesheet.

Three categories of difference, handled separately so nothing changes visually:

- 7 incidental-drift properties unioned in (overflow-wrap, word-break,
  min-width, user-select) -- fixes that landed on one page and were never
  back-ported. Also fixes latent long-school-name overflow on the page that
  lacked them.
- 4 genuinely visual class differences kept as explicit variants:
  genderBar, heroStatValue, genderSplitBoys, genderSplitGirls. font-weight on
  the genderSplit pair was reclassified from drift to visual -- unioning it
  would have bolded the primary page.
- 17 rules that exist only in the secondary stylesheet but target shared class
  names (.card, .sectionTitle, .metricCard). Applied flat these would restyle
  the primary page, so they are scoped under .secondaryScope, which only
  SecondarySchoolSections will carry.

Audited: every rule in both original stylesheets is a subset of the merged one
(0 MISSING, 0 LOST), excluding the documented variants.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Moves ~1,300 lines of section markup out of the two client views into
components/school/, mirroring the components/compare/ layout. Twelve section
components plus shared primitives, all server components. The only client
file is AdmissionsViewToggle, which owns the hidden/aria-pressed state and
receives both views as server-rendered children.

JSX was extracted mechanically rather than retyped, so the markup the CSS
modules depend on is verbatim.

Sharing follows measured similarity, not assumption:
- Finances (91%) shared. The secondary premises-cost card is gated behind a
  prop so primary pages are unchanged; enabling it is a one-line follow-up.
- Ofsted (80%) shared, but behind a variant prop. The headline similarity hid
  a real fork: on a school with no overall grade the primary page shows a
  "Not rated" badge while the secondary shows a four-area OEIF panel, and the
  disclaimer copy differs. Both preserved exactly; reconciling them is a
  human decision, not a side effect of a move.
- Admissions (14%) and History (40%) kept separate.

Not yet wired up -- the old views still render.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
page.tsx now composes the sections and passes them through SchoolDetailShell
as children, so ~1,300 lines of static markup stop shipping as client
JavaScript. The shell keeps what is genuinely interactive: back link, header
reveal, hero map, compare CTA, sticky nav and scroll-spy.

The scroll-spy already located sections via document.getElementById, so it
works unchanged against server-rendered children.

Charts needed a client wrapper: next/dynamic with ssr:false is illegal in a
Server Component, so components/school/charts.tsx is the boundary that keeps
Chart.js (64 KB gz) lazy and browser-only.

Measured on this build:
- school route client chunk: 8 KB gz (33 KB raw)
- total static JS across all chunks: 380.6 -> 350.7 KB gz
- section markup is absent from every client chunk ("Got their first choice",
  "Ofsted reports", "Most deprived" etc. all return 0 hits); shell strings
  still present, as expected
- shared baseline unchanged at 172 KB gz -- out of scope, as designed

The 14 characterization tests pass byte-identical to the commit that
introduced them. Only the render helper changed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
useSchools, useFilters, useMetrics and useSchoolDetails were imported by
nothing and were the only consumers of swr. All data fetching goes through
lib/api.ts on the server.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
test(e2e): cover the detail page server/client boundary
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m7s
PR Checks / Backend Smoke (pull_request) Successful in 8s
PR Checks / Build Backend (no push) (pull_request) Successful in 14s
PR Checks / Build Frontend (no push) (pull_request) Successful in 53s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 1m14s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 3m23s
ec2d12478e
Two journeys for what the split actually changed: the admissions toggle (the
one client island inside an otherwise server-rendered section) and the sticky
nav resolving to server-rendered sections. The nav is client-rendered from a
server-computed list while the sections render on the server, so a mismatch
between the two halves would only show up in a real browser.

Existing journeys already cover all-through and special-school pages.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

🤖 AI Code Review (Claude Code)

This PR moves the school detail page's ~750 lines of static section markup from two client components into server components composed behind a thin client shell (SchoolDetailShell), fetches national averages server-side instead of via a client useEffect, merges the primary/secondary CSS modules, and removes unused SWR hooks. The refactor is unusually well-executed: the risky next/dynamic({ssr:false})-in-a-server-component pitfall is correctly avoided via dedicated client chart wrappers, the Ofsted/Finances merges preserve both original code paths via explicit variants, and the primary/secondary section composers' render conditions match their corresponding buildNavItems functions exactly (verified against the pre-refactor source).

🟡 Minor

  • nextjs-app/components/school/SchoolDetailShell.tsx: The shell still computes computeSchoolFlags() plus isReportCard/oeifAreas/oeifAllSameGrade/ofstedInspectedDate/deprivationDesc/primaryAvg/secondaryAvg on every render (lines ~140-235), duplicating work already done in page.tsx, but none of those values (isSecondary, isPrimary, hasGenderSplit, hasInclusionData, hasSchoolLife, hasFinance, hasKS2/KS4Results, isSpecial, ks2Placeholder, suppressKs2/Ks4Comparison, primaryAvg, secondaryAvg, deprivationDesc, isReportCard, ofstedInspectedDate, oeifAllSameGrade, oeifAreas) are referenced in the shell's JSX anymore since that logic now lives in the section composers. Leftover dead code from the extraction, not a correctness issue.
## 🤖 AI Code Review (Claude Code) This PR moves the school detail page's ~750 lines of static section markup from two client components into server components composed behind a thin client shell (SchoolDetailShell), fetches national averages server-side instead of via a client useEffect, merges the primary/secondary CSS modules, and removes unused SWR hooks. The refactor is unusually well-executed: the risky `next/dynamic({ssr:false})`-in-a-server-component pitfall is correctly avoided via dedicated client chart wrappers, the Ofsted/Finances merges preserve both original code paths via explicit variants, and the primary/secondary section composers' render conditions match their corresponding buildNavItems functions exactly (verified against the pre-refactor source). ### 🟡 Minor - **nextjs-app/components/school/SchoolDetailShell.tsx**: The shell still computes computeSchoolFlags() plus isReportCard/oeifAreas/oeifAllSameGrade/ofstedInspectedDate/deprivationDesc/primaryAvg/secondaryAvg on every render (lines ~140-235), duplicating work already done in page.tsx, but none of those values (isSecondary, isPrimary, hasGenderSplit, hasInclusionData, hasSchoolLife, hasFinance, hasKS2/KS4Results, isSpecial, ks2Placeholder, suppressKs2/Ks4Comparison, primaryAvg, secondaryAvg, deprivationDesc, isReportCard, ofstedInspectedDate, oeifAllSameGrade, oeifAreas) are referenced in the shell's JSX anymore since that logic now lives in the section composers. Leftover dead code from the extraction, not a correctness issue.
tudor added 1 commit 2026-08-02 20:55:00 +00:00
refactor(detail): drop dead derived state from the shell
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m4s
PR Checks / Backend Smoke (pull_request) Successful in 7s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 47s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 1m12s
PR Checks / AI Code Review (Claude) (pull_request) Failing after 9m28s
803e68970c
Leftover from the mechanical extraction: the shell still called
computeSchoolFlags() and derived isReportCard / ofstedInspectedDate /
oeifAreas / oeifAllSameGrade / deprivationDesc / primaryAvg / secondaryAvg on
every render, duplicating work page.tsx already does. None of those values
were referenced in its JSX anymore -- that logic moved to the section
composers.

The chrome needs only four locally-derived values (latestResults, phase,
isAllThrough, hasLocation), all one-liners over props it already owns.

Removing them made seven props dead, which TypeScript caught at both call
sites: absenceData, ofsted, admissions, admissionsHistory, deprivation,
finance and nationalAvg now go straight to the section composers and never
reach the client component. The shell's surface is down to schoolInfo,
yearlyData, census, navItems and children.

No behaviour change: 155 tests pass and the characterization tests remain
byte-identical to the commit that introduced them.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tudor merged commit 5f961bf7f4 into main 2026-08-02 21:03:33 +00:00
Sign in to join this conversation.
No Reviewers
No labels
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: tudor/school_compare#84