Просмотр исходного кода

refactor: improve data loading and caching mechanisms in page tree

Yuki Takei 4 месяцев назад
Родитель
Сommit
6b3313b939

+ 77 - 42
apps/app/src/features/page-tree/client/components/SimplifiedItemsTree.tsx

@@ -1,5 +1,6 @@
 import type { FC } from 'react';
 import type { FC } from 'react';
-import { useCallback, useEffect, useRef, useState } from 'react';
+import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
+import type { ItemInstance } from '@headless-tree/core';
 import {
 import {
   asyncDataLoaderFeature,
   asyncDataLoaderFeature,
   hotkeysCoreFeature,
   hotkeysCoreFeature,
@@ -14,7 +15,7 @@ import { useSWRxRootPage } from '~/stores/page-listing';
 
 
 import { ROOT_PAGE_VIRTUAL_ID } from '../../constants';
 import { ROOT_PAGE_VIRTUAL_ID } from '../../constants';
 import { useAutoExpandAncestors } from '../hooks/use-auto-expand-ancestors';
 import { useAutoExpandAncestors } from '../hooks/use-auto-expand-ancestors';
-import { useDataLoader } from '../hooks/use-data-loader';
+import { clearChildrenCache, useDataLoader } from '../hooks/use-data-loader';
 import { usePageCreate } from '../hooks/use-page-create';
 import { usePageCreate } from '../hooks/use-page-create';
 import { usePageRename } from '../hooks/use-page-rename';
 import { usePageRename } from '../hooks/use-page-rename';
 import { useScrollToSelectedItem } from '../hooks/use-scroll-to-selected-item';
 import { useScrollToSelectedItem } from '../hooks/use-scroll-to-selected-item';
@@ -25,6 +26,25 @@ import {
   usePageTreeRevalidationEffect,
   usePageTreeRevalidationEffect,
 } from '../states/page-tree-update';
 } from '../states/page-tree-update';
 
 
+// Stable features array to avoid recreating on every render
+const TREE_FEATURES = [
+  asyncDataLoaderFeature,
+  selectionFeature,
+  hotkeysCoreFeature,
+  renamingFeature,
+];
+
+// Stable createLoadingItemData function
+const createLoadingItemData = (): IPageForTreeItem => ({
+  _id: '',
+  path: 'Loading...',
+  parent: '',
+  descendantCount: 0,
+  grant: 1,
+  isEmpty: false,
+  wip: false,
+});
+
 type Props = {
 type Props = {
   targetPath: string;
   targetPath: string;
   targetPathOrId?: string;
   targetPathOrId?: string;
@@ -67,64 +87,78 @@ export const SimplifiedItemsTree: FC<Props> = (props: Props) => {
   // Get creating parent id to determine if item should be treated as folder
   // Get creating parent id to determine if item should be treated as folder
   const creatingParentId = useCreatingParentId();
   const creatingParentId = useCreatingParentId();
 
 
-  // onRename handler for headless-tree
+  // Use refs to stabilize callbacks passed to useTree
+  // This prevents headless-tree from detecting config changes and refetching data
+  const creatingParentIdRef = useRef(creatingParentId);
+  creatingParentIdRef.current = creatingParentId;
+
+  const getPageNameRef = useRef(getPageName);
+  getPageNameRef.current = getPageName;
+
+  const renameRef = useRef(rename);
+  renameRef.current = rename;
+
+  const createFromPlaceholderRef = useRef(createFromPlaceholder);
+  createFromPlaceholderRef.current = createFromPlaceholder;
+
+  const isCreatingPlaceholderRef = useRef(isCreatingPlaceholder);
+  isCreatingPlaceholderRef.current = isCreatingPlaceholder;
+
+  const cancelCreatingRef = useRef(cancelCreating);
+  cancelCreatingRef.current = cancelCreating;
+
+  // Stable getItemName callback - receives ItemInstance from headless-tree
+  const getItemName = useCallback((item: ItemInstance<IPageForTreeItem>) => {
+    return getPageNameRef.current(item);
+  }, []);
+
+  // Stable isItemFolder callback
+  // IMPORTANT: Do NOT call item.getChildren() here as it triggers API calls for ALL visible items
+  const isItemFolder = useCallback((item: ItemInstance<IPageForTreeItem>) => {
+    const itemData = item.getItemData();
+    const currentCreatingParentId = creatingParentIdRef.current;
+    const isCreatingUnderThis = currentCreatingParentId === itemData._id;
+    if (isCreatingUnderThis) return true;
+
+    // Use descendantCount from the item data to determine if it's a folder
+    // This avoids triggering getChildrenWithData API calls
+    return itemData.descendantCount > 0;
+  }, []);
+
+  // Stable onRename handler for headless-tree
   // Handles both rename and create (for placeholder nodes)
   // Handles both rename and create (for placeholder nodes)
   const handleRename = useCallback(
   const handleRename = useCallback(
-    async (item, newValue: string) => {
-      if (isCreatingPlaceholder(item)) {
+    async (item: ItemInstance<IPageForTreeItem>, newValue: string) => {
+      if (isCreatingPlaceholderRef.current(item)) {
         // Placeholder node: create new page or cancel if empty
         // Placeholder node: create new page or cancel if empty
         if (newValue.trim() === '') {
         if (newValue.trim() === '') {
           // Empty value means cancel (Esc key or blur)
           // Empty value means cancel (Esc key or blur)
-          cancelCreating();
+          cancelCreatingRef.current();
         } else {
         } else {
-          await createFromPlaceholder(item, newValue);
+          await createFromPlaceholderRef.current(item, newValue);
         }
         }
       } else {
       } else {
         // Normal node: rename page
         // Normal node: rename page
-        await rename(item, newValue);
+        await renameRef.current(item, newValue);
       }
       }
       // Trigger re-render after operation
       // Trigger re-render after operation
       setRebuildTrigger((prev) => prev + 1);
       setRebuildTrigger((prev) => prev + 1);
     },
     },
-    [rename, createFromPlaceholder, isCreatingPlaceholder, cancelCreating],
+    [],
   );
   );
 
 
+  // Stable initial state
+  const initialState = useMemo(() => ({ expandedItems: [ROOT_PAGE_VIRTUAL_ID] }), []);
+
   const tree = useTree<IPageForTreeItem>({
   const tree = useTree<IPageForTreeItem>({
     rootItemId: ROOT_PAGE_VIRTUAL_ID,
     rootItemId: ROOT_PAGE_VIRTUAL_ID,
-    getItemName: (item) => getPageName(item),
-    initialState: { expandedItems: [ROOT_PAGE_VIRTUAL_ID] },
-    // Item is a folder if it has loaded children OR if it's currently in "creating" mode
-    // Use getChildren() to check actual cached children instead of descendantCount
-    isItemFolder: (item) => {
-      const itemData = item.getItemData();
-      const isCreatingUnderThis = creatingParentId === itemData._id;
-      if (isCreatingUnderThis) return true;
-
-      // Check cached children - getChildren() returns cached child items
-      const children = item.getChildren();
-      if (children.length > 0) return true;
-
-      // Fallback to descendantCount for items not yet expanded
-      return itemData.descendantCount > 0;
-    },
-    createLoadingItemData: () => ({
-      _id: '',
-      path: 'Loading...',
-      parent: '',
-      descendantCount: 0,
-      revision: '',
-      grant: 1,
-      isEmpty: false,
-      wip: false,
-    }),
+    getItemName,
+    initialState,
+    isItemFolder,
+    createLoadingItemData,
     dataLoader,
     dataLoader,
     onRename: handleRename,
     onRename: handleRename,
-    features: [
-      asyncDataLoaderFeature,
-      selectionFeature,
-      hotkeysCoreFeature,
-      renamingFeature,
-    ],
+    features: TREE_FEATURES,
   });
   });
 
 
   // Track local generation number
   // Track local generation number
@@ -152,7 +186,8 @@ export const SimplifiedItemsTree: FC<Props> = (props: Props) => {
       parentItem.expand();
       parentItem.expand();
     }
     }
 
 
-    // Invalidate children to load placeholder
+    // Clear cache for this parent and invalidate children to load placeholder
+    clearChildrenCache([creatingParentId]);
     parentItem?.invalidateChildrenIds(true);
     parentItem?.invalidateChildrenIds(true);
 
 
     // Trigger re-render
     // Trigger re-render

+ 98 - 23
apps/app/src/features/page-tree/client/hooks/use-data-loader.ts

@@ -1,4 +1,4 @@
-import { useCallback } from 'react';
+import { useMemo, useRef } from 'react';
 import type { TreeDataLoader } from '@headless-tree/core';
 import type { TreeDataLoader } from '@headless-tree/core';
 
 
 import { apiv3Get } from '~/client/util/apiv3-client';
 import { apiv3Get } from '~/client/util/apiv3-client';
@@ -27,6 +27,33 @@ function constructRootPageForVirtualRoot(
   };
   };
 }
 }
 
 
