fix(compare): drop the merge-on-fetch — cancellation alone fixes the race
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m3s
PR Checks / Backend Smoke (pull_request) Successful in 6s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 43s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 9s
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m3s
PR Checks / Backend Smoke (pull_request) Successful in 6s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 43s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 9s
Review finding: merging left a re-added school's old entry in the map forever, so the covered check served stale data and the map grew unboundedly. The cancelled flag already discards superseded responses, which is the actual race fix; replacing keeps the map bounded to the current selection and guarantees fresh refetches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
This commit is contained in:
@@ -133,15 +133,16 @@ export function ComparisonView({
|
|||||||
// Guard against out-of-order responses: while the basket hydrates from
|
// Guard against out-of-order responses: while the basket hydrates from
|
||||||
// localStorage it can transiently hold a DIFFERENT school set than the
|
// localStorage it can transiently hold a DIFFERENT school set than the
|
||||||
// URL, firing a fetch for schools the user is no longer comparing. That
|
// URL, firing a fetch for schools the user is no longer comparing. That
|
||||||
// stale response must not replace data for the current set — replacing
|
// stale response must not replace data for the current set — it blanked
|
||||||
// it blanked every section until a hard refresh. We (a) drop responses
|
// every section until a hard refresh. Cleanup marks the run cancelled
|
||||||
// from superseded effect runs and (b) merge rather than replace, so data
|
// when urnKey moves on, so only the current selection's response is
|
||||||
// for the current schools always survives.
|
// applied (replacing the map keeps it bounded and guarantees a re-added
|
||||||
|
// school is refetched fresh rather than served a lingering old entry).
|
||||||
let cancelled = false;
|
let cancelled = false;
|
||||||
fetchComparison(urnKey, { cache: 'no-store' })
|
fetchComparison(urnKey, { cache: 'no-store' })
|
||||||
.then((data) => {
|
.then((data) => {
|
||||||
if (cancelled) return;
|
if (cancelled) return;
|
||||||
setComparisonData((prev) => ({ ...(prev ?? {}), ...data.comparison }));
|
setComparisonData(data.comparison);
|
||||||
setNationalAverages(data.national_averages);
|
setNationalAverages(data.national_averages);
|
||||||
setBenchmarks(data.benchmarks);
|
setBenchmarks(data.benchmarks);
|
||||||
})
|
})
|
||||||
|
|||||||
Reference in New Issue
Block a user