Parcourir la source

refactor(page-tree): replace clearChildrenCache with invalidatePageTreeChildren for better cache management

Yuki Takei il y a 4 mois
Parent
commit
cdbb95276c

+ 2 - 2
apps/app/src/features/page-tree/client/components/SimplifiedItemsTree.spec.tsx

@@ -5,8 +5,8 @@ import { render, waitFor } from '@testing-library/react';
 
 import type { IPageForTreeItem } from '~/interfaces/page';
 
-import { clearChildrenCache } from '../hooks/use-data-loader';
 import type { TreeItemProps } from '../interfaces';
+import { invalidatePageTreeChildren } from '../services';
 import { SimplifiedItemsTree } from './SimplifiedItemsTree';
 
 // Mock the apiv3Get function
@@ -110,7 +110,7 @@ const TestWrapper: FC<{ children: React.ReactNode }> = ({ children }) => (
 describe('SimplifiedItemsTree', () => {
   beforeEach(() => {
     // Clear cache before each test
-    clearChildrenCache();
+    invalidatePageTreeChildren();
     // Reset mock
     mockApiv3Get.mockReset();
   });

+ 16 - 11
apps/app/src/features/page-tree/client/components/SimplifiedItemsTree.tsx

@@ -14,15 +14,19 @@ import { useSWRxRootPage } from '~/stores/page-listing';
 
 import { ROOT_PAGE_VIRTUAL_ID } from '../../constants';
 import { useAutoExpandAncestors } from '../hooks/use-auto-expand-ancestors';
-import { clearChildrenCache, useDataLoader } from '../hooks/use-data-loader';
+import { useDataLoader } from '../hooks/use-data-loader';
 import { useScrollToSelectedItem } from '../hooks/use-scroll-to-selected-item';
 import { useTreeItemHandlers } from '../hooks/use-tree-item-handlers';
 import type { TreeItemProps } from '../interfaces';
+import { invalidatePageTreeChildren } from '../services';
 import {
   usePageTreeInformationGeneration,
   usePageTreeRevalidationEffect,
 } from '../states/page-tree-update';
-import { useTreeRebuildTrigger, useTriggerTreeRebuild } from '../states/tree-rebuild';
+import {
+  useTreeRebuildTrigger,
+  useTriggerTreeRebuild,
+} from '../states/tree-rebuild';
 
 // Stable features array to avoid recreating on every render
 const TREE_FEATURES = [
@@ -79,15 +83,14 @@ export const SimplifiedItemsTree: FC<Props> = (props: Props) => {
 
   // Tree item handlers (rename, create, etc.) with stable callbacks for headless-tree
   // Note: triggerTreeRebuild is stable (from useSetAtom), so no need for useCallback wrapper
-  const {
-    getItemName,
-    isItemFolder,
-    handleRename,
-    creatingParentId,
-  } = useTreeItemHandlers(triggerTreeRebuild);
+  const { getItemName, isItemFolder, handleRename, creatingParentId } =
+    useTreeItemHandlers(triggerTreeRebuild);
 
   // Stable initial state
-  const initialState = useMemo(() => ({ expandedItems: [ROOT_PAGE_VIRTUAL_ID] }), []);
+  const initialState = useMemo(
+    () => ({ expandedItems: [ROOT_PAGE_VIRTUAL_ID] }),
+    [],
+  );
 
   const tree = useTree<IPageForTreeItem>({
     rootItemId: ROOT_PAGE_VIRTUAL_ID,
@@ -107,7 +110,9 @@ export const SimplifiedItemsTree: FC<Props> = (props: Props) => {
   // Refetch data when global generation is updated
   usePageTreeRevalidationEffect(tree, localGenerationRef.current, {
     // Update local generation number after revalidation
-    onRevalidated: () => { localGenerationRef.current = globalGeneration; },
+    onRevalidated: () => {
+      localGenerationRef.current = globalGeneration;
+    },
   });
 
   // Expand and rebuild tree when creatingParentId changes
@@ -126,7 +131,7 @@ export const SimplifiedItemsTree: FC<Props> = (props: Props) => {
     }
 
     // Clear cache for this parent and invalidate children to load placeholder
-    clearChildrenCache([creatingParentId]);
+    invalidatePageTreeChildren([creatingParentId]);
     parentItem?.invalidateChildrenIds(true);
 
     // Trigger re-render

+ 16 - 15
apps/app/src/features/page-tree/client/hooks/use-data-loader.integration.spec.tsx

@@ -20,12 +20,13 @@ import { createStore, Provider } from 'jotai';
 
 import type { IPageForTreeItem } from '~/interfaces/page';
 
+import { invalidatePageTreeChildren } from '../services';
 // Re-import the actions hook to use real implementation
 import {
   CREATING_PAGE_VIRTUAL_ID,
   usePageTreeCreateActions,
 } from '../states/page-tree-create';
-import { clearChildrenCache, useDataLoader } from './use-data-loader';
+import { useDataLoader } from './use-data-loader';
 
 /**
  * Type helper to extract getChildrenWithData from TreeDataLoader
@@ -87,8 +88,8 @@ describe('use-data-loader integration with Jotai atoms', () => {
   beforeEach(() => {
     // Create fresh store for each test
     store = createStore();
-    // Clear cache before each test
-    clearChildrenCache();
+    // Clear pending requests before each test
+    invalidatePageTreeChildren();
     // Reset mock
     mockApiv3Get.mockReset();
   });
@@ -124,8 +125,8 @@ describe('use-data-loader integration with Jotai atoms', () => {
         actionsResult.current.startCreating('parent-id', '/parent');
       });
 
-      // Clear cache to force re-fetch
-      clearChildrenCache(['parent-id']);
+      // Clear pending requests to force re-fetch
+      invalidatePageTreeChildren(['parent-id']);
 
       // Second call - should have placeholder because atom state changed
       const childrenAfter =
@@ -160,8 +161,8 @@ describe('use-data-loader integration with Jotai atoms', () => {
         actionsResult.current.startCreating('parent-id', '/parent');
       });
 
-      // Clear cache and fetch - should have placeholder
-      clearChildrenCache(['parent-id']);
+      // Clear pending requests and fetch - should have placeholder
+      invalidatePageTreeChildren(['parent-id']);
       const childrenWithPlaceholder =
         await getDataLoader(dataLoaderResult).getChildrenWithData('parent-id');
       expect(childrenWithPlaceholder).toHaveLength(2);
@@ -172,8 +173,8 @@ describe('use-data-loader integration with Jotai atoms', () => {
         actionsResult.current.cancelCreating();
       });
 
-      // Clear cache and fetch - should NOT have placeholder
-      clearChildrenCache(['parent-id']);
+      // Clear pending requests and fetch - should NOT have placeholder
+      invalidatePageTreeChildren(['parent-id']);
       const childrenAfterCancel =
         await getDataLoader(dataLoaderResult).getChildrenWithData('parent-id');
       expect(childrenAfterCancel).toHaveLength(1);
@@ -237,8 +238,8 @@ describe('use-data-loader integration with Jotai atoms', () => {
         actionsResult.current.startCreating('parent-id', '/parent');
       });
 
-      // Clear cache
-      clearChildrenCache(['parent-id']);
+      // Clear pending requests
+      invalidatePageTreeChildren(['parent-id']);
 
       // Call getChildrenWithData using the SAME dataLoader reference
       // This should still see the updated atom state
@@ -276,7 +277,7 @@ describe('use-data-loader integration with Jotai atoms', () => {
       act(() => {
         actionsResult.current.startCreating('parent-id', '/parent');
       });
-      clearChildrenCache(['parent-id']);
+      invalidatePageTreeChildren(['parent-id']);
       let children = await dataLoader.getChildrenWithData('parent-id');
       expect(children).toHaveLength(2);
       expect(children[0].id).toBe(CREATING_PAGE_VIRTUAL_ID);
@@ -285,7 +286,7 @@ describe('use-data-loader integration with Jotai atoms', () => {
       act(() => {
         actionsResult.current.cancelCreating();
       });
-      clearChildrenCache(['parent-id']);
+      invalidatePageTreeChildren(['parent-id']);
       children = await dataLoader.getChildrenWithData('parent-id');
       expect(children).toHaveLength(1);
       expect(children[0].id).toBe('existing-child');
@@ -294,7 +295,7 @@ describe('use-data-loader integration with Jotai atoms', () => {
       act(() => {
         actionsResult.current.startCreating('other-parent', '/other');
       });
-      clearChildrenCache(['parent-id', 'other-parent']);
+      invalidatePageTreeChildren(['parent-id', 'other-parent']);
 
       // Original parent should NOT have placeholder
       children = await dataLoader.getChildrenWithData('parent-id');
@@ -311,7 +312,7 @@ describe('use-data-loader integration with Jotai atoms', () => {
       act(() => {
         actionsResult.current.cancelCreating();
       });
-      clearChildrenCache(['other-parent']);
+      invalidatePageTreeChildren(['other-parent']);
       mockApiv3Get.mockResolvedValueOnce({ data: { children: [] } });
       children = await dataLoader.getChildrenWithData('other-parent');
       expect(children).toHaveLength(0);

+ 75 - 151
apps/app/src/features/page-tree/client/hooks/use-data-loader.spec.tsx

@@ -3,15 +3,18 @@ import { renderHook } from '@testing-library/react';
 import type { IPageForTreeItem } from '~/interfaces/page';
 
 import { ROOT_PAGE_VIRTUAL_ID } from '../../constants';
+import { invalidatePageTreeChildren } from '../services';
 import { CREATING_PAGE_VIRTUAL_ID } from '../states/page-tree-create';
-import { clearChildrenCache, useDataLoader } from './use-data-loader';
+import { useDataLoader } from './use-data-loader';
 
 /**
  * Type helper to extract getChildrenWithData from TreeDataLoader
  * TreeDataLoader is a union type, and we're using the variant with getChildrenWithData
  */
 type DataLoaderWithChildrenData = ReturnType<typeof useDataLoader> & {
-  getChildrenWithData: (itemId: string) => Promise<{ id: string; data: IPageForTreeItem }[]>;
+  getChildrenWithData: (
+    itemId: string,
+  ) => Promise<{ id: string; data: IPageForTreeItem }[]>;
 };
 
 // Mock the apiv3Get function
@@ -55,9 +58,9 @@ const createMockPage = (
 /**
  * Helper to get typed dataLoader with getChildrenWithData
  */
-const getDataLoader = (
-  result: { current: ReturnType<typeof useDataLoader> },
-): DataLoaderWithChildrenData => {
+const getDataLoader = (result: {
+  current: ReturnType<typeof useDataLoader>;
+}): DataLoaderWithChildrenData => {
   return result.current as DataLoaderWithChildrenData;
 };
 
@@ -66,8 +69,8 @@ describe('use-data-loader', () => {
   const ALL_PAGES_COUNT = 100;
 
   beforeEach(() => {
-    // Clear cache before each test
-    clearChildrenCache();
+    // Clear pending requests before each test
+    invalidatePageTreeChildren();
     // Reset mock
     mockApiv3Get.mockReset();
     // Reset creating state
@@ -158,7 +161,8 @@ describe('use-data-loader', () => {
           useDataLoader(ROOT_PAGE_ID, ALL_PAGES_COUNT),
         );
 
-        const children = await getDataLoader(result).getChildrenWithData('parent-id');
+        const children =
+          await getDataLoader(result).getChildrenWithData('parent-id');
 
         expect(children).toHaveLength(2);
         expect(children[0].id).toBe('child-1');
@@ -170,47 +174,7 @@ describe('use-data-loader', () => {
       });
     });
 
-    describe('cache behavior - API call count', () => {
-      test('should call API only once for same itemId (cache hit)', async () => {
-        const mockChildren = [createMockPage('child-1', '/parent/child-1')];
-        mockApiv3Get.mockResolvedValue({ data: { children: mockChildren } });
-
-        const { result } = renderHook(() =>
-          useDataLoader(ROOT_PAGE_ID, ALL_PAGES_COUNT),
-        );
-
-        // Call getChildrenWithData multiple times with the same ID
-        await getDataLoader(result).getChildrenWithData('parent-id');
-        await getDataLoader(result).getChildrenWithData('parent-id');
-        await getDataLoader(result).getChildrenWithData('parent-id');
-
-        // API should only be called once due to caching
-        expect(mockApiv3Get).toHaveBeenCalledTimes(1);
-      });
-
-      test('should call API once per unique itemId', async () => {
-        const mockChildren1 = [createMockPage('child-1', '/parent1/child-1')];
-        const mockChildren2 = [createMockPage('child-2', '/parent2/child-2')];
-        const mockChildren3 = [createMockPage('child-3', '/parent3/child-3')];
-
-        mockApiv3Get
-          .mockResolvedValueOnce({ data: { children: mockChildren1 } })
-          .mockResolvedValueOnce({ data: { children: mockChildren2 } })
-          .mockResolvedValueOnce({ data: { children: mockChildren3 } });
-
-        const { result } = renderHook(() =>
-          useDataLoader(ROOT_PAGE_ID, ALL_PAGES_COUNT),
-        );
-
-        // Call getChildrenWithData for different IDs
-        await getDataLoader(result).getChildrenWithData('parent-1');
-        await getDataLoader(result).getChildrenWithData('parent-2');
-        await getDataLoader(result).getChildrenWithData('parent-3');
-
-        // API should be called once per unique ID
-        expect(mockApiv3Get).toHaveBeenCalledTimes(3);
-      });
-
+    describe('concurrent request deduplication', () => {
       test('should deduplicate concurrent requests for the same itemId', async () => {
         const mockChildren = [createMockPage('child-1', '/parent/child-1')];
 
@@ -249,52 +213,41 @@ describe('use-data-loader', () => {
         expect(mockApiv3Get).toHaveBeenCalledTimes(1);
       });
 
-      test('should call API again after cache is cleared', async () => {
-        const mockChildren = [createMockPage('child-1', '/parent/child-1')];
-        mockApiv3Get.mockResolvedValue({ data: { children: mockChildren } });
-
-        const { result } = renderHook(() =>
-          useDataLoader(ROOT_PAGE_ID, ALL_PAGES_COUNT),
-        );
-
-        // First call
-        await getDataLoader(result).getChildrenWithData('parent-id');
-        expect(mockApiv3Get).toHaveBeenCalledTimes(1);
-
-        // Clear cache for specific ID
-        clearChildrenCache(['parent-id']);
-
-        // Second call after cache clear
-        await getDataLoader(result).getChildrenWithData('parent-id');
-        expect(mockApiv3Get).toHaveBeenCalledTimes(2);
-      });
-
-      test('should call API again after all cache is cleared', async () => {
+      test('should call API once per unique itemId for concurrent requests', async () => {
         const mockChildren1 = [createMockPage('child-1', '/parent1/child-1')];
         const mockChildren2 = [createMockPage('child-2', '/parent2/child-2')];
 
+        let resolvePromise1: ((value: unknown) => void) | undefined;
+        let resolvePromise2: ((value: unknown) => void) | undefined;
+
         mockApiv3Get
-          .mockResolvedValueOnce({ data: { children: mockChildren1 } })
-          .mockResolvedValueOnce({ data: { children: mockChildren2 } })
-          .mockResolvedValueOnce({ data: { children: mockChildren1 } })
-          .mockResolvedValueOnce({ data: { children: mockChildren2 } });
+          .mockReturnValueOnce(
+            new Promise((resolve) => {
+              resolvePromise1 = resolve;
+            }),
+          )
+          .mockReturnValueOnce(
+            new Promise((resolve) => {
+              resolvePromise2 = resolve;
+            }),
+          );
 
         const { result } = renderHook(() =>
           useDataLoader(ROOT_PAGE_ID, ALL_PAGES_COUNT),
         );
 
-        // First calls
-        await getDataLoader(result).getChildrenWithData('parent-1');
-        await getDataLoader(result).getChildrenWithData('parent-2');
-        expect(mockApiv3Get).toHaveBeenCalledTimes(2);
+        // Start concurrent requests for different IDs
+        const promise1 = getDataLoader(result).getChildrenWithData('parent-1');
+        const promise2 = getDataLoader(result).getChildrenWithData('parent-2');
+
+        // Resolve both
+        resolvePromise1?.({ data: { children: mockChildren1 } });
+        resolvePromise2?.({ data: { children: mockChildren2 } });
 
-        // Clear all cache
-        clearChildrenCache();
+        await Promise.all([promise1, promise2]);
 
-        // Calls after cache clear
-        await getDataLoader(result).getChildrenWithData('parent-1');
-        await getDataLoader(result).getChildrenWithData('parent-2');
-        expect(mockApiv3Get).toHaveBeenCalledTimes(4);
+        // API should be called once per unique ID
+        expect(mockApiv3Get).toHaveBeenCalledTimes(2);
       });
     });
 
@@ -351,7 +304,7 @@ describe('use-data-loader', () => {
     });
 
     describe('error handling', () => {
-      test('should remove cache entry on API error', async () => {
+      test('should allow retry after API error', async () => {
         const error = new Error('API Error');
         mockApiv3Get.mockRejectedValueOnce(error).mockResolvedValueOnce({
           data: { children: [createMockPage('child-1', '/parent/child-1')] },
@@ -361,13 +314,16 @@ describe('use-data-loader', () => {
           useDataLoader(ROOT_PAGE_ID, ALL_PAGES_COUNT),
         );
 
-        // First call - should fail
-        await expect(
-          getDataLoader(result).getChildrenWithData('parent-id'),
-        ).rejects.toThrow('API Error');
+        // First call - should fail and remove pending entry
+        try {
+          await getDataLoader(result).getChildrenWithData('parent-id');
+        } catch {
+          // Expected to fail
+        }
 
-        // Second call - should retry since cache entry was removed
-        const children = await getDataLoader(result).getChildrenWithData('parent-id');
+        // Second call - should retry since pending entry was removed
+        const children =
+          await getDataLoader(result).getChildrenWithData('parent-id');
 
         expect(children).toHaveLength(1);
         expect(mockApiv3Get).toHaveBeenCalledTimes(2);
@@ -375,55 +331,11 @@ describe('use-data-loader', () => {
     });
   });
 
-  describe('clearChildrenCache', () => {
-    test('should clear specific cache entries', async () => {
-      const mockChildren = [createMockPage('child-1', '/parent/child-1')];
-      mockApiv3Get.mockResolvedValue({ data: { children: mockChildren } });
-
-      const { result } = renderHook(() =>
-        useDataLoader(ROOT_PAGE_ID, ALL_PAGES_COUNT),
-      );
-
-      // Populate cache
-      await getDataLoader(result).getChildrenWithData('parent-1');
-      await getDataLoader(result).getChildrenWithData('parent-2');
-      expect(mockApiv3Get).toHaveBeenCalledTimes(2);
-
-      // Clear only parent-1
-      clearChildrenCache(['parent-1']);
-
-      // parent-1 should call API again, parent-2 should use cache
-      await getDataLoader(result).getChildrenWithData('parent-1');
-      await getDataLoader(result).getChildrenWithData('parent-2');
-      expect(mockApiv3Get).toHaveBeenCalledTimes(3);
-    });
-
-    test('should clear all cache entries when called without arguments', async () => {
-      const mockChildren = [createMockPage('child-1', '/parent/child-1')];
-      mockApiv3Get.mockResolvedValue({ data: { children: mockChildren } });
-
-      const { result } = renderHook(() =>
-        useDataLoader(ROOT_PAGE_ID, ALL_PAGES_COUNT),
-      );
-
-      // Populate cache
-      await getDataLoader(result).getChildrenWithData('parent-1');
-      await getDataLoader(result).getChildrenWithData('parent-2');
-      expect(mockApiv3Get).toHaveBeenCalledTimes(2);
-
-      // Clear all cache
-      clearChildrenCache();
-
-      // Both should call API again
-      await getDataLoader(result).getChildrenWithData('parent-1');
-      await getDataLoader(result).getChildrenWithData('parent-2');
-      expect(mockApiv3Get).toHaveBeenCalledTimes(4);
-    });
-  });
-
   describe('placeholder node for page creation', () => {
     test('should prepend placeholder node when parent is in creating mode', async () => {
-      const mockChildren = [createMockPage('existing-child', '/parent/existing')];
+      const mockChildren = [
+        createMockPage('existing-child', '/parent/existing'),
+      ];
       mockApiv3Get.mockResolvedValue({ data: { children: mockChildren } });
 
       // Set creating state BEFORE rendering the hook
@@ -434,7 +346,8 @@ describe('use-data-loader', () => {
         useDataLoader(ROOT_PAGE_ID, ALL_PAGES_COUNT),
       );
 
-      const children = await getDataLoader(result).getChildrenWithData('parent-id');
+      const children =
+        await getDataLoader(result).getChildrenWithData('parent-id');
 
       // Should have placeholder + existing children
       expect(children).toHaveLength(2);
@@ -448,7 +361,9 @@ describe('use-data-loader', () => {
     });
 
     test('should not add placeholder when parent is not in creating mode', async () => {
-      const mockChildren = [createMockPage('existing-child', '/parent/existing')];
+      const mockChildren = [
+        createMockPage('existing-child', '/parent/existing'),
+      ];
       mockApiv3Get.mockResolvedValue({ data: { children: mockChildren } });
 
       // Creating state is null (not creating)
@@ -459,7 +374,8 @@ describe('use-data-loader', () => {
         useDataLoader(ROOT_PAGE_ID, ALL_PAGES_COUNT),
       );
 
-      const children = await getDataLoader(result).getChildrenWithData('parent-id');
+      const children =
+        await getDataLoader(result).getChildrenWithData('parent-id');
 
       // Should only have existing children, no placeholder
       expect(children).toHaveLength(1);
@@ -467,7 +383,9 @@ describe('use-data-loader', () => {
     });
 
     test('should not add placeholder to different parent', async () => {
-      const mockChildren = [createMockPage('existing-child', '/other/existing')];
+      const mockChildren = [
+        createMockPage('existing-child', '/other/existing'),
+      ];
       mockApiv3Get.mockResolvedValue({ data: { children: mockChildren } });
 
       // Creating under 'parent-id', but fetching children of 'other-id'
@@ -478,7 +396,8 @@ describe('use-data-loader', () => {
         useDataLoader(ROOT_PAGE_ID, ALL_PAGES_COUNT),
       );
 
-      const children = await getDataLoader(result).getChildrenWithData('other-id');
+      const children =
+        await getDataLoader(result).getChildrenWithData('other-id');
 
       // Should only have existing children, no placeholder
       expect(children).toHaveLength(1);
@@ -497,7 +416,8 @@ describe('use-data-loader', () => {
         useDataLoader(ROOT_PAGE_ID, ALL_PAGES_COUNT),
       );
 
-      const children = await getDataLoader(result).getChildrenWithData('empty-parent-id');
+      const children =
+        await getDataLoader(result).getChildrenWithData('empty-parent-id');
 
       // Should have only the placeholder
       expect(children).toHaveLength(1);
@@ -507,7 +427,9 @@ describe('use-data-loader', () => {
     });
 
     test('should read creating state via refs when called after state change', async () => {
-      const mockChildren = [createMockPage('existing-child', '/parent/existing')];
+      const mockChildren = [
+        createMockPage('existing-child', '/parent/existing'),
+      ];
       mockApiv3Get.mockResolvedValue({ data: { children: mockChildren } });
 
       // Render hook WITHOUT creating state
@@ -516,8 +438,8 @@ describe('use-data-loader', () => {
       );
 
       // First call - no placeholder
-      clearChildrenCache();
-      const childrenBefore = await getDataLoader(result).getChildrenWithData('parent-id');
+      const childrenBefore =
+        await getDataLoader(result).getChildrenWithData('parent-id');
       expect(childrenBefore).toHaveLength(1);
       expect(childrenBefore[0].id).toBe('existing-child');
 
@@ -528,11 +450,13 @@ describe('use-data-loader', () => {
       // Rerender to update refs
       rerender();
 
-      // Clear cache to force re-fetch
-      clearChildrenCache();
-
       // Second call - should have placeholder because refs are updated
-      const childrenAfter = await getDataLoader(result).getChildrenWithData('parent-id');
+      // Note: Since sequential caching is now handled by headless-tree,
+      // we need to clear pending requests to get fresh data
+      invalidatePageTreeChildren();
+
+      const childrenAfter =
+        await getDataLoader(result).getChildrenWithData('parent-id');
       expect(childrenAfter).toHaveLength(2);
       expect(childrenAfter[0].id).toBe(CREATING_PAGE_VIRTUAL_ID);
       expect(childrenAfter[1].id).toBe('existing-child');

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

@@ -5,6 +5,7 @@ import { apiv3Get } from '~/client/util/apiv3-client';
 import type { IPageForTreeItem } from '~/interfaces/page';
 
 import { ROOT_PAGE_VIRTUAL_ID } from '../../constants';
+import { type ChildrenData, fetchAndCacheChildren } from '../services';
 import {
   CREATING_PAGE_VIRTUAL_ID,
   createPlaceholderPageData,
@@ -27,33 +28,6 @@ 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 = (
   rootPageId: string,
   allPagesCount: number,
@@ -72,6 +46,7 @@ export const useDataLoader = (
 
   // Memoize the entire dataLoader object to ensure reference stability
   // Only recreate when rootPageId or allPagesCount changes (which are truly needed for the API calls)
+  // Note: Creating state is read from refs inside callbacks to avoid triggering dataLoader recreation
   const dataLoader = useMemo<TreeDataLoader<IPageForTreeItem>>(() => {
     const getItem = async (itemId: string): Promise<IPageForTreeItem> => {
       // Virtual root (should rarely be called since it's provided by getChildrenWithData)
@@ -94,21 +69,9 @@ export const useDataLoader = (
       return response.data.item;
     };
 
-    const fetchChildrenFromApi = async (
+    const getChildrenWithData = 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) => {
+    ): Promise<ChildrenData> => {
       // Virtual root returns root page as its only child
       // Use actual MongoDB _id as tree item ID to avoid duplicate API calls
       if (itemId === ROOT_PAGE_VIRTUAL_ID) {
@@ -125,35 +88,7 @@ export const useDataLoader = (
         return [];
       }
 
-      // Check cache first
-      const now = Date.now();
-      const cached = childrenCache.get(itemId);
-
-      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;
-        }
-      }
+      const children = await fetchAndCacheChildren(itemId);
 
       // If this parent is in "creating" mode, prepend placeholder node
       // Read from refs to get current value without triggering dataLoader recreation

+ 5 - 0
apps/app/src/features/page-tree/client/services/index.ts

@@ -0,0 +1,5 @@
+export {
+  type ChildrenData,
+  fetchAndCacheChildren,
+  invalidatePageTreeChildren,
+} from './page-tree-children';

+ 53 - 0
apps/app/src/features/page-tree/client/services/page-tree-children.ts

@@ -0,0 +1,53 @@
+import { apiv3Get } from '~/client/util/apiv3-client';
+import type { IPageForTreeItem } from '~/interfaces/page';
+
+export type ChildrenData = { id: string; data: IPageForTreeItem }[];
+
+/**
+ * Pending requests for concurrent deduplication
+ * Note: Sequential cache is handled by headless-tree's internal cache
+ */
+const pending = new Map<string, Promise<ChildrenData>>();
+
+/**
+ * Clear pending requests (for cache invalidation scenarios)
+ */
+export const invalidatePageTreeChildren = (itemIds?: string[]): void => {
+  if (itemIds == null) {
+    pending.clear();
+  } else {
+    itemIds.forEach((id) => {
+      pending.delete(id);
+    });
+  }
+};
+
+/**
+ * Fetch children data with concurrent request deduplication
+ * Sequential caching is delegated to headless-tree's internal cache
+ */
+export const fetchAndCacheChildren = async (
+  itemId: string,
+): Promise<ChildrenData> => {
+  const existing = pending.get(itemId);
+  if (existing) return existing;
+
+  const promise = (async () => {
+    try {
+      const response = await apiv3Get<{ children: IPageForTreeItem[] }>(
+        '/page-listing/children',
+        { id: itemId },
+      );
+      return response.data.children.map((child) => ({
+        id: child._id,
+        data: child,
+      }));
+    } finally {
+      pending.delete(itemId);
+    }
+  })();
+
+  pending.set(itemId, promise);
+
+  return promise;
+};

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

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