+// Cache for children data to prevent duplicate API calls
+// Key: itemId, Value: { promise, data, timestamp }
+type CacheEntry = {
+  promise: Promise<{ id: string; data: IPageForTreeItem }[]>;
+  data?: { id: string; data: IPageForTreeItem }[];
+  timestamp: number;
+};
+
+// Module-level cache (persists across component remounts)
+const childrenCache = new Map<string, CacheEntry>();
+
+// Cache TTL in milliseconds (5 minutes)
+const CACHE_TTL = 5 * 60 * 1000;
+
+/**
+ * Clear cache for specific item IDs or all cache
+ */
+export const clearChildrenCache = (itemIds?: string[]): void => {
+  if (itemIds == null) {
+    childrenCache.clear();
+  } else {
+    itemIds.forEach((id) => {
+      childrenCache.delete(id);
+    });
+  }
+};
+
 export const useDataLoader = (
 export const useDataLoader = (
   rootPageId: string,
   rootPageId: string,
   allPagesCount: number,
   allPagesCount: number,
@@ -34,8 +61,19 @@ export const useDataLoader = (
   const creatingParentId = useCreatingParentId();
   const creatingParentId = useCreatingParentId();
   const creatingParentPath = useCreatingParentPath();
   const creatingParentPath = useCreatingParentPath();
 
 
-  const getItem = useCallback(
-    async (itemId: string): Promise<IPageForTreeItem> => {
+  // Use refs to avoid recreating dataLoader callbacks when creating state changes
+  // The creating state is accessed via refs so that:
+  // 1. The dataLoader reference stays stable (prevents headless-tree from refetching all data)
+  // 2. The actual creating state is still read at execution time (when invalidateChildrenIds is called)
+  const creatingParentIdRef = useRef(creatingParentId);
+  const creatingParentPathRef = useRef(creatingParentPath);
+  creatingParentIdRef.current = creatingParentId;
+  creatingParentPathRef.current = creatingParentPath;
+
+  // Memoize the entire dataLoader object to ensure reference stability
+  // Only recreate when rootPageId or allPagesCount changes (which are truly needed for the API calls)
+  const dataLoader = useMemo<TreeDataLoader<IPageForTreeItem>>(() => {
+    const getItem = async (itemId: string): Promise<IPageForTreeItem> => {
       // Virtual root (should rarely be called since it's provided by getChildrenWithData)
       // Virtual root (should rarely be called since it's provided by getChildrenWithData)
       if (itemId === ROOT_PAGE_VIRTUAL_ID) {
       if (itemId === ROOT_PAGE_VIRTUAL_ID) {
         return constructRootPageForVirtualRoot(rootPageId, allPagesCount);
         return constructRootPageForVirtualRoot(rootPageId, allPagesCount);
@@ -54,12 +92,23 @@ export const useDataLoader = (
         { id: itemId },
         { id: itemId },
       );
       );
       return response.data.item;
       return response.data.item;
-    },
-    [allPagesCount, rootPageId],
-  );
+    };
 
 
-  const getChildrenWithData = useCallback(
-    async (itemId: string) => {
+    const fetchChildrenFromApi = async (
+      itemId: string,
+    ): Promise<{ id: string; data: IPageForTreeItem }[]> => {
+      const response = await apiv3Get<{ children: IPageForTreeItem[] }>(
+        '/page-listing/children',
+        { id: itemId },
+      );
+
+      return response.data.children.map((child) => ({
+        id: child._id,
+        data: child,
+      }));
+    };
+
+    const getChildrenWithData = async (itemId: string) => {
       // Virtual root returns root page as its only child
       // Virtual root returns root page as its only child
       // Use actual MongoDB _id as tree item ID to avoid duplicate API calls
       // Use actual MongoDB _id as tree item ID to avoid duplicate API calls
       if (itemId === ROOT_PAGE_VIRTUAL_ID) {
       if (itemId === ROOT_PAGE_VIRTUAL_ID) {
@@ -76,22 +125,47 @@ export const useDataLoader = (
         return [];
         return [];
       }
       }
 
 
-      // For all pages (including root), fetch children using their _id
-      const response = await apiv3Get<{ children: IPageForTreeItem[] }>(
-        '/page-listing/children',
-        { id: itemId },
-      );
+      // Check cache first
+      const now = Date.now();
+      const cached = childrenCache.get(itemId);
 
 
-      const children = response.data.children.map((child) => ({
-        id: child._id,
-        data: child,
-      }));
+      let children: { id: string; data: IPageForTreeItem }[];
+
+      if (cached != null && now - cached.timestamp < CACHE_TTL) {
+        // Use cached data or wait for pending promise
+        if (cached.data != null) {
+          children = cached.data;
+        } else {
+          children = await cached.promise;
+        }
+      } else {
+        // Fetch from API and cache the promise to prevent duplicate requests
+        const promise = fetchChildrenFromApi(itemId);
+        const entry: CacheEntry = { promise, timestamp: now };
+        childrenCache.set(itemId, entry);
+
+        try {
+          children = await promise;
+          // Store the resolved data in cache
+          entry.data = children;
+        } catch (error) {
+          // Remove failed entry from cache
+          childrenCache.delete(itemId);
+          throw error;
+        }
+      }
 
 
       // If this parent is in "creating" mode, prepend placeholder node
       // If this parent is in "creating" mode, prepend placeholder node
-      if (creatingParentId === itemId && creatingParentPath != null) {
+      // Read from refs to get current value without triggering dataLoader recreation
+      const currentCreatingParentId = creatingParentIdRef.current;
+      const currentCreatingParentPath = creatingParentPathRef.current;
+      if (
+        currentCreatingParentId === itemId &&
+        currentCreatingParentPath != null
+      ) {
         const placeholderData = createPlaceholderPageData(
         const placeholderData = createPlaceholderPageData(
           itemId,
           itemId,
-          creatingParentPath,
+          currentCreatingParentPath,
         );
         );
         return [
         return [
           { id: CREATING_PAGE_VIRTUAL_ID, data: placeholderData },
           { id: CREATING_PAGE_VIRTUAL_ID, data: placeholderData },
@@ -100,9 +174,10 @@ export const useDataLoader = (
       }
       }
 
 
       return children;
       return children;
-    },
-    [allPagesCount, rootPageId, creatingParentId, creatingParentPath],
-  );
+    };
+
+    return { getItem, getChildrenWithData };
+  }, [allPagesCount, rootPageId]);
 
 
-  return { getItem, getChildrenWithData };
+  return dataLoader;
 };
 };

+ 5 - 2
apps/app/src/features/page-tree/client/states/page-tree-update.ts

@@ -3,6 +3,7 @@ import type { TreeInstance } from '@headless-tree/core';
 import { atom, useAtomValue, useSetAtom } from 'jotai';
 import { atom, useAtomValue, useSetAtom } from 'jotai';
 
 
 import { ROOT_PAGE_VIRTUAL_ID } from '../../constants';
 import { ROOT_PAGE_VIRTUAL_ID } from '../../constants';
+import { clearChildrenCache } from '../hooks/use-data-loader';
 
 
 // Update generation number
 // Update generation number
 const generationAtom = atom<number>(1);
 const generationAtom = atom<number>(1);
@@ -61,11 +62,13 @@ export const usePageTreeRevalidationEffect = (
     const shouldUpdateAll = globalLastUpdatedItemIds == null;
     const shouldUpdateAll = globalLastUpdatedItemIds == null;
 
 
     if (shouldUpdateAll) {
     if (shouldUpdateAll) {
-      // Full tree update: refetch from root
+      // Full tree update: clear all cache and refetch from root
+      clearChildrenCache();
       const root = getItemInstance(ROOT_PAGE_VIRTUAL_ID);
       const root = getItemInstance(ROOT_PAGE_VIRTUAL_ID);
       root?.invalidateChildrenIds(true);
       root?.invalidateChildrenIds(true);
     } else {
     } else {
-      // Partial update: refetch children of specified items
+      // Partial update: clear cache for specified items and refetch children
+      clearChildrenCache(globalLastUpdatedItemIds);
       globalLastUpdatedItemIds.forEach((itemId) => {
       globalLastUpdatedItemIds.forEach((itemId) => {
         const item = getItemInstance(itemId);
         const item = getItemInstance(itemId);
         // Invalidate children to refresh child list
         // Invalidate children to refresh child list