diff --git a/apps/client/src/features/auth/hooks/use-auth.ts b/apps/client/src/features/auth/hooks/use-auth.ts index 94ab3595..cd082e2c 100644 --- a/apps/client/src/features/auth/hooks/use-auth.ts +++ b/apps/client/src/features/auth/hooks/use-auth.ts @@ -23,6 +23,7 @@ import { acceptInvitation } from "@/features/workspace/services/workspace-servic import APP_ROUTE, { getPostLoginRedirect } from "@/lib/app-route.ts"; import { RESET } from "jotai/utils"; import { useTranslation } from "react-i18next"; +import { clearPersistedTreeCaches } from "@/features/page/tree/atoms/tree-data-atom"; export default function useAuth() { const { t } = useTranslation(); @@ -122,6 +123,11 @@ export default function useAuth() { const handleLogout = async () => { setCurrentUser(RESET); + // Purge the persisted sidebar tree caches (they contain page titles) so the + // cached page titles aren't left readable in localStorage on a shared + // machine. (Only the tree caches are swept; other localStorage entries + // remain.) + clearPersistedTreeCaches(); await logout(); window.location.replace(`${APP_ROUTE.AUTH.LOGIN}?logout=1`); }; diff --git a/apps/client/src/features/page/tree/atoms/open-tree-nodes-atom.ts b/apps/client/src/features/page/tree/atoms/open-tree-nodes-atom.ts index 9bbb7ff0..3c016206 100644 --- a/apps/client/src/features/page/tree/atoms/open-tree-nodes-atom.ts +++ b/apps/client/src/features/page/tree/atoms/open-tree-nodes-atom.ts @@ -13,20 +13,30 @@ export type OpenMap = Record; // `OpenMap | Promise` and break the functional-updater setter below). const openTreeNodesStorage = createJSONStorage(() => localStorage); +// Single source of truth for the open-map localStorage key prefix. Exported so +// the logout cache sweep (tree-data-atom.ts) removes keys by the SAME prefix +// used to write them — a rename here can never silently desync the cleanup. +export const OPEN_TREE_NODES_KEY_PREFIX = "openTreeNodes:"; + // One persisted open/closed map per (workspace, user). Scoping the localStorage // key prevents accounts that share a browser origin from leaking tree state. // `getOnInit: true` reads localStorage synchronously at atom init (not on mount), // so the first render already has the saved state — no collapse-then-expand // flicker on reload, and writes never run against an un-hydrated empty map. const openTreeNodesFamily = atomFamily((scopeKey: string) => - atomWithStorage(`openTreeNodes:${scopeKey}`, {}, openTreeNodesStorage, { - getOnInit: true, - }), + atomWithStorage( + `${OPEN_TREE_NODES_KEY_PREFIX}${scopeKey}`, + {}, + openTreeNodesStorage, + { getOnInit: true }, + ), ); // Resolve the storage scope from the current user. Fall back to "anon" for the // workspace/user parts when nothing is loaded yet (logged out / first paint). -const scopeKeyAtom = atom((get) => { +// Shared by the open-map atom below and the persisted tree-data atom +// (tree-data-atom.ts) so both caches are scoped identically. +export const scopeKeyAtom = atom((get) => { const currentUser = get(currentUserAtom); const workspaceId = currentUser?.workspace?.id ?? "anon"; const userId = currentUser?.user?.id ?? "anon"; diff --git a/apps/client/src/features/page/tree/atoms/tree-data-atom.test.ts b/apps/client/src/features/page/tree/atoms/tree-data-atom.test.ts new file mode 100644 index 00000000..62397ff1 --- /dev/null +++ b/apps/client/src/features/page/tree/atoms/tree-data-atom.test.ts @@ -0,0 +1,265 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import type { SpaceTreeNode } from "@/features/page/tree/types"; +import type { ICurrentUser } from "@/features/user/types/user.types"; + +// The persisted tree-data atom hydrates from localStorage ONCE, at family-atom +// creation (`getOnInit: true`). To exercise hydration deterministically each +// test imports a FRESH module instance (fresh atomFamily) after seeding the +// storage stub from vitest.setup.ts. jotai itself is externalized by vitest, so +// `createStore` can stay a static import — atoms are plain objects and any +// store works with any module instance. +import { createStore } from "jotai"; + +// Storage key for the default scope: no currentUser -> "anon:anon" (see +// scopeKeyAtom in open-tree-nodes-atom.ts) with the `v1` cache-shape version. +const ANON_KEY = "treeData:v1:anon:anon"; +const DEBOUNCE_MS = 500; + +async function freshImport() { + vi.resetModules(); + const treeDataModule = await import("./tree-data-atom"); + const userModule = await import( + "@/features/user/atoms/current-user-atom" + ); + return { + treeDataAtom: treeDataModule.treeDataAtom, + flushPendingTreeDataWrites: treeDataModule.flushPendingTreeDataWrites, + clearPersistedTreeCaches: treeDataModule.clearPersistedTreeCaches, + currentUserAtom: userModule.currentUserAtom, + }; +} + +function node(id: string): SpaceTreeNode { + return { + id, + slugId: `slug-${id}`, + name: id, + position: "a0", + spaceId: "space-1", + parentPageId: null as unknown as string, + hasChildren: false, + children: [], + }; +} + +// Every persisted tree key currently in storage — asserting on the whole +// prefix (not one known key) catches writes that resurrect under ANY scope. +function persistedTreeDataKeys(): string[] { + const keys: string[] = []; + for (let i = 0; i < localStorage.length; i++) { + const key = localStorage.key(i); + if (key !== null && key.startsWith("treeData:v1:")) keys.push(key); + } + return keys; +} + +function currentUser(workspaceId: string, userId: string): ICurrentUser { + return { + user: { id: userId }, + workspace: { id: workspaceId }, + } as unknown as ICurrentUser; +} + +beforeEach(() => { + localStorage.clear(); +}); + +afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); +}); + +describe("treeDataAtom (localStorage-persisted)", () => { + it("reads [] from a fresh store with empty storage", async () => { + const { treeDataAtom } = await freshImport(); + const store = createStore(); + + expect(store.get(treeDataAtom)).toEqual([]); + }); + + it("persists through the debounced setItem and hydrates a fresh module back", async () => { + vi.useFakeTimers(); + const setItemSpy = vi.spyOn(localStorage, "setItem"); + + const { treeDataAtom } = await freshImport(); + const store = createStore(); + + store.set(treeDataAtom, [node("a")]); + // Second write inside the debounce window — must coalesce into ONE flush + // carrying only the latest value. + vi.advanceTimersByTime(DEBOUNCE_MS / 2); + store.set(treeDataAtom, [node("a"), node("b")]); + + // Nothing flushed yet: the write is trailing-debounced. + expect(localStorage.getItem(ANON_KEY)).toBeNull(); + + vi.advanceTimersByTime(DEBOUNCE_MS + 100); + + expect(setItemSpy).toHaveBeenCalledTimes(1); + expect(JSON.parse(localStorage.getItem(ANON_KEY)!)).toEqual([ + node("a"), + node("b"), + ]); + + // A fresh module (fresh atom family -> getOnInit re-reads storage) and a + // fresh store hydrate the persisted tree back — the reload scenario. + const second = await freshImport(); + const store2 = createStore(); + expect(store2.get(second.treeDataAtom)).toEqual([node("a"), node("b")]); + }); + + it("reads [] (without throwing) when storage holds corrupted JSON", async () => { + localStorage.setItem(ANON_KEY, "{definitely not JSON!!!"); + + const { treeDataAtom } = await freshImport(); + const store = createStore(); + + expect(store.get(treeDataAtom)).toEqual([]); + }); + + it("reads [] when storage holds valid JSON of a non-array shape", async () => { + localStorage.setItem(ANON_KEY, JSON.stringify({ id: "not-a-tree" })); + + const { treeDataAtom } = await freshImport(); + const store = createStore(); + + expect(store.get(treeDataAtom)).toEqual([]); + }); + + it("supports functional-updater writes", async () => { + const { treeDataAtom } = await freshImport(); + const store = createStore(); + + store.set(treeDataAtom, [node("a")]); + store.set(treeDataAtom, (prev) => [...prev, node("b")]); + + expect(store.get(treeDataAtom).map((n) => n.id)).toEqual(["a", "b"]); + }); + + it("isolates trees between (workspace, user) scopes", async () => { + const { treeDataAtom, currentUserAtom } = await freshImport(); + const store = createStore(); + + store.set(currentUserAtom, currentUser("w1", "u1")); + store.set(treeDataAtom, [node("a")]); + expect(store.get(treeDataAtom).map((n) => n.id)).toEqual(["a"]); + + // Another account on the same browser origin must NOT see u1's tree. + store.set(currentUserAtom, currentUser("w2", "u2")); + expect(store.get(treeDataAtom)).toEqual([]); + + store.set(treeDataAtom, [node("b")]); + expect(store.get(treeDataAtom).map((n) => n.id)).toEqual(["b"]); + + // Switching back resolves the original scope's tree untouched. + store.set(currentUserAtom, currentUser("w1", "u1")); + expect(store.get(treeDataAtom).map((n) => n.id)).toEqual(["a"]); + }); + + it("clearPersistedTreeCaches removes all tree keys and discards pending writes", async () => { + vi.useFakeTimers(); + + // Stale caches across scopes plus an UNRELATED key that must survive. + localStorage.setItem("treeData:v1:a:b", JSON.stringify([node("stale")])); + localStorage.setItem("openTreeNodes:a:b", JSON.stringify({ p1: true })); + localStorage.setItem("currentUser", JSON.stringify({ user: { id: "b" } })); + + const { treeDataAtom, clearPersistedTreeCaches } = await freshImport(); + const store = createStore(); + + // Queue a debounced write (not flushed yet) for the anon scope. + store.set(treeDataAtom, [node("pending")]); + expect(localStorage.getItem(ANON_KEY)).toBeNull(); + + clearPersistedTreeCaches(); + + // Both prefixed caches are swept; the unrelated key is untouched. + expect(localStorage.getItem("treeData:v1:a:b")).toBeNull(); + expect(localStorage.getItem("openTreeNodes:a:b")).toBeNull(); + expect(localStorage.getItem("currentUser")).toBe( + JSON.stringify({ user: { id: "b" } }), + ); + + // The queued write was DISCARDED, not merely delayed: the debounce timer + // firing later must not resurrect a tree key after logout. + vi.advanceTimersByTime(DEBOUNCE_MS + 100); + expect(localStorage.getItem(ANON_KEY)).toBeNull(); + }); + + it("clearPersistedTreeCaches discards queued writes even when flushed DIRECTLY", async () => { + vi.useFakeTimers(); + + const { treeDataAtom, clearPersistedTreeCaches, flushPendingTreeDataWrites } = + await freshImport(); + const store = createStore(); + + // Queue a debounced write, then clear. Calling the flush directly (not via + // the debounce timer) isolates the pending-queue discard from the timer + // cancel: if the queue survived, this flush would resurrect the key even + // though the timer never fired. + store.set(treeDataAtom, [node("pending")]); + clearPersistedTreeCaches(); + flushPendingTreeDataWrites(); + + expect(localStorage.getItem(ANON_KEY)).toBeNull(); + expect(persistedTreeDataKeys()).toEqual([]); + }); + + it("skips persisting a tree over the size cap and warns exactly once", async () => { + vi.useFakeTimers(); + const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); + const setItemSpy = vi.spyOn(localStorage, "setItem"); + + const { treeDataAtom, flushPendingTreeDataWrites } = await freshImport(); + const store = createStore(); + + // One node whose name alone serializes to > MAX_SERIALIZED_LENGTH (~4M). + const huge = node("big"); + huge.name = "x".repeat(4_000_001); + + store.set(treeDataAtom, [huge]); + vi.advanceTimersByTime(DEBOUNCE_MS + 100); + + // The oversized serialization is skipped: the key is never written. + expect(localStorage.getItem(ANON_KEY)).toBeNull(); + expect(setItemSpy).not.toHaveBeenCalled(); + + // Editing the still-oversized tree fires another debounced write, but the + // "too large" warn is gated by the once-flag — no per-tick console spam. + store.set(treeDataAtom, [huge, node("big2")]); + vi.advanceTimersByTime(DEBOUNCE_MS + 100); + flushPendingTreeDataWrites(); + + expect(localStorage.getItem(ANON_KEY)).toBeNull(); + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy).toHaveBeenCalledWith( + "[tree] cached tree too large to persist; skipping", + ANON_KEY, + ); + }); + + it("disables persistence after clearPersistedTreeCaches: NEW writes never reach storage", async () => { + vi.useFakeTimers(); + + const { treeDataAtom, clearPersistedTreeCaches, flushPendingTreeDataWrites } = + await freshImport(); + const store = createStore(); + + clearPersistedTreeCaches(); + + // The resurrection scenario: a websocket tree event lands while `await + // logout()` is still in flight, AFTER the sweep. The write must not be + // queued, must not arm a new debounce timer, and must not survive the + // beforeunload flush fired by the logout redirect. + store.set(treeDataAtom, [node("late")]); + + vi.advanceTimersByTime(DEBOUNCE_MS + 100); + flushPendingTreeDataWrites(); // what the beforeunload handler runs + + expect(persistedTreeDataKeys()).toEqual([]); + + // Only PERSISTENCE is disabled: the in-memory atom keeps working, so the + // UI stays intact during the brief pre-redirect window. + expect(store.get(treeDataAtom).map((n) => n.id)).toEqual(["late"]); + }); +}); diff --git a/apps/client/src/features/page/tree/atoms/tree-data-atom.ts b/apps/client/src/features/page/tree/atoms/tree-data-atom.ts index 7d0ec503..411336aa 100644 --- a/apps/client/src/features/page/tree/atoms/tree-data-atom.ts +++ b/apps/client/src/features/page/tree/atoms/tree-data-atom.ts @@ -1,8 +1,206 @@ import { atom } from "jotai"; +import { atomFamily, atomWithStorage } from "jotai/utils"; import { SpaceTreeNode } from "@/features/page/tree/types"; import { appendNodeChildren } from "../utils"; +import { + OPEN_TREE_NODES_KEY_PREFIX, + scopeKeyAtom, +} from "./open-tree-nodes-atom"; -export const treeDataAtom = atom([]); +// The sidebar tree is persisted to localStorage so a page reload can paint the +// last-known tree IMMEDIATELY (no blank sidebar while the root query runs) and +// then reconcile with the server in the background. localStorage is a BOOT +// CACHE only — the in-memory atom stays the source of truth while the app runs. + +// Trailing-debounce machinery for the localStorage writes. The tree is +// rewritten on every lazy load / drag / socket event; serializing a large tree +// on each update would burn CPU and thrash the storage quota, so writes are +// coalesced (~500 ms per burst) and only the latest value per key is flushed. +const WRITE_DEBOUNCE_MS = 500; + +// Single source of truth for the tree-cache localStorage key prefix. The `v1` +// segment versions the cached node shape (bump it when SpaceTreeNode changes +// incompatibly). Shared by the storage key construction below AND the logout +// sweep in clearPersistedTreeCaches() so the two can never drift apart. +export const TREE_DATA_KEY_PREFIX = "treeData:v1:"; + +// Size guard: skip persisting trees whose JSON exceeds ~4M chars. localStorage +// quota is typically ~5 MB per origin; a huge tree must not evict everything +// else or spam QuotaExceededError on every debounce tick. +const MAX_SERIALIZED_LENGTH = 4_000_000; + +const pendingWrites = new Map(); +let flushTimer: ReturnType | null = null; +let writeFailureWarned = false; + +// Persistence kill-switch, armed by clearPersistedTreeCaches(). Once set, the +// debounced setItem and the flush become no-ops so nothing can be written back +// to localStorage AFTER the logout sweep: a websocket tree event landing while +// `await logout()` is still in flight would otherwise re-queue a write that +// the `beforeunload` flush (fired by the redirect) silently resurrects. +// Intentionally never reset: every caller of clearPersistedTreeCaches() +// immediately navigates away with a full page load +// (window.location.replace/href), so this module instance is torn down anyway. +// Only PERSISTENCE stops — the in-memory atoms keep working, so the UI stays +// intact during the brief pre-redirect window. +let persistenceDisabled = false; + +function writeNow(key: string, value: SpaceTreeNode[]): void { + try { + const serialized = JSON.stringify(value); + if (serialized.length > MAX_SERIALIZED_LENGTH) { + // Warn ONCE, like the quota branch below: a >4M-char tree re-serializes on + // every ~500ms debounce tick while it's edited, so an un-gated warn would + // spam the console on each flush. + if (!writeFailureWarned) { + writeFailureWarned = true; + console.warn("[tree] cached tree too large to persist; skipping", key); + } + return; + } + localStorage.setItem(key, serialized); + } catch (err) { + // QuotaExceededError, private mode, jsdom shims without working storage… + // The cache is best-effort: warn once, keep the in-memory tree working. + if (!writeFailureWarned) { + writeFailureWarned = true; + console.warn("[tree] failed to persist tree cache", err); + } + } +} + +// Exported so tests can force the debounced write synchronously; production +// code must never need it (the beforeunload hook below covers reloads). +export function flushPendingTreeDataWrites(): void { + if (flushTimer !== null) { + clearTimeout(flushTimer); + flushTimer = null; + } + if (persistenceDisabled) { + // Belt-and-braces: after logout nothing may reach localStorage, even via + // the beforeunload flush racing the redirect. Drop anything queued. + pendingWrites.clear(); + return; + } + for (const [key, value] of pendingWrites) { + writeNow(key, value); + } + pendingWrites.clear(); +} + +// Logout hygiene: the tree cache stores PAGE TITLES, so leaving it behind +// would keep them readable in localStorage on a shared machine after logout. +// Sweep by key prefix (not just the current scope) so stale scopes — old +// users, the `anon:anon` fallback — are purged too. Pending debounced writes +// are DISCARDED first (not flushed): a queued write firing after the sweep +// would silently resurrect a removed key. +export function clearPersistedTreeCaches(): void { + // Disable persistence FIRST so no write can be queued (or flushed) between + // the sweep below and the full-page navigation every caller performs next. + persistenceDisabled = true; + if (flushTimer !== null) { + clearTimeout(flushTimer); + flushTimer = null; + } + pendingWrites.clear(); + try { + // Collect matching keys BEFORE removing: deleting while iterating + // `localStorage.key(i)` shifts the indices and skips entries. + const keysToRemove: string[] = []; + for (let i = 0; i < localStorage.length; i++) { + const key = localStorage.key(i); + if ( + key !== null && + (key.startsWith(TREE_DATA_KEY_PREFIX) || + key.startsWith(OPEN_TREE_NODES_KEY_PREFIX)) + ) { + keysToRemove.push(key); + } + } + for (const key of keysToRemove) { + localStorage.removeItem(key); + } + } catch { + // Best-effort: disabled storage / jsdom shims must never break logout. + } +} + +// Flush the pending debounced write on unload so a reload right after a tree +// change doesn't lose the newest state (the debounce would otherwise eat it). +if ( + typeof window !== "undefined" && + typeof window.addEventListener === "function" +) { + window.addEventListener("beforeunload", flushPendingTreeDataWrites); +} + +// Custom sync storage for the tree cache. Deliberately NO `subscribe` key: +// cross-tab sync would REPLACE this tab's tree wholesale and clobber in-flight +// lazy loads; websockets already keep every open tab live. Each tab keeps its +// own in-memory tree — localStorage only seeds the next boot. +const treeDataStorage = { + getItem: (key: string, initialValue: SpaceTreeNode[]): SpaceTreeNode[] => { + // Defensive: jsdom test shims may lack methods, stored JSON may be + // corrupted or of a wrong shape. Any failure falls back to the empty tree. + try { + const raw = localStorage.getItem(key); + if (raw === null) return initialValue; + const parsed = JSON.parse(raw); + return Array.isArray(parsed) ? (parsed as SpaceTreeNode[]) : initialValue; + } catch { + return initialValue; + } + }, + setItem: (key: string, newValue: SpaceTreeNode[]): void => { + // After logout the cache must stay purged: neither queue the write nor arm + // a new flush timer (see persistenceDisabled above). The in-memory atom + // value is unaffected — only the localStorage mirror is frozen. + if (persistenceDisabled) return; + pendingWrites.set(key, newValue); + if (flushTimer !== null) clearTimeout(flushTimer); + flushTimer = setTimeout(flushPendingTreeDataWrites, WRITE_DEBOUNCE_MS); + }, + removeItem: (key: string): void => { + pendingWrites.delete(key); + try { + localStorage.removeItem(key); + } catch { + /* best-effort cache — ignore */ + } + }, +}; + +// One persisted tree per (workspace, user) — same scoping rationale as the +// open-map atom (accounts sharing a browser origin must not leak trees). +// `getOnInit: true` reads localStorage synchronously at atom init, so the very +// first render already has the cached tree — no blank-then-jump sidebar. +const treeDataFamily = atomFamily((scopeKey: string) => + atomWithStorage( + `${TREE_DATA_KEY_PREFIX}${scopeKey}`, + [], + treeDataStorage, + { getOnInit: true }, + ), +); + +// Public facade — same read value (SpaceTreeNode[]) and same setter shape +// (value OR functional updater) as the previous in-memory atom, transparently +// routed to the persisted tree of the current workspace/user. +export const treeDataAtom = atom( + (get) => get(treeDataFamily(get(scopeKeyAtom))), + ( + get, + set, + update: SpaceTreeNode[] | ((prev: SpaceTreeNode[]) => SpaceTreeNode[]), + ) => { + const target = treeDataFamily(get(scopeKeyAtom)); + const next = + typeof update === "function" + ? (update as (prev: SpaceTreeNode[]) => SpaceTreeNode[])(get(target)) + : update; + set(target, next); + }, +); // Atom export const appendNodeChildrenAtom = atom( diff --git a/apps/client/src/features/page/tree/components/space-tree.boot-cache.test.tsx b/apps/client/src/features/page/tree/components/space-tree.boot-cache.test.tsx new file mode 100644 index 00000000..4f248c81 --- /dev/null +++ b/apps/client/src/features/page/tree/components/space-tree.boot-cache.test.tsx @@ -0,0 +1,222 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { createRef } from "react"; +import { render, act, waitFor, cleanup } from "@testing-library/react"; + +// --- Mocks for the heavy / networked module graph --------------------------- +// Same isolation strategy as space-tree.expand-all.test.tsx: everything that +// would otherwise need a real server / router / DnD stack is mocked. Here we +// additionally CAPTURE the DocTree props (onToggle + data) so the test can +// drive a lazy-load expand exactly as a row click would, and we control +// fetchAllAncestorChildren to assert the fresh fetch happens. + +const fetchAllAncestorChildrenMock = vi.fn(); + +// Holder mutated by the DocTree stub each render so the test can read the +// latest tree it was handed and invoke its onToggle callback. +const docTree: { + onToggle?: (id: string, isOpen: boolean) => void | Promise; + data: unknown[]; +} = { data: [] }; + +vi.mock("@/features/page/services/page-service.ts", () => ({ + getSpaceTree: vi.fn(), + getPageBreadcrumbs: vi.fn(), +})); + +vi.mock("@/features/page/queries/page-query.ts", () => ({ + // No root pages and no further pages — the server data-load effect stays + // inert (isDataLoaded never flips), so refreshOpenBranches never runs and the + // test exercises ONLY the boot-prune + handleToggle lazy-load path against + // the hydrated cache we seed into the atom below. + useGetRootSidebarPagesQuery: () => ({ + data: undefined, + hasNextPage: false, + fetchNextPage: vi.fn(), + isFetching: false, + }), + usePageQuery: () => ({ data: undefined }), + fetchAllAncestorChildren: (...args: unknown[]) => + fetchAllAncestorChildrenMock(...args), +})); + +vi.mock("@/features/page/tree/hooks/use-tree-mutation.ts", () => ({ + useTreeMutation: () => ({ handleMove: vi.fn() }), +})); + +vi.mock("@mantine/notifications", () => ({ + notifications: { show: vi.fn() }, +})); + +vi.mock("react-i18next", () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); + +vi.mock("react-router-dom", () => ({ + useParams: () => ({ pageSlug: undefined }), +})); + +vi.mock("@/lib", () => ({ + extractPageSlugId: () => undefined, +})); + +vi.mock("@/lib/config.ts", () => ({ + isCompactPageTreeEnabled: () => false, +})); + +// Capture the props DocTree is rendered with instead of rendering anything. +vi.mock("./doc-tree", () => ({ + DocTree: (props: { onToggle: (id: string, isOpen: boolean) => void; data: unknown[] }) => { + docTree.onToggle = props.onToggle; + docTree.data = props.data; + return null; + }, + ROW_HEIGHT_COMPACT: 28, + ROW_HEIGHT_STANDARD: 32, +})); +vi.mock("./space-tree-row", () => ({ + SpaceTreeRow: () => null, +})); + +vi.mock("@mantine/core", () => ({ + Text: ({ children }: { children?: unknown }) => children ?? null, +})); + +// In-memory open-map (the real one is localStorage-backed and crashes under the +// jsdom shim). Empty at start of each test -> every branch is COLLAPSED, which +// is exactly the state we need to prove the boot-prune. `scopeKeyAtom` is +// re-exported because the persisted tree-data atom resolves its scope through it. +vi.mock("@/features/page/tree/atoms/open-tree-nodes-atom.ts", async () => { + const { atom } = await import("jotai"); + type OpenMap = Record; + const base = atom({}); + const openTreeNodesAtom = atom( + (get) => get(base), + (get, set, update: OpenMap | ((prev: OpenMap) => OpenMap)) => { + const next = + typeof update === "function" + ? (update as (prev: OpenMap) => OpenMap)(get(base)) + : update; + set(base, next); + }, + ); + const scopeKeyAtom = atom(() => "test-workspace:test-user"); + return { openTreeNodesAtom, scopeKeyAtom }; +}); + +import SpaceTree, { SpaceTreeApi } from "./space-tree"; +import { + treeDataAtom, + flushPendingTreeDataWrites, +} from "@/features/page/tree/atoms/tree-data-atom.ts"; +import { createStore, Provider } from "jotai"; +import type { SpaceTreeNode } from "@/features/page/tree/types.ts"; + +// The scopeKeyAtom mock resolves to this fixed scope, so the persisted +// tree-data atom hydrates from exactly this localStorage key at mount +// (getOnInit + atomWithStorage's onMount both read it). +const CACHE_KEY = "treeData:v1:test-workspace:test-user"; + +function child( + id: string, + parentPageId: string, + hasChildren = false, +): SpaceTreeNode { + return { + id, + slugId: `slug-${id}`, + name: id, + position: "a0", + spaceId: "space-1", + parentPageId, + hasChildren, + children: [], + }; +} + +// A hydrated boot cache: a COLLAPSED branch (not in the open-map) that still +// carries a stale cached child — the exact shape a previous session left behind +// after the branch was expanded then collapsed then persisted. +function cachedTreeWithCollapsedBranch(): SpaceTreeNode[] { + return [ + { + id: "branch", + slugId: "slug-branch", + name: "branch", + position: "a0", + spaceId: "space-1", + parentPageId: null as unknown as string, + hasChildren: true, + children: [child("stale", "branch")], + }, + ]; +} + +beforeEach(() => { + fetchAllAncestorChildrenMock.mockReset(); + docTree.onToggle = undefined; + docTree.data = []; + // Flush any pending debounced write from a previous test before clearing. + flushPendingTreeDataWrites(); + try { + localStorage.clear?.(); + } catch { + /* fresh store per test isolates state */ + } +}); + +afterEach(() => { + cleanup(); +}); + +describe("SpaceTree boot-cache prune (#159 #8 stale collapsed children)", () => { + it("drops a collapsed cached branch's children on boot and fetches fresh on first expand", async () => { + // Server returns FRESH children on the lazy-load: the stale cached child is + // gone, a renamed/new one takes its place. + fetchAllAncestorChildrenMock.mockResolvedValue([child("fresh", "branch")]); + + // Simulate the localStorage-hydrated boot cache: seed the persisted key + // BEFORE mount so the atom hydrates it (store.set would be clobbered by + // atomWithStorage's onMount re-reading storage — this is the real path). + localStorage.setItem( + CACHE_KEY, + JSON.stringify(cachedTreeWithCollapsedBranch()), + ); + + const store = createStore(); + const ref = createRef(); + render( + + + , + ); + + // Boot-prune ran at mount: the COLLAPSED branch's cached children were + // dropped to the unloaded shape ([]), so the stale child is no longer there. + const branchAfterBoot = docTree.data.find( + (n) => (n as SpaceTreeNode).id === "branch", + ) as SpaceTreeNode; + expect(branchAfterBoot.children).toEqual([]); + expect(branchAfterBoot.hasChildren).toBe(true); + + // First expand of the collapsed branch after boot must lazy-load fresh + // children (before this fix the cached children were kept and the fetch + // was skipped, showing stale data). + await act(async () => { + await docTree.onToggle!("branch", true); + }); + + expect(fetchAllAncestorChildrenMock).toHaveBeenCalledTimes(1); + expect(fetchAllAncestorChildrenMock).toHaveBeenCalledWith({ + pageId: "branch", + spaceId: "space-1", + }); + + // The fresh children replaced the stale cache in the live tree. + await waitFor(() => { + const branch = store + .get(treeDataAtom) + .find((n) => n.id === "branch")!; + expect(branch.children.map((c) => c.id)).toEqual(["fresh"]); + }); + }); +}); diff --git a/apps/client/src/features/page/tree/components/space-tree.expand-all.test.tsx b/apps/client/src/features/page/tree/components/space-tree.expand-all.test.tsx index 7f0e4313..c7e3cbc7 100644 --- a/apps/client/src/features/page/tree/components/space-tree.expand-all.test.tsx +++ b/apps/client/src/features/page/tree/components/space-tree.expand-all.test.tsx @@ -71,7 +71,8 @@ vi.mock("@mantine/core", () => ({ // getOnInit), which crashes under jsdom's localStorage shim here. Swap in a // plain in-memory atom with the same read value (OpenMap) and the same setter // shape (value OR functional updater) so the component's open-state logic runs -// unchanged while staying inside the test store. +// unchanged while staying inside the test store. `scopeKeyAtom` is also +// re-exported (the real module exports it for the persisted tree-data atom). vi.mock("@/features/page/tree/atoms/open-tree-nodes-atom.ts", async () => { const { atom } = await import("jotai"); type OpenMap = Record; @@ -86,11 +87,17 @@ vi.mock("@/features/page/tree/atoms/open-tree-nodes-atom.ts", async () => { set(base, next); }, ); - return { openTreeNodesAtom }; + // Fixed scope key: the tree-data atom family resolves through this, so all + // tests read/write the same (empty at start of each test) storage key. + const scopeKeyAtom = atom(() => "test-workspace:test-user"); + return { openTreeNodesAtom, scopeKeyAtom }; }); import SpaceTree, { SpaceTreeApi } from "./space-tree"; -import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom.ts"; +import { + treeDataAtom, + flushPendingTreeDataWrites, +} from "@/features/page/tree/atoms/tree-data-atom.ts"; import { openTreeNodesAtom } from "@/features/page/tree/atoms/open-tree-nodes-atom.ts"; import { createStore, Provider } from "jotai"; import type { SpaceTreeNode } from "@/features/page/tree/types.ts"; @@ -134,6 +141,10 @@ function renderTree(store: ReturnType) { beforeEach(() => { getSpaceTreeMock.mockReset(); notificationsShowMock.mockReset(); + // The tree-data atom persists via a ~500 ms trailing debounce; flush it NOW + // (cancelling the timer) so a previous test's pending write can't land in + // storage mid-test after the clear below. + flushPendingTreeDataWrites(); // jsdom's localStorage shim here lacks `clear`; guard it. Each test uses a // fresh jotai store anyway, so cross-test open-state never leaks. try { diff --git a/apps/client/src/features/page/tree/components/space-tree.tsx b/apps/client/src/features/page/tree/components/space-tree.tsx index affcbac3..85e4cdae 100644 --- a/apps/client/src/features/page/tree/components/space-tree.tsx +++ b/apps/client/src/features/page/tree/components/space-tree.tsx @@ -30,6 +30,7 @@ import { openBranches, closeIds, loadedOpenBranchIds, + pruneCollapsedChildren, } from "@/features/page/tree/utils/utils.ts"; import { SpaceTreeNode } from "@/features/page/tree/types.ts"; import { treeModel } from "@/features/page/tree/model/tree-model"; @@ -199,45 +200,81 @@ const SpaceTree = forwardRef(function SpaceTree( const openIdsRef = useRef(openIds); openIdsRef.current = openIds; - // Reconnect refresh (#159 #8): on a socket reconnect, re-fetch and reconcile - // the children of every currently-open, already-loaded branch of THIS space, + // Boot-cache hygiene (#159 #8): the localStorage-hydrated tree carries the + // children of every branch ever expanded, including ones now COLLAPSED. Their + // first expand would skip the lazy-load and render stale children (a + // rename/move/delete missed while offline). Drop the cached children of every + // COLLAPSED branch ONCE at mount so its first expand fetches fresh via + // handleToggle — exactly as it did before the tree was cached. OPEN branches + // keep their children and are refreshed by refreshOpenBranches instead, so + // this runs before any expand and never double-fetches an open branch. + const prunedBootCacheRef = useRef(false); + useEffect(() => { + if (prunedBootCacheRef.current) return; + prunedBootCacheRef.current = true; + setData((prev) => pruneCollapsedChildren(prev, openIdsRef.current)); + }, [setData]); + + // Re-fetch and reconcile the children of every currently-open, already-loaded + // branch of THIS space. Shared by the socket reconnect handler and the + // post-load cache refresh below. The ROOT level is reconciled separately by + // the root-query refetch + mergeRootTrees; an UNLOADED branch is skipped + // (lazy-load fetches it fresh on expand). Reads refs so it always sees the + // latest tree/open-state/space without re-creating the callback. + const refreshOpenBranches = useCallback(async () => { + const effectSpaceId = spaceIdRef.current; + const branchIds = loadedOpenBranchIds( + dataRef.current.filter((n) => n?.spaceId === effectSpaceId), + openIdsRef.current, + ); + if (branchIds.length === 0) return; + for (const id of branchIds) { + try { + // `fresh: true` bypasses the 30-min sidebar-pages cache so the + // reconcile sees the server's CURRENT children (handler-order + // independent — no reliance on the global reconnect invalidation). + const fresh = await fetchAllAncestorChildren( + { pageId: id, spaceId: effectSpaceId }, + { fresh: true }, + ); + if (spaceIdRef.current !== effectSpaceId) return; // space switched + setData((prev) => treeModel.reconcileChildren(prev, id, fresh)); + } catch (err) { + console.error("[tree] open branch refresh failed", err); + } + } + }, [setData]); + + // Reconnect refresh (#159 #8): on a socket reconnect, refresh open branches // so a move/rename/delete that happened INSIDE a loaded branch while events // were missed (laptop sleep / wifi gap) is reflected instead of left stale. - // The ROOT level is reconciled separately by the root-query refetch + - // mergeRootTrees; an UNLOADED branch is skipped (lazy-load fetches it fresh on - // expand). No first-connect guard is needed: space-tree usually mounts AFTER - // the initial connect, so every `connect` it sees is a reconnect; the rare + // No first-connect guard is needed: space-tree usually mounts AFTER the + // initial connect, so every `connect` it sees is a reconnect; the rare // initial-connect case has an empty tree, so the refresh is a harmless no-op. useEffect(() => { if (!socket) return; - const onConnect = async () => { - const effectSpaceId = spaceIdRef.current; - const branchIds = loadedOpenBranchIds( - dataRef.current.filter((n) => n?.spaceId === effectSpaceId), - openIdsRef.current, - ); - if (branchIds.length === 0) return; - for (const id of branchIds) { - try { - // `fresh: true` bypasses the 30-min sidebar-pages cache so the - // reconcile sees the server's CURRENT children (handler-order - // independent — no reliance on the global reconnect invalidation). - const fresh = await fetchAllAncestorChildren( - { pageId: id, spaceId: effectSpaceId }, - { fresh: true }, - ); - if (spaceIdRef.current !== effectSpaceId) return; // space switched - setData((prev) => treeModel.reconcileChildren(prev, id, fresh)); - } catch (err) { - console.error("[tree] reconnect branch refresh failed", err); - } - } + const onConnect = () => { + refreshOpenBranches(); }; socket.on("connect", onConnect); return () => { socket.off("connect", onConnect); }; - }, [socket, setData]); + }, [socket, refreshOpenBranches]); + + // Post-load cache refresh: the sidebar paints instantly from the + // localStorage-cached tree, so children of open branches may be stale. Once + // the server root set has been merged for this space (isDataLoaded flips + // true), refresh every open, already-loaded branch ONCE per space per mount. + // dataRef.current is already up to date here: refs are assigned during + // render, and this effect runs after the merge-triggered re-render commit. + const refreshedSpacesRef = useRef>(new Set()); + useEffect(() => { + if (!isDataLoaded) return; + if (refreshedSpacesRef.current.has(spaceId)) return; + refreshedSpacesRef.current.add(spaceId); + refreshOpenBranches(); + }, [isDataLoaded, spaceId, refreshOpenBranches]); const handleToggle = useCallback( async (id: string, isOpen: boolean) => { @@ -333,12 +370,17 @@ const SpaceTree = forwardRef(function SpaceTree( return (
+ {/* "No pages yet" only after the SERVER confirmed the space is empty — + never while just the localStorage cache is empty. */} {isDataLoaded && filteredData.length === 0 && ( {t("No pages yet")} )} - {isDataLoaded && filteredData.length > 0 && ( + {/* Cache-first paint: render as soon as ANY data exists (synchronous + localStorage hydration) instead of waiting for the server round-trip; + the background merge/refresh reconciles it afterwards. */} + {filteredData.length > 0 && ( data={filteredData} openIds={openIds} diff --git a/apps/client/src/features/page/tree/utils/utils.test.ts b/apps/client/src/features/page/tree/utils/utils.test.ts index 0eced376..5b48c0ba 100644 --- a/apps/client/src/features/page/tree/utils/utils.test.ts +++ b/apps/client/src/features/page/tree/utils/utils.test.ts @@ -8,6 +8,7 @@ import { closeIds, mergeRootTrees, loadedOpenBranchIds, + pruneCollapsedChildren, sortPositionKeys, pageToTreeNode, } from "./utils"; @@ -438,3 +439,62 @@ describe("loadedOpenBranchIds (#159 #8 reconnect refresh targets)", () => { expect(ids.sort()).toEqual(["a", "a1"]); }); }); + +describe("pruneCollapsedChildren", () => { + // Signature: pruneCollapsedChildren(tree: SpaceTreeNode[], openIds: + // ReadonlySet): SpaceTreeNode[]. Collapsed nodes (id NOT in openIds) + // are reset to `children: []` (hasChildren untouched); open nodes keep their + // children but are recursed into so a collapsed branch nested under an open + // one is still pruned. + // + // Fixture: + // open "p" (in openIds, hasChildren) + // └─ collapsed "c" (NOT in openIds) with STALE child "g" + // collapsed "t" (NOT in openIds) with child "t1" + // Only "p" is open. + function fixture() { + const grandchild = treeNode("g"); // stale, cached under the collapsed child + const collapsedChild = treeNode("c", [grandchild]); + const openParent = treeNode("p", [collapsedChild]); + const topCollapsed = treeNode("t", [treeNode("t1")]); + return { openParent, collapsedChild, topCollapsed }; + } + + it("keeps an OPEN parent's children and recurses to prune a nested collapsed branch; prunes a top-level collapsed node", () => { + const { openParent, topCollapsed } = fixture(); + const tree = [openParent, topCollapsed]; + const result = pruneCollapsedChildren(tree, new Set(["p"])); + + // (a) OPEN parent keeps its children (not cleared) and hasChildren stays true. + const p = result[0]; + expect(p.id).toBe("p"); + expect(p.hasChildren).toBe(true); + expect(p.children).toHaveLength(1); + + // (b) The nested COLLAPSED child under the open parent is pruned to + // `children: []` by the recursion, with hasChildren preserved. This is the + // open-keep + recurse branch that F1's empty-open-set fixture never hits. + const c = p.children[0]; + expect(c.id).toBe("c"); + expect(c.children).toEqual([]); + expect(c.hasChildren).toBe(true); + + // (c) The top-level collapsed node is pruned to `children: []`, hasChildren kept. + const t = result[1]; + expect(t.id).toBe("t"); + expect(t.children).toEqual([]); + expect(t.hasChildren).toBe(true); + }); + + it("does not mutate the input tree (returns fresh nodes)", () => { + const { openParent, collapsedChild, topCollapsed } = fixture(); + const tree = [openParent, topCollapsed]; + pruneCollapsedChildren(tree, new Set(["p"])); + + // Originals are untouched: the collapsed child still carries its stale grandchild. + expect(collapsedChild.children).toHaveLength(1); + expect(collapsedChild.children[0].id).toBe("g"); + expect(openParent.children[0]).toBe(collapsedChild); + expect(topCollapsed.children).toHaveLength(1); + }); +}); diff --git a/apps/client/src/features/page/tree/utils/utils.ts b/apps/client/src/features/page/tree/utils/utils.ts index d5084b00..1c61d5cf 100644 --- a/apps/client/src/features/page/tree/utils/utils.ts +++ b/apps/client/src/features/page/tree/utils/utils.ts @@ -293,6 +293,41 @@ export function loadedOpenBranchIds( return ids; } +/** + * Boot-cache hygiene (#159 #8): the persisted tree keeps the children of EVERY + * branch ever expanded — collapsing a branch never prunes them. So on reload a + * COLLAPSED branch hydrates with its old cached children, and `handleToggle` + * skips the lazy-load on first expand (children already present) → it shows + * STALE children (renamed / moved / deleted while the user was offline) with no + * reconcile. `refreshOpenBranches` only refreshes OPEN branches, so collapsed + * ones slip through. + * + * Fix: drop the cached children of every node NOT in the persisted open-set, + * resetting it to the canonical UNLOADED shape (`children: []`, `hasChildren` + * untouched — see pageToTreeNode). Its first expand then lazy-loads fresh, just + * as it did before the tree was cached to localStorage. OPEN branches keep + * their children (refreshOpenBranches reconciles those, so they must not be + * dropped here) and are recursed into so a collapsed branch nested under an + * open one is pruned too. + */ +export function pruneCollapsedChildren( + tree: SpaceTreeNode[], + openIds: ReadonlySet, +): SpaceTreeNode[] { + return tree.map((node) => { + const hasLoadedChildren = !!node.children && node.children.length > 0; + if (!openIds.has(node.id)) { + // Collapsed: drop the whole cached subtree so it reads as unloaded. + return hasLoadedChildren ? { ...node, children: [] } : node; + } + // Open: keep it, but recurse into its children (a nested collapsed branch + // must still be pruned). + return hasLoadedChildren + ? { ...node, children: pruneCollapsedChildren(node.children, openIds) } + : node; + }); +} + // Collect every node id in the tree (roots, branches, leaves). Used by // collapseAll to clear the open-state map for all current-space nodes. export function collectAllIds(nodes: SpaceTreeNode[]): string[] { diff --git a/apps/client/src/lib/api-client.ts b/apps/client/src/lib/api-client.ts index d42d4cb9..8ce35fcb 100644 --- a/apps/client/src/lib/api-client.ts +++ b/apps/client/src/lib/api-client.ts @@ -1,6 +1,7 @@ import axios, { AxiosInstance } from "axios"; import APP_ROUTE from "@/lib/app-route.ts"; import { isCloud } from "@/lib/config.ts"; +import { clearPersistedTreeCaches } from "@/features/page/tree/atoms/tree-data-atom"; const api: AxiosInstance = axios.create({ baseURL: "/api", @@ -71,6 +72,12 @@ function redirectToLogin() { "/invites", ]; if (!exemptPaths.some((path) => window.location.pathname.startsWith(path))) { + // Forced logout (401 / expired session) must purge the persisted sidebar + // tree caches too: they contain page titles, and on a shared machine most + // sessions end via cookie expiry — not the logout button — so this is the + // only cleanup that runs on that path. It also disables further cache + // persistence until the full page load below. + clearPersistedTreeCaches(); const redirectTo = window.location.pathname; if (redirectTo === APP_ROUTE.HOME) { window.location.href = APP_ROUTE.AUTH.LOGIN;