diff --git a/src/__tests__/query-client-server-gc-guard.test.ts b/src/__tests__/query-client-server-gc-guard.test.ts new file mode 100644 index 000000000..9080f92bd --- /dev/null +++ b/src/__tests__/query-client-server-gc-guard.test.ts @@ -0,0 +1,104 @@ +/** + * Every `QueryClient` a server render can build keeps no garbage-collection + * timer. + * + * A client component's `QueryClient` is built once per server render. With a + * finite `gcTime`, from its defaults or from a hook, each entry the render + * built started a timer of that length, and the timer kept the request's + * async context: the response, the Flight stream and the compressed body. + * Every page render stayed reachable for ten minutes after it was answered, + * and a scanner walking unknown paths filled the heap with not-found pages. + * `createAppQueryClient` (`src/lib/pwa/query-client-options.ts`) folds the + * library's server default (`Infinity`) back in; its behaviour is pinned by + * `src/lib/pwa/__tests__/query-client-server-retention.test.ts`. + * + * This guard keeps a client from being built around it: + * - a client module (`"use client"`) builds no `QueryClient` itself, it uses + * `createAppQueryClient`; + * - elsewhere, the only form allowed is a bare `new QueryClient()`, the + * server components' prefetch clients. Without options the library keeps + * `gcTime` at `Infinity` on the server, so a bare client schedules no timer + * unless a call site passes its own `gcTime`, which none of them do. + * + * Limit: the matcher is textual. A subclass of `QueryClient` declared outside + * the options module, or a client built through an alias, slips it. + */ +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; + +import { + readSource, + readSourceText, + scanSourceMatches, + stripComments, +} from "./helpers/source-files"; + +const SRC = join(process.cwd(), "src"); + +/** `new QueryClient(` with its argument list's first character. */ +const CONSTRUCTION = /\bnew\s+QueryClient\s*\(\s*(\S)?/g; + +const USE_CLIENT = /^\s*["']use client["']/; + +function isProductionSource(rel: string): boolean { + return ( + !rel.startsWith("generated/") && + !rel.includes("__tests__/") && + !rel.includes("__mocks__/") && + !/\.test\.tsx?$/.test(rel) + ); +} + +function constructions() { + return scanSourceMatches(SRC, CONSTRUCTION, { + fileFloor: 500, + // The dashboard, insights, medications, mood, coach and checkups pages + // each prefetch on a bare client. + matchFloor: 6, + include: isProductionSource, + stripComments: true, + }); +} + +describe("QueryClient server garbage-collection guard", () => { + it("builds no QueryClient inside a client module", () => { + const offenders = constructions() + .filter((match) => + USE_CLIENT.test( + stripComments(readSourceText(join(SRC, match.file))).trimStart(), + ), + ) + .map((match) => `src/${match.file}:${match.line}`); + + expect(offenders).toEqual([]); + }); + + it("passes no options to a QueryClient outside the app client", () => { + const offenders = constructions() + .filter((match) => !match.text.endsWith(")")) + .map((match) => `src/${match.file}:${match.line}`); + + expect(offenders).toEqual([]); + }); + + it("mounts the app client in Providers", () => { + const source = stripComments( + readSource("src/components/providers.tsx", "QueryClientProvider"), + ); + expect(source).toMatch(/useState\(\s*createAppQueryClient\s*\)/); + }); + + it("still matches the constructions it is meant to catch", () => { + const first = (code: string) => [...code.matchAll(CONSTRUCTION)][0]?.[0]; + expect(first("const c = new QueryClient();")).toBe("new QueryClient()"); + expect(first("new QueryClient({ defaultOptions })")).toBe( + "new QueryClient({", + ); + expect(first("new QueryClient(\n {\n defaultOptions })")).toBe( + "new QueryClient(\n {", + ); + expect(first("const c = createAppQueryClient();")).toBeUndefined(); + expect(USE_CLIENT.test('"use client";\n\nimport x from "y";')).toBe(true); + expect(USE_CLIENT.test('import x from "y";\n"use client";')).toBe(false); + }); +}); diff --git a/src/components/providers.tsx b/src/components/providers.tsx index 947ac9cbf..1c141b66e 100644 --- a/src/components/providers.tsx +++ b/src/components/providers.tsx @@ -1,10 +1,6 @@ "use client"; -import { - QueryClient, - QueryClientProvider, - useQueryClient, -} from "@tanstack/react-query"; +import { QueryClientProvider, useQueryClient } from "@tanstack/react-query"; import { usePathname } from "next/navigation"; import { createContext, @@ -49,7 +45,7 @@ import { startPersistingQueryCache, } from "@/lib/pwa/query-persister"; import { - QUERY_CLIENT_DEFAULT_OPTIONS, + createAppQueryClient, subscribeToMeaningfulVisibilityRefresh, } from "@/lib/pwa/query-client-options"; @@ -417,9 +413,9 @@ export function Providers({ children: ReactNode; initialLocale?: Locale; }) { - const [queryClient] = useState( - () => new QueryClient({ defaultOptions: QUERY_CLIENT_DEFAULT_OPTIONS }), - ); + // On the server this client lives for one render; `createAppQueryClient` + // keeps its entries from holding that render past the response. + const [queryClient] = useState(createAppQueryClient); return ( diff --git a/src/lib/pwa/__tests__/query-client-server-retention.test.ts b/src/lib/pwa/__tests__/query-client-server-retention.test.ts new file mode 100644 index 000000000..1a728b1b2 --- /dev/null +++ b/src/lib/pwa/__tests__/query-client-server-retention.test.ts @@ -0,0 +1,147 @@ +/** + * A server render's query cache must not outlive the request. + * + * `Providers` builds the app's `QueryClient` once per render. With a finite + * `gcTime` every entry that render built started a timer of that length, and + * a Node timer keeps the async context it was created in: on the server that + * is the request's, with the response, the Flight stream and the compressed + * body behind it. Production kept every page render, not-found pages for + * scanner probes included, reachable for ten minutes after it was answered. + * + * The scope test below reproduces that chain without a server: an object held + * only by an `AsyncLocalStorage` store stands in for the request, a query is + * built inside it, and after the client is dropped the object must be + * collectable. Its positive control runs the same steps on a plain client with + * the same defaults and must see the object survive, so the detector is shown + * to fire before its silence is trusted. + */ +import { AsyncLocalStorage } from "node:async_hooks"; +import { setFlagsFromString } from "node:v8"; +import { runInNewContext } from "node:vm"; + +import { + QueryClient, + QueryObserver, + environmentManager, +} from "@tanstack/react-query"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +import { + QUERY_CLIENT_DEFAULT_OPTIONS, + createAppQueryClient, +} from "../query-client-options"; + +setFlagsFromString("--expose-gc"); +const forceGc: () => void = runInNewContext("gc"); +setFlagsFromString("--no-expose-gc"); + +const TEN_MINUTES = 10 * 60 * 1000; +const FIVE_MINUTES = 5 * 60 * 1000; + +afterEach(() => { + vi.useRealTimers(); + environmentManager.setIsServer(() => typeof window === "undefined"); +}); + +/** + * What a render does: one observer per hook, some on the client's default + * `gcTime`, some with their own, disabled ones included. + */ +function renderQueries(client: QueryClient): void { + new QueryObserver(client, { queryKey: ["me"], queryFn: () => "me" }); + new QueryObserver(client, { + queryKey: ["chart"], + queryFn: () => "chart", + gcTime: FIVE_MINUTES, + }); + new QueryObserver(client, { + queryKey: ["disabled"], + queryFn: () => "disabled", + enabled: false, + gcTime: FIVE_MINUTES, + }); +} + +/** + * Renders inside a request scope, lets the client go, collects, and reports + * whether the request's response survived. + */ +async function requestSurvivesRender( + makeClient: () => QueryClient, +): Promise<{ survived: boolean; client: WeakRef }> { + const storage = new AsyncLocalStorage<{ response: object }>(); + const { request, client } = storage.run( + { response: { body: new Uint8Array(64 * 1024) } }, + () => { + const queryClient = makeClient(); + renderQueries(queryClient); + return { + request: new WeakRef(storage.getStore()!.response), + client: new WeakRef(queryClient), + }; + }, + ); + + for (let i = 0; i < 5; i++) { + await new Promise((resolve) => setImmediate(resolve)); + forceGc(); + } + return { survived: request.deref() !== undefined, client }; +} + +describe("AppQueryClient on the server", () => { + it("schedules no garbage-collection timer for any entry", () => { + vi.useFakeTimers(); + environmentManager.setIsServer(() => true); + + const client = createAppQueryClient(); + renderQueries(client); + + expect(client.getQueryCache().getAll()).toHaveLength(3); + for (const query of client.getQueryCache().getAll()) { + expect(query.gcTime).toBe(Infinity); + } + expect(vi.getTimerCount()).toBe(0); + }); + + it("leaves the request that rendered it collectable", async () => { + environmentManager.setIsServer(() => true); + + const { survived } = await requestSurvivesRender(createAppQueryClient); + + expect(survived).toBe(false); + }); + + it("positive control: a plain client with the same defaults keeps the request alive", async () => { + environmentManager.setIsServer(() => true); + + const { survived, client } = await requestSurvivesRender( + () => new QueryClient({ defaultOptions: QUERY_CLIENT_DEFAULT_OPTIONS }), + ); + + expect(survived).toBe(true); + // Release the pending timers so the worker is not left holding them. + client.deref()?.clear(); + }); +}); + +describe("AppQueryClient in the browser", () => { + it("keeps the configured gcTime, from the defaults and from the call site", () => { + vi.useFakeTimers(); + environmentManager.setIsServer(() => false); + + const client = createAppQueryClient(); + renderQueries(client); + + const byKey = new Map( + client + .getQueryCache() + .getAll() + .map((query) => [query.queryKey[0], query.gcTime]), + ); + expect(byKey.get("me")).toBe(TEN_MINUTES); + expect(byKey.get("chart")).toBe(FIVE_MINUTES); + expect(vi.getTimerCount()).toBeGreaterThan(0); + client.clear(); + }); +}); diff --git a/src/lib/pwa/query-client-options.ts b/src/lib/pwa/query-client-options.ts index 1d944428f..e82045220 100644 --- a/src/lib/pwa/query-client-options.ts +++ b/src/lib/pwa/query-client-options.ts @@ -1,4 +1,12 @@ -import type { DefaultOptions, QueryClient } from "@tanstack/react-query"; +import { + QueryClient, + environmentManager, + type DefaultError, + type DefaultOptions, + type DefaultedQueryObserverOptions, + type QueryKey, + type QueryObserverOptions, +} from "@tanstack/react-query"; import { recordScopedQueryKeyHashFn } from "@/lib/query-keys/record-scope"; @@ -79,3 +87,72 @@ export const QUERY_CLIENT_DEFAULT_OPTIONS: DefaultOptions = { networkMode: "always", }, }; + +/** + * The app's `QueryClient`. It differs from a plain one in a single rule: on + * the server, no cache entry schedules its own garbage collection. + * + * `Providers` creates this client once per render, so on the server it lives + * exactly as long as one request. A finite `gcTime` (the ten minutes above, or + * a call site's own) makes every entry the render builds start a timer of that + * length, `useQuery` with `enabled: false` included. A Node timer keeps the + * async context it was created in, and that context is the request's: its + * response, the React Flight stream, the compressed body. So each server render + * stayed reachable for ten minutes after it was answered, about 250 KB per + * request, and a scanner walking a few hundred unknown paths pushed the heap + * into the hundreds of megabytes on not-found pages alone. + * + * The library already defaults `gcTime` to `Infinity` on the server for this + * reason; an explicit value from the defaults or from a hook overrides it. + * Folding `Infinity` back in at `defaultQueryOptions`, the one merge point every + * observer, cache build and hydration goes through, restores that default + * whatever a caller passes. The server cache then goes with the client at the + * end of the request. In the browser nothing changes. + * + * Mutations need no counterpart: a mutation is built when `mutate` runs, which + * a server render never does. + */ +export class AppQueryClient extends QueryClient { + override defaultQueryOptions< + TQueryFnData = unknown, + TError = DefaultError, + TData = TQueryFnData, + TQueryData = TQueryFnData, + TQueryKey extends QueryKey = QueryKey, + TPageParam = never, + >( + options: + | QueryObserverOptions< + TQueryFnData, + TError, + TData, + TQueryData, + TQueryKey, + TPageParam + > + | DefaultedQueryObserverOptions< + TQueryFnData, + TError, + TData, + TQueryData, + TQueryKey + >, + ): DefaultedQueryObserverOptions< + TQueryFnData, + TError, + TData, + TQueryData, + TQueryKey + > { + const defaulted = super.defaultQueryOptions(options); + if (!environmentManager.isServer() || defaulted.gcTime === Infinity) { + return defaulted; + } + return { ...defaulted, gcTime: Infinity }; + } +} + +/** The client `Providers` mounts: the app's defaults on an `AppQueryClient`. */ +export function createAppQueryClient(): QueryClient { + return new AppQueryClient({ defaultOptions: QUERY_CLIENT_DEFAULT_OPTIONS }); +}