diff --git a/docs/hooks.md b/docs/hooks.md index 1cc4108..7d86e75 100644 --- a/docs/hooks.md +++ b/docs/hooks.md @@ -9,6 +9,7 @@ contract changes. | Hook | Source | Status | | --- | --- | --- | | `useApi` | `src/lib/useApi.ts` | Exported | +| `useApiMutation` | `src/lib/useApiMutation.ts` | Exported | | `useClipboard` | `src/lib/useClipboard.ts` | Exported | | `useDebounce` | `src/lib/useDebounce.ts` | Exported | | `useLocalState` | `src/lib/useLocalState.ts` | Exported | @@ -30,18 +31,21 @@ import { useApi } from "@/lib/useApi"; Return shape: ```ts -type ApiErrorKind = "timeout" | "generic"; +type ApiErrorKind = "timeout" | "rate_limited" | "generic"; type State = - | { status: "loading" } + | { status: "loading"; refetch: () => void } | { status: "error"; error: string; errorKind: ApiErrorKind; isTimeout: boolean; + isRateLimited: boolean; + retryAfterMs: number | null; retry: () => void; + refetch: () => void; } - | { status: "ok"; data: T }; + | { status: "ok"; data: T; refetch: () => void }; ``` Parameters: @@ -52,43 +56,143 @@ Parameters: Behaviour and gotchas: - This is a client hook and must be used from a client component. -- The first state is `{ status: "loading" }`. +- The first state is `{ status: "loading"; refetch }`. - When `path` changes, the hook dispatches a fresh loading state and fetches the new path. - If the component unmounts or `path` changes before a response settles, the - stale response is ignored through an internal cancellation flag. + stale response is ignored through an internal cancellation flag and the + in-flight request is aborted. - `path: null` skips fetching and leaves the existing state unchanged. -- Detects `ApiTimeoutError` and sets `errorKind: "timeout"`, `isTimeout: true`, and `"Request timed out. Please try again."`. Generic errors set `errorKind: "generic"`, `isTimeout: false`, and `Error.message` (or `"failed to load"`). -- Provides a `retry()` callback affordance on the error state to trigger a refetch of the path. + Calling `refetch()` while `path` is `null` is a no-op. +- Detects `ApiTimeoutError` and sets `errorKind: "timeout"`, `isTimeout: true`, and `"Request timed out. Please try again."`. Detects `ApiRateLimitedError` and sets `errorKind: "rate_limited"`, `isRateLimited: true`, and `retryAfterMs`. Generic errors set `errorKind: "generic"`, `isTimeout: false`, and `Error.message` (or `"failed to load"`). +- Provides a stable `refetch()` callback on every status. Calling it aborts any + in-flight request and re-runs the fetch for the current `path`. +- Error states also expose `retry()`, which is the same callback as `refetch`, + kept for existing callers. +- When the browser fires an `online` event while the hook is in an error state, + the request is automatically retried. -Minimal real usage, based on `src/app/changelog/page.tsx`: +Minimal real usage, based on `src/app/agents/[agent]/page.tsx`: ```tsx "use client"; +import { ErrorMessage } from "@/components/ErrorMessage"; import { Spinner } from "@/components/Spinner"; import { useApi } from "@/lib/useApi"; -type Entry = { version: string; date: string; notes: string[] }; +type Usage = { items: { serviceId: string; total: number }[] }; -export function ChangelogPreview() { - const state = useApi<{ entries: Entry[] }>("/api/v1/changelog"); +export function AgentUsagePreview({ agent }: { agent: string }) { + const state = useApi( + `/api/v1/agents/${encodeURIComponent(agent)}/usage`, + ); if (state.status === "loading") { - return ; + return ; } if (state.status === "error") { - return

{state.error}

; + return ( + + ); } - return

{state.data.entries.length} entries

; + return

{state.data.items.length} services

; } ``` Use this hook for simple GET-backed client views that can be represented as -loading, error, or successful data. For write actions or request bodies, use the -helpers in `src/lib/apiClient.ts` directly. +loading, error, or successful data. For write actions or request bodies, prefer +`useApiMutation` below. + +## `useApiMutation` + +```ts +function useApiMutation( + mutationFn: ( + variables: TVariables, + options: { signal: AbortSignal }, + ) => Promise, +): { + mutate: (variables: TVariables) => Promise; + status: "idle" | "pending" | "success" | "error"; + error: string | null; + reset: () => void; +}; +``` + +Import from: + +```ts +import { useApiMutation } from "@/lib/useApiMutation"; +``` + +Parameters: + +- `mutationFn`: async writer that receives call variables and an AbortSignal. + Forward the signal into `apiPost` / `apiDelete` / `apiPatch` so the shared + client can cancel the underlying fetch. + +Return shape: + +- `mutate(variables)`: starts the mutation, sets `status` to `"pending"`, and + resolves with `TData` on success. On failure it sets `status` to `"error"`, + mirrors a display-ready message on `error`, and rethrows. +- `status`: `"idle"` | `"pending"` | `"success"` | `"error"`. +- `error`: display-ready message when `status` is `"error"`, otherwise `null`. +- `reset()`: returns to `"idle"`, clears `error`, and aborts any in-flight + mutation. + +Behaviour and gotchas: + +- This is a client hook and must be used from a client component. +- Calling `mutate` while a previous mutation is still pending aborts the older + request and ignores its late response. +- Unmount aborts the current request and ignores late responses, matching the + `useApi` cancellation contract. +- Aborted / superseded mutations do not flip `status` to `"error"`. +- Non-Error rejections fall back to `"failed to mutate"`. + +Minimal real usage, based on `src/app/services/new/page.tsx`: + +```tsx +"use client"; + +import { apiPost } from "@/lib/apiClient"; +import { useApiMutation } from "@/lib/useApiMutation"; + +type CreateServiceBody = { + serviceId: string; + priceStroops: number; +}; + +export function RegisterServiceButton(props: CreateServiceBody) { + const { mutate, status, error } = useApiMutation( + (body: CreateServiceBody, { signal }) => + apiPost("/api/v1/services", body, { signal }), + ); + + return ( +
+ + {error &&

{error}

} +
+ ); +} +``` + +Use this hook for POST / DELETE / PATCH flows that need a shared pending, error, +and success state machine without copying AbortController cleanup into each +page. ## `usePolling` @@ -389,4 +493,5 @@ Use this hook in components that need to react to connectivity changes, such as ## Coverage Note This reference covers every hook exported from `src/lib` at the time of writing: -`useApi`, `useClipboard`, `useOnlineStatus`, `usePolling`, `useDebounce`, and `useLocalState`. +`useApi`, `useApiMutation`, `useClipboard`, `useOnlineStatus`, `usePolling`, +`useDebounce`, and `useLocalState`. diff --git a/src/app/agents/[agent]/page.test.tsx b/src/app/agents/[agent]/page.test.tsx index 656a983..a6066d0 100644 --- a/src/app/agents/[agent]/page.test.tsx +++ b/src/app/agents/[agent]/page.test.tsx @@ -1,4 +1,5 @@ import { render, screen, waitFor } from "@testing-library/react"; +import { act } from "react"; import AgentDetailPage from "./page"; import { apiGet } from "@/lib/apiClient"; @@ -184,6 +185,44 @@ describe("AgentDetailPage", () => { expect(await screen.findByRole("alert")).toHaveTextContent( "Backend usage offline", ); + expect( + screen.getByRole("button", { name: /try again/i }), + ).toBeInTheDocument(); + }); + + it("refetches usage when Try again is clicked after an error", async () => { + let usageCalls = 0; + + apiGetMock.mockImplementation((path: string) => { + if (path.endsWith("/usage")) { + usageCalls += 1; + if (usageCalls === 1) { + return Promise.reject(new Error("Backend usage offline")) as never; + } + return Promise.resolve({ + agent: "agent-retry", + items: [{ serviceId: "svc-retry", total: 3 }], + }) as never; + } + if (path.endsWith("/total")) { + return Promise.resolve({ total: 10 }) as never; + } + return Promise.reject(new Error(`unexpected: ${path}`)) as never; + }); + + renderPage("agent-retry"); + + expect(await screen.findByRole("alert")).toHaveTextContent( + "Backend usage offline", + ); + + act(() => { + screen.getByRole("button", { name: /try again/i }).click(); + }); + + expect(await screen.findByText("svc-retry")).toBeInTheDocument(); + expect(screen.queryByRole("alert")).not.toBeInTheDocument(); + expect(usageCalls).toBe(2); }); it("hides the spinner after a usage error", async () => { diff --git a/src/app/agents/[agent]/page.tsx b/src/app/agents/[agent]/page.tsx index 0376245..79ca173 100644 --- a/src/app/agents/[agent]/page.tsx +++ b/src/app/agents/[agent]/page.tsx @@ -3,6 +3,7 @@ import { use, useEffect, useState } from "react"; import { Breadcrumb } from "@/components/Breadcrumb"; import { EmptyState } from "@/components/EmptyState"; +import { ErrorMessage } from "@/components/ErrorMessage"; import { PageShell } from "@/components/PageShell"; import { Spinner } from "@/components/Spinner"; import { apiGet } from "@/lib/apiClient"; @@ -39,7 +40,6 @@ export default function AgentDetailPage({ }, [agent, encodedAgent]); const items = usageState.status === "ok" ? usageState.data.items : null; - const error = usageState.status === "error" ? usageState.error : null; const total = totalState?.agent === agent ? totalState.total : null; return ( @@ -68,10 +68,11 @@ export default function AgentDetailPage({ )} - {error && ( -

- {error} -

+ {usageState.status === "error" && ( + )}

Lifetime total:{" "} diff --git a/src/app/services/new/page.test.tsx b/src/app/services/new/page.test.tsx index 8f5a3b5..1712be6 100644 --- a/src/app/services/new/page.test.tsx +++ b/src/app/services/new/page.test.tsx @@ -151,10 +151,14 @@ describe("NewServicePage", () => { fireEvent.submit(screen.getByRole("button", { name: /register service/i })); await waitFor(() => { - expect(apiPostMock).toHaveBeenCalledWith("/api/v1/services", { - serviceId: "test-service", - priceStroops: 100, - }); + expect(apiPostMock).toHaveBeenCalledWith( + "/api/v1/services", + { + serviceId: "test-service", + priceStroops: 100, + }, + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); }); await waitFor(() => { @@ -176,10 +180,14 @@ describe("NewServicePage", () => { fireEvent.submit(screen.getByRole("button", { name: /register service/i })); await waitFor(() => { - expect(apiPostMock).toHaveBeenCalledWith("/api/v1/services", { - serviceId: "existing-service", - priceStroops: 50, - }); + expect(apiPostMock).toHaveBeenCalledWith( + "/api/v1/services", + { + serviceId: "existing-service", + priceStroops: 50, + }, + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); }); // Check page-level alert @@ -262,10 +270,14 @@ describe("NewServicePage", () => { fireEvent.submit(screen.getByRole("button", { name: /register service/i })); await waitFor(() => { - expect(apiPostMock).toHaveBeenCalledWith("/api/v1/services", { - serviceId: "free-svc", - priceStroops: 0, - }); + expect(apiPostMock).toHaveBeenCalledWith( + "/api/v1/services", + { + serviceId: "free-svc", + priceStroops: 0, + }, + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); }); await waitFor(() => { @@ -282,10 +294,14 @@ describe("NewServicePage", () => { fireEvent.submit(screen.getByRole("button", { name: /register service/i })); await waitFor(() => { - expect(apiPostMock).toHaveBeenCalledWith("/api/v1/services", { - serviceId: "", - priceStroops: 50, - }); + expect(apiPostMock).toHaveBeenCalledWith( + "/api/v1/services", + { + serviceId: "", + priceStroops: 50, + }, + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); }); }); diff --git a/src/app/services/new/page.tsx b/src/app/services/new/page.tsx index 07aecdb..07547e2 100644 --- a/src/app/services/new/page.tsx +++ b/src/app/services/new/page.tsx @@ -1,24 +1,35 @@ "use client"; -import { useState } from "react"; import { useRouter } from "next/navigation"; +import { useState } from "react"; import { apiPost } from "@/lib/apiClient"; import { PageShell } from "@/components/PageShell"; import { TextField } from "@/components/TextField"; import { Button } from "@/components/Button"; import { parseNonNegativeInt } from "@/lib/validateNumber"; +import { useApiMutation } from "@/lib/useApiMutation"; + +type CreateServiceBody = { + serviceId: string; + priceStroops: number; +}; export default function NewServicePage() { const router = useRouter(); const [serviceId, setServiceId] = useState(""); const [priceStroops, setPriceStroops] = useState(""); const [priceError, setPriceError] = useState(null); - const [error, setError] = useState(null); - const [loading, setLoading] = useState(false); + + const { mutate, status, error, reset } = useApiMutation( + (body: CreateServiceBody, { signal }) => + apiPost("/api/v1/services", body, { signal }), + ); + + const loading = status === "pending"; const onSubmit = async (e: React.FormEvent) => { e.preventDefault(); - setError(null); + reset(); setPriceError(null); const parsed = parseNonNegativeInt(priceStroops); @@ -27,17 +38,14 @@ export default function NewServicePage() { return; } - setLoading(true); try { - await apiPost("/api/v1/services", { + await mutate({ serviceId, priceStroops: parsed.value, }); router.push("/services"); - } catch (err) { - setError((err as Error).message); - } finally { - setLoading(false); + } catch { + // Error message is already mirrored on the mutation `error` state. } }; @@ -81,4 +89,3 @@ export default function NewServicePage() { ); } - diff --git a/src/lib/__tests__/useApi.test.tsx b/src/lib/__tests__/useApi.test.tsx index c637e17..a612c57 100644 --- a/src/lib/__tests__/useApi.test.tsx +++ b/src/lib/__tests__/useApi.test.tsx @@ -33,14 +33,35 @@ function Probe({ path }: { path: string | null }) { const state = useApi(path); if (state.status === "ok") { - return ok:{state.data.label}; + return ( +

+ ok:{state.data.label} + +
+ ); } if (state.status === "error") { - return error:{state.error}; + return ( +
+ error:{state.error} + +
+ ); } - return loading; + return ( +
+ loading + +
+ ); } function DetailedProbe({ path }: { path: string | null }) { @@ -57,15 +78,32 @@ function DetailedProbe({ path }: { path: string | null }) { + ); } if (state.status === "ok") { - return ok:{state.data.label}; + return ( +
+ ok:{state.data.label} + +
+ ); } - return loading; + return ( +
+ loading + +
+ ); } describe("useApi", () => { @@ -164,6 +202,196 @@ describe("useApi", () => { expect(screen.getByTestId("state")).toHaveTextContent("ok:retried success"); }); + it("exposes a stable refetch callback on every status", async () => { + const seen: Array<() => void> = []; + + function IdentityProbe({ path }: { path: string }) { + const state = useApi(path); + seen.push(state.refetch); + + if (state.status === "ok") { + return ( +
+ ok:{state.data.label} + +
+ ); + } + + if (state.status === "error") { + return ( +
+ error:{state.error} + +
+ ); + } + + return ( +
+ loading + +
+ ); + } + + const first = createDeferred(); + apiGetMock.mockReturnValueOnce(first.promise); + + render(); + expect(seen.length).toBeGreaterThan(0); + const initialRefetch = seen[0]; + + await act(async () => { + first.resolve({ label: "initial" }); + await first.promise; + }); + + expect(screen.getByTestId("state")).toHaveTextContent("ok:initial"); + expect(seen.every((fn) => fn === initialRefetch)).toBe(true); + + apiGetMock.mockRejectedValueOnce(new Error("boom")); + act(() => { + screen.getByTestId("refetch-btn").click(); + }); + + await waitFor(() => { + expect(screen.getByTestId("state")).toHaveTextContent("error:boom"); + }); + + expect(seen.every((fn) => fn === initialRefetch)).toBe(true); + }); + + it("refetch cancels an in-flight request and loads fresh data", async () => { + const first = createDeferred(); + const second = createDeferred(); + let firstSignal: AbortSignal | undefined; + + apiGetMock + .mockImplementationOnce((_path, init) => { + firstSignal = init?.signal as AbortSignal; + return first.promise; + }) + .mockReturnValueOnce(second.promise); + + render(); + + expect(apiGetMock).toHaveBeenCalledTimes(1); + expect(firstSignal?.aborted).toBe(false); + + act(() => { + screen.getByTestId("refetch-btn").click(); + }); + + expect(apiGetMock).toHaveBeenCalledTimes(2); + expect(firstSignal?.aborted).toBe(true); + expect(screen.getByTestId("state")).toHaveTextContent("loading"); + + await act(async () => { + first.resolve({ label: "stale" }); + await first.promise; + }); + + expect(screen.getByTestId("state")).toHaveTextContent("loading"); + + await act(async () => { + second.resolve({ label: "fresh" }); + await second.promise; + }); + + expect(screen.getByTestId("state")).toHaveTextContent("ok:fresh"); + }); + + it("refetch from an error state recovers to ok", async () => { + apiGetMock.mockRejectedValueOnce(new Error("temporary failure")); + + render(); + + await waitFor(() => { + expect(screen.getByTestId("error-message")).toHaveTextContent( + "temporary failure", + ); + }); + + const recovered = createDeferred(); + apiGetMock.mockReturnValueOnce(recovered.promise); + + act(() => { + screen.getByTestId("refetch-btn").click(); + }); + + expect(screen.getByTestId("state")).toHaveTextContent("loading"); + + await act(async () => { + recovered.resolve({ label: "recovered" }); + await recovered.promise; + }); + + expect(screen.getByTestId("state")).toHaveTextContent("ok:recovered"); + }); + + it("refetch is a no-op while path is null", () => { + render(); + + act(() => { + screen.getByTestId("refetch-btn").click(); + }); + + expect(apiGetMock).not.toHaveBeenCalled(); + expect(screen.getByTestId("state")).toHaveTextContent("loading"); + }); + + it("automatically refetches when the browser comes back online after an error", async () => { + apiGetMock.mockRejectedValueOnce(new Error("offline failure")); + + render(); + + await waitFor(() => { + expect(screen.getByTestId("state")).toHaveTextContent( + "error:offline failure", + ); + }); + + const recovered = createDeferred(); + apiGetMock.mockReturnValueOnce(recovered.promise); + + act(() => { + window.dispatchEvent(new Event("online")); + }); + + expect(screen.getByTestId("state")).toHaveTextContent("loading"); + + await act(async () => { + recovered.resolve({ label: "back online" }); + await recovered.promise; + }); + + expect(screen.getByTestId("state")).toHaveTextContent("ok:back online"); + }); + + it("ignores online events while the request is already successful", async () => { + apiGetMock.mockResolvedValueOnce({ label: "stable" }); + + render(); + + await waitFor(() => { + expect(screen.getByTestId("state")).toHaveTextContent("ok:stable"); + }); + + act(() => { + window.dispatchEvent(new Event("online")); + }); + + expect(apiGetMock).toHaveBeenCalledTimes(1); + expect(screen.getByTestId("state")).toHaveTextContent("ok:stable"); + }); + it("covers timeout branch using fake timers", async () => { jest.useFakeTimers(); diff --git a/src/lib/__tests__/useApiMutation.test.tsx b/src/lib/__tests__/useApiMutation.test.tsx new file mode 100644 index 0000000..37fb10e --- /dev/null +++ b/src/lib/__tests__/useApiMutation.test.tsx @@ -0,0 +1,464 @@ +import { render, screen, waitFor } from "@testing-library/react"; +import { act } from "react"; +import { useApiMutation } from "../useApiMutation"; + +function createDeferred() { + let resolve!: (value: T) => void; + let reject!: (reason?: unknown) => void; + const promise = new Promise((promiseResolve, promiseReject) => { + resolve = promiseResolve; + reject = promiseReject; + }); + return { promise, resolve, reject }; +} + +type Payload = { id: string }; +type Vars = { label: string }; + +function Probe({ + mutationFn, +}: { + mutationFn: ( + variables: Vars, + options: { signal: AbortSignal }, + ) => Promise; +}) { + const { mutate, status, error, reset } = useApiMutation(mutationFn); + + return ( +
+ {status} + {error ?? ""} + + +
+ ); +} + +describe("useApiMutation", () => { + it("starts idle and transitions to success with mutate", async () => { + const request = createDeferred(); + const mutationFn = jest.fn(() => request.promise); + + render(); + + expect(screen.getByTestId("status")).toHaveTextContent("idle"); + expect(screen.getByTestId("error")).toHaveTextContent(""); + + act(() => { + screen.getByTestId("mutate-btn").click(); + }); + + expect(screen.getByTestId("status")).toHaveTextContent("pending"); + expect(mutationFn).toHaveBeenCalledWith( + { label: "ci" }, + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); + + await act(async () => { + request.resolve({ id: "1" }); + await request.promise; + }); + + expect(screen.getByTestId("status")).toHaveTextContent("success"); + expect(screen.getByTestId("error")).toHaveTextContent(""); + }); + + it("transitions to error when the mutation rejects", async () => { + const mutationFn = jest.fn(() => + Promise.reject(new Error("backend unavailable")), + ); + + render(); + + act(() => { + screen.getByTestId("mutate-btn").click(); + }); + + await waitFor(() => { + expect(screen.getByTestId("status")).toHaveTextContent("error"); + expect(screen.getByTestId("error")).toHaveTextContent( + "backend unavailable", + ); + }); + }); + + it("falls back to a generic error message for non-Error rejections", async () => { + const mutationFn = jest.fn(() => Promise.reject({})); + + render(); + + act(() => { + screen.getByTestId("mutate-btn").click(); + }); + + await waitFor(() => { + expect(screen.getByTestId("status")).toHaveTextContent("error"); + expect(screen.getByTestId("error")).toHaveTextContent("failed to mutate"); + }); + }); + + it("reset returns to idle and clears error", async () => { + const mutationFn = jest.fn(() => Promise.reject(new Error("boom"))); + + render(); + + act(() => { + screen.getByTestId("mutate-btn").click(); + }); + + await waitFor(() => { + expect(screen.getByTestId("status")).toHaveTextContent("error"); + }); + + act(() => { + screen.getByTestId("reset-btn").click(); + }); + + expect(screen.getByTestId("status")).toHaveTextContent("idle"); + expect(screen.getByTestId("error")).toHaveTextContent(""); + }); + + it("reset aborts an in-flight mutation without applying late success", async () => { + const request = createDeferred(); + let signal: AbortSignal | undefined; + const mutationFn = jest.fn((_vars: Vars, opts: { signal: AbortSignal }) => { + signal = opts.signal; + return request.promise; + }); + + render(); + + act(() => { + screen.getByTestId("mutate-btn").click(); + }); + + expect(screen.getByTestId("status")).toHaveTextContent("pending"); + expect(signal?.aborted).toBe(false); + + act(() => { + screen.getByTestId("reset-btn").click(); + }); + + expect(signal?.aborted).toBe(true); + expect(screen.getByTestId("status")).toHaveTextContent("idle"); + + await act(async () => { + request.resolve({ id: "late" }); + await request.promise.catch(() => {}); + }); + + expect(screen.getByTestId("status")).toHaveTextContent("idle"); + expect(screen.getByTestId("error")).toHaveTextContent(""); + }); + + it("a newer mutate aborts the previous request and ignores its result", async () => { + const first = createDeferred(); + const second = createDeferred(); + let firstSignal: AbortSignal | undefined; + let call = 0; + + const mutationFn = jest.fn((_vars: Vars, opts: { signal: AbortSignal }) => { + call += 1; + if (call === 1) { + firstSignal = opts.signal; + return first.promise; + } + return second.promise; + }); + + render(); + + act(() => { + screen.getByTestId("mutate-btn").click(); + }); + expect(screen.getByTestId("status")).toHaveTextContent("pending"); + + act(() => { + screen.getByTestId("mutate-btn").click(); + }); + + expect(mutationFn).toHaveBeenCalledTimes(2); + expect(firstSignal?.aborted).toBe(true); + expect(screen.getByTestId("status")).toHaveTextContent("pending"); + + await act(async () => { + first.resolve({ id: "stale" }); + await first.promise; + }); + + expect(screen.getByTestId("status")).toHaveTextContent("pending"); + + await act(async () => { + second.resolve({ id: "fresh" }); + await second.promise; + }); + + expect(screen.getByTestId("status")).toHaveTextContent("success"); + }); + + it("aborts in-flight mutations on unmount without updating state", async () => { + const request = createDeferred(); + let signal: AbortSignal | undefined; + const consoleError = jest + .spyOn(console, "error") + .mockImplementation(() => {}); + + const mutationFn = jest.fn((_vars: Vars, opts: { signal: AbortSignal }) => { + signal = opts.signal; + return request.promise; + }); + + const { unmount } = render(); + + act(() => { + screen.getByTestId("mutate-btn").click(); + }); + + expect(signal?.aborted).toBe(false); + unmount(); + expect(signal?.aborted).toBe(true); + + await act(async () => { + request.resolve({ id: "late" }); + await request.promise; + }); + + expect(consoleError).not.toHaveBeenCalled(); + consoleError.mockRestore(); + }); + + it("does not treat AbortError as a mutation error state", async () => { + const mutationFn = jest.fn( + (_vars: Vars, opts: { signal: AbortSignal }) => + new Promise((_resolve, reject) => { + opts.signal.addEventListener("abort", () => { + const err = new Error("aborted"); + err.name = "AbortError"; + reject(err); + }); + }), + ); + + render(); + + act(() => { + screen.getByTestId("mutate-btn").click(); + }); + + expect(screen.getByTestId("status")).toHaveTextContent("pending"); + + act(() => { + screen.getByTestId("reset-btn").click(); + }); + + await waitFor(() => { + expect(screen.getByTestId("status")).toHaveTextContent("idle"); + expect(screen.getByTestId("error")).toHaveTextContent(""); + }); + }); + + it("clears a previous error when a new mutate starts", async () => { + const mutationFn = jest + .fn() + .mockRejectedValueOnce(new Error("first failure")) + .mockImplementationOnce( + () => new Promise(() => {}), + ); + + render(); + + act(() => { + screen.getByTestId("mutate-btn").click(); + }); + + await waitFor(() => { + expect(screen.getByTestId("error")).toHaveTextContent("first failure"); + }); + + act(() => { + screen.getByTestId("mutate-btn").click(); + }); + + expect(screen.getByTestId("status")).toHaveTextContent("pending"); + expect(screen.getByTestId("error")).toHaveTextContent(""); + }); + + it("returns the resolved data from mutate", async () => { + const mutationFn = jest.fn(async () => ({ id: "created" })); + let result: Payload | undefined; + + function ReturnProbe() { + const { mutate, status } = useApiMutation(mutationFn); + return ( +
+ {status} + +
+ ); + } + + render(); + + act(() => { + screen.getByTestId("mutate-btn").click(); + }); + + await waitFor(() => { + expect(screen.getByTestId("status")).toHaveTextContent("success"); + }); + + expect(result).toEqual({ id: "created" }); + }); + + it("rethrows the rejection from mutate after setting error state", async () => { + const mutationFn = jest.fn(() => Promise.reject(new Error("nope"))); + let caught: Error | undefined; + + function ThrowProbe() { + const { mutate, status, error } = useApiMutation(mutationFn); + return ( +
+ {status} + {error ?? ""} + +
+ ); + } + + render(); + + act(() => { + screen.getByTestId("mutate-btn").click(); + }); + + await waitFor(() => { + expect(screen.getByTestId("status")).toHaveTextContent("error"); + expect(caught?.message).toBe("nope"); + }); + }); + + it("falls back to a generic error message for Error with an empty message", async () => { + const mutationFn = jest.fn(() => Promise.reject(new Error(""))); + + render(); + + act(() => { + screen.getByTestId("mutate-btn").click(); + }); + + await waitFor(() => { + expect(screen.getByTestId("status")).toHaveTextContent("error"); + expect(screen.getByTestId("error")).toHaveTextContent("failed to mutate"); + }); + }); + + it("ignores a late rejection from a superseded mutation", async () => { + const first = createDeferred(); + const second = createDeferred(); + let call = 0; + + const mutationFn = jest.fn(() => { + call += 1; + return call === 1 ? first.promise : second.promise; + }); + + render(); + + act(() => { + screen.getByTestId("mutate-btn").click(); + }); + act(() => { + screen.getByTestId("mutate-btn").click(); + }); + + await act(async () => { + first.reject(new Error("stale failure")); + await first.promise.catch(() => {}); + }); + + expect(screen.getByTestId("status")).toHaveTextContent("pending"); + expect(screen.getByTestId("error")).toHaveTextContent(""); + + await act(async () => { + second.resolve({ id: "fresh" }); + await second.promise; + }); + + expect(screen.getByTestId("status")).toHaveTextContent("success"); + }); + + it("uses the latest mutationFn when mutate is called", async () => { + const firstFn = jest.fn(async () => ({ id: "first" })); + const secondFn = jest.fn(async () => ({ id: "second" })); + + function SwapProbe({ + fn, + }: { + fn: ( + variables: Vars, + options: { signal: AbortSignal }, + ) => Promise; + }) { + const { mutate, status } = useApiMutation(fn); + return ( +
+ {status} + +
+ ); + } + + const { rerender } = render(); + rerender(); + + act(() => { + screen.getByTestId("mutate-btn").click(); + }); + + await waitFor(() => { + expect(screen.getByTestId("status")).toHaveTextContent("success"); + }); + + expect(firstFn).not.toHaveBeenCalled(); + expect(secondFn).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/lib/useApi.ts b/src/lib/useApi.ts index 30f45a7..99d5d1d 100644 --- a/src/lib/useApi.ts +++ b/src/lib/useApi.ts @@ -12,14 +12,21 @@ export type ApiErrorState = { isTimeout: boolean; isRateLimited: boolean; retryAfterMs: number | null; + /** Error-state alias of `refetch` for existing callers. */ retry: () => void; }; -export type State = +type FetchState = | { status: "loading" } | ApiErrorState | { status: "ok"; data: T }; +/** + * Discriminated fetch status plus a stable `refetch` handle available in every + * status. Narrowing on `status` continues to work for existing consumers. + */ +export type State = FetchState & { refetch: () => void }; + /** * Fetch JSON from the AgentPay backend and react to path changes. * @@ -27,6 +34,9 @@ export type State = * stale paths are ignored after unmount or path changes, so consumers do not * need to add their own "is mounted" guard around this hook. * + * Call `refetch()` to re-run the current path request; any in-flight request is + * aborted first. + * * @example * const state = useApi<{ items: AppEvent[] }>("/api/v1/events?limit=100"); * if (state.status === "loading") return ; @@ -34,7 +44,7 @@ export type State = * return ( *
*

{state.error}

- * {state.isTimeout && } + * {state.isTimeout && } *
* ); * } @@ -42,17 +52,19 @@ export type State = */ export function useApi(path: string | null): State { const [state, dispatch] = useReducer( - (_state: State, action: State) => action, - { status: "loading" } as State + (_state: FetchState, action: FetchState) => action, + { status: "loading" } as FetchState ); const [reloadToken, bumpReloadToken] = useReducer((s: number) => s + 1, 0); - const retry = useCallback(() => { + const refetch = useCallback(() => { bumpReloadToken(); }, []); const stateRef = useRef(state); - stateRef.current = state; + useEffect(() => { + stateRef.current = state; + }, [state]); useEffect(() => { const handleOnline = () => { @@ -101,15 +113,14 @@ export function useApi(path: string | null): State { isTimeout, isRateLimited, retryAfterMs, - retry, + retry: refetch, }); }); return () => { cancelled = true; controller.abort(); }; - }, [path, reloadToken, retry]); + }, [path, reloadToken, refetch]); - return state; + return { ...state, refetch }; } - diff --git a/src/lib/useApiMutation.ts b/src/lib/useApiMutation.ts new file mode 100644 index 0000000..95c491f --- /dev/null +++ b/src/lib/useApiMutation.ts @@ -0,0 +1,116 @@ +"use client"; + +import { useCallback, useEffect, useRef, useState } from "react"; + +export type MutationStatus = "idle" | "pending" | "success" | "error"; + +export type UseApiMutationResult = { + mutate: (variables: TVariables) => Promise; + status: MutationStatus; + error: string | null; + reset: () => void; +}; + +/** + * Run a write mutation (POST / DELETE / PATCH) with pending, error, and success + * state — the mutation counterpart to `useApi`. + * + * Pass an async `mutationFn` that receives the call variables and an AbortSignal. + * In-flight requests are aborted when the component unmounts or when a newer + * `mutate` call supersedes them; late responses are ignored. + * + * @example + * const { mutate, status, error, reset } = useApiMutation( + * (body: { label: string }, { signal }) => + * apiPost<{ key: string }>("/api/v1/api-keys", body, { signal }), + * ); + * + * await mutate({ label: "ci" }); + */ +export function useApiMutation( + mutationFn: ( + variables: TVariables, + options: { signal: AbortSignal }, + ) => Promise, +): UseApiMutationResult { + const [status, setStatus] = useState("idle"); + const [error, setError] = useState(null); + + const mutationFnRef = useRef(mutationFn); + useEffect(() => { + mutationFnRef.current = mutationFn; + }, [mutationFn]); + + const controllerRef = useRef(null); + const generationRef = useRef(0); + const mountedRef = useRef(true); + + useEffect(() => { + mountedRef.current = true; + return () => { + mountedRef.current = false; + generationRef.current += 1; + controllerRef.current?.abort(); + controllerRef.current = null; + }; + }, []); + + const reset = useCallback(() => { + generationRef.current += 1; + controllerRef.current?.abort(); + controllerRef.current = null; + setStatus("idle"); + setError(null); + }, []); + + const mutate = useCallback(async (variables: TVariables): Promise => { + controllerRef.current?.abort(); + + const controller = new AbortController(); + controllerRef.current = controller; + const generation = ++generationRef.current; + + if (mountedRef.current) { + setStatus("pending"); + setError(null); + } + + try { + const data = await mutationFnRef.current(variables, { + signal: controller.signal, + }); + + if (generation !== generationRef.current || !mountedRef.current) { + return data; + } + + setStatus("success"); + setError(null); + return data; + } catch (err) { + const aborted = + controller.signal.aborted || + (err instanceof Error && err.name === "AbortError"); + + if (aborted || generation !== generationRef.current || !mountedRef.current) { + throw err; + } + + const message = + err instanceof Error && err.message + ? err.message + : "failed to mutate"; + const normalized = err instanceof Error ? err : new Error(message); + + setStatus("error"); + setError(message); + throw normalized; + } finally { + if (controllerRef.current === controller) { + controllerRef.current = null; + } + } + }, []); + + return { mutate, status, error, reset }; +}