refactor useFetchCurrentPage

apps/app/src/states/page/use-fetch-current-page.spec.tsx

@@ -285,27 +285,6 @@ describe('useFetchCurrentPage - Integration Test', () => {
     });
   });
 
-  it('should set pageNotFoundAtom on 404 error for a non-creatable path', async () => {
-    // Arrange
-    const notCreatablePath = '/user';
-    const apiError = {
-      response: {
-        status: 404,
-      },
-    };
-    mockedApiv3Get.mockRejectedValue(apiError);
-
-    // Act
-    const { result } = renderHookWithProvider();
-    await result.current.fetchCurrentPage({ path: notCreatablePath });
-
-    // Assert
-    await waitFor(() => {
-      expect(store.get(pageNotFoundAtom)).toBe(true);
-      expect(store.get(pageErrorAtom)).toEqual(apiError);
-    });
-  });
-
   it('should handle path with permalink and convert to pageId for API call', async () => {
     // Arrange: A path that looks like a permalink
     const permalinkPath = '/58a4569921a8424d00a1aa0e';

apps/app/src/states/page/use-fetch-current-page.ts

@@ -19,6 +19,8 @@ import {
   pageErrorAtom,
   pageLoadingAtom,
   pageNotFoundAtom,
+  remoteRevisionBodyAtom,
+  remoteRevisionIdAtom,
   shareLinkIdAtom,
 } from './internal-atoms';
 
@@ -30,6 +32,125 @@ type FetchPageArgs = {
   revisionId?: string;
 };
 
+/**
+ * Process path to handle URL decoding and hash fragment removal
+ */
+const decodeAndRemoveFragment = (pathname?: string): string | undefined => {
+  if (pathname == null) return;
+
+  // Decode path
+  let decodedPathname: string;
+  try {
+    decodedPathname = decodeURIComponent(pathname);
+  } catch {
+    decodedPathname = pathname;
+  }
+
+  // Strip hash fragment from path to properly detect permalinks
+  try {
+    const url = new URL(decodedPathname, 'http://example.com');
+    return url.pathname;
+  } catch {
+    // Fallback to simple split if URL parsing fails
+    return decodedPathname.split('#')[0];
+  }
+};
+
+/**
+ * Check if cached data should be used to prevent unnecessary fetching
+ */
+const shouldUseCachedData = (
+  args: FetchPageArgs | undefined,
+  currentPageId: string | undefined,
+  currentPageData: IPagePopulatedToShowRevision | undefined,
+  decodedPathname?: string,
+): boolean => {
+  // Guard clause to prevent unnecessary fetching by pageId
+  if (args?.pageId != null && args.pageId === currentPageId) {
+    return true;
+  }
+
+  // Guard clause to prevent unnecessary fetching by path
+  if (decodedPathname != null) {
+    if (
+      isPermalink(decodedPathname) &&
+      removeHeadingSlash(decodedPathname) === currentPageId
+    ) {
+      return true;
+    }
+    if (decodedPathname === currentPageData?.path) {
+      return true;
+    }
+  }
+
+  return false;
+};
+
+type BuildApiParamsArgs = {
+  fetchPageArgs: Omit<FetchPageArgs, 'path'> | undefined;
+  decodedPathname: string | undefined;
+  currentPageId: string | undefined;
+  shareLinkId: string | undefined;
+};
+type ApiParams = { params: Record<string, string>; shouldSkip: boolean };
+
+/**
+ * Build API parameters for page fetching
+ */
+const buildApiParams = ({
+  fetchPageArgs,
+  decodedPathname,
+  currentPageId,
+  shareLinkId,
+}: BuildApiParamsArgs): ApiParams => {
+  const revisionId =
+    fetchPageArgs?.revisionId ??
+    (isClient()
+      ? new URLSearchParams(window.location.search).get('revisionId')
+      : undefined);
+
+  const params: {
+    path?: string;
+    pageId?: string;
+    revisionId?: string;
+    shareLinkId?: string;
+  } = {};
+
+  if (shareLinkId != null) {
+    params.shareLinkId = shareLinkId;
+  }
+  if (revisionId != null) {
+    params.revisionId = revisionId;
+  }
+
+  // priority A: pageId > permalink > path
+  if (fetchPageArgs?.pageId != null) {
+    params.pageId = fetchPageArgs.pageId;
+  } else if (decodedPathname != null) {
+    if (isPermalink(decodedPathname)) {
+      params.pageId = removeHeadingSlash(decodedPathname);
+    } else {
+      params.path = decodedPathname;
+    }
+    // priority B: currentPageId > permalink(by location) > path(by location)
+  } else if (currentPageId != null) {
+    params.pageId = currentPageId;
+  } else if (isClient()) {
+    try {
+      params.path = decodeURIComponent(window.location.pathname);
+    } catch {
+      params.path = window.location.pathname;
+    }
+  } else {
+    logger.warn(
+      'Either path or pageId must be provided when not in a browser environment',
+    );
+    return { params, shouldSkip: true };
+  }
+
+  return { params, shouldSkip: false };
+};
+
 /**
  * Simplified page fetching hook using Jotai state management
  * All state is managed through atoms for consistent global state
@@ -57,88 +178,32 @@ export const useFetchCurrentPage = (): {
         const currentPageData = get(currentPageDataAtom);
 
         // Process path first to handle permalinks and strip hash fragments
-        let decodedPath: string | undefined;
-        if (args?.path != null) {
-          try {
-            decodedPath = decodeURIComponent(args.path);
-          } catch (e) {
-            decodedPath = args.path;
-          }
-        }
-
-        // Strip hash fragment from path to properly detect permalinks
-        let pathWithoutHash: string | undefined;
-        if (decodedPath != null) {
-          try {
-            const url = new URL(decodedPath, 'http://example.com');
-            pathWithoutHash = url.pathname;
-          } catch {
-            // Fallback to simple split if URL parsing fails
-            pathWithoutHash = decodedPath.split('#')[0];
-          }
-        }
+        const decodedPathname = decodeAndRemoveFragment(args?.path);
 
         // Guard clause to prevent unnecessary fetching
-        if (args?.pageId != null && args.pageId === currentPageId) {
+        if (
+          shouldUseCachedData(
+            args,
+            currentPageId,
+            currentPageData,
+            decodedPathname,
+          )
+        ) {
           return currentPageData ?? null;
         }
-        if (pathWithoutHash != null) {
-          if (
-            isPermalink(pathWithoutHash) &&
-            removeHeadingSlash(pathWithoutHash) === currentPageId
-          ) {
-            return currentPageData ?? null;
-          }
-          if (pathWithoutHash === currentPageData?.path) {
-            return currentPageData ?? null;
-          }
-        }
 
         set(pageLoadingAtom, true);
         set(pageErrorAtom, null);
 
-        // determine parameters
-        const pageId = args?.pageId;
-        const revisionId =
-          args?.revisionId ??
-          (isClient()
-            ? new URLSearchParams(window.location.search).get('revisionId')
-            : undefined);
-
-        // params for API
-        const params: {
-          path?: string;
-          pageId?: string;
-          revisionId?: string;
-          shareLinkId?: string;
-        } = {};
-        if (shareLinkId != null) {
-          params.shareLinkId = shareLinkId;
-        }
-        if (revisionId != null) {
-          params.revisionId = revisionId;
-        }
+        // Build API parameters
+        const { params, shouldSkip } = buildApiParams({
+          fetchPageArgs: args,
+          decodedPathname,
+          currentPageId,
+          shareLinkId,
+        });
 
-        // priority: pageId > permalink > path
-        if (pageId != null) {
-          params.pageId = pageId;
-        } else if (pathWithoutHash != null && isPermalink(pathWithoutHash)) {
-          params.pageId = removeHeadingSlash(pathWithoutHash);
-        } else if (decodedPath != null) {
-          params.path = decodedPath;
-        }
-        // if args is empty, get from global state
-        else if (currentPageId != null) {
-          params.pageId = currentPageId;
-        } else if (isClient()) {
-          try {
-            params.path = decodeURIComponent(window.location.pathname);
-          } catch (e) {
-            params.path = window.location.pathname;
-          }
-        } else {
-          // TODO: https://github.com/weseek/growi/pull/9118
-          // throw new Error('Either path or pageId must be provided when not in a browser environment');
+        if (shouldSkip) {
           set(pageLoadingAtom, false);
           return null;
         }
@@ -155,17 +220,24 @@ export const useFetchCurrentPage = (): {
 
           return newData;
         } catch (err) {
-          if (Array.isArray(err) && err.length > 0 && isErrorV3(err[0])) {
-            const error = err[0];
+          if (!Array.isArray(err) || err.length === 0) {
+            set(pageErrorAtom, new Error('Unknown error'));
+            logger.error('Unhandled error when fetching current page:', err);
+            return null;
+          }
+
+          const error = err[0];
+          if (isErrorV3(error)) {
             set(pageErrorAtom, error);
 
             if (isIPageNotFoundInfo(error.args)) {
               set(pageNotFoundAtom, true);
               set(isForbiddenAtom, error.args.isForbidden ?? false);
               set(currentPageDataAtom, undefined);
+              set(currentPageIdAtom, undefined);
+              set(remoteRevisionIdAtom, undefined);
+              set(remoteRevisionBodyAtom, undefined);
             }
-          } else {
-            logger.error('Unhandled error when fetching current page:', err);
           }
         } finally {
           set(pageLoadingAtom, false);