fix(dashboard): use primitive deps in onMetrics effect to break render loop
Using a metrics object as a useEffect dep causes the loop: onMetrics → setMetricsById → Dashboard re-renders → ServerWorkspaceRows re-renders → useQueries runs → TanStack Query cache subscriber fires (Sidebar's setNow) → query result objects are new references → metrics useMemo returns new object → effect dep changed → onMetrics again → ∞ Fix: depend on the five primitive values (workspaceCount, conclusionCount, queueActive, queuePending, health) directly. React compares primitives by value, so the effect only fires when actual data changes, not on reference churn from re-renders. Also adds server-workspace-rows.test.tsx with three focused unit tests: correct health:ok report, stability after load (no re-fire), and health transition coverage.
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
import { useQueries } from "@tanstack/react-query";
|
||||
import { motion } from "framer-motion";
|
||||
import { ChevronRight, CircleDot } from "lucide-react";
|
||||
import { useEffect, useMemo, useRef } from "react";
|
||||
import { useEffect, useMemo } from "react";
|
||||
import {
|
||||
scopedConclusionsCountOptions,
|
||||
scopedQueueStatusOptions,
|
||||
@@ -28,16 +28,6 @@ interface Props {
|
||||
|
||||
const WORKSPACE_PAGE_SIZE = 100;
|
||||
|
||||
function metricsEqual(a: FleetRowMetrics, b: FleetRowMetrics): boolean {
|
||||
return (
|
||||
a.workspaceCount === b.workspaceCount &&
|
||||
a.conclusionCount === b.conclusionCount &&
|
||||
a.queueActive === b.queueActive &&
|
||||
a.queuePending === b.queuePending &&
|
||||
a.health === b.health
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Renders one `<tr>` per workspace on a single server (instance), labelled
|
||||
* `<workspace> (<server>)`, and reports the server's summed metrics up so the
|
||||
@@ -80,24 +70,19 @@ export function ServerWorkspaceRows({ instance, onOpenWorkspace, onMetrics }: Pr
|
||||
: workspacesQ.isSuccess
|
||||
? "ok"
|
||||
: "loading";
|
||||
const metrics: FleetRowMetrics = useMemo(
|
||||
() => ({
|
||||
// Dep array uses primitives only — an object dep (e.g. the metrics shape) would
|
||||
// create a new reference on each render even when values are unchanged, causing
|
||||
// onMetrics → setMetricsById → re-render → new object → onMetrics … loop.
|
||||
useEffect(() => {
|
||||
onMetrics(instance.id, {
|
||||
workspaceCount: totalWorkspaces,
|
||||
conclusionCount,
|
||||
queueActive,
|
||||
queuePending,
|
||||
lastSeen: null,
|
||||
health,
|
||||
}),
|
||||
[totalWorkspaces, conclusionCount, queueActive, queuePending, health],
|
||||
);
|
||||
|
||||
const lastReported = useRef<FleetRowMetrics | null>(null);
|
||||
useEffect(() => {
|
||||
if (lastReported.current && metricsEqual(lastReported.current, metrics)) return;
|
||||
lastReported.current = metrics;
|
||||
onMetrics(instance.id, metrics);
|
||||
}, [instance.id, metrics, onMetrics]);
|
||||
});
|
||||
}, [instance.id, totalWorkspaces, conclusionCount, queueActive, queuePending, health, onMetrics]);
|
||||
|
||||
if (workspacesQ.isError) {
|
||||
return (
|
||||
|
||||
Reference in New Issue
Block a user