Skip to content

Commit dd573b9

Browse files
committed
fix(ui-kit): make StateBoundary's onFailureNotify edge-triggered
StateBoundary's onFailureNotify effect was level-triggered instead of edge-triggered: it re-fired on any render where isError stayed true and onRetry/onFailureNotify (ordinary function props) changed identity, not just on the false->true transition its own comment describes. Track the previous isError value via a ref (mirroring the wasError pattern in mcp-version-badge.tsx) so it only fires once per actual error-state entry, and still re-fires correctly if the boundary leaves and re-enters error. Closes #7436
1 parent a3f2baa commit dd573b9

2 files changed

Lines changed: 59 additions & 3 deletions

File tree

apps/loopover-ui/src/components/site/state-views.test.tsx

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,3 +138,52 @@ describe("StateBoundary retry/refresh actions (#793 regression guard)", () => {
138138
expect(screen.getByText("content")).toBeTruthy();
139139
});
140140
});
141+
142+
describe("StateBoundary onFailureNotify edge-triggered dedupe (#7436 regression)", () => {
143+
it("fires onFailureNotify exactly once across re-renders while isError stays true, even as onRetry's identity changes", () => {
144+
notifyApiFailure.mockClear();
145+
// Each render passes a brand-new onRetry closure -- this mirrors the real app wrapper, which
146+
// also constructs a fresh onFailureNotify arrow function on every render (#6506) -- so an
147+
// unrelated re-render while the boundary is still in the error state must not re-notify.
148+
const { rerender } = render(
149+
<StateBoundary isError errorLabel="Widgets" onRetry={() => {}}>
150+
<div>content</div>
151+
</StateBoundary>,
152+
);
153+
expect(notifyApiFailure).toHaveBeenCalledTimes(1);
154+
155+
for (let i = 0; i < 3; i++) {
156+
rerender(
157+
<StateBoundary isError errorLabel="Widgets" onRetry={() => {}}>
158+
<div>content</div>
159+
</StateBoundary>,
160+
);
161+
}
162+
163+
expect(notifyApiFailure).toHaveBeenCalledTimes(1);
164+
});
165+
166+
it("fires again exactly once on a fresh false->true transition after leaving the error state", () => {
167+
notifyApiFailure.mockClear();
168+
const { rerender } = render(
169+
<StateBoundary isError errorLabel="Widgets" onRetry={() => {}}>
170+
<div>content</div>
171+
</StateBoundary>,
172+
);
173+
expect(notifyApiFailure).toHaveBeenCalledTimes(1);
174+
175+
rerender(
176+
<StateBoundary isError={false} errorLabel="Widgets" onRetry={() => {}}>
177+
<div>content</div>
178+
</StateBoundary>,
179+
);
180+
expect(notifyApiFailure).toHaveBeenCalledTimes(1);
181+
182+
rerender(
183+
<StateBoundary isError errorLabel="Widgets" onRetry={() => {}}>
184+
<div>content</div>
185+
</StateBoundary>,
186+
);
187+
expect(notifyApiFailure).toHaveBeenCalledTimes(2);
188+
});
189+
});

packages/loopover-ui-kit/src/components/state-views.tsx

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { Loader2, Inbox, AlertTriangle, RefreshCw, WifiOff } from "lucide-react";
2-
import { useEffect, type ReactNode } from "react";
2+
import { useEffect, useRef, type ReactNode } from "react";
33
import { toast } from "sonner";
44

55
import { cn } from "../utils";
@@ -264,9 +264,16 @@ export function StateBoundary({
264264
? undefined
265265
: "The data source did not respond. Retry the request, or check back once the service has recovered.");
266266

267-
// When this boundary flips into the error state, surface a toast with Retry.
267+
// When this boundary flips into the error state, surface a toast with Retry. Tracked via a ref
268+
// (mirroring the `wasError` pattern in apps/loopover-ui/src/components/site/mcp-version-badge.tsx)
269+
// so this only fires on the false->true edge, not on every re-render while isError stays true --
270+
// onRetry/onFailureNotify are ordinary function props that get new identities on unrelated
271+
// re-renders, and without edge detection those identity changes alone would re-run this effect.
272+
const wasErrorRef = useRef(false);
268273
useEffect(() => {
269-
if (isError && errorLabel) {
274+
const enteredError = isError && !wasErrorRef.current;
275+
wasErrorRef.current = Boolean(isError);
276+
if (enteredError && errorLabel) {
270277
onFailureNotify?.({
271278
label: errorLabel,
272279
kind: errorKind ?? "network",

0 commit comments

Comments
 (0)