From 9633d66eea3662ddda2d0f0c7621dde93760cb91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 18 Jun 2026 17:48:57 +0200 Subject: [PATCH] fix(pwa): restore persisted query cache as gap-fill, never overwrite live data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit restorePersistedQueryCache hydrated the dehydrated snapshot unconditionally, so a restore that resolves in an effect AFTER the page has already fetched a fresh result overwrote that live query with the stale (often empty) on-disk copy — a read opened straight after a mutation then showed pre-mutation data. Filter the dehydrated state to keys the live cache does not already hold before hydrate, so restore only fills gaps; a query already present is by definition fresher than disk. Scope the post-restore invalidation to the keys actually hydrated so a live query is never disturbed. Adds an invariant test covering the clobber case. --- src/lib/pwa/__tests__/query-persister.test.ts | 82 +++++++++++++++++++ src/lib/pwa/query-persister.ts | 33 +++++++- 2 files changed, 114 insertions(+), 1 deletion(-) diff --git a/src/lib/pwa/__tests__/query-persister.test.ts b/src/lib/pwa/__tests__/query-persister.test.ts index 8f085b01d..161c6c14a 100644 --- a/src/lib/pwa/__tests__/query-persister.test.ts +++ b/src/lib/pwa/__tests__/query-persister.test.ts @@ -134,8 +134,90 @@ describe("persister round-trips through a fake IndexedDB", () => { weightKg: 81, }); + // Restored data must paint instantly BUT be marked stale so the first + // observer background-refetches — otherwise the client's 5-minute + // staleTime treats a minutes-old snapshot as fresh and a list opened + // right after a mutation serves the stale pre-mutation copy. + const restored = matchingBuild + .getQueryCache() + .find({ queryKey: ["dashboard", "snapshot"] }); + expect(restored?.isStale()).toBe(true); + const foreignBuild = new QueryClient(); await restorePersistedQueryCache(foreignBuild, "v9.9.9"); expect(foreignBuild.getQueryData(["dashboard", "snapshot"])).toBeUndefined(); }); + + it("never clobbers a query the live cache already holds", async () => { + // The read-after-write invariant: restore runs in an effect, possibly + // AFTER the page already fetched a fresh (post-mutation) list. A + // persisted STALE copy under the same key must not overwrite it — a + // present query is always fresher than disk. + const store = new Map(); + const makeReq = (run: () => void) => { + const req: Record = {}; + queueMicrotask(() => { + run(); + (req.onsuccess as (() => void) | undefined)?.(); + }); + return req; + }; + const fakeDb = { + close() {}, + transaction() { + return { + objectStore() { + return { + put(value: unknown, key: string) { + store.set(key, value); + return makeReq(() => {}); + }, + get(key: string) { + const req = makeReq(() => {}); + (req as { result?: unknown }).result = store.get(key); + return req; + }, + delete(key: string) { + store.delete(key); + return makeReq(() => {}); + }, + }; + }, + set oncomplete(fn: () => void) { + queueMicrotask(fn); + }, + set onerror(_fn: () => void) {}, + }; + }, + }; + (globalThis as { indexedDB?: unknown }).indexedDB = { + open() { + const req: Record = { result: fakeDb }; + queueMicrotask(() => (req.onsuccess as () => void)?.()); + return req; + }, + }; + + // Persist a STALE (empty) measurements list. + const source = new QueryClient(); + source.setQueryData(["measurements", "list"], []); + const stop = startPersistingQueryCache(source, "v1.18.6"); + vi.useFakeTimers(); + source.setQueryData(["measurements", "list"], []); + vi.advanceTimersByTime(1100); + vi.useRealTimers(); + await new Promise((r) => setTimeout(r, 0)); + stop(); + + // The live client already fetched the FRESH (post-mutation) row. + const live = new QueryClient(); + live.setQueryData(["measurements", "list"], [{ id: "fresh-row" }]); + + await restorePersistedQueryCache(live, "v1.18.6"); + + // The fresh row survived — the empty persisted snapshot did not win. + expect(live.getQueryData(["measurements", "list"])).toEqual([ + { id: "fresh-row" }, + ]); + }); }); diff --git a/src/lib/pwa/query-persister.ts b/src/lib/pwa/query-persister.ts index 3d7f5a578..cc85009ab 100644 --- a/src/lib/pwa/query-persister.ts +++ b/src/lib/pwa/query-persister.ts @@ -106,7 +106,38 @@ export async function restorePersistedQueryCache( await clearPersistedQueryCache(); return; } - hydrate(queryClient, payload.state); + + // Never clobber a query the live client already holds. `hydrate` calls + // `setData` per dehydrated query unconditionally, so a snapshot restored + // in an effect AFTER the first paint would overwrite a query the page + // already fetched/prefetched — replacing the fresh (post-mutation) list + // with the stale persisted (often empty) copy. That was the read-after- + // write break across dashboard/measurements/medications: the restore won + // the race against the in-flight fetch. Drop every dehydrated query whose + // key the cache already carries so restore only ever FILLS gaps, never + // overwrites; a query already present is by definition fresher than disk. + const cache = queryClient.getQueryCache(); + const filtered: DehydratedState = { + ...payload.state, + queries: payload.state.queries.filter( + (q) => cache.get(q.queryHash) === undefined, + ), + }; + if (filtered.queries.length === 0) return; + + hydrate(queryClient, filtered); + // Restored queries carry their original `dataUpdatedAt`, so with the + // client's 5-minute `staleTime` a snapshot saved minutes ago hydrates as + // *fresh* and the first observer never refetches — a list opened straight + // after a create/update then serves the stale pre-mutation copy. Marking + // every restored query invalidated makes it stale-but-shown: the persisted + // data still paints instantly, and the first mount triggers a background + // refetch. Online that lands fresh data within ms; offline the refetch + // fails and the hydrated copy stays on screen — the offline value holds. + // Scoped to the keys we just hydrated so we never disturb a live query. + for (const dq of filtered.queries) { + cache.get(dq.queryHash)?.invalidate(); + } } catch { // Persistence is best-effort; never block boot on a cache restore. }