From fcbe840c740734828cae24c45bca6c2a53b5067a Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sun, 5 Jul 2026 00:34:51 +0300 Subject: [PATCH 1/2] perf(client): cut background re-renders + duplicate work (#344) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Outside the editor the UI did background work on every tree event, socket reconnect, and navigation. Tree infra (virtualization/memo/O(N) utils) was already good — the cost was in the subscriptions and duplicates around it. Client-only; behavior 1:1. - Setter-only atom subscriptions → useSetAtom: space-tree-row, use-tree-mutation, use-tree-socket no longer subscribe every visible row to the WHOLE treeDataAtom value (a tree event re-rendered all ~20-30 rows, bypassing the DocTreeRow memo). space-tree-node-menu / mention-list read the tree imperatively (store.get) in their handlers only. breadcrumb.tsx uses a selectAtom slice (ancestor chain + field equality) instead of the whole-tree subscription. - Socket handler cleanup (BUG): use-tree-socket + use-query-subscription now socket.off() their named handlers on cleanup (were accumulating listeners on every reconnect → duplicated invalidations/tree-walks). Mirrors use-notification-socket. - Field-update tree path: invalidateOnUpdatePage does a pointwise patch of the cached embed subtrees instead of a blanket invalidatePageTree() (refetch storm); structural events keep the blanket invalidate. - usePageMetaQuery: a content-less select slice for the 13 peripheral subscribers that read only title/permissions/id, so they stop re-rendering every ~3s while typing / on every collab page.updated (page.tsx keeps the full query for content). - page.tsx: skeleton + placeholderData keepPreviousData (no blank flash on nav). - Removed refetchOnMount:true where socket/mutation invalidation already keeps the cache fresh (favorite/space/space-watcher/workspace). KEPT it on the 3 queries with NO other freshness path (trash-list, created-by, recent-changes) — the global default is refetchOnMount:false, so those overrides are load-bearing. - Small: resize mousemove/up attached only while dragging; per-row emoji-picker keydown gated on `opened`; AiChatWindow queries enabled only when the window is open. Gate: client tsc 0, client vitest page+websocket 200 passed (+editor suites), build ok. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../layouts/global/global-app-shell.tsx | 10 +- .../client/src/components/ui/emoji-picker.tsx | 26 +++-- .../ai-chat/components/ai-chat-window.tsx | 12 ++- .../features/ai-chat/queries/ai-chat-query.ts | 16 ++- .../components/comment-list-with-tabs.tsx | 4 +- .../editor/components/link/link-view.tsx | 4 +- .../components/mention/mention-list.tsx | 18 ++-- .../components/mention/mention-view.tsx | 4 +- .../favorite/queries/favorite-query.ts | 2 - .../components/page-details-aside.tsx | 4 +- .../components/breadcrumbs/breadcrumb.tsx | 69 ++++++++++-- .../components/header/page-header-menu.tsx | 6 +- .../page/components/temporary-note-banner.tsx | 4 +- .../src/features/page/queries/page-query.ts | 102 +++++++++++++++++- .../trash/components/deleted-page-banner.tsx | 4 +- .../tree/components/space-tree-node-menu.tsx | 12 ++- .../page/tree/components/space-tree-row.tsx | 8 +- .../components/space-tree.boot-cache.test.tsx | 1 + .../components/space-tree.expand-all.test.tsx | 1 + .../page/tree/components/space-tree.tsx | 4 +- .../page/tree/hooks/use-tree-mutation.ts | 7 +- .../features/share/components/share-modal.tsx | 4 +- .../src/features/space/queries/space-query.ts | 1 - .../space/queries/space-watcher-query.ts | 1 - .../websocket/use-query-subscription.ts | 13 ++- .../src/features/websocket/use-tree-socket.ts | 22 +++- .../workspace/queries/workspace-query.ts | 1 - apps/client/src/pages/page/page-redirect.tsx | 4 +- apps/client/src/pages/page/page.tsx | 21 +++- 29 files changed, 303 insertions(+), 82 deletions(-) diff --git a/apps/client/src/components/layouts/global/global-app-shell.tsx b/apps/client/src/components/layouts/global/global-app-shell.tsx index 1b41011e..ea4ab05c 100644 --- a/apps/client/src/components/layouts/global/global-app-shell.tsx +++ b/apps/client/src/components/layouts/global/global-app-shell.tsx @@ -67,14 +67,20 @@ export default function GlobalAppShell({ ); useEffect(() => { - //https://codesandbox.io/p/sandbox/kz9de + // Attach the global mousemove/mouseup only WHILE resizing (started on the + // handle's mousedown via startResizing → isResizing=true) and detach on + // mouseup (stopResizing → isResizing=false). Previously these listeners were + // attached for the whole app lifetime, so every mouse move over the app ran + // the resize handler. + // https://codesandbox.io/p/sandbox/kz9de + if (!isResizing) return; window.addEventListener("mousemove", resize); window.addEventListener("mouseup", stopResizing); return () => { window.removeEventListener("mousemove", resize); window.removeEventListener("mouseup", stopResizing); }; - }, [resize, stopResizing]); + }, [isResizing, resize, stopResizing]); const location = useLocation(); const isSettingsRoute = location.pathname.startsWith("/settings"); diff --git a/apps/client/src/components/ui/emoji-picker.tsx b/apps/client/src/components/ui/emoji-picker.tsx index c360998a..a36e3ef8 100644 --- a/apps/client/src/components/ui/emoji-picker.tsx +++ b/apps/client/src/components/ui/emoji-picker.tsx @@ -5,7 +5,7 @@ import { Button, useMantineColorScheme, } from "@mantine/core"; -import { useClickOutside, useDisclosure, useWindowEvent } from "@mantine/hooks"; +import { useClickOutside, useDisclosure } from "@mantine/hooks"; import { Suspense } from "react"; import { useTranslation } from "react-i18next"; @@ -57,14 +57,22 @@ function EmojiPicker({ [dropdown, target], ); - // We need this because the default Mantine popover closeOnEscape does not work - useWindowEvent("keydown", (event) => { - if (opened && event.key === "Escape") { - event.stopPropagation(); - event.preventDefault(); - handlers.close(); - } - }); + // We need this because the default Mantine popover closeOnEscape does not work. + // Attach the global keydown ONLY while the picker is open (every tree row + // renders an EmojiPicker, so an always-on window listener meant ~20-30 idle + // keydown handlers firing on each keystroke). + useEffect(() => { + if (!opened) return; + const handleKeydown = (event: KeyboardEvent) => { + if (event.key === "Escape") { + event.stopPropagation(); + event.preventDefault(); + handlers.close(); + } + }; + window.addEventListener("keydown", handleKeydown); + return () => window.removeEventListener("keydown", handleKeydown); + }, [opened, handlers]); // emoji-mart's built-in autoFocus calls .focus() without preventScroll, which // makes the browser scroll every scrollable ancestor of the search input to diff --git a/apps/client/src/features/ai-chat/components/ai-chat-window.tsx b/apps/client/src/features/ai-chat/components/ai-chat-window.tsx index c26bfa2d..e2d6bdd4 100644 --- a/apps/client/src/features/ai-chat/components/ai-chat-window.tsx +++ b/apps/client/src/features/ai-chat/components/ai-chat-window.tsx @@ -36,7 +36,7 @@ import { desktopSidebarAtom, mobileSidebarAtom, } from "@/components/layouts/global/hooks/atoms/sidebar-atom.ts"; -import { usePageQuery } from "@/features/page/queries/page-query.ts"; +import { usePageMetaQuery } from "@/features/page/queries/page-query.ts"; import { extractPageSlugId } from "@/lib"; import { AI_CHATS_RQ_KEY, @@ -219,7 +219,9 @@ export default function AiChatWindow() { // left partly off-screen). const [geom, setGeom] = useAtom(aiChatWindowGeomAtom); - const { data: chats } = useAiChatsQuery(); + // Gated on windowOpen: the chat list is only needed once the window is open, + // so a closed window issues no chat-list request/refetch on navigation. + const { data: chats } = useAiChatsQuery(windowOpen); // Roles for the new-chat picker (any member may list them). Only fetched while // the window is open. const { data: roles } = useAiRolesQuery(windowOpen); @@ -231,8 +233,10 @@ export default function AiChatWindow() { [roles], ); + // Gated on windowOpen too: no message history is fetched while the window is + // closed (it is loaded when the window opens with an active chat). const { data: messageRows, isLoading: messagesLoading } = - useAiChatMessagesQuery(activeChatId ?? undefined); + useAiChatMessagesQuery(activeChatId ?? undefined, windowOpen); // The page the user is currently viewing. AiChatWindow lives in a pathless // parent layout route, so useParams() can't see :pageSlug. Match the full @@ -244,7 +248,7 @@ export default function AiChatWindow() { // reads/writes via its CASL-enforced page tools using the id. const pageRouteMatch = useMatch("/s/:spaceSlug/p/:pageSlug"); const pageSlug = pageRouteMatch?.params?.pageSlug; - const { data: openPageData } = usePageQuery({ + const { data: openPageData } = usePageMetaQuery({ pageId: extractPageSlugId(pageSlug), }); const openPage = openPageData diff --git a/apps/client/src/features/ai-chat/queries/ai-chat-query.ts b/apps/client/src/features/ai-chat/queries/ai-chat-query.ts index 37cf70f8..887a4770 100644 --- a/apps/client/src/features/ai-chat/queries/ai-chat-query.ts +++ b/apps/client/src/features/ai-chat/queries/ai-chat-query.ts @@ -52,8 +52,12 @@ export const AI_CHAT_MESSAGES_RQ_KEY = (chatId: string) => [ chatId, ]; -/** Paginated list of the current user's chats (auto-loads further pages). */ -export function useAiChatsQuery() { +/** + * Paginated list of the current user's chats (auto-loads further pages). + * `enabled` (default true) lets the AI chat window skip fetching while it is + * closed — the list is only needed once the window is open. + */ +export function useAiChatsQuery(enabled: boolean = true) { const query = useInfiniteQuery({ queryKey: AI_CHATS_RQ_KEY, queryFn: ({ pageParam }) => @@ -61,6 +65,7 @@ export function useAiChatsQuery() { initialPageParam: undefined as string | undefined, getNextPageParam: (lastPage) => lastPage.meta.hasNextPage ? (lastPage.meta.nextCursor ?? undefined) : undefined, + enabled, }); const data = useMemo | undefined>(() => { @@ -83,7 +88,10 @@ export function useAiChatsQuery() { * Load all persisted messages of a chat (oldest first), flattening the * paginated server response. Used to seed `useChat` initial messages. */ -export function useAiChatMessagesQuery(chatId: string | undefined) { +export function useAiChatMessagesQuery( + chatId: string | undefined, + enabled: boolean = true, +) { const query = useInfiniteQuery({ queryKey: AI_CHAT_MESSAGES_RQ_KEY(chatId ?? ""), queryFn: ({ pageParam }) => @@ -91,7 +99,7 @@ export function useAiChatMessagesQuery(chatId: string | undefined) { initialPageParam: undefined as string | undefined, getNextPageParam: (lastPage) => lastPage.meta.hasNextPage ? (lastPage.meta.nextCursor ?? undefined) : undefined, - enabled: !!chatId, + enabled: !!chatId && enabled, }); // useInfiniteQuery only fetches the first page on its own. The hook's contract diff --git a/apps/client/src/features/comment/components/comment-list-with-tabs.tsx b/apps/client/src/features/comment/components/comment-list-with-tabs.tsx index b5e62e8e..1b667e47 100644 --- a/apps/client/src/features/comment/components/comment-list-with-tabs.tsx +++ b/apps/client/src/features/comment/components/comment-list-with-tabs.tsx @@ -22,7 +22,7 @@ import CommentEditor from "@/features/comment/components/comment-editor"; import CommentActions from "@/features/comment/components/comment-actions"; import { useFocusWithin } from "@mantine/hooks"; import { IComment } from "@/features/comment/types/comment.types.ts"; -import { usePageQuery } from "@/features/page/queries/page-query.ts"; +import { usePageMetaQuery } from "@/features/page/queries/page-query.ts"; import { extractPageSlugId } from "@/lib"; import { useTranslation } from "react-i18next"; import { useGetSpaceBySlugQuery } from "@/features/space/queries/space-query.ts"; @@ -56,7 +56,7 @@ export function buildChildrenByParent( function CommentListWithTabs({ onClose }: CommentListWithTabsProps) { const { t } = useTranslation(); const { pageSlug } = useParams(); - const { data: page } = usePageQuery({ pageId: extractPageSlugId(pageSlug) }); + const { data: page } = usePageMetaQuery({ pageId: extractPageSlugId(pageSlug) }); const { data: comments, isLoading: isCommentsLoading, diff --git a/apps/client/src/features/editor/components/link/link-view.tsx b/apps/client/src/features/editor/components/link/link-view.tsx index 46227ef9..d4b503f4 100644 --- a/apps/client/src/features/editor/components/link/link-view.tsx +++ b/apps/client/src/features/editor/components/link/link-view.tsx @@ -24,7 +24,7 @@ import classes from "./link.module.css"; import { useTranslation } from "react-i18next"; import { INTERNAL_LINK_REGEX } from "@/lib/constants"; import { LinkEditorPanel } from "@/features/editor/components/link/link-editor-panel.tsx"; -import { usePageQuery } from "@/features/page/queries/page-query.ts"; +import { usePageMetaQuery } from "@/features/page/queries/page-query.ts"; import { useSharePageQuery } from "@/features/share/queries/share-query.ts"; import { buildSharedPageUrl } from "@/features/page/page.utils.ts"; import { extractPageSlugId } from "@/lib"; @@ -83,7 +83,7 @@ export default function LinkView(props: MarkViewProps) { const isPopoverVisible = popoverState !== "closed"; const activeView = isPopoverVisible ? popoverState : lastOpenState.current; - const { data: linkedPage } = usePageQuery({ + const { data: linkedPage } = usePageMetaQuery({ pageId: isPopoverVisible && slugId && !isShareRoute ? slugId : null, }); diff --git a/apps/client/src/features/editor/components/mention/mention-list.tsx b/apps/client/src/features/editor/components/mention/mention-list.tsx index a098a1af..8e9a2b4b 100644 --- a/apps/client/src/features/editor/components/mention/mention-list.tsx +++ b/apps/client/src/features/editor/components/mention/mention-list.tsx @@ -25,7 +25,7 @@ import { IconFileDescription, IconPlus } from "@tabler/icons-react"; import { useSpaceQuery } from "@/features/space/queries/space-query.ts"; import { useParams } from "react-router-dom"; import { v7 as uuid7 } from "uuid"; -import { useAtom } from "jotai"; +import { useAtom, useSetAtom, useStore } from "jotai"; import { currentUserAtom } from "@/features/user/atoms/current-user-atom.ts"; import { MentionListProps, @@ -34,7 +34,7 @@ import { import { IPage } from "@/features/page/types/page.types"; import { useCreatePageMutation, - usePageQuery, + usePageMetaQuery, } from "@/features/page/queries/page-query"; import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom"; import { treeModel } from "@/features/page/tree/model/tree-model"; @@ -50,12 +50,16 @@ const MentionList = forwardRef((props, ref) => { const [countAnnouncement, setCountAnnouncement] = useState(""); const [selectionAnnouncement, setSelectionAnnouncement] = useState(""); const { pageSlug, spaceSlug } = useParams(); - const { data: page } = usePageQuery({ pageId: extractPageSlugId(pageSlug) }); + const { data: page } = usePageMetaQuery({ pageId: extractPageSlugId(pageSlug) }); const { data: space } = useSpaceQuery(spaceSlug); const [currentUser] = useAtom(currentUserAtom); const [renderItems, setRenderItems] = useState([]); const { t } = useTranslation(); - const [data, setData] = useAtom(treeDataAtom); + // Setter-only: the tree value is read only imperatively inside createPage + // (via `store` below), never at render, so useSetAtom avoids re-rendering the + // mention popup on any tree event. + const setData = useSetAtom(treeDataAtom); + const store = useStore(); const createPageMutation = useCreatePageMutation(); const emit = useQueryEmit(); const isInCommentContext = props.isInCommentContext ?? false; @@ -272,9 +276,11 @@ const MentionList = forwardRef((props, ref) => { children: [], }; - const lastIndex = data.length; + // Read the live tree imperatively at call time. + const currentTree = store.get(treeDataAtom); + const lastIndex = currentTree.length; - setData(treeModel.insert(data, parentId, newNode, lastIndex)); + setData(treeModel.insert(currentTree, parentId, newNode, lastIndex)); props.command({ id: uuid7(), diff --git a/apps/client/src/features/editor/components/mention/mention-view.tsx b/apps/client/src/features/editor/components/mention/mention-view.tsx index fa0b6d52..7a88b44f 100644 --- a/apps/client/src/features/editor/components/mention/mention-view.tsx +++ b/apps/client/src/features/editor/components/mention/mention-view.tsx @@ -2,7 +2,7 @@ import { NodeViewProps, NodeViewWrapper } from "@tiptap/react"; import { ActionIcon, Anchor, Text } from "@mantine/core"; import { IconFileDescription } from "@tabler/icons-react"; import { Link, useLocation, useNavigate, useParams } from "react-router-dom"; -import { usePageQuery } from "@/features/page/queries/page-query.ts"; +import { usePageMetaQuery } from "@/features/page/queries/page-query.ts"; import { useSharePageQuery } from "@/features/share/queries/share-query.ts"; import { buildPageUrl, @@ -36,7 +36,7 @@ export function MentionContent({ attrs }: { attrs: MentionAttrs }) { data: page, isLoading, isError, - } = usePageQuery({ pageId: isPageMention && !isShareRoute ? slugId : null }); + } = usePageMetaQuery({ pageId: isPageMention && !isShareRoute ? slugId : null }); const { data: sharedPage } = useSharePageQuery({ pageId: isPageMention && isShareRoute ? slugId : undefined, diff --git a/apps/client/src/features/favorite/queries/favorite-query.ts b/apps/client/src/features/favorite/queries/favorite-query.ts index 51fd7856..eb83c4ac 100644 --- a/apps/client/src/features/favorite/queries/favorite-query.ts +++ b/apps/client/src/features/favorite/queries/favorite-query.ts @@ -24,7 +24,6 @@ export function useFavoritesQuery(type?: FavoriteType, spaceId?: string) { initialPageParam: undefined as string | undefined, getNextPageParam: (lastPage) => lastPage.meta.hasNextPage ? lastPage.meta.nextCursor : undefined, - refetchOnMount: true, }); } @@ -32,7 +31,6 @@ export function useFavoriteIds(type: FavoriteType, spaceId?: string): Set getFavoriteIds(type, spaceId), - refetchOnMount: true, }); const items = data?.items; diff --git a/apps/client/src/features/page-details/components/page-details-aside.tsx b/apps/client/src/features/page-details/components/page-details-aside.tsx index 89c51027..b6285828 100644 --- a/apps/client/src/features/page-details/components/page-details-aside.tsx +++ b/apps/client/src/features/page-details/components/page-details-aside.tsx @@ -12,7 +12,7 @@ import { useAtomValue } from "jotai"; import { useParams } from "react-router-dom"; import { useTranslation } from "react-i18next"; import { extractPageSlugId } from "@/lib"; -import { usePageQuery } from "@/features/page/queries/page-query.ts"; +import { usePageMetaQuery } from "@/features/page/queries/page-query.ts"; import { pageEditorAtom } from "@/features/editor/atoms/editor-atoms.ts"; import { useBacklinksCountQuery } from "@/features/page-details/queries/backlinks-query.ts"; import { BacklinksModal } from "./backlinks-modal"; @@ -23,7 +23,7 @@ import { LabelsSection } from "@/features/label/components/labels-section.tsx"; export function PageDetailsAside() { const { pageSlug } = useParams(); - const { data: page } = usePageQuery({ + const { data: page } = usePageMetaQuery({ pageId: extractPageSlugId(pageSlug), }); const pageEditor = useAtomValue(pageEditorAtom); diff --git a/apps/client/src/features/page/components/breadcrumbs/breadcrumb.tsx b/apps/client/src/features/page/components/breadcrumbs/breadcrumb.tsx index feec4a5b..a74ce7aa 100644 --- a/apps/client/src/features/page/components/breadcrumbs/breadcrumb.tsx +++ b/apps/client/src/features/page/components/breadcrumbs/breadcrumb.tsx @@ -1,7 +1,9 @@ import { useAtomValue } from "jotai"; +import { selectAtom } from "jotai/utils"; import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom.ts"; -import React, { useCallback, useEffect, useState } from "react"; +import React, { useCallback, useEffect, useMemo, useState } from "react"; import { computeBreadcrumbState } from "./breadcrumb.utils"; +import { findBreadcrumbPath } from "@/features/page/tree/utils"; import { Button, Anchor, @@ -18,7 +20,7 @@ import { SpaceTreeNode } from "@/features/page/tree/types.ts"; import { IPage } from "@/features/page/types/page.types.ts"; import { buildPageUrl } from "@/features/page/page.utils.ts"; import { - usePageQuery, + usePageMetaQuery, usePageBreadcrumbsQuery, } from "@/features/page/queries/page-query.ts"; import { extractPageSlugId } from "@/lib"; @@ -32,39 +34,84 @@ function getTitle(name: string, icon: string) { return name; } +/** + * Equality over a breadcrumb chain by the only fields the breadcrumb renders + * (id, slugId, name, icon). Lets the selectAtom below hand back the SAME + * reference when an unrelated tree mutation leaves THIS page's ancestor chain + * visually unchanged, so the breadcrumb no longer re-renders on every tree + * event (it previously subscribed to the whole treeDataAtom). + */ +function breadcrumbPathEqual( + a: SpaceTreeNode[] | null, + b: SpaceTreeNode[] | null, +): boolean { + if (a === b) return true; + if (!a || !b || a.length !== b.length) return false; + for (let i = 0; i < a.length; i++) { + if ( + a[i].id !== b[i].id || + a[i].slugId !== b[i].slugId || + a[i].name !== b[i].name || + a[i].icon !== b[i].icon + ) { + return false; + } + } + return true; +} + export default function Breadcrumb() { const { t } = useTranslation(); - const treeData = useAtomValue(treeDataAtom); const [breadcrumbNodes, setBreadcrumbNodes] = useState< SpaceTreeNode[] | null >(null); const { pageSlug, spaceSlug } = useParams(); - const { data: currentPage } = usePageQuery({ + const { data: currentPage } = usePageMetaQuery({ pageId: extractPageSlugId(pageSlug), }); + const currentPageId = currentPage?.id; // The page's own ancestor chain, fetched independently of the lazily-built // sidebar tree so a deep page doesn't render a blank breadcrumb for seconds // while the tree backfills (#218). - const { data: ancestors } = usePageBreadcrumbsQuery(currentPage?.id); + const { data: ancestors } = usePageBreadcrumbsQuery(currentPageId); const isMobile = useMediaQuery("(max-width: 48em)"); + // Narrowed subscription: instead of subscribing to the whole treeDataAtom and + // recomputing on every tree event, derive ONLY the current page's ancestor + // chain. The custom equality returns the previous reference when that chain is + // visually unchanged, so an unrelated tree mutation no longer re-renders this + // component. Mirrors computeBreadcrumbState's tree-hit branch + // (findBreadcrumbPath); the tree-miss/ancestors fallback is applied below. + const treePathAtom = useMemo( + () => + selectAtom( + treeDataAtom, + (tree): SpaceTreeNode[] | null => + currentPageId ? findBreadcrumbPath(tree, currentPageId) : null, + breadcrumbPathEqual, + ), + [currentPageId], + ); + const treePath = useAtomValue(treePathAtom); + useEffect(() => { if (!currentPage) return; // Selection/mapping + stale-clearing live in a pure, unit-tested helper - // (#218). It resolves the correct chain when possible and, on a transient - // miss, clears a chain left over from a previously-viewed page instead of - // showing the wrong trail — while keeping a chain already resolved for THIS - // page to avoid a blank flash. + // (#218). The tree-hit chain (treePath) always wins when present; otherwise + // fall back to the page's own ancestors and the stale-clearing logic — this + // reproduces computeBreadcrumbState(fullTree, ancestors, …) exactly, since + // its tree-hit branch is precisely findBreadcrumbPath(fullTree, pageId). setBreadcrumbNodes((previous) => + treePath ?? computeBreadcrumbState( - treeData, + null, ancestors as IPage[] | undefined, currentPage.id, previous, ), ); - }, [currentPage?.id, treeData, ancestors]); + }, [currentPage?.id, treePath, ancestors]); const HiddenNodesTooltipContent = () => breadcrumbNodes?.slice(1, -1).map((node) => ( diff --git a/apps/client/src/features/page/components/header/page-header-menu.tsx b/apps/client/src/features/page/components/header/page-header-menu.tsx index 732d9136..dfd4059a 100644 --- a/apps/client/src/features/page/components/header/page-header-menu.tsx +++ b/apps/client/src/features/page/components/header/page-header-menu.tsx @@ -24,7 +24,7 @@ import { historyAtoms } from "@/features/page-history/atoms/history-atoms.ts"; import { useDisclosure, useHotkeys } from "@mantine/hooks"; import { useClipboard } from "@/hooks/use-clipboard"; import { useParams } from "react-router-dom"; -import { usePageQuery } from "@/features/page/queries/page-query.ts"; +import { usePageMetaQuery } from "@/features/page/queries/page-query.ts"; import { useToggleTemporaryMutation, syncTemporaryExpiresInCache, @@ -67,7 +67,7 @@ export default function PageHeaderMenu({ readOnly }: PageHeaderMenuProps) { const commentsTriggerProps = useAsideTriggerProps("comments"); const tocTriggerProps = useAsideTriggerProps("toc"); const { pageSlug } = useParams(); - const { data: page } = usePageQuery({ + const { data: page } = usePageMetaQuery({ pageId: extractPageSlugId(pageSlug), }); const isDeleted = !!page?.deletedAt; @@ -146,7 +146,7 @@ function PageActionMenu({ readOnly }: PageActionMenuProps) { const [, setHistoryModalOpen] = useAtom(historyAtoms); const clipboard = useClipboard({ timeout: 500 }); const { pageSlug, spaceSlug } = useParams(); - const { data: page, isLoading } = usePageQuery({ + const { data: page, isLoading } = usePageMetaQuery({ pageId: extractPageSlugId(pageSlug), }); const { handleDelete } = useTreeMutation(page?.spaceId ?? ""); diff --git a/apps/client/src/features/page/components/temporary-note-banner.tsx b/apps/client/src/features/page/components/temporary-note-banner.tsx index d30087f1..dcd81088 100644 --- a/apps/client/src/features/page/components/temporary-note-banner.tsx +++ b/apps/client/src/features/page/components/temporary-note-banner.tsx @@ -10,7 +10,7 @@ import { IconClockHour4, IconTrash } from "@tabler/icons-react"; import { useState } from "react"; import { Trans, useTranslation } from "react-i18next"; import { useTimeAgo } from "@/hooks/use-time-ago.tsx"; -import { usePageQuery } from "@/features/page/queries/page-query.ts"; +import { usePageMetaQuery } from "@/features/page/queries/page-query.ts"; import { useTreeMutation } from "@/features/page/tree/hooks/use-tree-mutation.ts"; import { useToggleTemporaryMutation, @@ -35,7 +35,7 @@ type TemporaryNoteBannerProps = { */ export function TemporaryNoteBanner({ slugId }: TemporaryNoteBannerProps) { const { t } = useTranslation(); - const { data: page } = usePageQuery({ pageId: slugId }); + const { data: page } = usePageMetaQuery({ pageId: slugId }); const { data: space } = useGetSpaceBySlugQuery(page?.space?.slug); const spaceAbility = useSpaceAbility(space?.membership?.permissions); const expiresTimeAgo = useTimeAgo(page?.temporaryExpiresAt); diff --git a/apps/client/src/features/page/queries/page-query.ts b/apps/client/src/features/page/queries/page-query.ts index 7837f8db..6abed784 100644 --- a/apps/client/src/features/page/queries/page-query.ts +++ b/apps/client/src/features/page/queries/page-query.ts @@ -51,6 +51,10 @@ export function usePageQuery( queryFn: () => getPageById(pageInput), enabled: !!pageInput.pageId, staleTime: 5 * 60 * 1000, + // Keep the previously-loaded page visible while navigating to a new one + // instead of flashing a blank/skeleton frame (the new page's content + // streams in when ready). isLoading stays true only for the very first load. + placeholderData: keepPreviousData, }); useEffect(() => { @@ -66,6 +70,61 @@ export function usePageQuery( return query; } +/** + * A page view that omits the large, frequently-changing `content` field. Every + * other field is preserved, so consumers that read only metadata (title, icon, + * permissions, id, creator, timestamps, …) keep working unchanged. + */ +export type IPageMeta = Omit; + +function selectPageMeta(page: IPage): IPageMeta { + // Drop `content`; react-query's structural sharing (replaceEqualDeep) then + // returns the SAME reference whenever the remaining fields are unchanged, so a + // pure content churn (typing / debouncedUpdateContent, collab `page.updated`) + // no longer changes this slice's identity and its ~13 subscribers don't + // re-render on every keystroke wave. + const { content: _content, ...meta } = page; + return meta as IPageMeta; +} + +/** + * Metadata-only variant of {@link usePageQuery}. Shares the SAME query cache + * entry (`["pages", pageId]`, full object incl. content), but this hook returns + * a stable content-less slice so peripheral subscribers stop re-rendering on + * every content update. Use it anywhere the full `content` is not read. + */ +export function usePageMetaQuery( + pageInput: Partial, +): UseQueryResult { + const query = useQuery({ + queryKey: ["pages", pageInput.pageId], + queryFn: () => getPageById(pageInput), + enabled: !!pageInput.pageId, + staleTime: 5 * 60 * 1000, + select: selectPageMeta, + // Match usePageQuery: keep the previous page's metadata visible while + // navigating so the periphery (header, breadcrumb, …) doesn't flash blank. + placeholderData: keepPreviousData, + }); + + // Mirror usePageQuery's cross-key alias write so a page fetched by one + // identifier is also cached under the other. The cache stores the FULL page + // (select only narrows what THIS hook returns), so read the full object back + // from the cache and alias THAT — never the content-less slice. + useEffect(() => { + if (!query.data) return; + const full = queryClient.getQueryData(["pages", pageInput.pageId]); + if (!full) return; + if (isValidUuid(pageInput.pageId)) { + queryClient.setQueryData(["pages", full.slugId], full); + } else { + queryClient.setQueryData(["pages", full.id], full); + } + }, [query.data]); + + return query; +} + export function useCreatePageMutation() { const { t } = useTranslation(); return useMutation>({ @@ -351,6 +410,10 @@ export function useRecentChangesQuery(spaceId?: string) { initialPageParam: undefined as string | undefined, 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. refetchOnMount: true, }); } @@ -367,6 +430,9 @@ export function useCreatedByQuery(params?: { initialPageParam: undefined as string | undefined, getNextPageParam: (lastPage) => lastPage.meta.hasNextPage ? lastPage.meta.nextCursor : undefined, + // KEEP refetchOnMount:true: the "created-by" key is never invalidated (no + // socket/mutation path), so the mount refetch is its ONLY freshness mechanism + // — without it the list shows stale cache on navigation. refetchOnMount: true, }); } @@ -380,8 +446,12 @@ export function useDeletedPagesQuery( queryFn: () => getDeletedPages(spaceId, params), enabled: !!spaceId, placeholderData: keepPreviousData, - refetchOnMount: true, 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. + refetchOnMount: true, }); } @@ -516,7 +586,35 @@ export function invalidateOnUpdatePage( title: string, icon: string, ) { - invalidatePageTree(); + // Scoped page-tree refresh (was a blanket `invalidatePageTree()`): this is the + // FIELD-only update path (title/icon — no structural change), and the sidebar + // tree is already updated pointwise (applyUpdateOne / optimistic setData) plus + // via the sidebar-pages cache below. Invalidating ALL ["page-tree"] queries + // here refetched every open recursive subpages-embed block on each + // rename/icon-change — pure duplicate work. Instead patch just the affected + // node IN PLACE in every cached embed subtree: same visible result, no network + // churn, no full embed-tree rebuild. Structural events (create/move/delete) + // keep the blanket invalidate in their own helpers. + const pageTreeMatches = queryClient.getQueriesData({ + queryKey: ["page-tree"], + }); + pageTreeMatches.forEach(([key, items]) => { + if (!items || !items.some((p) => p.id === id)) return; + queryClient.setQueryData(key, (old) => + old?.map((p) => + p.id === id + ? { + ...p, + // Guard undefined so a title-only event can't wipe the icon (and + // vice versa) in the embed cache. + ...(title !== undefined ? { title } : {}), + ...(icon !== undefined ? { icon } : {}), + } + : p, + ), + ); + }); + let queryKey: QueryKey = null; if (parentPageId === null) { queryKey = ["root-sidebar-pages", spaceId]; diff --git a/apps/client/src/features/page/trash/components/deleted-page-banner.tsx b/apps/client/src/features/page/trash/components/deleted-page-banner.tsx index f01a6ab4..74f34e65 100644 --- a/apps/client/src/features/page/trash/components/deleted-page-banner.tsx +++ b/apps/client/src/features/page/trash/components/deleted-page-banner.tsx @@ -7,7 +7,7 @@ import { useRestorePageModal } from "@/features/page/hooks/use-restore-page-moda import { useDeletePageModal } from "@/features/page/hooks/use-delete-page-modal.tsx"; import { useDeletePageMutation, - usePageQuery, + usePageMetaQuery, useRestorePageMutation, } from "@/features/page/queries/page-query.ts"; import { getSpaceUrl } from "@/lib/config.ts"; @@ -25,7 +25,7 @@ type DeletedPageBannerProps = { export function DeletedPageBanner({ slugId }: DeletedPageBannerProps) { const { t } = useTranslation(); const navigate = useNavigate(); - const { data: page } = usePageQuery({ pageId: slugId }); + const { data: page } = usePageMetaQuery({ pageId: slugId }); const { data: space } = useGetSpaceBySlugQuery(page?.space?.slug); const spaceAbility = useSpaceAbility(space?.membership?.permissions); const deletedTimeAgo = useTimeAgo(page?.deletedAt); diff --git a/apps/client/src/features/page/tree/components/space-tree-node-menu.tsx b/apps/client/src/features/page/tree/components/space-tree-node-menu.tsx index 63ddd98e..a689f52f 100644 --- a/apps/client/src/features/page/tree/components/space-tree-node-menu.tsx +++ b/apps/client/src/features/page/tree/components/space-tree-node-menu.tsx @@ -1,4 +1,4 @@ -import { useAtom } from "jotai"; +import { useSetAtom, useStore } from "jotai"; import { useTranslation } from "react-i18next"; import { useParams } from "react-router-dom"; import { ActionIcon, Menu, rem } from "@mantine/core"; @@ -52,7 +52,11 @@ export function NodeMenu({ node, canEdit }: NodeMenuProps) { const clipboard = useClipboard({ timeout: 500 }); const { spaceSlug } = useParams(); const { handleDelete } = useTreeMutation(node.spaceId); - const [data, setData] = useAtom(treeDataAtom); + // Setter-only: the tree value is read only imperatively inside the duplicate + // handler (via `store` below), never at render, so useSetAtom avoids + // re-rendering every row's NodeMenu on any tree event. + const setData = useSetAtom(treeDataAtom); + const store = useStore(); const emit = useQueryEmit(); const [exportOpened, { open: openExportModal, close: closeExportModal }] = useDisclosure(false); @@ -125,8 +129,8 @@ export function NodeMenu({ node, canEdit }: NodeMenuProps) { try { const duplicatedPage = await duplicatePage({ pageId: node.id }); - // figure out parent + insertion index - const siblings = treeModel.siblingsOf(data, node.id); + // figure out parent + insertion index (read the live tree imperatively) + const siblings = treeModel.siblingsOf(store.get(treeDataAtom), node.id); const parentId = siblings?.parentId ?? null; const currentIndex = siblings?.index ?? 0; const newIndex = currentIndex + 1; diff --git a/apps/client/src/features/page/tree/components/space-tree-row.tsx b/apps/client/src/features/page/tree/components/space-tree-row.tsx index ef117e8a..ca62168e 100644 --- a/apps/client/src/features/page/tree/components/space-tree-row.tsx +++ b/apps/client/src/features/page/tree/components/space-tree-row.tsx @@ -1,6 +1,6 @@ import { useRef } from "react"; import { Link, useParams } from "react-router-dom"; -import { useAtom } from "jotai"; +import { useAtom, useSetAtom } from "jotai"; import { useTranslation } from "react-i18next"; import { ActionIcon, rem, Tooltip } from "@mantine/core"; import { @@ -51,7 +51,11 @@ export function SpaceTreeRow({ const { t } = useTranslation(); const { spaceSlug } = useParams(); const updatePageMutation = useUpdatePageMutation(); - const [, setTreeData] = useAtom(treeDataAtom); + // Setter-only: subscribing to the whole treeDataAtom (via useAtom) re-rendered + // every virtualized row on any tree event, bypassing the DocTreeRow memo. This + // row never reads the tree value, only writes it, so useSetAtom avoids the + // value subscription. + const setTreeData = useSetAtom(treeDataAtom); const emit = useQueryEmit(); const timerRef = useRef | null>(null); const [mobileSidebarOpened] = useAtom(mobileSidebarAtom); 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 index 4f248c81..d6fee37b 100644 --- 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 @@ -35,6 +35,7 @@ vi.mock("@/features/page/queries/page-query.ts", () => ({ isFetching: false, }), usePageQuery: () => ({ data: undefined }), + usePageMetaQuery: () => ({ data: undefined }), fetchAllAncestorChildren: (...args: unknown[]) => fetchAllAncestorChildrenMock(...args), })); 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 c7e3cbc7..8504caef 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 @@ -26,6 +26,7 @@ vi.mock("@/features/page/queries/page-query.ts", () => ({ isFetching: false, }), usePageQuery: () => ({ data: undefined }), + usePageMetaQuery: () => ({ data: undefined }), fetchAllAncestorChildren: vi.fn(), })); 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 85e4cdae..98317797 100644 --- a/apps/client/src/features/page/tree/components/space-tree.tsx +++ b/apps/client/src/features/page/tree/components/space-tree.tsx @@ -15,7 +15,7 @@ import { notifications } from "@mantine/notifications"; import { fetchAllAncestorChildren, useGetRootSidebarPagesQuery, - usePageQuery, + usePageMetaQuery, } from "@/features/page/queries/page-query.ts"; import classes from "@/features/page/tree/styles/tree.module.css"; import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom.ts"; @@ -76,7 +76,7 @@ const SpaceTree = forwardRef(function SpaceTree( const [isDataLoaded, setIsDataLoaded] = useState(false); const spaceIdRef = useRef(spaceId); spaceIdRef.current = spaceId; - const { data: currentPage } = usePageQuery({ + const { data: currentPage } = usePageMetaQuery({ pageId: extractPageSlugId(pageSlug), }); diff --git a/apps/client/src/features/page/tree/hooks/use-tree-mutation.ts b/apps/client/src/features/page/tree/hooks/use-tree-mutation.ts index 7aff8141..5f6a1061 100644 --- a/apps/client/src/features/page/tree/hooks/use-tree-mutation.ts +++ b/apps/client/src/features/page/tree/hooks/use-tree-mutation.ts @@ -1,5 +1,5 @@ import { useCallback } from "react"; -import { useAtom, useSetAtom, useStore } from "jotai"; +import { useSetAtom, useStore } from "jotai"; import { notifications } from "@mantine/notifications"; import { useTranslation } from "react-i18next"; import { useNavigate, useParams } from "react-router-dom"; @@ -34,7 +34,10 @@ export type UseTreeMutation = { export function useTreeMutation(spaceId: string): UseTreeMutation { const { t } = useTranslation(); - const [, setData] = useAtom(treeDataAtom); + // Setter-only: this hook never reads the tree reactively (handlers read the + // live value imperatively via `store` below), so useSetAtom avoids + // re-rendering SpaceSidebar on every tree event. + const setData = useSetAtom(treeDataAtom); // `store` reads the *current* treeDataAtom imperatively in handlers — avoids // stale-closure issues when the caller updates the tree (e.g. lazy-load // children) and then immediately invokes a handler. diff --git a/apps/client/src/features/share/components/share-modal.tsx b/apps/client/src/features/share/components/share-modal.tsx index 20a67766..49609cb9 100644 --- a/apps/client/src/features/share/components/share-modal.tsx +++ b/apps/client/src/features/share/components/share-modal.tsx @@ -20,7 +20,7 @@ import { import { Link, useParams } from "react-router-dom"; import { extractPageSlugId, getPageIcon } from "@/lib"; import { useTranslation } from "react-i18next"; -import { usePageQuery } from "@/features/page/queries/page-query.ts"; +import { usePageMetaQuery } from "@/features/page/queries/page-query.ts"; import CopyTextButton from "@/components/common/copy.tsx"; import { getAppUrl } from "@/lib/config.ts"; import { buildPageUrl } from "@/features/page/page.utils.ts"; @@ -37,7 +37,7 @@ export default function ShareModal({ readOnly }: ShareModalProps) { const { t } = useTranslation(); const { pageSlug } = useParams(); const pageSlugId = extractPageSlugId(pageSlug); - const { data: page } = usePageQuery({ pageId: pageSlugId }); + const { data: page } = usePageMetaQuery({ pageId: pageSlugId }); const pageId = page?.id; const { data: share } = useShareForPageQuery(pageId); const { spaceSlug } = useParams(); diff --git a/apps/client/src/features/space/queries/space-query.ts b/apps/client/src/features/space/queries/space-query.ts index dd259326..96425808 100644 --- a/apps/client/src/features/space/queries/space-query.ts +++ b/apps/client/src/features/space/queries/space-query.ts @@ -38,7 +38,6 @@ export function useGetSpacesQuery( queryKey: ["spaces", params], queryFn: () => getSpaces(params), placeholderData: keepPreviousData, - refetchOnMount: true, }); } diff --git a/apps/client/src/features/space/queries/space-watcher-query.ts b/apps/client/src/features/space/queries/space-watcher-query.ts index 84642bb5..e890fd83 100644 --- a/apps/client/src/features/space/queries/space-watcher-query.ts +++ b/apps/client/src/features/space/queries/space-watcher-query.ts @@ -16,7 +16,6 @@ export function useWatchedSpaceIds(): Set { const { data } = useQuery({ queryKey: [WATCHED_SPACE_IDS_KEY], queryFn: () => getWatchedSpaceIds(), - refetchOnMount: true, }); const items = data?.items; diff --git a/apps/client/src/features/websocket/use-query-subscription.ts b/apps/client/src/features/websocket/use-query-subscription.ts index 4bb11c5c..b826673d 100644 --- a/apps/client/src/features/websocket/use-query-subscription.ts +++ b/apps/client/src/features/websocket/use-query-subscription.ts @@ -19,7 +19,11 @@ export const useQuerySubscription = () => { const [socket] = useAtom(socketAtom); React.useEffect(() => { - socket?.on("message", (event) => { + if (!socket) return; + // Named handler + off() cleanup (mirrors use-notification-socket). Without + // cleanup, every socket recreation / effect re-run stacked another listener, + // so a single broadcast fired duplicated invalidateQueries / setQueryData. + const handleMessage = (event) => { const data: WebSocketEvent = event; let entity = null; @@ -163,6 +167,11 @@ export const useQuerySubscription = () => { }); break; } - }); + }; + + socket.on("message", handleMessage); + return () => { + socket.off("message", handleMessage); + }; }, [queryClient, socket]); }; diff --git a/apps/client/src/features/websocket/use-tree-socket.ts b/apps/client/src/features/websocket/use-tree-socket.ts index 29b02ce7..68bd001d 100644 --- a/apps/client/src/features/websocket/use-tree-socket.ts +++ b/apps/client/src/features/websocket/use-tree-socket.ts @@ -1,6 +1,6 @@ import { useEffect } from "react"; import { socketAtom } from "@/features/websocket/atoms/socket-atom.ts"; -import { useAtom } from "jotai"; +import { useAtom, useSetAtom } from "jotai"; import { treeDataAtom } from "@/features/page/tree/atoms/tree-data-atom.ts"; import { WebSocketEvent } from "@/features/websocket/types"; import { SpaceTreeNode } from "@/features/page/tree/types.ts"; @@ -16,7 +16,10 @@ import localEmitter from "@/lib/local-emitter.ts"; export const useTreeSocket = () => { const [socket] = useAtom(socketAtom); - const [, setTreeData] = useAtom(treeDataAtom); + // Setter-only: this hook writes the tree from socket events but never reads it + // reactively, so useSetAtom avoids re-rendering UserProvider (its host) on + // every tree event. + const setTreeData = useSetAtom(treeDataAtom); const queryClient = useQueryClient(); useEffect(() => { @@ -37,7 +40,11 @@ export const useTreeSocket = () => { }, []); useEffect(() => { - socket?.on("message", (event: WebSocketEvent) => { + if (!socket) return; + // Named handler + off() cleanup (mirrors use-notification-socket). Without + // cleanup, every socket recreation / effect re-run stacked another listener, + // so a single broadcast fired duplicated tree walks after each reconnect. + const handleMessage = (event: WebSocketEvent) => { switch (event.operation) { case "updateOne": if (event.entity[0] === "pages") { @@ -64,6 +71,11 @@ export const useTreeSocket = () => { }); break; } - }); - }, [socket]); + }; + + socket.on("message", handleMessage); + return () => { + socket.off("message", handleMessage); + }; + }, [socket, queryClient, setTreeData]); }; diff --git a/apps/client/src/features/workspace/queries/workspace-query.ts b/apps/client/src/features/workspace/queries/workspace-query.ts index 035d3d85..1d34da83 100644 --- a/apps/client/src/features/workspace/queries/workspace-query.ts +++ b/apps/client/src/features/workspace/queries/workspace-query.ts @@ -243,6 +243,5 @@ export function useAppVersion( queryFn: () => getAppVersion(), staleTime: 60 * 60 * 1000, // 1 hr enabled: isEnabled, - refetchOnMount: true, }); } diff --git a/apps/client/src/pages/page/page-redirect.tsx b/apps/client/src/pages/page/page-redirect.tsx index c7d3f3ec..cc9b0ad2 100644 --- a/apps/client/src/pages/page/page-redirect.tsx +++ b/apps/client/src/pages/page/page-redirect.tsx @@ -1,6 +1,6 @@ import { useNavigate, useParams } from "react-router-dom"; import { useEffect } from "react"; -import { usePageQuery } from "@/features/page/queries/page-query"; +import { usePageMetaQuery } from "@/features/page/queries/page-query"; import { buildPageUrl } from "@/features/page/page.utils.ts"; import { extractPageSlugId } from "@/lib"; import { Error404 } from "@/components/ui/error-404.tsx"; @@ -11,7 +11,7 @@ export default function PageRedirect() { data: page, isLoading: pageIsLoading, isError, - } = usePageQuery({ pageId: extractPageSlugId(pageSlug) }); + } = usePageMetaQuery({ pageId: extractPageSlugId(pageSlug) }); const navigate = useNavigate(); useEffect(() => { diff --git a/apps/client/src/pages/page/page.tsx b/apps/client/src/pages/page/page.tsx index a94faf58..6689d5d6 100644 --- a/apps/client/src/pages/page/page.tsx +++ b/apps/client/src/pages/page/page.tsx @@ -10,7 +10,7 @@ import { useTranslation } from "react-i18next"; import React from "react"; import { EmptyState } from "@/components/ui/empty-state.tsx"; import { IconAlertTriangle, IconFileOff } from "@tabler/icons-react"; -import { Button } from "@mantine/core"; +import { Button, Skeleton } from "@mantine/core"; import { Link } from "react-router-dom"; import { ErrorBoundary } from "react-error-boundary"; const MemoizedFullEditor = React.memo(FullEditor); @@ -58,7 +58,7 @@ function PageContent({ pageSlug }: { pageSlug: string | undefined }) { (space?.settings?.comments?.allowViewerComments === true); if (isLoading) { - return <>; + return ; } if (isError || !page) { @@ -87,7 +87,7 @@ function PageContent({ pageSlug }: { pageSlug: string | undefined }) { } if (!space) { - return <>; + return ; } return ( @@ -116,3 +116,18 @@ function PageContent({ pageSlug }: { pageSlug: string | undefined }) { ) ); } + +// Lightweight loading placeholder shown instead of a blank fragment while the +// page (or its space) is loading, so navigation into a not-yet-cached page no +// longer flashes empty. Approximates the title + first content lines. +function PageSkeleton() { + return ( +
+ + + + + +
+ ); +} -- 2.52.0 From 9d1b033fe86a10b1fb25f602be675af4a7101d39 Mon Sep 17 00:00:00 2001 From: agent_coder Date: Sun, 5 Jul 2026 01:05:32 +0300 Subject: [PATCH 2/2] fix(#344 review F1-F4): test-mock coverage + getSpaces freshness + comment/test fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - F1 [blocking]: share-modal.test.tsx + comment-content-view.test.tsx mocked page-query without usePageMetaQuery → 3 tests threw (ShareModal uses it directly, comment-content-view via MentionContent). Added usePageMetaQuery to both mocks (the space-tree mocks were already fixed; these two were missed). - F2: restored refetchOnMount:true on useGetSpacesQuery — ["spaces"] is invalidated only by same-tab mutations (no socket path), so a cross-actor change (an admin adding/removing THIS user from a space) left the list stale until a hard reload. The other refetchOnMount removals (favorites/watched — per-user, same-tab-only gap) stay removed. - F3: corrected the trash-list + recent-changes KEEP comments — both keys ARE invalidated (trash-list by 3 mutations, recent-changes by page CRUD), but invalidateQueries only marks an UNMOUNTED query stale without refetching, so the mount refetch closes the gap. The old "never invalidated" wording was wrong and risked a maintainer deleting a live invalidation as dead code. - F4: tests for the two load-bearing pure paths — invalidate-on-update-page (the undefined-guard: a title-only event keeps the icon; sibling/unrelated subtrees untouched) and breadcrumb-path-equal (equal chain → true; any id/slugId/name/ icon change or length diff → false; both-null → true). Exported breadcrumbPathEqual for the test. Gate: client tsc 0; the 4 affected/new test files 33 passed. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../components/comment-content-view.test.tsx | 1 + .../breadcrumbs/breadcrumb-path-equal.test.ts | 53 +++++++++++ .../components/breadcrumbs/breadcrumb.tsx | 2 +- .../queries/invalidate-on-update-page.test.ts | 90 +++++++++++++++++++ .../src/features/page/queries/page-query.ts | 18 ++-- .../share/components/share-modal.test.tsx | 1 + .../src/features/space/queries/space-query.ts | 6 ++ 7 files changed, 163 insertions(+), 8 deletions(-) create mode 100644 apps/client/src/features/page/components/breadcrumbs/breadcrumb-path-equal.test.ts create mode 100644 apps/client/src/features/page/queries/invalidate-on-update-page.test.ts 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, }); } -- 2.52.0