Skip to content

Commit 2840506

Browse files
authored
fix(ui): drop superseded useApiResource responses when the path changes mid-flight (#7790)
useApiResource had no cancellation guard, so when path changes (pagination offsets, free-text repo input, analytics window selection) an older request's response could resolve after a newer one and silently overwrite it — showing the wrong page/repo while the surrounding UI reflects the newer request. Tag each load with an incrementing request id and drop the result after the await if a newer load has superseded it. Fixes the class for every useApiResource consumer. Closes #7785
1 parent 07064df commit 2840506

2 files changed

Lines changed: 40 additions & 1 deletion

File tree

apps/loopover-ui/src/lib/api/use-api-resource.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,3 +86,32 @@ describe("useApiResource errorKind/errorStatus (#793)", () => {
8686
expect(state.errorKind).toBeUndefined();
8787
});
8888
});
89+
90+
describe("useApiResource stale-response guard (#7785)", () => {
91+
it("drops a superseded response when the path changed before it resolved", async () => {
92+
apiFetch.mockReset();
93+
let resolveOld!: (value: unknown) => void;
94+
let resolveNew!: (value: unknown) => void;
95+
apiFetch
96+
.mockImplementationOnce(() => new Promise((resolve) => (resolveOld = resolve)))
97+
.mockImplementationOnce(() => new Promise((resolve) => (resolveNew = resolve)));
98+
99+
const { result, rerender } = renderHook(
100+
({ path }) => useApiResource<{ page: number }>(path, "Thing"),
101+
{ initialProps: { path: "/v1/thing?offset=0" } },
102+
);
103+
// Change the path so a second load starts while the first request is still in flight.
104+
rerender({ path: "/v1/thing?offset=20" });
105+
106+
// The NEWER (second) request resolves first and is applied.
107+
resolveNew({ ok: true, data: { page: 2 }, status: 200, durationMs: 1 });
108+
await waitFor(() => expect(result.current.status).toBe("ready"));
109+
expect(result.current.data).toEqual({ page: 2 });
110+
111+
// The STALE (first) request resolves last — it must be dropped, not overwrite the current page.
112+
resolveOld({ ok: true, data: { page: 1 }, status: 200, durationMs: 1 });
113+
await Promise.resolve();
114+
await Promise.resolve();
115+
expect(result.current.data).toEqual({ page: 2 });
116+
});
117+
});

apps/loopover-ui/src/lib/api/use-api-resource.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useCallback, useEffect, useState } from "react";
1+
import { useCallback, useEffect, useRef, useState } from "react";
22

33
import { getApiOrigin } from "./origin";
44
import { apiFetch, type ApiFailureKind } from "./request";
@@ -34,7 +34,15 @@ export function useApiResource<T>(
3434
loadedAt: null,
3535
});
3636

37+
const requestIdRef = useRef(0);
38+
3739
const load = useCallback(async () => {
40+
// Guard against out-of-order responses (#7785): when `path` changes (pagination offsets, free-text repo input,
41+
// window selection) a new load starts while an older apiFetch is still in flight. Tag each load and, after the
42+
// await, drop the result if a newer load has since superseded it — otherwise a stale page's response resolves
43+
// last and silently overwrites the current one while the surrounding UI reflects the newer request.
44+
const requestId = requestIdRef.current + 1;
45+
requestIdRef.current = requestId;
3846
if (!enabled) {
3947
setState({ status: "error", data: null, error: "disabled", loadedAt: null });
4048
return;
@@ -47,6 +55,8 @@ export function useApiResource<T>(
4755
headers,
4856
credentials: "include",
4957
});
58+
// A newer load superseded this one (the path changed mid-flight); drop this stale response entirely.
59+
if (requestId !== requestIdRef.current) return;
5060
if (result.ok) {
5161
setState({ status: "ready", data: result.data, error: null, loadedAt: Date.now() });
5262
} else {

0 commit comments

Comments
 (0)