From 26cce98d8d7c4806fac93a5b62102c074e6e232e Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Mon, 22 Jun 2026 02:35:22 +0300 Subject: [PATCH] refactor(offline-sync): share query keys/options between hooks and offline warm The 'Make available offline' warm path re-typed React Query key literals and re-declared queryKey+queryFn pairs that the feature hooks already owned, so the two could silently drift (a hook key change would leave the warm cache under a stale key). Centralize them so there is one source: - Add pageKeys (page-query.ts) and spaceKeys (space-query.ts) key factories and route the inline key literals through them. Partial-match keys and 2-element spaceMembers invalidations are deliberately left inline so their effective key VALUE (and invalidation breadth) is unchanged. - Add queryOptions factories sidebarPagesQueryOptions and spaceByIdQueryOptions, consumed by both the hooks (fetchAllAncestorChildren, useGetSpaceBySlugQuery) and the warm path. Comments reuse the existing RQ_KEY factory. The warm path also stops silently succeeding: warmInfiniteAll returns a boolean and logs failures; makePageAvailableOffline is best-effort (never throws) and returns { ok, failed[] }, recording each failed step by label; the tree menu caller now shows a success or error toast from result.ok. Removed the unused slugId/parentPageId params from the offline params type. This is a behavior-preserving centralization: effective query keys, queryFns, staleTime and enabled are unchanged for every hook. Co-Authored-By: Claude Opus 4.8 --- .../src/features/offline/make-offline.test.ts | 94 +++++++++++- .../src/features/offline/make-offline.ts | 137 +++++++++++------- .../src/features/page/queries/page-query.ts | 101 ++++++++----- .../tree/components/space-tree-node-menu.tsx | 23 ++- .../src/features/space/queries/space-query.ts | 64 +++++--- 5 files changed, 309 insertions(+), 110 deletions(-) diff --git a/apps/client/src/features/offline/make-offline.test.ts b/apps/client/src/features/offline/make-offline.test.ts index 28989413..f54019ba 100644 --- a/apps/client/src/features/offline/make-offline.test.ts +++ b/apps/client/src/features/offline/make-offline.test.ts @@ -46,18 +46,37 @@ vi.mock("@hocuspocus/provider", () => ({ }), })); -import { warmInfiniteAll, warmPageYdoc } from "./make-offline"; +import { + warmInfiniteAll, + warmPageYdoc, + makePageAvailableOffline, +} from "./make-offline"; import { queryClient } from "@/main.tsx"; +import { + getPageById, + getPageBreadcrumbs, + getSidebarPages, +} from "@/features/page/services/page-service"; +import { getPageComments } from "@/features/comment/services/comment-service"; const setQueryData = (queryClient as any).setQueryData as ReturnType< typeof vi.fn >; +const prefetchQuery = (queryClient as any).prefetchQuery as ReturnType< + typeof vi.fn +>; beforeEach(() => { // Clear call history WITHOUT wiping the mock implementations the vi.mock // factories installed (vi.clearAllMocks would drop the constructor return // objects and break the provider/idb/yjs spies). setQueryData.mockClear(); + prefetchQuery.mockReset(); + prefetchQuery.mockResolvedValue(undefined); + (getPageById as ReturnType).mockReset(); + (getPageBreadcrumbs as ReturnType).mockReset(); + (getSidebarPages as ReturnType).mockReset(); + (getPageComments as ReturnType).mockReset(); h.ydocDestroy.mockClear(); h.idbDestroy.mockClear(); h.providerOn.mockClear(); @@ -117,13 +136,80 @@ describe("warmInfiniteAll", () => { expect(payload.pages).toHaveLength(2); }); - it("swallows errors and never writes the cache on failure", async () => { - const fetchPage = vi.fn().mockRejectedValue(new Error("network")); + it("returns true on success", async () => { + const fetchPage = vi + .fn() + .mockResolvedValue({ items: [], meta: { nextCursor: null } }); await expect( warmInfiniteAll(["comments", "p1"], fetchPage), - ).resolves.toBeUndefined(); + ).resolves.toBe(true); + }); + + it("reports errors (returns false) and never writes the cache on failure", async () => { + const fetchPage = vi.fn().mockRejectedValue(new Error("network")); + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + + await expect( + warmInfiniteAll(["comments", "p1"], fetchPage), + ).resolves.toBe(false); expect(setQueryData).not.toHaveBeenCalled(); + expect(errorSpy).toHaveBeenCalled(); + + errorSpy.mockRestore(); + }); +}); + +describe("makePageAvailableOffline", () => { + const okPage = { + id: "uuid-1", + slugId: "slug-1", + space: { slug: "space-slug" }, + }; + + it("returns ok:true with no failures when every step succeeds", async () => { + (getPageById as ReturnType).mockResolvedValue(okPage); + (getPageBreadcrumbs as ReturnType).mockResolvedValue([]); + (getSidebarPages as ReturnType).mockResolvedValue({ + items: [], + meta: { nextCursor: null }, + }); + (getPageComments as ReturnType).mockResolvedValue({ + items: [], + meta: { nextCursor: null }, + }); + + const result = await makePageAvailableOffline({ + pageId: "uuid-1", + spaceId: "space-uuid", + }); + + expect(result).toEqual({ ok: true, failed: [] }); + }); + + it("returns ok:false with the failed step label when a warm step fails", async () => { + const errorSpy = vi.spyOn(console, "error").mockImplementation(() => {}); + (getPageById as ReturnType).mockResolvedValue(okPage); + (getPageBreadcrumbs as ReturnType).mockResolvedValue([]); + (getSidebarPages as ReturnType).mockResolvedValue({ + items: [], + meta: { nextCursor: null }, + }); + // Comments warm fails -> labeled "comments". + (getPageComments as ReturnType).mockRejectedValue( + new Error("network"), + ); + + const result = await makePageAvailableOffline({ + pageId: "uuid-1", + spaceId: "space-uuid", + }); + + expect(result.ok).toBe(false); + expect(result.failed).toContain("comments"); + expect(errorSpy).toHaveBeenCalled(); + + errorSpy.mockRestore(); }); }); diff --git a/apps/client/src/features/offline/make-offline.ts b/apps/client/src/features/offline/make-offline.ts index 86c77664..b4ccd458 100644 --- a/apps/client/src/features/offline/make-offline.ts +++ b/apps/client/src/features/offline/make-offline.ts @@ -7,9 +7,13 @@ import { getPageById, getPageBreadcrumbs, getSidebarPages, - getAllSidebarPages, } from "@/features/page/services/page-service"; -import { getSpaceById } from "@/features/space/services/space-service.ts"; +import { + pageKeys, + sidebarPagesQueryOptions, +} from "@/features/page/queries/page-query"; +import { spaceByIdQueryOptions } from "@/features/space/queries/space-query"; +import { RQ_KEY } from "@/features/comment/queries/comment-query"; import { getPageComments } from "@/features/comment/services/comment-service"; import { IPage } from "@/features/page/types/page.types"; import { IPagination } from "@/lib/types.ts"; @@ -23,15 +27,19 @@ import { IPagination } from "@/lib/types.ts"; * spinning forever offline, and silently truncates large lists. This walks the * cursor chain until it runs out (or hits maxPages) so the whole list is cached. * - * Best-effort: any failure is swallowed so a partial/failed warm never throws. + * Best-effort: a failure does not throw (a partial/failed warm is still useful), + * but it is reported — the error is logged with context and `false` is returned + * so the caller can record the failed step instead of silently succeeding. + * + * Returns true if the whole list was paginated and written, false on any error. * * Exported for unit testing of the cursor-walk / cache-write behavior. */ export async function warmInfiniteAll( - queryKey: unknown[], + queryKey: readonly unknown[], fetchPage: (cursor: string | undefined) => Promise>, maxPages = 50, -): Promise { +): Promise { try { const pages: IPagination[] = []; const pageParams: (string | undefined)[] = []; @@ -46,90 +54,112 @@ export async function warmInfiniteAll( } queryClient.setQueryData(queryKey, { pages, pageParams }); - } catch { - // best-effort + return true; + } catch (error) { + console.error("warmInfiniteAll failed", { queryKey, error }); + return false; } } export interface MakePageAvailableOfflineParams { pageId: string; - slugId?: string; spaceId?: string; - parentPageId?: string; +} + +/** + * Outcome of {@link makePageAvailableOffline}. `ok` is true only when every warm + * step succeeded; `failed` lists the labels of the steps that failed (a subset + * of: "page", "space", "tree", "breadcrumbs", "comments"). + */ +export interface MakePageAvailableOfflineResult { + ok: boolean; + failed: string[]; } /** * Best-effort prefetch of a page's read queries so they get persisted to * IndexedDB and become readable offline. * - * Each prefetch is isolated in try/catch — this function NEVER throws to its - * caller. Only meaningful while online (the underlying requests must succeed). + * Each step is isolated and this function does NOT throw — a partial warm is + * still useful. Instead of silently succeeding, every failed step is logged + * with a label and recorded in the returned result: `{ ok, failed }` where + * `ok` is true only if no step failed and `failed` lists the failed step + * labels. Only meaningful while online (the underlying requests must succeed). */ export async function makePageAvailableOffline({ pageId, spaceId, -}: MakePageAvailableOfflineParams): Promise { +}: MakePageAvailableOfflineParams): Promise { + const failed: string[] = []; + // Fetch the page document ONCE and write it under BOTH cache keys, exactly - // like usePageQuery's onData effect. Every page consumer reads ["pages", - // ] (usePageQuery keys on the slugId for routed reads), so warming - // only ["pages", ] would leave the offline page blank. + // like usePageQuery's onData effect. Every page consumer reads + // pageKeys.detail(slugId) (usePageQuery keys on the slugId for routed reads), + // so warming only the uuid key would leave the offline page blank. let page: IPage | undefined; try { page = await getPageById({ pageId }); - queryClient.setQueryData(["pages", page.slugId], page); - queryClient.setQueryData(["pages", page.id], page); - } catch { - // best-effort + queryClient.setQueryData(pageKeys.detail(page.slugId), page); + queryClient.setQueryData(pageKeys.detail(page.id), page); + } catch (error) { + console.error("makePageAvailableOffline: page step failed", { + pageId, + error, + }); + failed.push("page"); } // Warm the space — page.tsx renders nothing until the space query resolves - // (useGetSpaceBySlugQuery → ["space", ]). Awaited (not the - // fire-and-forget prefetchSpace) so the space is actually persisted before - // the caller fires its success toast. Matches the hook's key/fn exactly. + // (useGetSpaceBySlugQuery). Awaited (not the fire-and-forget prefetchSpace) so + // the space is actually persisted before the caller fires its toast. Shares + // spaceByIdQueryOptions so the key/fn cannot drift from the hook. try { const spaceSlug = page?.space?.slug; if (spaceSlug) { - await queryClient.prefetchQuery({ - queryKey: ["space", spaceSlug], - queryFn: () => getSpaceById(spaceSlug), - }); + await queryClient.prefetchQuery(spaceByIdQueryOptions(spaceSlug)); } - } catch { - // best-effort + } catch (error) { + console.error("makePageAvailableOffline: space step failed", { + pageId, + error, + }); + failed.push("space"); } // Warm the sidebar tree root so the WHOLE root level renders offline (matches - // useGetRootSidebarPagesQuery's ["root-sidebar-pages", spaceId] infinite - // key/fn). Fully paginated so large root levels are not truncated at 100. + // useGetRootSidebarPagesQuery's pageKeys.rootSidebar(spaceId) infinite cache). + // Fully paginated so large root levels are not truncated at 100. if (spaceId) { - await warmInfiniteAll(["root-sidebar-pages", spaceId], (cursor) => + const ok = await warmInfiniteAll(pageKeys.rootSidebar(spaceId), (cursor) => getSidebarPages({ spaceId, cursor, limit: 100 }), ); + if (!ok) failed.push("tree"); } // Warm the children of the page and of every ancestor so the path to this - // page is expandable offline. We MIRROR fetchAllAncestorChildren exactly — - // same regular ["sidebar-pages", { pageId, spaceId }] key, same - // getAllSidebarPages fn (which aggregates ALL children pages, so nothing is - // truncated at 100), same 30min staleTime — otherwise the warmed cache would - // never be read by the offline tree. - const warmSidebarChildren = async (id: string) => { + // page is expandable offline. We MIRROR fetchAllAncestorChildren exactly via + // sidebarPagesQueryOptions — same pageKeys.sidebar({ pageId, spaceId }) key, + // same getAllSidebarPages fn (which aggregates ALL children pages, so nothing + // is truncated at 100), same 30min staleTime — otherwise the warmed cache + // would never be read by the offline tree. + const warmSidebarChildren = async (id: string): Promise => { try { // Keep EXACTLY { pageId, spaceId } so the key hashes identically to // fetchAllAncestorChildren's (no parentPageId, no extra fields). const params = { pageId: id, spaceId }; - await queryClient.prefetchQuery({ - queryKey: ["sidebar-pages", params], - queryFn: () => getAllSidebarPages(params), - staleTime: 30 * 60 * 1000, + await queryClient.prefetchQuery(sidebarPagesQueryOptions(params)); + return true; + } catch (error) { + console.error("makePageAvailableOffline: tree node step failed", { + pageId: id, + error, }); - } catch { - // best-effort per node + return false; } }; // The page's own children. - await warmSidebarChildren(pageId); + if (!(await warmSidebarChildren(pageId))) failed.push("tree"); // Each ancestor's children. Use the breadcrumbs endpoint ONLY to discover the // ancestor ids — we intentionally do NOT cache the breadcrumbs themselves @@ -141,20 +171,29 @@ export async function makePageAvailableOffline({ for (const ancestor of ancestors ?? []) { const ancestorId = ancestor?.id; if (!ancestorId || ancestorId === pageId) continue; - await warmSidebarChildren(ancestorId); + if (!(await warmSidebarChildren(ancestorId))) failed.push("tree"); } - } catch { - // best-effort + } catch (error) { + console.error("makePageAvailableOffline: breadcrumbs step failed", { + pageId, + error, + }); + failed.push("breadcrumbs"); } - // Comments (matches useCommentsQuery's ["comments", pageId] infinite cache). + // Comments (matches useCommentsQuery's RQ_KEY(pageId) infinite cache). // useCommentsQuery reports isLoading while hasNextPage is true, so warming // only the first page leaves the offline comments panel spinning forever on // pages with >100 comments. Fully paginate so the last cached page has no // nextCursor and the panel settles offline. - await warmInfiniteAll(["comments", pageId], (cursor) => + const commentsOk = await warmInfiniteAll(RQ_KEY(pageId), (cursor) => getPageComments({ pageId, cursor, limit: 100 }), ); + if (!commentsOk) failed.push("comments"); + + // Dedupe — the tree label can be recorded once per failed node/ancestor. + const uniqueFailed = [...new Set(failed)]; + return { ok: uniqueFailed.length === 0, failed: uniqueFailed }; } /** diff --git a/apps/client/src/features/page/queries/page-query.ts b/apps/client/src/features/page/queries/page-query.ts index ee44b775..d224e235 100644 --- a/apps/client/src/features/page/queries/page-query.ts +++ b/apps/client/src/features/page/queries/page-query.ts @@ -1,6 +1,7 @@ import { InfiniteData, QueryKey, + queryOptions, useInfiniteQuery, UseInfiniteQueryResult, useMutation, @@ -43,11 +44,36 @@ import { SpaceTreeNode } from "@/features/page/tree/types"; import { useQueryEmit } from "@/features/websocket/use-query-emit"; import { moveToTrashNotificationMessage } from "@/features/page/components/move-to-trash-notification"; +/** + * Centralized React Query key factories for page queries. The hooks below and + * the offline warm path (features/offline/make-offline.ts) share these so the + * runtime keys can never silently drift apart. + */ +export const pageKeys = { + detail: (idOrSlug: string) => ["pages", idOrSlug] as const, + sidebar: (data: unknown) => ["sidebar-pages", data] as const, + rootSidebar: (spaceId: string) => ["root-sidebar-pages", spaceId] as const, + breadcrumbs: (pageId: string) => ["breadcrumbs", pageId] as const, + recentChanges: (spaceId?: string) => ["recent-changes", spaceId] as const, +}; + +/** + * Shared queryOptions for the sidebar-pages (ancestor children) query. Both + * fetchAllAncestorChildren and the offline warm path consume this so the key, + * queryFn and staleTime stay identical. + */ +export const sidebarPagesQueryOptions = (params: SidebarPagesParams) => + queryOptions({ + queryKey: pageKeys.sidebar(params), + queryFn: () => getAllSidebarPages(params), + staleTime: 30 * 60 * 1000, + }); + export function usePageQuery( pageInput: Partial, ): UseQueryResult { const query = useQuery({ - queryKey: ["pages", pageInput.pageId], + queryKey: pageKeys.detail(pageInput.pageId), queryFn: () => getPageById(pageInput), enabled: !!pageInput.pageId, staleTime: 5 * 60 * 1000, @@ -56,9 +82,9 @@ export function usePageQuery( useEffect(() => { if (query.data) { if (isValidUuid(pageInput.pageId)) { - queryClient.setQueryData(["pages", query.data.slugId], query.data); + queryClient.setQueryData(pageKeys.detail(query.data.slugId), query.data); } else { - queryClient.setQueryData(["pages", query.data.id], query.data); + queryClient.setQueryData(pageKeys.detail(query.data.id), query.data); } } }, [query.data]); @@ -80,18 +106,20 @@ export function useCreatePageMutation() { } export function updatePageData(data: IPage) { - const pageBySlug = queryClient.getQueryData(["pages", data.slugId]); - const pageById = queryClient.getQueryData(["pages", data.id]); + const pageBySlug = queryClient.getQueryData( + pageKeys.detail(data.slugId), + ); + const pageById = queryClient.getQueryData(pageKeys.detail(data.id)); if (pageBySlug) { - queryClient.setQueryData(["pages", data.slugId], { + queryClient.setQueryData(pageKeys.detail(data.slugId), { ...pageBySlug, ...data, }); } if (pageById) { - queryClient.setQueryData(["pages", data.id], { ...pageById, ...data }); + queryClient.setQueryData(pageKeys.detail(data.id), { ...pageById, ...data }); } invalidateOnUpdatePage( @@ -145,11 +173,11 @@ export function useRemovePageMutation() { }); // Stamp deletedAt so a re-visit shows the trash banner, not stale state. - const cached = queryClient.getQueryData(["pages", pageId]); + const cached = queryClient.getQueryData(pageKeys.detail(pageId)); if (cached) { const stamped = { ...cached, deletedAt: new Date() }; - queryClient.setQueryData(["pages", cached.id], stamped); - queryClient.setQueryData(["pages", cached.slugId], stamped); + queryClient.setQueryData(pageKeys.detail(cached.id), stamped); + queryClient.setQueryData(pageKeys.detail(cached.slugId), stamped); } invalidateOnDeletePage(pageId); @@ -270,8 +298,11 @@ export function useRestorePageMutation() { // Replace would strip space/permissions/content and break the editor. const merge = (cached: IPage | undefined) => cached ? { ...cached, ...restoredPage } : cached; - queryClient.setQueryData(["pages", restoredPage.id], merge); - queryClient.setQueryData(["pages", restoredPage.slugId], merge); + queryClient.setQueryData(pageKeys.detail(restoredPage.id), merge); + queryClient.setQueryData( + pageKeys.detail(restoredPage.slugId), + merge, + ); }, onError: (error) => { notifications.show({ @@ -286,7 +317,7 @@ export function useGetSidebarPagesQuery( data: SidebarPagesParams | null, ): UseInfiniteQueryResult, unknown>> { return useInfiniteQuery({ - queryKey: ["sidebar-pages", data], + queryKey: pageKeys.sidebar(data), enabled: !!data?.pageId || !!data?.spaceId, queryFn: ({ pageParam }) => getSidebarPages({ ...data, cursor: pageParam, limit: 100 }), @@ -297,7 +328,7 @@ export function useGetSidebarPagesQuery( export function useGetRootSidebarPagesQuery(data: SidebarPagesParams) { return useInfiniteQuery({ - queryKey: ["root-sidebar-pages", data.spaceId], + queryKey: pageKeys.rootSidebar(data.spaceId), queryFn: async ({ pageParam }) => { return getSidebarPages({ spaceId: data.spaceId, @@ -323,7 +354,7 @@ export function usePageBreadcrumbsQuery( pageId: string, ): UseQueryResult, Error> { return useQuery({ - queryKey: ["breadcrumbs", pageId], + queryKey: pageKeys.breadcrumbs(pageId), queryFn: () => getPageBreadcrumbs(pageId), enabled: !!pageId, }); @@ -335,10 +366,12 @@ export async function fetchAllAncestorChildren( // refresh (#159 #8), which must NOT receive the 30-min-cached children. opts?: { fresh?: boolean }, ) { - // not using a hook here, so we can call it inside a useEffect hook + // not using a hook here, so we can call it inside a useEffect hook. Reuse the + // shared sidebarPagesQueryOptions (key + queryFn) so the offline warm path and + // this fetch never drift, but override staleTime for the `fresh` reconnect + // refresh (#159 #8), which must force a server refetch (staleTime 0). const response = await queryClient.fetchQuery({ - queryKey: ["sidebar-pages", params], - queryFn: () => getAllSidebarPages(params), + ...sidebarPagesQueryOptions(params), staleTime: opts?.fresh ? 0 : 30 * 60 * 1000, }); @@ -348,7 +381,7 @@ export async function fetchAllAncestorChildren( export function useRecentChangesQuery(spaceId?: string) { return useInfiniteQuery({ - queryKey: ["recent-changes", spaceId], + queryKey: pageKeys.recentChanges(spaceId), queryFn: ({ pageParam }) => getRecentChanges({ spaceId, cursor: pageParam, limit: 15 }), initialPageParam: undefined as string | undefined, @@ -414,12 +447,12 @@ export function invalidateOnCreatePage(data: Partial) { let queryKey: QueryKey = null; if (data.parentPageId === null) { - queryKey = ["root-sidebar-pages", data.spaceId]; + queryKey = pageKeys.rootSidebar(data.spaceId); } else { - queryKey = [ - "sidebar-pages", - { pageId: data.parentPageId, spaceId: data.spaceId }, - ]; + queryKey = pageKeys.sidebar({ + pageId: data.parentPageId, + spaceId: data.spaceId, + }); } //update all sidebar pages @@ -479,7 +512,7 @@ export function invalidateOnCreatePage(data: Partial) { //update root sidebar pages haschildern const rootSideBarMatches = queryClient.getQueriesData({ - queryKey: ["root-sidebar-pages", data.spaceId], + queryKey: pageKeys.rootSidebar(data.spaceId), exact: false, }); @@ -503,7 +536,7 @@ export function invalidateOnCreatePage(data: Partial) { //update recent changes queryClient.invalidateQueries({ - queryKey: ["recent-changes", data.spaceId], + queryKey: pageKeys.recentChanges(data.spaceId), }); } @@ -517,9 +550,9 @@ export function invalidateOnUpdatePage( invalidatePageTree(); let queryKey: QueryKey = null; if (parentPageId === null) { - queryKey = ["root-sidebar-pages", spaceId]; + queryKey = pageKeys.rootSidebar(spaceId); } else { - queryKey = ["sidebar-pages", { pageId: parentPageId, spaceId: spaceId }]; + queryKey = pageKeys.sidebar({ pageId: parentPageId, spaceId: spaceId }); } //update all sidebar pages queryClient.setQueryData>>( @@ -542,7 +575,7 @@ export function invalidateOnUpdatePage( //update recent changes queryClient.invalidateQueries({ - queryKey: ["recent-changes", spaceId], + queryKey: pageKeys.recentChanges(spaceId), }); } @@ -557,8 +590,8 @@ export function updateCacheOnMovePage( // Remove page from old parent's cache const oldQueryKey = oldParentId === null - ? ["root-sidebar-pages", spaceId] - : ["sidebar-pages", { pageId: oldParentId, spaceId }]; + ? pageKeys.rootSidebar(spaceId) + : pageKeys.sidebar({ pageId: oldParentId, spaceId }); queryClient.setQueryData>>( oldQueryKey, @@ -578,7 +611,7 @@ export function updateCacheOnMovePage( if (oldParentId !== null) { const oldParentCache = queryClient.getQueryData< InfiniteData> - >(["sidebar-pages", { pageId: oldParentId, spaceId }]); + >(pageKeys.sidebar({ pageId: oldParentId, spaceId })); const remainingChildren = oldParentCache?.pages.flatMap((p) => p.items).length ?? 0; @@ -616,8 +649,8 @@ export function updateCacheOnMovePage( // Add page to new parent's cache const newQueryKey = newParentId === null - ? ["root-sidebar-pages", spaceId] - : ["sidebar-pages", { pageId: newParentId, spaceId }]; + ? pageKeys.rootSidebar(spaceId) + : pageKeys.sidebar({ pageId: newParentId, spaceId }); queryClient.setQueryData>>>( newQueryKey, diff --git a/apps/client/src/features/page/tree/components/space-tree-node-menu.tsx b/apps/client/src/features/page/tree/components/space-tree-node-menu.tsx index d051a963..d74a2ad9 100644 --- a/apps/client/src/features/page/tree/components/space-tree-node-menu.tsx +++ b/apps/client/src/features/page/tree/components/space-tree-node-menu.tsx @@ -83,18 +83,29 @@ export function NodeMenu({ node, canEdit }: NodeMenuProps) { const handleMakeAvailableOffline = async () => { notifications.show({ message: t("Saving page for offline use...") }); try { - // Prefetch read queries so they get persisted to IndexedDB. - await makePageAvailableOffline({ + // Prefetch read queries so they get persisted to IndexedDB. The result + // reports whether every warm step succeeded. + const result = await makePageAvailableOffline({ pageId: node.id, - slugId: node.slugId, spaceId: node.spaceId, - parentPageId: node.parentPageId, }); // Best-effort: warm the page's Yjs document into IndexedDB. await warmPageYdoc(node.id, getCollaborationUrl(), collabQuery?.token); - notifications.show({ message: t("Page is now available offline") }); + + if (result.ok) { + notifications.show({ message: t("Page is now available offline") }); + } else { + // Partial warm — the page may still be partly usable offline, but some + // queries failed to cache, so surface it as an error rather than a + // silent success. + notifications.show({ + message: t("Failed to make page available offline"), + color: "red", + }); + } } catch { - // makePageAvailableOffline / warmPageYdoc never throw, but stay safe. + // makePageAvailableOffline no longer throws, but warmPageYdoc and other + // unexpected failures stay guarded here. notifications.show({ message: t("Failed to make page available offline"), color: "red", diff --git a/apps/client/src/features/space/queries/space-query.ts b/apps/client/src/features/space/queries/space-query.ts index dd259326..8752787b 100644 --- a/apps/client/src/features/space/queries/space-query.ts +++ b/apps/client/src/features/space/queries/space-query.ts @@ -1,5 +1,6 @@ import { keepPreviousData, + queryOptions, useInfiniteQuery, useMutation, useQuery, @@ -31,11 +32,37 @@ import { getRecentChanges } from "@/features/page/services/page-service.ts"; import { useEffect } from "react"; import { validate as isValidUuid } from "uuid"; +/** + * Centralized React Query key factories for space queries. The hooks below and + * the offline warm path (features/offline/make-offline.ts) share these so the + * runtime keys can never silently drift apart. + */ +export const spaceKeys = { + detail: (idOrSlug: string) => ["space", idOrSlug] as const, + list: (params?: QueryParams) => ["spaces", params] as const, + members: (spaceId: string, query?: string) => + ["spaceMembers", spaceId, query] as const, +}; + +/** + * Shared queryOptions for fetching a space by id/slug. Both + * useGetSpaceBySlugQuery and the offline warm path consume this so the key, + * queryFn and staleTime stay identical. (`enabled` is intentionally omitted — + * prefetchQuery ignores it anyway and the warm path always passes a real id; + * the hook reapplies `enabled` itself.) + */ +export const spaceByIdQueryOptions = (spaceId: string) => + queryOptions({ + queryKey: spaceKeys.detail(spaceId), + queryFn: () => getSpaceById(spaceId), + staleTime: 5 * 60 * 1000, + }); + export function useGetSpacesQuery( params?: QueryParams, ): UseQueryResult, Error> { return useQuery({ - queryKey: ["spaces", params], + queryKey: spaceKeys.list(params), queryFn: () => getSpaces(params), placeholderData: keepPreviousData, refetchOnMount: true, @@ -44,16 +71,16 @@ export function useGetSpacesQuery( export function useSpaceQuery(spaceId: string): UseQueryResult { const query = useQuery({ - queryKey: ["space", spaceId], + queryKey: spaceKeys.detail(spaceId), queryFn: () => getSpaceById(spaceId), enabled: !!spaceId, }); useEffect(() => { if (query.data) { if (isValidUuid(spaceId)) { - queryClient.setQueryData(["space", query.data.slug], query.data); + queryClient.setQueryData(spaceKeys.detail(query.data.slug), query.data); } else { - queryClient.setQueryData(["space", query.data.id], query.data); + queryClient.setQueryData(spaceKeys.detail(query.data.id), query.data); } } }, [query.data]); @@ -62,8 +89,11 @@ export function useSpaceQuery(spaceId: string): UseQueryResult { } export const prefetchSpace = (spaceSlug: string, spaceId?: string) => { + // Note: intentionally NOT using spaceByIdQueryOptions here — that factory sets + // a 5min staleTime which would let this prefetch skip fetching fresh data; + // prefetchSpace must always refetch (default staleTime: 0). queryClient.prefetchQuery({ - queryKey: ["space", spaceSlug], + queryKey: spaceKeys.detail(spaceSlug), queryFn: () => getSpaceById(spaceSlug), }); @@ -100,10 +130,8 @@ export function useGetSpaceBySlugQuery( spaceId: string, ): UseQueryResult { return useQuery({ - queryKey: ["space", spaceId], - queryFn: () => getSpaceById(spaceId), + ...spaceByIdQueryOptions(spaceId), enabled: !!spaceId, - staleTime: 5 * 60 * 1000, }); } @@ -116,14 +144,16 @@ export function useUpdateSpaceMutation() { onSuccess: (data, variables) => { notifications.show({ message: t("Space updated successfully") }); - const space = queryClient.getQueryData([ - "space", - variables.spaceId, - ]) as ISpace; + const space = queryClient.getQueryData( + spaceKeys.detail(variables.spaceId), + ) as ISpace; if (space) { const updatedSpace = { ...space, ...data }; - queryClient.setQueryData(["space", variables.spaceId], updatedSpace); - queryClient.setQueryData(["space", data.slug], updatedSpace); + queryClient.setQueryData( + spaceKeys.detail(variables.spaceId), + updatedSpace, + ); + queryClient.setQueryData(spaceKeys.detail(data.slug), updatedSpace); } queryClient.invalidateQueries({ @@ -148,7 +178,7 @@ export function useDeleteSpaceMutation() { if (variables.slug) { queryClient.removeQueries({ - queryKey: ["space", variables.slug], + queryKey: spaceKeys.detail(variables.slug), exact: true, }); } @@ -156,7 +186,7 @@ export function useDeleteSpaceMutation() { // Remove space-specific queries if (variables.id) { queryClient.removeQueries({ - queryKey: ["space", variables.id], + queryKey: spaceKeys.detail(variables.id), exact: true, }); @@ -196,7 +226,7 @@ export function useSpaceMembersInfiniteQuery( query?: string, ) { return useInfiniteQuery({ - queryKey: ["spaceMembers", spaceId, query], + queryKey: spaceKeys.members(spaceId, query), queryFn: ({ pageParam }) => getSpaceMembers(spaceId, { cursor: pageParam, limit: 50, query }), enabled: !!spaceId,