diff --git a/apps/client/src/features/editor/full-editor.tsx b/apps/client/src/features/editor/full-editor.tsx index 5836ea08..116c01f0 100644 --- a/apps/client/src/features/editor/full-editor.tsx +++ b/apps/client/src/features/editor/full-editor.tsx @@ -82,14 +82,18 @@ export function FullEditor({ // AI title generation is gated by the general AI chat flag (the same toggle // that enables the chat agent); the server enforces it too (#199). const isTitleGenEnabled = workspace?.settings?.ai?.chat === true; - const fullPageWidth = user.settings?.preferences?.fullPageWidth; + // `user` can momentarily be null during logout teardown (the currentUser atom + // is reset before this subtree unmounts). Optional-chain every access so the + // teardown render does not throw "Cannot read properties of null (reading + // 'settings')". + const fullPageWidth = user?.settings?.preferences?.fullPageWidth; const editorToolbarEnabled = - user.settings?.preferences?.editorToolbar ?? false; + user?.settings?.preferences?.editorToolbar ?? false; const [currentPageEditMode, setCurrentPageEditMode] = useAtom( currentPageEditModeAtom, ); const userPageEditMode = - user.settings?.preferences?.pageEditMode ?? PageEditMode.Edit; + user?.settings?.preferences?.pageEditMode ?? PageEditMode.Edit; const isEditMode = currentPageEditMode === PageEditMode.Edit; // Single shared Y.Doc + HocuspocusProvider for both the title and body diff --git a/apps/client/src/features/home/components/new-note-button.tsx b/apps/client/src/features/home/components/new-note-button.tsx index 43649bc6..c87fead1 100644 --- a/apps/client/src/features/home/components/new-note-button.tsx +++ b/apps/client/src/features/home/components/new-note-button.tsx @@ -3,6 +3,8 @@ import { IconHourglass, IconPlus } from "@tabler/icons-react"; import { ReactNode } from "react"; import { useNavigate } from "react-router-dom"; import { useTranslation } from "react-i18next"; +import { onlineManager } from "@tanstack/react-query"; +import { notifications } from "@mantine/notifications"; import { useGetSpacesQuery } from "@/features/space/queries/space-query.ts"; import { useCreatePageMutation } from "@/features/page/queries/page-query.ts"; import { buildPageUrl } from "@/features/page/page.utils.ts"; @@ -36,21 +38,39 @@ function CreateNoteButton({ const createPageMutation = useCreatePageMutation(); const createNote = async (space: ISpace) => { + // `spaceId`/`temporary` are accepted by the create-page endpoint but are + // not part of the shared `IPageInput` type; cast to satisfy the mutation + // signature. + const variables = { + spaceId: space.id, + ...(temporary ? { temporary: true } : {}), + } as any; + + if (!onlineManager.isOnline()) { + // Offline: the create is PAUSED and queued — its promise will not resolve + // until we are back online, so awaiting it here would spin the button + // forever. Fire it without awaiting (it persists and replays on reconnect) + // and tell the user it was saved offline instead of leaving a dead spinner. + createPageMutation.mutate(variables); + notifications.show({ + color: "blue", + message: t("You're offline. This note will be created once you reconnect."), + }); + return; + } + try { - // `spaceId`/`temporary` are accepted by the create-page endpoint but are - // not part of the shared `IPageInput` type; cast to satisfy the mutation - // signature. - const createdPage = await createPageMutation.mutateAsync({ - spaceId: space.id, - ...(temporary ? { temporary: true } : {}), - } as any); + const createdPage = await createPageMutation.mutateAsync(variables); navigate(buildPageUrl(space.slug, createdPage.slugId, createdPage.title)); } catch { // useCreatePageMutation already surfaces a red notification on error. } }; - const isPending = createPageMutation.isPending; + // A paused (offline) mutation stays `isPending`, so gate the spinner on it NOT + // being paused — otherwise the button would spin forever after an offline + // create. The offline path above gives its own "saved offline" feedback. + const isPending = createPageMutation.isPending && !createPageMutation.isPaused; // Exactly one writable space → create directly, no picker needed. if (writableSpaces.length === 1) { diff --git a/apps/client/src/features/offline/clear-offline-cache.ts b/apps/client/src/features/offline/clear-offline-cache.ts index 25b0bcf2..dd9dd543 100644 --- a/apps/client/src/features/offline/clear-offline-cache.ts +++ b/apps/client/src/features/offline/clear-offline-cache.ts @@ -1,7 +1,11 @@ import { del } from "idb-keyval"; import { queryClient } from "@/main.tsx"; -import { OFFLINE_CACHE_KEY } from "./query-persister"; +import { + OFFLINE_CACHE_KEY, + freezeOfflinePersistence, + unfreezeOfflinePersistence, +} from "./query-persister"; import { PAGE_YDOC_NAME_PREFIX } from "@/features/editor/page-ydoc-name"; /** @@ -31,19 +35,27 @@ import { PAGE_YDOC_NAME_PREFIX } from "@/features/editor/page-ydoc-name"; * service-worker-capable browsers). */ export async function clearOfflineCache(): Promise { - // 1a. Drop the in-memory query cache immediately. - try { - queryClient.clear(); - } catch { - // best-effort: ignore in-memory cache reset failures - } + // Freeze the throttled persister BEFORE touching the cache so the + // queryClient.clear() below cannot trigger a late re-write of the (still + // nearly-full) dehydrated snapshot after we del() the key — which would + // otherwise resurrect the previous user's persisted data in IndexedDB. + // Re-enabled in `finally` so the next (sign-in) session persists normally. + freezeOfflinePersistence(); - // 1b. Delete the persisted RQ cache from IndexedDB. try { - await del(OFFLINE_CACHE_KEY); - } catch { - // best-effort: ignore persisted-cache deletion failures - } + // 1a. Drop the in-memory query cache immediately. + try { + queryClient.clear(); + } catch { + // best-effort: ignore in-memory cache reset failures + } + + // 1b. Delete the persisted RQ cache from IndexedDB. + try { + await del(OFFLINE_CACHE_KEY); + } catch { + // best-effort: ignore persisted-cache deletion failures + } // 2. Delete the Yjs page IndexedDB databases (`page.`). // `indexedDB.databases()` is not implemented everywhere (e.g. Firefox); when @@ -91,4 +103,10 @@ export async function clearOfflineCache(): Promise { } catch { // best-effort: ignore Cache Storage failures } + } finally { + // Re-enable persistence for the next session (sign-in continues running in + // the same tab; logout reloads via window.location.replace, so this is a + // harmless no-op there). + unfreezeOfflinePersistence(); + } } diff --git a/apps/client/src/features/offline/offline-resume.test.ts b/apps/client/src/features/offline/offline-resume.test.ts new file mode 100644 index 00000000..ea95dbe2 --- /dev/null +++ b/apps/client/src/features/offline/offline-resume.test.ts @@ -0,0 +1,128 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { QueryClient, onlineManager } from "@tanstack/react-query"; +import { + persistQueryClientRestore, + persistQueryClientSave, +} from "@tanstack/react-query-persist-client"; + +// Stub the network services so a replayed mutation hits a spy, not the network. +const h = vi.hoisted(() => ({ + createPage: vi.fn(), + movePage: vi.fn(), + createComment: vi.fn(), +})); + +vi.mock("@/features/page/services/page-service", () => ({ + createPage: h.createPage, + movePage: h.movePage, +})); +vi.mock("@/features/comment/services/comment-service", () => ({ + createComment: h.createComment, +})); +vi.mock("@/features/page/queries/page-query", () => ({ + invalidateOnCreatePage: vi.fn(), +})); + +// In-memory idb-keyval so the REAL queryPersister round-trips through a fake +// store (the actual persist -> reload -> restore path, not a hand-built blob). +const store = new Map(); +vi.mock("idb-keyval", () => ({ + get: vi.fn((k: string) => Promise.resolve(store.get(k) ?? undefined)), + set: vi.fn((k: string, v: string) => { + store.set(k, v); + return Promise.resolve(); + }), + del: vi.fn((k: string) => { + store.delete(k); + return Promise.resolve(); + }), +})); + +import { queryPersister } from "./query-persister"; +import { + offlineMutationKeys, + registerOfflineMutationDefaults, +} from "./offline-mutations"; + +const BUSTER = "test-buster"; + +beforeEach(() => { + store.clear(); + h.createPage.mockReset().mockResolvedValue({ id: "new-page" }); + h.movePage.mockReset().mockResolvedValue(undefined); + h.createComment.mockReset().mockResolvedValue({ id: "new-comment" }); +}); + +afterEach(() => { + // onlineManager is a global singleton; leave it in the default online state. + onlineManager.setOnline(true); +}); + +describe("offline paused-mutation resume across a reload", () => { + // This is the #120 silent-data-loss reproduction: a paused mutation persisted + // to IndexedDB while offline, then the tab RELOADS while still offline, must + // resume on reconnect. It exercises the real persister round-trip plus the two + // boot-time fixes the app wiring relies on: + // (a) onlineManager seeded to the real offline state so the later reconnect + // is a true offline->online transition that auto-resumes, and + // (b) resumePausedMutations() called after the persister restores (what the + // PersistQueryClientProvider onSuccess does), with mutation defaults + // registered BEFORE the resume so the rehydrated mutation has a fn. + it("replays a rehydrated paused create on reconnect (mutationFn fires)", async () => { + // --- Tab 1, OFFLINE: user creates a page; it pauses and gets persisted. --- + onlineManager.setOnline(false); // (a) boot seeded offline + + const client1 = new QueryClient(); + registerOfflineMutationDefaults(client1); + const observer = client1.getMutationCache().build(client1, { + mutationKey: offlineMutationKeys.createPage, + }); + observer.state.isPaused = true; + observer.state.status = "pending"; + observer.state.variables = { spaceId: "s1", title: "Offline page" }; + + await persistQueryClientSave({ + // Cast: persist-client-core and react-query may resolve to different + // @tanstack/query-core copies whose QueryClient brands are nominally + // incompatible (see query-persister.ts). Structurally identical at runtime. + queryClient: client1 as any, + persister: queryPersister, + buster: BUSTER, + dehydrateOptions: { shouldDehydrateMutation: () => true }, + }); + // The paused mutation is now in the persisted store. + expect(store.size).toBe(1); + + // --- RELOAD while still offline: fresh client restores from the SAME + // persister. Defaults are registered BEFORE restore/resume. --- + const client2 = new QueryClient(); + registerOfflineMutationDefaults(client2); + client2.mount(); // subscribes to onlineManager (auto-resume on reconnect) + + await persistQueryClientRestore({ + queryClient: client2 as any, + persister: queryPersister, + buster: BUSTER, + }); + expect(client2.getMutationCache().getAll()).toHaveLength(1); + + // (b) onSuccess wiring resumes after restore — but we are still OFFLINE, so + // the mutation must stay paused and NOT fire yet. + await client2.resumePausedMutations(); + expect(h.createPage).not.toHaveBeenCalled(); + + // --- RECONNECT: the offline->online transition auto-resumes the paused + // mutation and its registered default mutationFn finally fires. --- + onlineManager.setOnline(true); + + await vi.waitFor(() => { + expect(h.createPage).toHaveBeenCalledTimes(1); + }); + expect(h.createPage).toHaveBeenCalledWith({ + spaceId: "s1", + title: "Offline page", + }); + + client2.unmount(); + }); +}); diff --git a/apps/client/src/features/offline/query-persister.test.ts b/apps/client/src/features/offline/query-persister.test.ts index eb65245f..afdcd515 100644 --- a/apps/client/src/features/offline/query-persister.test.ts +++ b/apps/client/src/features/offline/query-persister.test.ts @@ -1,7 +1,19 @@ -import { describe, it, expect } from "vitest"; +import { describe, it, expect, vi, afterEach } from "vitest"; + +// In-memory idb-keyval so we can observe whether the persister actually writes. +const h = vi.hoisted(() => ({ + get: vi.fn(() => Promise.resolve(undefined)), + set: vi.fn(() => Promise.resolve()), + del: vi.fn(() => Promise.resolve()), +})); +vi.mock("idb-keyval", () => h); + import { shouldDehydrateOfflineQuery, OFFLINE_PERSIST_ROOTS, + queryPersister, + freezeOfflinePersistence, + unfreezeOfflinePersistence, } from "./query-persister"; // Small helper to build the structural query shape the predicate reads. @@ -87,3 +99,30 @@ describe("OFFLINE_PERSIST_ROOTS", () => { expect(OFFLINE_PERSIST_ROOTS.has("trash")).toBe(false); }); }); + +describe("freeze/unfreeze persistence (logout no-late-write guard)", () => { + const dummyClient = { + timestamp: Date.now(), + buster: "", + clientState: { mutations: [], queries: [] }, + } as any; + + afterEach(() => { + // Always leave persistence enabled so other tests/sessions persist normally. + unfreezeOfflinePersistence(); + h.set.mockClear(); + }); + + it("does NOT write to storage while frozen", async () => { + freezeOfflinePersistence(); + await queryPersister.persistClient(dummyClient); + expect(h.set).not.toHaveBeenCalled(); + }); + + it("resumes writing to storage once unfrozen", async () => { + freezeOfflinePersistence(); + unfreezeOfflinePersistence(); + await queryPersister.persistClient(dummyClient); + expect(h.set).toHaveBeenCalledTimes(1); + }); +}); diff --git a/apps/client/src/features/offline/query-persister.ts b/apps/client/src/features/offline/query-persister.ts index fabe7683..0d50ac08 100644 --- a/apps/client/src/features/offline/query-persister.ts +++ b/apps/client/src/features/offline/query-persister.ts @@ -23,12 +23,38 @@ const idbStorage = { removeItem: (key: string) => del(key), }; -export const queryPersister = createAsyncStoragePersister({ +const basePersister = createAsyncStoragePersister({ storage: idbStorage, key: OFFLINE_CACHE_KEY, throttleTime: 1000, }); +// When frozen, persistClient becomes a no-op so no new dehydrated snapshot is +// written to IndexedDB. This closes a logout data-leak race: clearing the cache +// (queryClient.clear()) fires `removed` cache events, each of which the persist +// subscription turns into a throttled persistClient call. The FIRST such call +// dehydrates a still-nearly-full snapshot and its async write can land AFTER the +// del() that clears the key, resurrecting the previous user's data (~180KB) in +// IndexedDB. Freezing before clear()/del() prevents any such rewrite. Re-enabled +// afterwards so the next (sign-in) session persists normally. See +// clear-offline-cache.ts. +let persistFrozen = false; + +export function freezeOfflinePersistence(): void { + persistFrozen = true; +} + +export function unfreezeOfflinePersistence(): void { + persistFrozen = false; +} + +export const queryPersister = { + persistClient: (persistedClient: Parameters[0]) => + persistFrozen ? Promise.resolve() : basePersister.persistClient(persistedClient), + restoreClient: () => basePersister.restoreClient(), + removeClient: () => basePersister.removeClient(), +}; + // Only navigation/read query roots are persisted for offline reading. // Volatile/auth queries (collab tokens, trash lists) are intentionally excluded. // diff --git a/apps/client/src/main.tsx b/apps/client/src/main.tsx index 6ee4233a..a683065d 100644 --- a/apps/client/src/main.tsx +++ b/apps/client/src/main.tsx @@ -11,7 +11,7 @@ import { MantineProvider } from "@mantine/core"; import { BrowserRouter } from "react-router-dom"; import { ModalsProvider } from "@mantine/modals"; import { Notifications } from "@mantine/notifications"; -import { QueryClient } from "@tanstack/react-query"; +import { QueryClient, onlineManager } from "@tanstack/react-query"; import { PersistQueryClientProvider } from "@tanstack/react-query-persist-client"; import { HelmetProvider } from "react-helmet-async"; import "./i18n"; @@ -47,9 +47,21 @@ export const queryClient = new QueryClient({ // Register default mutationFns for the offline-relevant structural mutations so // a paused mutation restored from IndexedDB after an offline reload still has a // mutationFn and is replayed by resumePausedMutations() on reconnect (instead -// of silently no-op'ing and dropping the offline create/move/comment). +// of silently no-op'ing and dropping the offline create/move/comment). MUST run +// before any resumePausedMutations() so rehydrated paused mutations have a fn. registerOfflineMutationDefaults(queryClient); +// Seed TanStack Query's onlineManager from the REAL connectivity state at boot. +// It defaults to `online: true` and only flips on window online/offline events, +// so a tab that COLD-BOOTS offline would wrongly believe it is online: paused +// mutations restored from IndexedDB would never get a later offline->online +// transition to trigger their replay, and the offline UI affordances could not +// tell they are offline. Seeding here makes the first real `online` event a true +// transition that auto-resumes the rehydrated paused mutations (#120 data loss). +if (typeof navigator !== "undefined" && "onLine" in navigator) { + onlineManager.setOnline(navigator.onLine); +} + if (isCloud() && isPostHogEnabled) { posthog.init(getPostHogKey(), { api_host: getPostHogHost(), @@ -76,6 +88,16 @@ root.render( shouldDehydrateQuery: shouldDehydrateOfflineQuery, }, }} + // After the persister finishes rehydrating, replay any paused + // mutations restored from IndexedDB. If we are back online this fires + // them immediately; if still offline they stay paused and TanStack's + // onlineManager auto-resumes them on the next online transition (which + // is now a true transition thanks to the onlineManager seeding above). + // Without this, a paused mutation persisted while offline and then + // reloaded would never resume and the user's work would be lost (#120). + onSuccess={() => { + queryClient.resumePausedMutations(); + }} > {/* Skip SW registration inside the Capacitor native WebView — the diff --git a/apps/client/src/pages/page/page.tsx b/apps/client/src/pages/page/page.tsx index a94faf58..3c395937 100644 --- a/apps/client/src/pages/page/page.tsx +++ b/apps/client/src/pages/page/page.tsx @@ -9,6 +9,7 @@ import { useGetSpaceBySlugQuery } from "@/features/space/queries/space-query.ts" import { useTranslation } from "react-i18next"; import React from "react"; import { EmptyState } from "@/components/ui/empty-state.tsx"; +import { OfflineFallback } from "@/features/offline/offline-fallback.tsx"; import { IconAlertTriangle, IconFileOff } from "@tabler/icons-react"; import { Button } from "@mantine/core"; import { Link } from "react-router-dom"; @@ -62,7 +63,19 @@ function PageContent({ pageSlug }: { pageSlug: string | undefined }) { } if (isError || !page) { - if ([401, 403, 404].includes(error?.["status"])) { + // An offline fetch of a page that was never saved for offline use yields a + // network error with NO HTTP status (status is undefined), which would + // otherwise fall through to the generic "Error fetching page data." state. + // When we are offline (or the failure is a network error with no status), + // show the dedicated "You're offline — this page isn't saved for offline" + // fallback instead, so the user understands why the page won't load. + const httpStatus = error?.["status"]; + const isOffline = + typeof navigator !== "undefined" && navigator.onLine === false; + if (isOffline || (isError && httpStatus == null)) { + return ; + } + if ([401, 403, 404].includes(httpStatus)) { return (