From 23961d2280fd4f5b145c407d4ceb6ca4e891e9c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 4 Oct 2026 15:29:30 +0200 Subject: [PATCH] perf(medications): stop re-sending the read a nav prefetch just made The medications list and the compliance summary refetch on every mount, so a take or skip logged on another device shows on return (#316). The nav link's hover / touch / focus intent and the route-commit preload prefetch both reads too, so every visit through a link sent each of them twice: the prefetch, then the identical request from the mount a few hundred milliseconds after the first answer landed. The mount now skips its refetch only when the cached data answers a prefetch that went to the network for this visit: within the last 10 s, and since the route last left /medications. A direct load, a return with an older entry, or a quick return whose prefetch found the entry still fresh keep the refetch on mount. --- src/app/medications/page-client.tsx | 5 +- src/app/medications/page.tsx | 4 +- src/components/providers.tsx | 9 +- .../__tests__/prefetch-medications.test.ts | 129 +++++++++++++++++- src/lib/queries/prefetch-medications.ts | 92 +++++++++++-- .../use-medication-compliance-summary.ts | 8 +- 6 files changed, 229 insertions(+), 18 deletions(-) diff --git a/src/app/medications/page-client.tsx b/src/app/medications/page-client.tsx index 7d13852ec..b37d4ef51 100644 --- a/src/app/medications/page-client.tsx +++ b/src/app/medications/page-client.tsx @@ -55,6 +55,7 @@ import { Wrench, } from "lucide-react"; import { apiGet } from "@/lib/api/api-fetch"; +import { refetchMedicationsOnMount } from "@/lib/queries/prefetch-medications"; import { useMedicationComplianceSummaryAll } from "@/lib/queries/use-medication-compliance-summary"; import { useMedicationListLayout } from "@/lib/queries/use-medication-list-layout"; import { applyMedicationOrder } from "@/lib/medications/medication-order"; @@ -345,7 +346,9 @@ export default function MedicationsPageClient() { // client cache can't know to invalidate a cross-device write. "always" // makes a return-visit fetch fresh; the per-user server cache keeps it // cheap. Mutation-driven invalidation still covers same-tab writes. - refetchOnMount: "always", + // A prefetch this same navigation fired (nav-link intent, route commit) + // already is that fresh fetch, so it is not sent twice. + refetchOnMount: refetchMedicationsOnMount("list"), }); // v1.30.1 M12 — pull-to-refresh parity with labs/checkups/mood/measurements. diff --git a/src/app/medications/page.tsx b/src/app/medications/page.tsx index 201e4e239..a240df37f 100644 --- a/src/app/medications/page.tsx +++ b/src/app/medications/page.tsx @@ -39,7 +39,9 @@ import MedicationsPageClient from "./page-client"; * event, so the list must re-verify on every mount. With the prefetch in * place the hydrated cards paint immediately and that refetch runs in the * background — the "empty then fills" flash is gone WITHOUT dropping the - * freshness guarantee. + * freshness guarantee. The one exception is data a prefetch fired by this + * same navigation just fetched (`refetchMedicationsOnMount`): that already + * is the re-verification, so it is not sent twice. * - Record identity: the session comes from `getUnswitchedSession()`, which * answers null while the browser is acting on somebody else's record. * `/api/medications` IS delegable, so the client cell corrects itself on its diff --git a/src/components/providers.tsx b/src/components/providers.tsx index 2b626370c..e482e04d9 100644 --- a/src/components/providers.tsx +++ b/src/components/providers.tsx @@ -34,7 +34,10 @@ import { getRecordSessionTransition, subscribeToRecordSessionTransition, } from "@/lib/query-keys/record-session-transition"; -import { prefetchMedicationsList } from "@/lib/queries/prefetch-medications"; +import { + forgetMedicationsPrefetch, + prefetchMedicationsList, +} from "@/lib/queries/prefetch-medications"; import { getRecordScope, subscribeToRecordScope, @@ -245,6 +248,10 @@ function DashboardSnapshotPreloader() { // intent, so this is the fallback for direct loads + reloads. if (pathname === "/medications") { prefetchMedicationsList(queryClient, controller.signal); + } else { + // Off the page: a later prefetch starts a new visit, and an earlier one + // must not excuse the next mount's re-verification. + forgetMedicationsPrefetch(); } // A transition to an unresolved, refused, or different record scope must // abort a preloader before its response can be adopted by the next route. diff --git a/src/lib/queries/__tests__/prefetch-medications.test.ts b/src/lib/queries/__tests__/prefetch-medications.test.ts index bf718c69c..22ee0f750 100644 --- a/src/lib/queries/__tests__/prefetch-medications.test.ts +++ b/src/lib/queries/__tests__/prefetch-medications.test.ts @@ -1,12 +1,17 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { QueryClient } from "@tanstack/react-query"; +import { QueryClient, QueryObserver } from "@tanstack/react-query"; import { QUERY_CLIENT_DEFAULT_OPTIONS } from "@/lib/pwa/query-client-options"; import { queryKeys } from "@/lib/query-keys"; import { __resetRecordScopeForTests, setRecordScope, } from "@/lib/query-keys/record-scope"; -import { prefetchMedicationsList } from "../prefetch-medications"; +import { + MEDICATIONS_PREFETCH_COVERS_MOUNT_MS, + forgetMedicationsPrefetch, + prefetchMedicationsList, + refetchMedicationsOnMount, +} from "../prefetch-medications"; function envelopeResponse(data: unknown): Response { return new Response(JSON.stringify({ data, error: null }), { @@ -17,7 +22,9 @@ function envelopeResponse(data: unknown): Response { afterEach(() => { __resetRecordScopeForTests(); + forgetMedicationsPrefetch(); vi.unstubAllGlobals(); + vi.useRealTimers(); }); describe("prefetchMedicationsList", () => { @@ -70,3 +77,121 @@ describe("prefetchMedicationsList", () => { }); }); }); + +/** + * The page cells re-verify on every mount (#316), but a prefetch fired by the + * same navigation already is that fresh read. Before, the hover / route-commit + * prefetch landed and the mount sent the identical request again, on every + * visit, for both the list and the compliance summary. + */ +describe("refetchMedicationsOnMount", () => { + function setup() { + const fetchMock = vi.fn(async () => envelopeResponse([])); + vi.stubGlobal("fetch", fetchMock); + const client = new QueryClient({ + defaultOptions: QUERY_CLIENT_DEFAULT_OPTIONS, + }); + const calls = (path: string) => + fetchMock.mock.calls.filter((call) => (call as unknown[])[0] === path) + .length; + return { client, calls }; + } + + /** Mount a page observer the way the medications page does. */ + async function mountList(client: QueryClient) { + const mountFetch = vi.fn(async () => []); + const observer = new QueryObserver(client, { + queryKey: queryKeys.medications(), + queryFn: mountFetch, + refetchOnMount: refetchMedicationsOnMount("list"), + }); + const unsubscribe = observer.subscribe(() => {}); + await Promise.resolve(); + return { refetched: mountFetch.mock.calls.length > 0, unsubscribe }; + } + + it("does not repeat a prefetch this navigation just answered", async () => { + const { client, calls } = setup(); + prefetchMedicationsList(client); + await vi.waitFor(() => expect(calls("/api/medications")).toBe(1)); + await vi.waitFor(() => + expect(client.getQueryState(queryKeys.medications())?.status).toBe( + "success", + ), + ); + + const { refetched, unsubscribe } = await mountList(client); + expect(refetched).toBe(false); + unsubscribe(); + }); + + it("still refetches on mount when the cache holds an older read", async () => { + const { client } = setup(); + const queryFn = vi.fn(async () => []); + await client.fetchQuery({ queryKey: queryKeys.medications(), queryFn }); + + const { refetched, unsubscribe } = await mountList(client); + // No prefetch answered this visit: the #316 re-verification runs. + expect(refetched).toBe(true); + unsubscribe(); + }); + + it("refetches once the prefetch is older than the visit window", async () => { + vi.useFakeTimers({ toFake: ["Date"] }); + const { client, calls } = setup(); + prefetchMedicationsList(client); + await vi.waitFor(() => + expect(client.getQueryState(queryKeys.medications())?.status).toBe( + "success", + ), + ); + expect(calls("/api/medications")).toBe(1); + + vi.setSystemTime(Date.now() + MEDICATIONS_PREFETCH_COVERS_MOUNT_MS + 1); + const { refetched, unsubscribe } = await mountList(client); + expect(refetched).toBe(true); + unsubscribe(); + }); + + it("a prefetch skipped as fresh does not excuse the mount refetch", async () => { + // The return-visit case: the list was read moments ago by a mount, so + // the nav-link prefetch does not go to the network, and the remount + // must still re-verify. + const { client, calls } = setup(); + await client.fetchQuery({ + queryKey: queryKeys.medications(), + queryFn: async () => [], + }); + prefetchMedicationsList(client); + await Promise.resolve(); + expect(calls("/api/medications")).toBe(0); + + const { refetched, unsubscribe } = await mountList(client); + expect(refetched).toBe(true); + unsubscribe(); + }); + + it("a prefetch from an earlier visit does not excuse a later mount", async () => { + // Load the page (route-commit prefetch), leave, come straight back: the + // nav-link prefetch finds the entry fresh and stays off the network, so + // only the mount can re-verify, and it must. + const { client, calls } = setup(); + prefetchMedicationsList(client); + await vi.waitFor(() => + expect(client.getQueryState(queryKeys.medications())?.status).toBe( + "success", + ), + ); + const first = await mountList(client); + expect(first.refetched).toBe(false); + first.unsubscribe(); + + forgetMedicationsPrefetch(); // the route left /medications + prefetchMedicationsList(client); // hover on the way back: still fresh + expect(calls("/api/medications")).toBe(1); + + const second = await mountList(client); + expect(second.refetched).toBe(true); + second.unsubscribe(); + }); +}); diff --git a/src/lib/queries/prefetch-medications.ts b/src/lib/queries/prefetch-medications.ts index b9902f6e3..6058fc137 100644 --- a/src/lib/queries/prefetch-medications.ts +++ b/src/lib/queries/prefetch-medications.ts @@ -29,20 +29,92 @@ import { apiGet } from "@/lib/api/api-fetch"; */ export const MEDICATIONS_LIST_STALE_TIME_MS = 15_000; +/** + * How long after a prefetch STARTED its data still counts as this visit's + * fresh read. Wide enough to span hover → click → route commit → mount on a + * slow device; far below any gap in which another surface could have + * written something the user would expect to see. + */ +export const MEDICATIONS_PREFETCH_COVERS_MOUNT_MS = 10_000; + +type MedicationsRead = "list" | "compliance"; + +/** When the last prefetch that actually went to the network started. */ +const prefetchStartedAt = new Map(); + +function prefetchRead( + queryClient: QueryClient, + read: MedicationsRead, + queryKey: readonly unknown[], + path: string, + signal?: AbortSignal, +): void { + const state = queryClient.getQueryState(queryKey); + const willFetch = + state?.fetchStatus !== "fetching" && + (!state || + state.dataUpdatedAt < Date.now() - MEDICATIONS_LIST_STALE_TIME_MS); + if (willFetch) prefetchStartedAt.set(read, Date.now()); + void queryClient.prefetchQuery({ + queryKey, + queryFn: () => apiGet(path, { signal }), + staleTime: MEDICATIONS_LIST_STALE_TIME_MS, + }); +} + export function prefetchMedicationsList( queryClient: QueryClient, signal?: AbortSignal, ): void { - void queryClient.prefetchQuery({ - queryKey: queryKeys.medications(), - queryFn: () => apiGet("/api/medications", { signal }), - staleTime: MEDICATIONS_LIST_STALE_TIME_MS, - }); - void queryClient.prefetchQuery({ - queryKey: queryKeys.medicationComplianceSummary(), - queryFn: () => apiGet("/api/medications/compliance", { signal }), - staleTime: MEDICATIONS_LIST_STALE_TIME_MS, - }); + prefetchRead( + queryClient, + "list", + queryKeys.medications(), + "/api/medications", + signal, + ); + prefetchRead( + queryClient, + "compliance", + queryKeys.medicationComplianceSummary(), + "/api/medications/compliance", + signal, + ); +} + +/** + * `refetchOnMount` for the medications reads that must re-verify on every + * visit (#316: a take or skip on another device produces no event here). + * + * Still "always", except when the cached data is the answer to a prefetch + * that this same navigation fired moments ago: the hover / touch / focus + * intent on the nav link, or the route-commit preload. Leaving the page ends + * that navigation (`forgetMedicationsPrefetch`). Re-asking then sent + * the identical request a second time, a few hundred milliseconds after the + * first answer landed, on every visit to the page. A return to the page with + * an older cache entry, or a direct load, still refetches on mount. + */ +export function refetchMedicationsOnMount( + read: MedicationsRead, +): (query: { state: { dataUpdatedAt: number } }) => boolean | "always" { + return (query) => { + const startedAt = prefetchStartedAt.get(read); + const answeredThisVisit = + startedAt !== undefined && + query.state.dataUpdatedAt >= startedAt && + Date.now() - startedAt < MEDICATIONS_PREFETCH_COVERS_MOUNT_MS; + return answeredThisVisit ? false : "always"; + }; +} + +/** + * End the visit a recorded prefetch belongs to. Called whenever the route is + * anything but `/medications`, so a prefetch only ever excuses the mounts of + * the visit it was fired for: leave the page and come back within seconds, + * and the remount still re-verifies. + */ +export function forgetMedicationsPrefetch(): void { + prefetchStartedAt.clear(); } /** diff --git a/src/lib/queries/use-medication-compliance-summary.ts b/src/lib/queries/use-medication-compliance-summary.ts index a3aa4c03b..4a8b6d9e8 100644 --- a/src/lib/queries/use-medication-compliance-summary.ts +++ b/src/lib/queries/use-medication-compliance-summary.ts @@ -4,6 +4,7 @@ import { useQuery } from "@tanstack/react-query"; import { apiGet } from "@/lib/api/api-fetch"; import { queryKeys } from "@/lib/query-keys"; +import { refetchMedicationsOnMount } from "@/lib/queries/prefetch-medications"; import type { ComplianceDisplay } from "@/lib/analytics/compliance"; /** @@ -69,9 +70,10 @@ export function useMedicationComplianceSummary(medicationId: string): { // another surface (the iOS app) leaves this stale for up to the // window, so refetch on every mount: returning to the page reflects // cross-device intakes without a manual reload. The batched read is - // one request per visit, server-cached. + // one request per visit, server-cached. A prefetch fired by this same + // navigation already is that request (see `refetchMedicationsOnMount`). staleTime: 5 * 60 * 1000, - refetchOnMount: "always", + refetchOnMount: refetchMedicationsOnMount("compliance"), select: (rows: MedicationComplianceSummaryEntry[]) => rows.find((row) => row.medicationId === medicationId) ?? null, }); @@ -92,7 +94,7 @@ export function useMedicationComplianceSummaryAll(): { queryFn: fetchComplianceSummary, staleTime: 5 * 60 * 1000, // v1.16.12 (#316) — fresh on return-navigation (see the per-row hook). - refetchOnMount: "always", + refetchOnMount: refetchMedicationsOnMount("compliance"), }); return { data }; }