From 619e3a1189a2a9ece1d4e7e96729acdbb04d59b0 Mon Sep 17 00:00:00 2001 From: Tudor Date: Tue, 14 Jul 2026 13:06:52 +0100 Subject: [PATCH] perf(compare): fetch only on school-set changes; use SSR payload; parallel page fetches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Metric changes no longer refire /api/compare (the data is already client-side; the picker is presentational) — the fetch effect depends only on the URN set, with URL sync split into its own effect. - The initial client fetch is skipped when the SSR payload already covers the selected schools; national averages + benchmarks now arrive via SSR props so nothing is lost by skipping. - page.tsx fetches comparison and metrics in parallel. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB --- nextjs-app/app/compare/page.tsx | 28 +++++----- nextjs-app/components/ComparisonView.tsx | 66 ++++++++++++++++-------- 2 files changed, 58 insertions(+), 36 deletions(-) diff --git a/nextjs-app/app/compare/page.tsx b/nextjs-app/app/compare/page.tsx index 31689a5..0b2b4e4 100644 --- a/nextjs-app/app/compare/page.tsx +++ b/nextjs-app/app/compare/page.tsx @@ -32,26 +32,24 @@ export default async function ComparePage({ searchParams }: ComparePageProps) { const selectedMetric = metricParam || 'rwm_expected_pct'; try { - // Fetch comparison data if URNs provided - let comparisonData = null; - if (urns.length > 0) { - try { - const response = await fetchComparison(urnsParam!); - comparisonData = response.comparison; - } catch (error) { - console.error('Failed to fetch comparison:', error); - } - } + // Fetch comparison + metrics in parallel — they are independent. + const [comparisonResponse, metricsResponse] = await Promise.all([ + urns.length > 0 + ? fetchComparison(urnsParam!).catch((error) => { + console.error('Failed to fetch comparison:', error); + return null; + }) + : Promise.resolve(null), + fetchMetrics(), + ]); - // Fetch available metrics - const metricsResponse = await fetchMetrics(); - - // Metrics is already an array const metricsArray = metricsResponse?.metrics || []; return ( | null; + initialNationalAverages?: NationalAverages; + initialBenchmarks?: Benchmarks; initialUrns: number[]; metrics: MetricDefinition[]; selectedMetric: string; @@ -42,6 +44,8 @@ interface ComparisonViewProps { export function ComparisonView({ initialData, + initialNationalAverages, + initialBenchmarks, initialUrns, metrics, selectedMetric: initialMetric, @@ -54,8 +58,10 @@ export function ComparisonView({ const [selectedMetric, setSelectedMetric] = useState(initialMetric); const [isModalOpen, setIsModalOpen] = useState(false); const [comparisonData, setComparisonData] = useState(initialData); - const [nationalAverages, setNationalAverages] = useState(); - const [benchmarks, setBenchmarks] = useState(); + const [nationalAverages, setNationalAverages] = useState( + initialNationalAverages, + ); + const [benchmarks, setBenchmarks] = useState(initialBenchmarks); const [shareConfirm, setShareConfirm] = useState(false); const [comparePhase, setComparePhase] = useState<'primary' | 'secondary'>('primary'); // Tracks whether the user has explicitly clicked a phase tab. @@ -81,13 +87,16 @@ export function ComparisonView({ } }, [isInitialized]); // eslint-disable-line react-hooks/exhaustive-deps - // Sync URL with selected schools + metric, and (re)fetch the comparison. + const urnKey = selectedSchools.map((s) => s.urn).join(','); + + // Sync the URL with the selection + metric. Pure navigation state — no + // fetching here: metric changes are presentational (the data is already + // client-side) and must not refire the comparison request. useEffect(() => { - const urns = selectedSchools.map((s) => s.urn).join(','); const params = new URLSearchParams(searchParams); - if (urns) { - params.set('urns', urns); + if (urnKey) { + params.set('urns', urnKey); } else { params.delete('urns'); } @@ -96,26 +105,41 @@ export function ComparisonView({ const newUrl = `${pathname}?${params.toString()}`; router.replace(newUrl, { scroll: false }); + }, [urnKey, selectedMetric, pathname, searchParams, router]); - if (selectedSchools.length > 0) { - fetchComparison(urns, { cache: 'no-store' }) - .then((data) => { - setComparisonData(data.comparison); - setNationalAverages(data.national_averages); - setBenchmarks(data.benchmarks); - }) - .catch((err) => { - // Keep whatever we already have (SSR data or a previous fetch) rather - // than blanking the page — a transient refetch failure shouldn't - // destroy a working comparison the user is looking at. - console.error('Failed to fetch comparison:', err); - }); - } else { + // Fetch only when the school set changes. The very first run is skipped + // when the SSR payload already covers the current set — no double-fetch + // of data the server just rendered. + const firstFetchRef = useRef(true); + useEffect(() => { + if (!urnKey) { setComparisonData(null); setNationalAverages(undefined); setBenchmarks(undefined); + return; } - }, [selectedSchools, selectedMetric, pathname, searchParams, router]); + + if (firstFetchRef.current) { + firstFetchRef.current = false; + const ssrUrns = new Set(Object.keys(initialData ?? {})); + const covered = urnKey.split(',').every((urn) => ssrUrns.has(urn)); + if (covered && ssrUrns.size > 0) return; + } + + fetchComparison(urnKey, { cache: 'no-store' }) + .then((data) => { + setComparisonData(data.comparison); + setNationalAverages(data.national_averages); + setBenchmarks(data.benchmarks); + }) + .catch((err) => { + // Keep whatever we already have (SSR data or a previous fetch) rather + // than blanking the page — a transient refetch failure shouldn't + // destroy a working comparison the user is looking at. + console.error('Failed to fetch comparison:', err); + }); + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [urnKey]); // Classify schools by phase using comparison data const classifySchool = (school: School): 'primary' | 'secondary' => {