diff --git a/apps/client/src/features/comment/components/comment-content-view.test.tsx b/apps/client/src/features/comment/components/comment-content-view.test.tsx index 96e01961..812deb43 100644 --- a/apps/client/src/features/comment/components/comment-content-view.test.tsx +++ b/apps/client/src/features/comment/components/comment-content-view.test.tsx @@ -15,6 +15,7 @@ vi.mock("@/features/comment/components/comment-editor", () => ({ // case renders in isolation. vi.mock("@/features/page/queries/page-query.ts", () => ({ usePageQuery: () => ({ data: undefined, isLoading: false, isError: false }), + usePageMetaQuery: () => ({ data: undefined, isLoading: false, isError: false }), })); vi.mock("@/features/share/queries/share-query.ts", () => ({ useSharePageQuery: () => ({ data: undefined }), diff --git a/apps/client/src/features/page/components/breadcrumbs/breadcrumb-path-equal.test.ts b/apps/client/src/features/page/components/breadcrumbs/breadcrumb-path-equal.test.ts new file mode 100644 index 00000000..0002736d --- /dev/null +++ b/apps/client/src/features/page/components/breadcrumbs/breadcrumb-path-equal.test.ts @@ -0,0 +1,53 @@ +import { describe, it, expect, vi } from "vitest"; +import { SpaceTreeNode } from "@/features/page/tree/types"; + +// breadcrumb.tsx transitively imports @/main.tsx (via usePageMetaQuery -> +// queryClient), whose module body calls ReactDOM.createRoot on a null root in +// jsdom. Stub it so importing the pure helper under test doesn't run that +// (breadcrumbPathEqual does not use queryClient, so a dummy is enough). +vi.mock("@/main.tsx", () => ({ queryClient: {} })); + +import { breadcrumbPathEqual } from "./breadcrumb"; + +// breadcrumbPathEqual is the ONLY point where a false-positive equality would +// leave a stale/incorrect breadcrumb trail on screen: it decides whether the +// selectAtom hands back the same reference (no re-render) for the ancestor chain. +// Pin both directions — a too-loose equality goes stale on a rename; a too-tight +// one loses the perf win. +const node = (over: Partial): SpaceTreeNode => + ({ id: "a", slugId: "sa", name: "A", icon: "📄", ...over }) as SpaceTreeNode; + +describe("breadcrumbPathEqual", () => { + it("both null → true", () => { + expect(breadcrumbPathEqual(null, null)).toBe(true); + }); + + it("same reference → true", () => { + const p = [node({})]; + expect(breadcrumbPathEqual(p, p)).toBe(true); + }); + + it("equal by id/slugId/name/icon (different arrays) → true", () => { + expect(breadcrumbPathEqual([node({})], [node({})])).toBe(true); + }); + + it("one side null → false", () => { + expect(breadcrumbPathEqual([node({})], null)).toBe(false); + expect(breadcrumbPathEqual(null, [node({})])).toBe(false); + }); + + it("different length → false", () => { + expect( + breadcrumbPathEqual([node({})], [node({}), node({ id: "b" })]), + ).toBe(false); + }); + + it.each(["name", "icon", "slugId", "id"] as const)( + "a changed %s → false (breadcrumb must re-render)", + (field) => { + expect( + breadcrumbPathEqual([node({})], [node({ [field]: "CHANGED" })]), + ).toBe(false); + }, + ); +}); diff --git a/apps/client/src/features/page/components/breadcrumbs/breadcrumb.tsx b/apps/client/src/features/page/components/breadcrumbs/breadcrumb.tsx index a74ce7aa..1f988947 100644 --- a/apps/client/src/features/page/components/breadcrumbs/breadcrumb.tsx +++ b/apps/client/src/features/page/components/breadcrumbs/breadcrumb.tsx @@ -41,7 +41,7 @@ function getTitle(name: string, icon: string) { * visually unchanged, so the breadcrumb no longer re-renders on every tree * event (it previously subscribed to the whole treeDataAtom). */ -function breadcrumbPathEqual( +export function breadcrumbPathEqual( a: SpaceTreeNode[] | null, b: SpaceTreeNode[] | null, ): boolean { diff --git a/apps/client/src/features/page/queries/invalidate-on-update-page.test.ts b/apps/client/src/features/page/queries/invalidate-on-update-page.test.ts new file mode 100644 index 00000000..38b7f883 --- /dev/null +++ b/apps/client/src/features/page/queries/invalidate-on-update-page.test.ts @@ -0,0 +1,90 @@ +import { describe, it, expect, beforeEach, vi } from "vitest"; +import type { IPage } from "@/features/page/types/page.types"; + +// A fresh QueryClient stands in for the app singleton (importing the real +// @/main.tsx would run ReactDOM.createRoot, which has no DOM root in jsdom). The +// factory constructs it (QueryClient can't be referenced in vi.hoisted — that +// runs before imports resolve); we import the SAME mocked instance back to seed +// and assert on it. +vi.mock("@/main.tsx", async () => { + const { QueryClient } = await import("@tanstack/react-query"); + return { queryClient: new QueryClient() }; +}); + +import { queryClient as h_qc } from "@/main.tsx"; +import { invalidateOnUpdatePage } from "./page-query"; + +const h = { qc: h_qc }; + +// invalidateOnUpdatePage is the field-only (title/icon) tree path: instead of a +// blanket invalidate it patches the affected node IN PLACE in every cached embed +// subtree. The undefined-guard is LOAD-BEARING: a title-only socket event carries +// icon:undefined, and without the guard `{...p, icon: undefined}` would WIPE the +// icon in every cached subtree. +const page = (over: Partial): IPage => + ({ id: "p1", title: "Old", icon: "📄", spaceId: "s1" }) as IPage & + typeof over as IPage; + +describe("invalidateOnUpdatePage — pointwise embed-cache patch", () => { + beforeEach(() => { + h.qc.clear(); + }); + + it("title-only event updates title but PRESERVES the icon (undefined-guard)", () => { + const key = ["page-tree", "parent-1"]; + h.qc.setQueryData(key, [ + { id: "p1", title: "Old", icon: "📄", spaceId: "s1" } as IPage, + { id: "p2", title: "Other", icon: "📁", spaceId: "s1" } as IPage, + ]); + + // icon passed as undefined (a title-only update) + invalidateOnUpdatePage( + "s1", + "parent-1", + "p1", + "New Title", + undefined as unknown as string, + ); + + const patched = h.qc.getQueryData(key)!; + const p1 = patched.find((p) => p.id === "p1")!; + const p2 = patched.find((p) => p.id === "p2")!; + expect(p1.title).toBe("New Title"); + expect(p1.icon).toBe("📄"); // preserved, not wiped + // Sibling node untouched. + expect(p2.title).toBe("Other"); + expect(p2.icon).toBe("📁"); + }); + + it("icon-only event updates icon but preserves the title", () => { + const key = ["page-tree", "parent-1"]; + h.qc.setQueryData(key, [ + { id: "p1", title: "Keep", icon: "📄", spaceId: "s1" } as IPage, + ]); + + invalidateOnUpdatePage( + "s1", + "parent-1", + "p1", + undefined as unknown as string, + "🚀", + ); + + const p1 = h.qc.getQueryData(key)!.find((p) => p.id === "p1")!; + expect(p1.icon).toBe("🚀"); + expect(p1.title).toBe("Keep"); + }); + + it("does not touch a subtree that lacks the updated node", () => { + const otherKey = ["page-tree", "unrelated"]; + const before = [ + { id: "x1", title: "X", icon: "❌", spaceId: "s1" } as IPage, + ]; + h.qc.setQueryData(otherKey, before); + + invalidateOnUpdatePage("s1", "parent-1", "p1", "New", "🚀"); + + // Same reference back — the subtree without p1 is left as-is. + expect(h.qc.getQueryData(otherKey)).toBe(before); + }); +}); diff --git a/apps/client/src/features/page/queries/page-query.ts b/apps/client/src/features/page/queries/page-query.ts index 6abed784..525070b8 100644 --- a/apps/client/src/features/page/queries/page-query.ts +++ b/apps/client/src/features/page/queries/page-query.ts @@ -411,9 +411,11 @@ export function useRecentChangesQuery(spaceId?: string) { getNextPageParam: (lastPage) => lastPage.meta.hasNextPage ? lastPage.meta.nextCursor : undefined, // KEEP refetchOnMount:true (against the global default false): recent-changes - // is invalidated only while a mounted observer exists, and the widget isn't - // always mounted — an event that lands while it's unmounted marks it stale but - // the global refetchOnMount:false would not re-fetch on remount → stale list. + // IS invalidated on page create/update/move/delete, but invalidateQueries only + // marks an UNMOUNTED query stale — it doesn't refetch it. The widget isn't + // always mounted, so an event that lands while it's unmounted leaves it stale, + // and the global refetchOnMount:false would not re-fetch on remount. The mount + // refetch closes that gap. refetchOnMount: true, }); } @@ -447,10 +449,12 @@ export function useDeletedPagesQuery( enabled: !!spaceId, placeholderData: keepPreviousData, staleTime: 0, - // KEEP refetchOnMount:true: the "trash-list" key is never invalidated (no - // socket/mutation path), so opening the trash after deleting/restoring a page - // must refetch on mount — the global refetchOnMount:false would show a stale - // trash missing the just-deleted page until a hard reload. + // KEEP refetchOnMount:true: ["trash-list"] IS invalidated by the + // move-to-trash / delete / restore mutations, but invalidateQueries only marks + // an unmounted query stale — it doesn't refetch it. The trash panel isn't + // usually mounted when a page is trashed, so on opening it the global + // refetchOnMount:false would show a stale list; the mount refetch closes that. + // (Do NOT remove the three trash-list invalidations — they are not dead code.) refetchOnMount: true, }); } diff --git a/apps/client/src/features/share/components/share-modal.test.tsx b/apps/client/src/features/share/components/share-modal.test.tsx index c3d96afd..d9c9d01f 100644 --- a/apps/client/src/features/share/components/share-modal.test.tsx +++ b/apps/client/src/features/share/components/share-modal.test.tsx @@ -28,6 +28,7 @@ vi.mock("@/features/share/queries/share-query.ts", () => ({ vi.mock("@/features/page/queries/page-query.ts", () => ({ usePageQuery: () => ({ data: { id: "page-1", title: "Doc" } }), + usePageMetaQuery: () => ({ data: { id: "page-1", title: "Doc" } }), })); vi.mock("@/features/space/queries/space-query.ts", () => ({ diff --git a/apps/client/src/features/space/queries/space-query.ts b/apps/client/src/features/space/queries/space-query.ts index 96425808..b98f258c 100644 --- a/apps/client/src/features/space/queries/space-query.ts +++ b/apps/client/src/features/space/queries/space-query.ts @@ -38,6 +38,12 @@ export function useGetSpacesQuery( queryKey: ["spaces", params], queryFn: () => getSpaces(params), placeholderData: keepPreviousData, + // KEEP refetchOnMount:true (against the global default false): the ["spaces"] + // key is invalidated only by same-tab mutations (no socket path), so a + // cross-actor change — an admin adding/removing THIS user from a space — has + // no local mutation or socket event and would leave the space list stale until + // a hard reload. The mount refetch is its only cross-actor freshness path. + refetchOnMount: true, }); }