Compare commits

...

2 Commits

Author SHA1 Message Date
agent_coder e9d5d493d3 fix(#290 review): drop stale cached children on boot + gate size warn (F1/F2/F3)
F1 (MEDIUM regression): a collapsed-but-cached branch showed STALE children on
re-expand after reload (the cache keeps children of any ever-expanded branch;
refreshOpenBranches only refreshes OPEN branches, but the fetch guard skips a branch
that has cached children). New pruneCollapsedChildren(tree, openIds) resets children
to [] (keeps hasChildren) for every node NOT in the persisted open-set, recursing
into open nodes — a once-per-mount boot effect. A pruned collapsed branch is then the
'unloaded' shape handleToggle re-fetches, so its first expand reconciles fresh (as
pre-cache). Open branches keep their children (refreshOpenBranches handles them, no
double fetch). Test: a collapsed cached branch with a stale child fetches fresh on
first expand after boot.
F2: gate the >4MB size-guard console.warn behind the writeFailureWarned once-flag
(like the quota branch) so editing a huge tree no longer re-warns every ~500ms; test
that an oversized tree is not persisted + warns exactly once.
F3: narrow the use-auth privacy comment (only tree caches are swept; other
localStorage entries remain).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-02 22:25:05 +03:00
claude_code b349676eae feat(tree): paint sidebar tree instantly from localStorage boot cache
On page reload the sidebar tree rendered nothing until every root page
was fetched (paginated), and children of expanded branches arrived even
later (breadcrumbs effect / socket connect) — the tree visibly jumped a
couple of seconds after load.

- treeDataAtom is now a facade over atomFamily(atomWithStorage) keyed
  treeData:v1:{workspaceId}:{userId} with getOnInit: true — the cached
  tree hydrates synchronously and paints on the very first render,
  together with the already-persisted open-branches map. Public atom
  interface unchanged (value or functional updater), all call sites
  untouched.
- Custom sync storage: debounced writes (500ms, coalesced, size guard,
  beforeunload flush), defensive reads (corrupted JSON -> []), no
  cross-tab subscribe (localStorage is a boot cache only).
- SpaceTree renders on cached data immediately; "No pages yet" still
  waits for the server. Once server roots merge, open loaded branches
  are re-fetched fresh and reconciled once per space (shared
  refreshOpenBranches, also used by the socket reconnect handler).
- Logout hygiene: clearPersistedTreeCaches() purges treeData:v1:* and
  openTreeNodes:* by prefix and disables further persistence (kill
  switch closes the websocket-write-vs-beforeunload-flush resurrection
  race). Wired into both handleLogout and the 401 redirectToLogin path,
  since cached trees contain page titles.
- Tests: tree-data-atom.test.ts (hydration, debounce round-trip,
  corrupted JSON, scope isolation, logout purge, persistence kill
  switch); expand-all suite adapted. 144 tree tests / full client suite
  green, tsc clean.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-02 15:51:15 +03:00
9 changed files with 834 additions and 38 deletions
@@ -23,6 +23,7 @@ import { acceptInvitation } from "@/features/workspace/services/workspace-servic
import APP_ROUTE, { getPostLoginRedirect } from "@/lib/app-route.ts"; import APP_ROUTE, { getPostLoginRedirect } from "@/lib/app-route.ts";
import { RESET } from "jotai/utils"; import { RESET } from "jotai/utils";
import { useTranslation } from "react-i18next"; import { useTranslation } from "react-i18next";
import { clearPersistedTreeCaches } from "@/features/page/tree/atoms/tree-data-atom";
export default function useAuth() { export default function useAuth() {
const { t } = useTranslation(); const { t } = useTranslation();
@@ -122,6 +123,11 @@ export default function useAuth() {
const handleLogout = async () => { const handleLogout = async () => {
setCurrentUser(RESET); 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(); await logout();
window.location.replace(`${APP_ROUTE.AUTH.LOGIN}?logout=1`); window.location.replace(`${APP_ROUTE.AUTH.LOGIN}?logout=1`);
}; };
@@ -13,20 +13,30 @@ export type OpenMap = Record<string, boolean>;
// `OpenMap | Promise<OpenMap>` and break the functional-updater setter below). // `OpenMap | Promise<OpenMap>` and break the functional-updater setter below).
const openTreeNodesStorage = createJSONStorage<OpenMap>(() => localStorage); const openTreeNodesStorage = createJSONStorage<OpenMap>(() => 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 // One persisted open/closed map per (workspace, user). Scoping the localStorage
// key prevents accounts that share a browser origin from leaking tree state. // key prevents accounts that share a browser origin from leaking tree state.
// `getOnInit: true` reads localStorage synchronously at atom init (not on mount), // `getOnInit: true` reads localStorage synchronously at atom init (not on mount),
// so the first render already has the saved state — no collapse-then-expand // 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. // flicker on reload, and writes never run against an un-hydrated empty map.
const openTreeNodesFamily = atomFamily((scopeKey: string) => const openTreeNodesFamily = atomFamily((scopeKey: string) =>
atomWithStorage<OpenMap>(`openTreeNodes:${scopeKey}`, {}, openTreeNodesStorage, { atomWithStorage<OpenMap>(
getOnInit: true, `${OPEN_TREE_NODES_KEY_PREFIX}${scopeKey}`,
}), {},
openTreeNodesStorage,
{ getOnInit: true },
),
); );
// Resolve the storage scope from the current user. Fall back to "anon" for the // 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). // 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 currentUser = get(currentUserAtom);
const workspaceId = currentUser?.workspace?.id ?? "anon"; const workspaceId = currentUser?.workspace?.id ?? "anon";
const userId = currentUser?.user?.id ?? "anon"; const userId = currentUser?.user?.id ?? "anon";
@@ -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"]);
});
});
@@ -1,8 +1,206 @@
import { atom } from "jotai"; import { atom } from "jotai";
import { atomFamily, atomWithStorage } from "jotai/utils";
import { SpaceTreeNode } from "@/features/page/tree/types"; import { SpaceTreeNode } from "@/features/page/tree/types";
import { appendNodeChildren } from "../utils"; import { appendNodeChildren } from "../utils";
import {
OPEN_TREE_NODES_KEY_PREFIX,
scopeKeyAtom,
} from "./open-tree-nodes-atom";
export const treeDataAtom = atom<SpaceTreeNode[]>([]); // 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<string, SpaceTreeNode[]>();
let flushTimer: ReturnType<typeof setTimeout> | 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<SpaceTreeNode[]>(
`${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 // Atom
export const appendNodeChildrenAtom = atom( export const appendNodeChildrenAtom = atom(
@@ -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<void>;
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<string, boolean>;
const base = atom<OpenMap>({});
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<SpaceTreeApi>();
render(
<Provider store={store}>
<SpaceTree ref={ref} spaceId="space-1" readOnly={false} />
</Provider>,
);
// 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"]);
});
});
});
@@ -71,7 +71,8 @@ vi.mock("@mantine/core", () => ({
// getOnInit), which crashes under jsdom's localStorage shim here. Swap in a // 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 // 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 // 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 () => { vi.mock("@/features/page/tree/atoms/open-tree-nodes-atom.ts", async () => {
const { atom } = await import("jotai"); const { atom } = await import("jotai");
type OpenMap = Record<string, boolean>; type OpenMap = Record<string, boolean>;
@@ -86,11 +87,17 @@ vi.mock("@/features/page/tree/atoms/open-tree-nodes-atom.ts", async () => {
set(base, next); 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 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 { openTreeNodesAtom } from "@/features/page/tree/atoms/open-tree-nodes-atom.ts";
import { createStore, Provider } from "jotai"; import { createStore, Provider } from "jotai";
import type { SpaceTreeNode } from "@/features/page/tree/types.ts"; import type { SpaceTreeNode } from "@/features/page/tree/types.ts";
@@ -134,6 +141,10 @@ function renderTree(store: ReturnType<typeof createStore>) {
beforeEach(() => { beforeEach(() => {
getSpaceTreeMock.mockReset(); getSpaceTreeMock.mockReset();
notificationsShowMock.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 // jsdom's localStorage shim here lacks `clear`; guard it. Each test uses a
// fresh jotai store anyway, so cross-test open-state never leaks. // fresh jotai store anyway, so cross-test open-state never leaks.
try { try {
@@ -30,6 +30,7 @@ import {
openBranches, openBranches,
closeIds, closeIds,
loadedOpenBranchIds, loadedOpenBranchIds,
pruneCollapsedChildren,
} from "@/features/page/tree/utils/utils.ts"; } from "@/features/page/tree/utils/utils.ts";
import { SpaceTreeNode } from "@/features/page/tree/types.ts"; import { SpaceTreeNode } from "@/features/page/tree/types.ts";
import { treeModel } from "@/features/page/tree/model/tree-model"; import { treeModel } from "@/features/page/tree/model/tree-model";
@@ -199,45 +200,81 @@ const SpaceTree = forwardRef<SpaceTreeApi, SpaceTreeProps>(function SpaceTree(
const openIdsRef = useRef(openIds); const openIdsRef = useRef(openIds);
openIdsRef.current = openIds; openIdsRef.current = openIds;
// Reconnect refresh (#159 #8): on a socket reconnect, re-fetch and reconcile // Boot-cache hygiene (#159 #8): the localStorage-hydrated tree carries the
// the children of every currently-open, already-loaded branch of THIS space, // 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 // 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. // were missed (laptop sleep / wifi gap) is reflected instead of left stale.
// The ROOT level is reconciled separately by the root-query refetch + // No first-connect guard is needed: space-tree usually mounts AFTER the
// mergeRootTrees; an UNLOADED branch is skipped (lazy-load fetches it fresh on // initial connect, so every `connect` it sees is a reconnect; the rare
// 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
// initial-connect case has an empty tree, so the refresh is a harmless no-op. // initial-connect case has an empty tree, so the refresh is a harmless no-op.
useEffect(() => { useEffect(() => {
if (!socket) return; if (!socket) return;
const onConnect = async () => { const onConnect = () => {
const effectSpaceId = spaceIdRef.current; refreshOpenBranches();
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);
}
}
}; };
socket.on("connect", onConnect); socket.on("connect", onConnect);
return () => { return () => {
socket.off("connect", onConnect); 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<Set<string>>(new Set());
useEffect(() => {
if (!isDataLoaded) return;
if (refreshedSpacesRef.current.has(spaceId)) return;
refreshedSpacesRef.current.add(spaceId);
refreshOpenBranches();
}, [isDataLoaded, spaceId, refreshOpenBranches]);
const handleToggle = useCallback( const handleToggle = useCallback(
async (id: string, isOpen: boolean) => { async (id: string, isOpen: boolean) => {
@@ -333,12 +370,17 @@ const SpaceTree = forwardRef<SpaceTreeApi, SpaceTreeProps>(function SpaceTree(
return ( return (
<div className={classes.treeContainer}> <div className={classes.treeContainer}>
{/* "No pages yet" only after the SERVER confirmed the space is empty —
never while just the localStorage cache is empty. */}
{isDataLoaded && filteredData.length === 0 && ( {isDataLoaded && filteredData.length === 0 && (
<Text size="xs" c="dimmed" py="xs" px="sm"> <Text size="xs" c="dimmed" py="xs" px="sm">
{t("No pages yet")} {t("No pages yet")}
</Text> </Text>
)} )}
{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 && (
<DocTree<SpaceTreeNode> <DocTree<SpaceTreeNode>
data={filteredData} data={filteredData}
openIds={openIds} openIds={openIds}
@@ -293,6 +293,41 @@ export function loadedOpenBranchIds(
return ids; 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<string>,
): 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 // Collect every node id in the tree (roots, branches, leaves). Used by
// collapseAll to clear the open-state map for all current-space nodes. // collapseAll to clear the open-state map for all current-space nodes.
export function collectAllIds(nodes: SpaceTreeNode[]): string[] { export function collectAllIds(nodes: SpaceTreeNode[]): string[] {
+7
View File
@@ -1,6 +1,7 @@
import axios, { AxiosInstance } from "axios"; import axios, { AxiosInstance } from "axios";
import APP_ROUTE from "@/lib/app-route.ts"; import APP_ROUTE from "@/lib/app-route.ts";
import { isCloud } from "@/lib/config.ts"; import { isCloud } from "@/lib/config.ts";
import { clearPersistedTreeCaches } from "@/features/page/tree/atoms/tree-data-atom";
const api: AxiosInstance = axios.create({ const api: AxiosInstance = axios.create({
baseURL: "/api", baseURL: "/api",
@@ -71,6 +72,12 @@ function redirectToLogin() {
"/invites", "/invites",
]; ];
if (!exemptPaths.some((path) => window.location.pathname.startsWith(path))) { 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; const redirectTo = window.location.pathname;
if (redirectTo === APP_ROUTE.HOME) { if (redirectTo === APP_ROUTE.HOME) {
window.location.href = APP_ROUTE.AUTH.LOGIN; window.location.href = APP_ROUTE.AUTH.LOGIN;