fix(dashboard): stop the chart-row render loop on tab resume

A user reported the dashboard crashing on returning to a backgrounded
tab. The on-focus snapshot refetch re-rendered the page, and each chart
received a fresh inline `() => markChartReady(id)` handler every render;
the chart's data-ready notify effect, keyed on that ever-changing prop,
re-fired on every commit. That per-commit passive effect kept the chart
row's anchored popovers re-committing until the browser tripped its
render-depth guard and the page fell back to its error card.

Latch the notify behind a ref so it fires once, when the chart's initial
query settles, and depend on the load flag alone. The handler is read
through a ref so a late-arriving callback still fires without re-arming
the effect. Extract the pure fire-once decision into a small helper with
direct unit coverage (no DOM runner, per project convention).
This commit is contained in:
Marc-André Bombeck committed 2026-06-23 21:45:42 +02:00
1 parent 009d923c87
commit 2c112c3323
8 files changed
+210 -10

No files matched your search

+29 -2
View File
@@ -23,6 +23,7 @@ import {
useMemo,
useEffect,
useCallback,
useRef,
type ComponentType,
} from "react";
import { Skeleton } from "@/components/ui/skeleton";
@@ -60,6 +61,7 @@ import {
computeWindowStats,
type MetricWindowStats,
} from "@/lib/charts/window-stats";
import { shouldFireDataReady } from "@/lib/charts/data-ready-latch";
const TIME_RANGES_KEYS = [
{
@@ -1015,9 +1017,34 @@ export function HealthChart({
// range-tab change creates a new cache entry but the gate has long
// latched by then), so this fires exactly when the first paintable
// state — chart, empty-window card, or error fallback — is available.
//
// v1.20.1 — fire exactly once, gated on a ref rather than on the
// `onDataReady` prop identity. The dashboard passes an inline
// `() => markChartReady(id)` closure, so `onDataReady` is a NEW
// reference on every parent render; keying the effect on it re-ran the
// notify on every commit (e.g. a snapshot refetch on tab-resume), and
// that per-commit passive effect kept the chart-row's Radix-Popper
// anchors re-committing until React tripped its update-depth guard
// (#185). The ready signal is monotonic — once a chart has settled it
// stays settled — so latch it behind a ref and depend on `isLoading`
// alone. `onDataReady` is read through a ref so a late-arriving handler
// still fires without re-arming the effect.
const onDataReadyRef = useRef(onDataReady);
useEffect(() => {
if (!isLoading) onDataReady?.();
}, [isLoading, onDataReady]);
onDataReadyRef.current = onDataReady;
}, [onDataReady]);
const dataReadyFiredRef = useRef(false);
useEffect(() => {
if (
!shouldFireDataReady({
isLoading,
alreadyFired: dataReadyFiredRef.current,
})
)
return;
dataReadyFiredRef.current = true;
onDataReadyRef.current?.();
}, [isLoading]);
const chartData = useMemo(() => {
if (!data?.length) return data;
@@ -33,7 +33,7 @@
* the wrapper itself stays intact.
*/
import { useQuery } from "@tanstack/react-query";
import { useEffect, useMemo, useState } from "react";
import { useEffect, useMemo, useState, useRef } from "react";
import { queryKeys } from "@/lib/query-keys";
import {
ResponsiveContainer,
@@ -63,6 +63,7 @@ import { useChartOverlayPrefs } from "@/hooks/use-chart-overlay-prefs";
import { useViewportWidth } from "@/hooks/use-viewport-width";
import { chooseTickInterval } from "@/lib/charts/x-axis-density";
import { apiGet } from "@/lib/api/api-fetch";
import { shouldFireDataReady } from "@/lib/charts/data-ready-latch";
interface DailyCompliancePoint {
/** Berlin calendar day, "YYYY-MM-DD". */
@@ -255,9 +256,29 @@ export function MedicationComplianceChart({
// v1.16.0 — report the settled query to the dashboard's shared
// reveal gate (see `onDataReady` prop doc).
//
// v1.20.1 — fire once via a ref instead of keying on the unstable
// `onDataReady` prop. See the matching note in `health-chart.tsx`: the
// dashboard hands every chart a fresh `() => markChartReady(id)` closure
// each render, so depending on it re-ran this notify on every commit and
// the per-commit passive effect kept the Radix-Popper tile anchors
// re-committing until React's update-depth guard tripped (#185).
const onDataReadyRef = useRef(onDataReady);
useEffect(() => {
if (!isLoading) onDataReady?.();
}, [isLoading, onDataReady]);
onDataReadyRef.current = onDataReady;
}, [onDataReady]);
const dataReadyFiredRef = useRef(false);
useEffect(() => {
if (
!shouldFireDataReady({
isLoading,
alreadyFired: dataReadyFiredRef.current,
})
)
return;
dataReadyFiredRef.current = true;
onDataReadyRef.current?.();
}, [isLoading]);
const chartData = useMemo(
() =>
+24 -3
View File
@@ -14,7 +14,7 @@ import {
ReferenceArea,
ReferenceLine,
} from "recharts";
import { useEffect, useState, useMemo } from "react";
import { useEffect, useState, useMemo, useRef } from "react";
import { Button } from "@/components/ui/button";
import { Skeleton } from "@/components/ui/skeleton";
import { Card, CardHeader, CardTitle, CardContent } from "@/components/ui/card";
@@ -40,6 +40,7 @@ import { useChartOverlayPrefs } from "@/hooks/use-chart-overlay-prefs";
import { useViewportWidth } from "@/hooks/use-viewport-width";
import { computeTickPositions } from "@/lib/charts/x-axis-density";
import { CHART_HEIGHT_PX } from "@/lib/charts/constants";
import { shouldFireDataReady } from "@/lib/charts/data-ready-latch";
import { moodLabelKeyForScore } from "@/lib/mood/labels";
import { apiGet } from "@/lib/api/api-fetch";
@@ -361,9 +362,29 @@ export function MoodChart({
// gate (see `onDataReady` prop doc). Must run BEFORE the
// empty-entries early return below so a no-data mount still unblocks
// the row.
//
// v1.20.1 — fire once via a ref instead of keying on the unstable
// `onDataReady` prop. See the matching note in `health-chart.tsx`: the
// dashboard hands every chart a fresh `() => markChartReady(id)` closure
// each render, so depending on it re-ran this notify on every commit and
// the per-commit passive effect kept the Radix-Popper tile anchors
// re-committing until React's update-depth guard tripped (#185).
const onDataReadyRef = useRef(onDataReady);
useEffect(() => {
if (!isLoading) onDataReady?.();
}, [isLoading, onDataReady]);
onDataReadyRef.current = onDataReady;
}, [onDataReady]);
const dataReadyFiredRef = useRef(false);
useEffect(() => {
if (
!shouldFireDataReady({
isLoading,
alreadyFired: dataReadyFiredRef.current,
})
)
return;
dataReadyFiredRef.current = true;
onDataReadyRef.current?.();
}, [isLoading]);
// v1.4.43 W2-CHART-GATE — raw mood-entry count across every day in
// the window. `entries[].samples` carries the per-day raw count
@@ -0,0 +1,88 @@
/**
* v1.20.1 — regression cover for the dashboard chart-row render loop.
*
* A user reported the dashboard crashing (blank error card) on returning
* to a backgrounded tab. The snapshot query refetches on window focus,
* re-rendering the page; every chart was handed an inline
* `() => markChartReady(id)` closure, so the chart's `onDataReady` notify
* effect — keyed on that ever-changing prop — re-fired on every commit.
* That per-commit passive effect kept the tile strip's Radix-Popper
* anchors re-committing until React tripped its update-depth guard
* (minified React error #185).
*
* The fix latches the notify behind a ref so it fires exactly once on the
* first non-loading commit and depends on `isLoading` alone. This pins the
* pure decision and the charts' wiring so the latch cannot silently revert
* to the per-render notify.
*/
import { describe, it, expect } from "vitest";
import { readFileSync } from "node:fs";
import { join } from "node:path";
import { shouldFireDataReady } from "@/lib/charts/data-ready-latch";
describe("shouldFireDataReady", () => {
it("holds while the chart's initial query is still loading", () => {
expect(shouldFireDataReady({ isLoading: true, alreadyFired: false })).toBe(
false,
);
});
it("fires once on the first non-loading commit", () => {
expect(shouldFireDataReady({ isLoading: false, alreadyFired: false })).toBe(
true,
);
});
it("never re-fires after it has fired once, even though the chart keeps re-rendering", () => {
// Models the tab-resume path: the parent re-renders repeatedly (a new
// `onDataReady` closure each time) while `isLoading` stays false. The
// latch must decline every subsequent commit so the notify cannot
// drive an unbounded render loop.
let fired = false;
let fireCount = 0;
for (let commit = 0; commit < 60; commit += 1) {
if (shouldFireDataReady({ isLoading: false, alreadyFired: fired })) {
fired = true;
fireCount += 1;
}
}
expect(fireCount).toBe(1);
});
it("a chart that never loaded data still fires once the moment it settles", () => {
// Loading first, then settled — fires on the settle commit, then holds.
expect(shouldFireDataReady({ isLoading: true, alreadyFired: false })).toBe(
false,
);
expect(shouldFireDataReady({ isLoading: false, alreadyFired: false })).toBe(
true,
);
expect(shouldFireDataReady({ isLoading: false, alreadyFired: true })).toBe(
false,
);
});
});
describe("chart notify wiring", () => {
const charts = [
"src/components/charts/health-chart.tsx",
"src/components/charts/mood-chart.tsx",
"src/components/charts/medication-compliance-chart.tsx",
];
for (const rel of charts) {
const src = readFileSync(join(process.cwd(), rel), "utf8");
it(`${rel} latches the data-ready notify once, not per render`, () => {
// Routes through the pure latch.
expect(src).toContain("shouldFireDataReady(");
// Reads the handler through a ref so a changing prop identity never
// re-arms the effect.
expect(src).toContain("onDataReadyRef.current");
// The notify effect depends on `isLoading` ALONE — never on the
// unstable `onDataReady` prop (the loop trigger).
expect(src).not.toMatch(/\}, \[isLoading, onDataReady\]\)/);
});
}
});
+35
View File
@@ -0,0 +1,35 @@
/**
* v1.20.1 — pure decision behind the charts' one-shot `onDataReady`
* notify.
*
* The dashboard's shared reveal gate (`useDashboardChartReveal`) listens
* for each chart reporting its data query settled. Every chart receives
* an inline `() => markChartReady(id)` closure from the dashboard, so the
* `onDataReady` prop is a NEW reference on every parent render. Keying the
* notify effect on that prop re-ran it on every commit — and a refetch on
* tab-resume (`refetchOnWindowFocus`) re-rendered the page repeatedly,
* which kept that per-commit passive effect firing and the chart row's
* Radix-Popper anchors re-committing until React tripped its update-depth
* guard (the minified React error #185 a user reported on returning to a
* backgrounded tab).
*
* The ready signal is monotonic: once a chart's initial query has settled
* it never un-settles (a later range-tab change opens a fresh cache entry,
* but the gate has long latched by then). So the notify must fire exactly
* once — on the first non-loading commit — and never again, regardless of
* the `onDataReady` prop identity.
*
* This helper is the pure core of that latch, exported so the gate has
* direct unit coverage without mounting React (project convention:
* SSR-only tests, no DOM runner). The chart wires it to a `useRef` flag.
*/
export function shouldFireDataReady(input: {
/** The chart's query is still on its initial load. */
isLoading: boolean;
/** Whether the notify has already fired this mount. */
alreadyFired: boolean;
}): boolean {
if (input.isLoading) return false;
if (input.alreadyFired) return false;
return true;
}