fix(status): stop the freshness watcher updating TimeRangePicker during sibling renders - #1510
Merged
Merged
Conversation
…ng sibling renders useAnalyticsFreshness called setState synchronously from its QueryCache subscription. useQuery registers new queries during render and RQ fires the cache event synchronously, so any component whose render introduced a new analytics query (a KPI tile's previous-window fetch, a MetricPanel on a fresh refresh window) made React log 'Cannot update a component (TimeRangePicker) while rendering a different component'. Reimplement on useSyncExternalStore with the notification deferred through notifyManager.batchCalls — the same bridge RQ's useIsFetching uses. A bare uSES onStoreChange still schedules the cross-fiber update mid-render; the regression test fails on both the old setState version and the bare-uSES version, passes only with the deferral. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZEaYdKFXuQw2Hf6XpPdEV
Coverage Report
File Coverage
|
||||||||||||||||||||||||||||||||||||||
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the useAnalyticsFreshness hook to use useSyncExternalStore and notifyManager.batchCalls to prevent React warnings about mid-render updates, and adds a regression test. The reviewer suggests splitting the hook into two separate useSyncExternalStore calls for primitive values (isFetching and lastFetchedAt). This avoids mutating a ref during the render phase, which is a React anti-pattern, and simplifies the code by removing the need for the FreshnessSnapshot interface.
Gemini review: splitting isFetching / lastFetchedAt into separate useSyncExternalStore calls returning primitives removes the ref-cached object snapshot and the FreshnessSnapshot interface entirely — Object.is handles primitive stability for free. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZEaYdKFXuQw2Hf6XpPdEV
dawsontoth
approved these changes
Jul 15, 2026
dawsontoth
pushed a commit
that referenced
this pull request
Jul 22, 2026
…ry metric Two harnesses derived from the runtime-verification passes: - src/testSetup/failOnRenderPhaseUpdate.ts (global vitest setup): any test during which React logs 'Cannot update a component while rendering a different component' now fails with the substituted component names. This class shipped silently twice (Transitioner/router rebuild, and the analytics freshness watcher fixed in #1510) and was only ever caught by watching a real browser console. Narrow match; intentional console.error paths are unaffected. Zero offenders in the current suite. - registry-smoke.test.tsx: renders all 24 specRegistry metrics through the real MetricRenderer + PanelErrorBoundary with synthetic records derived from each spec's required fields (quantile columns, confidence-gate- clearing counts, timestamp: 'id' mirroring). Two tiers: no metric may trip the boundary; every line/stacked-area metric (derived from spec.primitive, not a hardcoded list) must emit an actual chart. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UZEaYdKFXuQw2Hf6XpPdEV
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
useAnalyticsFreshness(the "Updated Xs ago" / spinning-refresh watcher behindTimeRangePicker) calledsetStatesynchronously inside itsQueryCache.subscribecallback.useQueryregisters new queries during render, and React Query dispatches the cache event synchronously — so any component whose render introduces a new analytics query key made React log:Latent on stage today (a refresh tick minting fresh window keys can trigger it); the enhancement-batch PRs (#1506 especially, whose KPI tiles mount previous-window queries) exercise it on every Health-tab visit. Caught during a runtime verification pass over the merged batch; with this fix the same browser run reports zero console errors.
Change
Reimplements the hook the way RQ's own
useIsFetchingbridges cache events to React 18:useSyncExternalStorewith the store notification deferred throughnotifyManager.batchCalls, and a referentially-stable snapshot cached in a ref. Behavior ofisFetching/lastFetchedAt/nowis unchanged.Where to focus
onStoreChangestill warns (it schedules a cross-fiber update mid-render all the same). The new regression test was verified to fail on the old setState implementation and on the bare-uSES variant — theinitialDatain the test is what makes the mid-render event value-changing, without which the old code's functional-setState bailout hides the bug.getSnapshotscans the analytics-prefixed cache entries per picker render — same complexity class as RQ'suseIsFetchingand as the previous implementation's event handler.Comment generated by kAIle (Claude Fable 5)