mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 13:01:00 +00:00
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.
This commit is contained in:
1 parent
2c1369f01b
commit
23961d2280
6 files changed
+229
-18
No files matched your search
@@ -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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -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<MedicationsRead, number>();
|
||||
|
||||
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();
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -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 };
|
||||
}
|
||||
Reference in new issue
Block a user