From dc0371b6049bb29e53f81a1216484109ea2498d8 Mon Sep 17 00:00:00 2001 From: Sergey Krylov Date: Wed, 4 Nov 2026 10:19:23 +0300 Subject: [PATCH] refactor: drop dead query helpers and simplify read deadline (WealthLedger/frontend#141) --- src/shared/api/authenticated-query.test.tsx | 92 +++++++++------------ src/shared/api/authenticated-query.ts | 22 +---- src/shared/api/client.ts | 55 ++++++------ src/shared/api/resource-query.test.tsx | 73 ---------------- src/shared/api/resource-query.ts | 59 +------------ 5 files changed, 72 insertions(+), 229 deletions(-) diff --git a/src/shared/api/authenticated-query.test.tsx b/src/shared/api/authenticated-query.test.tsx index 4699a91..b72b1fa 100644 --- a/src/shared/api/authenticated-query.test.tsx +++ b/src/shared/api/authenticated-query.test.tsx @@ -6,10 +6,7 @@ import { createQueryWrapper } from "@/test/query"; import { resource } from "@/test/resource"; import { server } from "@/test/server"; -import { - useAuthenticatedQueries, - useAuthenticatedQuery, -} from "./authenticated-query"; +import { useAuthenticatedQuery } from "./authenticated-query"; import { apiQueryClient, configureAuthClient } from "./client"; const useSingleQuery = () => @@ -17,58 +14,49 @@ const useSingleQuery = () => apiQueryClient.queryOptions("get", "/api/v1/brokerage-accounts"), ); -const useMultipleQueries = () => - useAuthenticatedQueries([ - apiQueryClient.queryOptions("get", "/api/v1/brokerage-accounts"), - ])[0]; - describe("useAuthenticatedQuery", () => { - it.each(["single", "multiple"])( - "keeps %s protected queries idle without a session and loads after session configuration", - async (mode) => { - const requests = vi.fn(); - server.use( - http.get("*/api/v1/brokerage-accounts", () => { - requests(); + it("keeps protected queries idle without a session and loads after session configuration", async () => { + const requests = vi.fn(); + server.use( + http.get("*/api/v1/brokerage-accounts", () => { + requests(); - return HttpResponse.json( - resource([ - { - id: "account-1", - name: "Основной", - openedOn: "2024-08-13", - type: "BROKERAGE", - }, - ]), - ); - }), - ); + return HttpResponse.json( + resource([ + { + id: "account-1", + name: "Основной", + openedOn: "2024-08-13", + type: "BROKERAGE", + }, + ]), + ); + }), + ); - const { result } = renderHook( - mode === "single" ? useSingleQuery : useMultipleQueries, - { wrapper: createQueryWrapper().wrapper }, - ); + const { result } = renderHook(useSingleQuery, { + wrapper: createQueryWrapper().wrapper, + }); - await new Promise(resolve => setTimeout(resolve, 0)); - expect(requests).not.toHaveBeenCalled(); - expect(result.current?.fetchStatus).toBe("idle"); + await new Promise(resolve => setTimeout(resolve, 0)); + expect(requests).not.toHaveBeenCalled(); + expect(result.current?.fetchStatus).toBe("idle"); - configureAuthClient({ - accessToken: () => "token", - clear: vi.fn(), - refresh: () => Promise.resolve(true), - }); + configureAuthClient({ + accessToken: () => "token", + clear: vi.fn(), + refresh: () => Promise.resolve(true), + }); - await waitFor(() => expect(result.current?.isSuccess).toBe(true)); - expect(requests).toHaveBeenCalledOnce(); - expect(result.current?.data?.data).toEqual([ - { - id: "account-1", - name: "Основной", - openedOn: "2024-08-13", - type: "BROKERAGE", - }, - ]); - }, - ); + await waitFor(() => expect(result.current?.isSuccess).toBe(true)); + expect(requests).toHaveBeenCalledOnce(); + expect(result.current?.data?.data).toEqual([ + { + id: "account-1", + name: "Основной", + openedOn: "2024-08-13", + type: "BROKERAGE", + }, + ]); + }); }); diff --git a/src/shared/api/authenticated-query.ts b/src/shared/api/authenticated-query.ts index bbe0a09..06be1e8 100644 --- a/src/shared/api/authenticated-query.ts +++ b/src/shared/api/authenticated-query.ts @@ -1,6 +1,5 @@ import { type QueryKey, - useQueries, useQuery, type UseQueryOptions, type UseQueryResult, @@ -12,25 +11,6 @@ import { subscribeAuthenticatedSession, } from "./client"; -function useAuthenticatedQueries( - queries: UseQueryOptions[], -) { - const hasSession = useHasAuthenticatedSession(); - - return useQueries({ - queries: queries.map( - (options): UseQueryOptions => ({ - ...options, - enabled: query => - hasSession - && (typeof options.enabled === "function" - ? options.enabled(query) - : (options.enabled ?? true)), - }), - ), - }); -} - function useAuthenticatedQuery< TQueryFunctionData = unknown, TError = Error, @@ -58,4 +38,4 @@ function useHasAuthenticatedSession() { ); } -export { useAuthenticatedQueries, useAuthenticatedQuery }; +export { useAuthenticatedQuery }; diff --git a/src/shared/api/client.ts b/src/shared/api/client.ts index c95f2b3..e04a4a8 100644 --- a/src/shared/api/client.ts +++ b/src/shared/api/client.ts @@ -208,51 +208,50 @@ function parseJson(body: string): unknown { } async function readAttempt(request: Request): Promise { - const controller = new AbortController(); - const failure: { reason?: "abort" | "timeout" } = {}; - - const { promise: deadline, reject: rejectDeadline } - = Promise.withResolvers(); - - const timeout = setTimeout(() => { - failure.reason = "timeout"; - controller.abort(); - rejectDeadline(abortError()); - }, GET_DEADLINE_MS); - - const abort = () => { - failure.reason = "abort"; - controller.abort(); - rejectDeadline(abortError()); - }; - - request.signal.addEventListener("abort", abort, { once: true }); - const timedRequest = new Request(request, { signal: controller.signal }); + // Manual timeout (not AbortSignal.timeout) so fake-timer tests can advance it. + const timeout = new AbortController(); + const timer = setTimeout(() => timeout.abort(), GET_DEADLINE_MS); + const signal = AbortSignal.any([request.signal, timeout.signal]); try { - const response = await Promise.race([fetch(timedRequest), deadline]); - const body = await Promise.race([response.text(), deadline]); - const problem = parseJson(body); + // MSW keeps a stalled body readable after abort, so keep the race. + const response = await Promise.race([ + fetch(new Request(request, { signal })), + rejectOnAbort(signal), + ]); + + const body = await Promise.race([response.text(), rejectOnAbort(signal)]); return { - problem, + problem: parseJson(body), reason: "transient", response: new Response(body, response), }; } catch (error) { - if (failure.reason === "abort") return { reason: "abort" }; - if (failure.reason === "timeout" || isNetworkFailure(error)) + if (request.signal.aborted) return { reason: "abort" }; + // Timeout or network failure: retryable. + if (timeout.signal.aborted || isNetworkFailure(error)) return { reason: "transient" }; throw error; } finally { - clearTimeout(timeout); - request.signal.removeEventListener("abort", abort); + clearTimeout(timer); } } +function rejectOnAbort(signal: AbortSignal): Promise { + return new Promise((_resolve, reject) => { + // The signal is either already aborted or one of two parents firing once. + signal.addEventListener( + "abort", + () => reject(abortError()), + { once: true }, + ); + }); +} + function retryAfterMs(response: Response): number | undefined { const header = response.headers.get("Retry-After"); if (!header) return undefined; diff --git a/src/shared/api/resource-query.test.tsx b/src/shared/api/resource-query.test.tsx index a6ddf93..fa2743b 100644 --- a/src/shared/api/resource-query.test.tsx +++ b/src/shared/api/resource-query.test.tsx @@ -37,38 +37,6 @@ function auth() { } describe("persistent resource query lifecycle", () => { - it("observes once into the canonical cache without another import", async () => { - auth(); - const modes: (null | string)[] = []; - let observations = 0; - server.use( - http.get( - "*/api/v1/brokerage-accounts/:accountId/portfolio", - ({ request }) => { - const mode = new URL(request.url).searchParams.get("sync"); - modes.push(mode); - if (mode) observations += 1; - - return HttpResponse.json( - resource(snapshot, { - dataRevision: observations > 0 ? "new" : "old", - }), - ); - }, - ), - ); - const { queryClient, wrapper } = createQueryWrapper(); - const { result } = renderHook(() => usePrivateResource("a"), { wrapper }); - await waitFor(() => expect(result.current.isSuccess).toBe(true)); - await act(() => result.current.observe()); - await waitFor(() => expect(result.current.meta?.dataRevision).toBe("new")); - - expect(modes).toEqual([null, "observe"]); - expect(queryClient.getQueryCache().getAll()).toHaveLength(1); - expect( - queryClient.getQueryData(resourceOptions("a").queryKey), - ).toMatchObject({ data: snapshot, meta: { dataRevision: "new" } }); - }); it("keeps successful HTTP state distinct from missing or failed source data", async () => { auth(); server.use( @@ -136,47 +104,6 @@ describe("persistent resource query lifecycle", () => { expect(result.current.isSuccess).toBe(true); expect(result.current.isPending).toBe(false); }); - it("keeps the observed snapshot when an older query finishes later", async () => { - auth(); - - const { promise: blocked, resolve: release } - = Promise.withResolvers(); - - let isStarted = false; - server.use( - http.get( - "*/api/v1/brokerage-accounts/:accountId/portfolio", - async ({ request }) => { - if (!new URL(request.url).searchParams.has("sync")) { - isStarted = true; - await blocked; - - return HttpResponse.json(resource({ ...snapshot, isStale: true })); - } - - return HttpResponse.json(resource(snapshot)); - }, - ), - ); - - const { result } = renderHook(() => usePrivateResource("a"), { - wrapper: createQueryWrapper().wrapper, - }); - - await waitFor(() => expect(isStarted).toBe(true)); - // Capture the observation promise before releasing the old handler; the - // final assertion must follow its settlement, not the handler release. - let observation: Promise = Promise.resolve(); - await act(async () => { - observation = result.current.observe() ?? Promise.resolve(); - await observation; - }); - await act(async () => { - release(); - await observation; - }); - expect(result.current.data).toEqual(snapshot); - }); it("clears private snapshots on confirmed access loss and fences another pending success", async () => { auth(); let isDenied = false; diff --git a/src/shared/api/resource-query.ts b/src/shared/api/resource-query.ts index 46e77e7..d71833e 100644 --- a/src/shared/api/resource-query.ts +++ b/src/shared/api/resource-query.ts @@ -1,6 +1,5 @@ import { type QueryClient, - type QueryFunctionContext, useQueryClient, type UseQueryOptions, } from "@tanstack/react-query"; @@ -9,7 +8,6 @@ import { useEffect } from "react"; import type { components, paths } from "./generated/schema"; import { useAuthenticatedQuery } from "./authenticated-query"; -import { getAuthenticatedSessionSnapshot } from "./client"; import { apiErrorCode, isBrokerageAccessLoss } from "./problem-details"; type InvestmentResourceMeta = components["schemas"]["InvestmentResourceMetaDto"]; @@ -31,7 +29,7 @@ type ResourcePath = { type ResourceQueryKey = readonly [ "get", ResourcePath, - { params?: { path?: object; query?: { sync?: "observe" } } }?, + { params?: { path?: object; query?: object } }?, ]; function invalidateResourceAccess( @@ -57,22 +55,6 @@ function invalidateResourceAccess( }); } -// Keep endpoint and parameters intact. The generated queryFn accepts the -// optional sync mode even for a cache key originally created without init. -function observationKey(key: K): K { - const [method, path, init] = key as unknown as [ - string, - string, - { params?: { path?: object; query?: object } }?, - ]; - - return [method, path, { - ...init, - params: { ...init?.params, query: { ...init?.params?.query, sync: "observe" } }, - }] as unknown as K; -} - -// Observation changes transport mode, never the generated cache identity. function useResourceQuery< T extends Resource, E = unknown, @@ -92,50 +74,17 @@ function useResourceQuery< if (group) invalidateResourceAccess(client, group, query.error); }, [client, group, query.error]); - const observe = async () => { - const queryFunction = options.queryFn; - if ( - typeof queryFunction !== "function" - || !query.isEnabled - || !getAuthenticatedSessionSnapshot() - ) - return; - // Cancellation is synchronous; start the replacement in the same boundary. - void client.cancelQueries({ exact: true, queryKey: options.queryKey }); - - return client.query({ - ...options, - queryFn: async (context: QueryFunctionContext) => { - const queryKey = observationKey(context.queryKey); - - return queryFunction({ ...context, queryKey }); - }, - retry: false, - // Keep the cache envelope independent of observer projection. - select: undefined, - staleTime: 0, - }); - }; - const data: T["data"] | undefined = query.data?.data; const meta: T["meta"] | undefined = query.data?.meta; // Project resource fields; refetch still returns the transport envelope. + // queryKey is kept for screen-sync cache bookkeeping. return { + // eslint-disable-next-line @tanstack/query/no-rest-destructuring -- Single projection point; consumers already depend on the whole query state. + ...query, data: data ?? undefined, - error: query.error, - fetchStatus: query.fetchStatus, - isEnabled: query.isEnabled, - isError: query.isError, - isFetching: query.isFetching, - isLoading: query.isLoading, - isPending: query.isPending, - isSuccess: query.isSuccess, meta, - observe, queryKey: options.queryKey, - refetch: query.refetch, - status: query.status, sync: query.data && "sync" in query.data ? query.data.sync : undefined, }; } -- 2.54.0