perf(compare): fetch only on school-set changes; use SSR payload; parallel page fetches
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 9m46s
PR Checks / Backend Smoke (pull_request) Successful in 7s
PR Checks / Build Backend (no push) (pull_request) Successful in 18s
PR Checks / Build Frontend (no push) (pull_request) Successful in 47s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 36s
PR Checks / AI Code Review (Claude) (pull_request) Failing after 5m4s
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 9m46s
PR Checks / Backend Smoke (pull_request) Successful in 7s
PR Checks / Build Backend (no push) (pull_request) Successful in 18s
PR Checks / Build Frontend (no push) (pull_request) Successful in 47s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 36s
PR Checks / AI Code Review (Claude) (pull_request) Failing after 5m4s
- 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
This commit is contained in:
@@ -32,26 +32,24 @@ export default async function ComparePage({ searchParams }: ComparePageProps) {
|
|||||||
const selectedMetric = metricParam || 'rwm_expected_pct';
|
const selectedMetric = metricParam || 'rwm_expected_pct';
|
||||||
|
|
||||||
try {
|
try {
|
||||||
// Fetch comparison data if URNs provided
|
// Fetch comparison + metrics in parallel — they are independent.
|
||||||
let comparisonData = null;
|
const [comparisonResponse, metricsResponse] = await Promise.all([
|
||||||
if (urns.length > 0) {
|
urns.length > 0
|
||||||
try {
|
? fetchComparison(urnsParam!).catch((error) => {
|
||||||
const response = await fetchComparison(urnsParam!);
|
console.error('Failed to fetch comparison:', error);
|
||||||
comparisonData = response.comparison;
|
return null;
|
||||||
} catch (error) {
|
})
|
||||||
console.error('Failed to fetch comparison:', error);
|
: Promise.resolve(null),
|
||||||
}
|
fetchMetrics(),
|
||||||
}
|
]);
|
||||||
|
|
||||||
// Fetch available metrics
|
|
||||||
const metricsResponse = await fetchMetrics();
|
|
||||||
|
|
||||||
// Metrics is already an array
|
|
||||||
const metricsArray = metricsResponse?.metrics || [];
|
const metricsArray = metricsResponse?.metrics || [];
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<ComparisonView
|
<ComparisonView
|
||||||
initialData={comparisonData}
|
initialData={comparisonResponse?.comparison ?? null}
|
||||||
|
initialNationalAverages={comparisonResponse?.national_averages}
|
||||||
|
initialBenchmarks={comparisonResponse?.benchmarks}
|
||||||
initialUrns={urns}
|
initialUrns={urns}
|
||||||
metrics={metricsArray}
|
metrics={metricsArray}
|
||||||
selectedMetric={selectedMetric}
|
selectedMetric={selectedMetric}
|
||||||
|
|||||||
@@ -35,6 +35,8 @@ import styles from './ComparisonView.module.css';
|
|||||||
|
|
||||||
interface ComparisonViewProps {
|
interface ComparisonViewProps {
|
||||||
initialData: Record<string, ComparisonData> | null;
|
initialData: Record<string, ComparisonData> | null;
|
||||||
|
initialNationalAverages?: NationalAverages;
|
||||||
|
initialBenchmarks?: Benchmarks;
|
||||||
initialUrns: number[];
|
initialUrns: number[];
|
||||||
metrics: MetricDefinition[];
|
metrics: MetricDefinition[];
|
||||||
selectedMetric: string;
|
selectedMetric: string;
|
||||||
@@ -42,6 +44,8 @@ interface ComparisonViewProps {
|
|||||||
|
|
||||||
export function ComparisonView({
|
export function ComparisonView({
|
||||||
initialData,
|
initialData,
|
||||||
|
initialNationalAverages,
|
||||||
|
initialBenchmarks,
|
||||||
initialUrns,
|
initialUrns,
|
||||||
metrics,
|
metrics,
|
||||||
selectedMetric: initialMetric,
|
selectedMetric: initialMetric,
|
||||||
@@ -54,8 +58,10 @@ export function ComparisonView({
|
|||||||
const [selectedMetric, setSelectedMetric] = useState(initialMetric);
|
const [selectedMetric, setSelectedMetric] = useState(initialMetric);
|
||||||
const [isModalOpen, setIsModalOpen] = useState(false);
|
const [isModalOpen, setIsModalOpen] = useState(false);
|
||||||
const [comparisonData, setComparisonData] = useState(initialData);
|
const [comparisonData, setComparisonData] = useState(initialData);
|
||||||
const [nationalAverages, setNationalAverages] = useState<NationalAverages | undefined>();
|
const [nationalAverages, setNationalAverages] = useState<NationalAverages | undefined>(
|
||||||
const [benchmarks, setBenchmarks] = useState<Benchmarks | undefined>();
|
initialNationalAverages,
|
||||||
|
);
|
||||||
|
const [benchmarks, setBenchmarks] = useState<Benchmarks | undefined>(initialBenchmarks);
|
||||||
const [shareConfirm, setShareConfirm] = useState(false);
|
const [shareConfirm, setShareConfirm] = useState(false);
|
||||||
const [comparePhase, setComparePhase] = useState<'primary' | 'secondary'>('primary');
|
const [comparePhase, setComparePhase] = useState<'primary' | 'secondary'>('primary');
|
||||||
// Tracks whether the user has explicitly clicked a phase tab.
|
// 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
|
}, [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(() => {
|
useEffect(() => {
|
||||||
const urns = selectedSchools.map((s) => s.urn).join(',');
|
|
||||||
const params = new URLSearchParams(searchParams);
|
const params = new URLSearchParams(searchParams);
|
||||||
|
|
||||||
if (urns) {
|
if (urnKey) {
|
||||||
params.set('urns', urns);
|
params.set('urns', urnKey);
|
||||||
} else {
|
} else {
|
||||||
params.delete('urns');
|
params.delete('urns');
|
||||||
}
|
}
|
||||||
@@ -96,26 +105,41 @@ export function ComparisonView({
|
|||||||
|
|
||||||
const newUrl = `${pathname}?${params.toString()}`;
|
const newUrl = `${pathname}?${params.toString()}`;
|
||||||
router.replace(newUrl, { scroll: false });
|
router.replace(newUrl, { scroll: false });
|
||||||
|
}, [urnKey, selectedMetric, pathname, searchParams, router]);
|
||||||
|
|
||||||
if (selectedSchools.length > 0) {
|
// Fetch only when the school set changes. The very first run is skipped
|
||||||
fetchComparison(urns, { cache: 'no-store' })
|
// when the SSR payload already covers the current set — no double-fetch
|
||||||
.then((data) => {
|
// of data the server just rendered.
|
||||||
setComparisonData(data.comparison);
|
const firstFetchRef = useRef(true);
|
||||||
setNationalAverages(data.national_averages);
|
useEffect(() => {
|
||||||
setBenchmarks(data.benchmarks);
|
if (!urnKey) {
|
||||||
})
|
|
||||||
.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 {
|
|
||||||
setComparisonData(null);
|
setComparisonData(null);
|
||||||
setNationalAverages(undefined);
|
setNationalAverages(undefined);
|
||||||
setBenchmarks(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
|
// Classify schools by phase using comparison data
|
||||||
const classifySchool = (school: School): 'primary' | 'secondary' => {
|
const classifySchool = (school: School): 'primary' | 'secondary' => {
|
||||||
|
|||||||
Reference in New Issue
Block a user