refactor

apps/app/src/pages/[[...path]].page.tsx

@@ -9,8 +9,6 @@ import type {
   IDataWithMeta,
 } from '@growi/core';
 import { isClient } from '@growi/core/dist/utils';
-import { isPermalink } from '@growi/core/dist/utils/page-path-utils';
-import { removeHeadingSlash } from '@growi/core/dist/utils/path-utils';
 import type {
   GetServerSideProps, GetServerSidePropsContext,
 } from 'next';
@@ -42,6 +40,7 @@ import { useSetupGlobalSocket, useSetupGlobalSocketForPage } from '~/stores/webs
 import { useSWRMUTxCurrentPageYjsData } from '~/stores/yjs';
 
 import { useSameRouteNavigation, useShallowRouting } from './[[...path]]/hooks';
+import { extractPageIdFromPathname } from './[[...path]]/navigation-utils';
 import { getServerSidePropsForInitial, getServerSidePropsForSameRoute } from './[[...path]]/server-side-props';
 import type {
   Props, InitialProps, SameRouteEachProps, IPageToShowRevisionWithMeta,
@@ -114,13 +113,6 @@ const GrowiContextualSubNavigation = (props: GrowiContextualSubNavigationProps):
   );
 };
 
-const extractPageIdFromPathname = (pathname: string): string | null => {
-  if (isPermalink(pathname)) {
-    return removeHeadingSlash(pathname);
-  }
-  return null;
-};
-
 const isInitialProps = (props: Props): props is (InitialProps & SameRouteEachProps) => {
   return 'isNextjsRoutingTypeInitial' in props && props.isNextjsRoutingTypeInitial;
 };
@@ -157,20 +149,20 @@ const Page: NextPageWithLayout<Props> = (props: Props) => {
   useSameRouteNavigation(props, extractPageIdFromPathname, isInitialProps);
   useShallowRouting(props);
 
-  // Load current yjs data - optimize to avoid unnecessary calls
+  // Optimized effects with minimal dependencies
   useEffect(() => {
-    if (pageId != null && currentPage?.revision?._id != null && !isNotFound) {
+    // Load YJS data only when revision changes and page exists
+    if (pageId && currentPage?.revision?._id && !isNotFound) {
       mutateCurrentPageYjsDataFromApi();
     }
   }, [currentPage?.revision?._id, mutateCurrentPageYjsDataFromApi, isNotFound, pageId]);
 
-  // Initialize mutateEditingMarkdown only once per page change
-  // Use currentPagePath not props.currentPathname to avoid unnecessary updates
   useEffect(() => {
-    if (currentPagePath != null) {
+    // Initialize editing markdown only when page path changes
+    if (currentPagePath) {
       mutateEditingMarkdown(currentPage?.revision?.body);
     }
-  }, [mutateEditingMarkdown, currentPage?.revision?.body, currentPagePath]);
+  }, [currentPagePath, currentPage?.revision?.body, mutateEditingMarkdown]);
 
   // If the data on the page changes without router.push, pageWithMeta remains old because getServerSideProps() is not executed
   // So preferentially take page data from useSWRxCurrentPage

apps/app/src/pages/[[...path]]/hooks/index.ts

@@ -1,9 +1,11 @@
 /**
- * Custom hooks for [[...path]].page.tsx component
+ * Custom hooks and utilities for [[...path]].page.tsx component
  *
  * This module exports navigation and routing hooks that were extracted
  * from the main page component to improve maintainability and testability.
+ * Also includes optimized utility functions for common navigation operations.
  */
 
 export { useSameRouteNavigation } from '../use-same-route-navigation';
 export { useShallowRouting } from '../use-shallow-routing';
+export { extractPageIdFromPathname, createNavigationState, isSamePage } from '../navigation-utils';

apps/app/src/pages/[[...path]]/navigation-utils.ts

@@ -0,0 +1,58 @@
+import { isPermalink } from '@growi/core/dist/utils/page-path-utils';
+import { removeHeadingSlash } from '@growi/core/dist/utils/path-utils';
+
+/**
+ * Optimized utility functions for navigation handling
+ * Centralized logic to reduce code duplication and improve maintainability
+ */
+
+/**
+ * Extract pageId from pathname efficiently
+ * Returns null for non-permalink paths to optimize conditional checks
+ */
+export const extractPageIdFromPathname = (pathname: string): string | null => {
+  return isPermalink(pathname) ? removeHeadingSlash(pathname) : null;
+};
+
+/**
+ * Check if two paths represent the same page
+ * Optimized for common comparison scenarios
+ */
+export const isSamePage = (pathA?: string | null, pathB?: string | null): boolean => {
+  if (!pathA || !pathB) return false;
+  return pathA === pathB;
+};
+
+/**
+ * Batch state checker for navigation decisions
+ * Combines multiple boolean checks into a single calculation
+ */
+export const createNavigationState = (params: {
+  pageId?: string | null;
+  currentPagePath?: string;
+  targetPathname: string;
+  targetPageId: string | null;
+  hasInitialData: boolean;
+  skipSSR: boolean;
+}): {
+  isPageIdMismatch: boolean;
+  isPathMismatch: boolean;
+  needsPathBasedFetch: boolean;
+  shouldFetch: boolean;
+} => {
+  const {
+    pageId, currentPagePath, targetPathname, targetPageId, hasInitialData, skipSSR,
+  } = params;
+
+  const isPageIdMismatch = pageId != null && targetPageId != null && pageId !== targetPageId;
+  const isPathMismatch = pageId != null && targetPageId == null && currentPagePath !== targetPathname;
+  const needsPathBasedFetch = targetPageId == null && !isPathMismatch
+    && (skipSSR || !currentPagePath || currentPagePath !== targetPathname);
+
+  return {
+    isPageIdMismatch,
+    isPathMismatch,
+    needsPathBasedFetch,
+    shouldFetch: !hasInitialData && (isPageIdMismatch || isPathMismatch || needsPathBasedFetch),
+  };
+};

apps/app/src/pages/[[...path]]/use-same-route-navigation.ts

@@ -1,19 +1,20 @@
-import { useEffect } from 'react';
+import { useEffect, useMemo } from 'react';
 
 import {
   useCurrentPageData, useFetchCurrentPage, useCurrentPageId,
 } from '../../states/page';
 import { useEditingMarkdown } from '../../stores/editor';
 
+import { extractPageIdFromPathname, createNavigationState } from './navigation-utils';
 import type { Props, InitialProps, SameRouteEachProps } from './types';
 
 /**
  * Custom hook for handling same-route navigation and fetching page data when needed
- * Handles permalink navigation, path-based navigation, and browser back/forward scenarios
+ * Optimized for minimal re-renders and efficient state updates using centralized navigation state
  */
 export const useSameRouteNavigation = (
     props: Props,
-    extractPageIdFromPathname: (pathname: string) => string | null,
+    _extractPageIdFromPathname: (pathname: string) => string | null, // Legacy parameter for backward compatibility
     isInitialProps: (props: Props) => props is (InitialProps & SameRouteEachProps),
 ): void => {
   const [currentPage] = useCurrentPageData();
@@ -21,58 +22,50 @@ export const useSameRouteNavigation = (
   const { fetchCurrentPage } = useFetchCurrentPage();
   const { mutate: mutateEditingMarkdown } = useEditingMarkdown();
 
-  // Handle same-route navigation: fetch page data when needed
-  useEffect(() => {
-    const currentPathname = props.currentPathname;
-    const newPageId = extractPageIdFromPathname(currentPathname);
+  // Memoize navigation state using centralized logic
+  const navigationState = useMemo(() => {
+    const targetPathname = props.currentPathname;
+    const targetPageId = extractPageIdFromPathname(targetPathname);
     const skipSSR = isInitialProps(props) ? props.skipSSR : false;
+    const hasInitialData = isInitialProps(props) && !skipSSR;
 
-    // Skip if we have initial props with complete data
-    if (isInitialProps(props) && !skipSSR) {
-      return;
-    }
+    return createNavigationState({
+      pageId,
+      currentPagePath: currentPage?.path,
+      targetPathname,
+      targetPageId,
+      hasInitialData,
+      skipSSR,
+    });
+  }, [props, isInitialProps, pageId, currentPage?.path]);
 
-    // Check if current pageId matches the URL (important for browser back/forward)
-    const isPageIdMismatch = pageId != null && newPageId != null && pageId !== newPageId;
-    const isPathMismatch = pageId != null && newPageId == null && currentPage?.path !== currentPathname;
+  // Handle same-route navigation with optimized batch operations
+  useEffect(() => {
+    if (!navigationState.shouldFetch) return;
 
-    // Determine if we need to fetch new page data based on different scenarios:
-    const needsFetch = (
-      // 1. Permalink navigation with different pageId (includes browser back/forward)
-      isPageIdMismatch
-      // 2. Path-based navigation mismatch (includes browser back/forward to path-based pages)
-      || isPathMismatch
-      // 3. Path-based navigation (no pageId) or forced client-side rendering
-      || (newPageId == null && !isPathMismatch && (
-        // Force fetch if skipSSR is true
-        skipSSR
-        // Or if we don't have current page data for this path
-        || (!currentPage || currentPage.path !== currentPathname)
-      ))
-    );
+    // Batch state updates for optimal performance
+    const updatePageState = async(): Promise<void> => {
+      const targetPageId = extractPageIdFromPathname(props.currentPathname);
 
-    if (needsFetch) {
-      const mutatePageData = async() => {
-        // Clear old data first to prevent using stale pageId
-        if (isPageIdMismatch || isPathMismatch) {
-          setCurrentPageId(undefined);
-        }
+      // Clear stale pageId atomically when needed
+      if (navigationState.isPageIdMismatch || navigationState.isPathMismatch) {
+        setCurrentPageId(undefined);
+      }
 
-        // Update pageId if we have a new one from permalink
-        if (newPageId != null) {
-          setCurrentPageId(newPageId);
-        }
+      // Set new pageId if available
+      if (targetPageId) {
+        setCurrentPageId(targetPageId);
+      }
 
-        // fetchCurrentPage will handle both pageId and path-based navigation
-        const pageData = await fetchCurrentPage(currentPathname);
-        mutateEditingMarkdown(pageData?.revision?.body);
-      };
+      // Fetch and update page data
+      const pageData = await fetchCurrentPage(props.currentPathname);
+      if (pageData?.revision?.body !== undefined) {
+        mutateEditingMarkdown(pageData.revision.body);
+      }
+    };
 
-      mutatePageData();
-    }
-  // Remove fetchCurrentPage, mutateEditingMarkdown, setCurrentPageId from deps to prevent infinite loop
-  // They are stable functions and don't need to be in dependencies
-  // Use currentPage?.path instead of currentPage to be more specific
-  // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [props.currentPathname, pageId, currentPage?.path]);
+    updatePageState();
+    // Stable functions from hooks are omitted from dependencies to prevent infinite loops
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [navigationState.shouldFetch, navigationState.isPageIdMismatch, navigationState.isPathMismatch, props.currentPathname]);
 };

apps/app/src/pages/[[...path]]/use-shallow-routing.ts

@@ -1,4 +1,4 @@
-import { useEffect } from 'react';
+import { useEffect, useRef } from 'react';
 
 import { isClient } from '@growi/core/dist/utils';
 import { useRouter } from 'next/router';
@@ -7,19 +7,28 @@ import type { Props } from './types';
 
 /**
  * Custom hook for syncing pathname by Shallow Routing
- * Optimized to avoid unnecessary updates and handle URL synchronization
+ * Optimized to minimize unnecessary router operations and re-renders
  */
 export const useShallowRouting = (props: Props): void => {
   const router = useRouter();
+  const lastPathnameRef = useRef<string>();
 
-  // Sync pathname by Shallow Routing - optimize to avoid unnecessary updates
+  // Sync pathname by Shallow Routing with performance optimization
   useEffect(() => {
-    if (isClient() && props.currentPathname) {
-      const decodedURI = decodeURI(window.location.pathname);
-      if (decodedURI !== props.currentPathname) {
-        const { search, hash } = window.location;
-        router.replace(`${props.currentPathname}${search}${hash}`, undefined, { shallow: true });
-      }
+    if (!isClient() || !props.currentPathname) return;
+
+    // Skip if pathname hasn't changed (prevents unnecessary operations)
+    if (lastPathnameRef.current === props.currentPathname) return;
+
+    const currentURL = decodeURI(window.location.pathname);
+
+    // Only update if URLs actually differ
+    if (currentURL !== props.currentPathname) {
+      const { search, hash } = window.location;
+      router.replace(`${props.currentPathname}${search}${hash}`, undefined, { shallow: true });
     }
+
+    // Update reference for next comparison
+    lastPathnameRef.current = props.currentPathname;
   }, [props.currentPathname, router]);
 };

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

@@ -30,68 +30,42 @@ export const useFetchCurrentPage = (): {
 
   const fetchCurrentPage = useAtomCallback(
     useCallback(async(get, set, currentPathname?: string) => {
-      let currentPageId = get(currentPageIdAtom);
-
-      // Get current path - prefer provided pathname, fallback to URL
-      const currentPath = currentPathname
-        || (isClient() ? decodeURIComponent(window.location.pathname) : '');
-
-      // Validate pageId against current URL to prevent fetching wrong page on browser back/forward
-      if (currentPageId) {
-        const expectedPageId = isPermalink(currentPath) ? removeHeadingSlash(currentPath) : null;
+      const currentPath = currentPathname || (isClient() ? decodeURIComponent(window.location.pathname) : '');
 
-        // If we have a pageId but it doesn't match the current URL, clear it
-        if (expectedPageId && currentPageId !== expectedPageId) {
-          currentPageId = expectedPageId;
-          set(currentPageIdAtom, currentPageId);
-        }
-        // If current path is not a permalink but we have a pageId, it's likely a mismatch
-        else if (!expectedPageId && isPermalink(`/${currentPageId}`)) {
-          // Current URL is path-based but we have a pageId from previous navigation
-          currentPageId = undefined;
-          set(currentPageIdAtom, undefined);
-        }
-      }
+      // Determine target pageId from current path
+      const targetPageId = isPermalink(currentPath) ? removeHeadingSlash(currentPath) : null;
+      let currentPageId = get(currentPageIdAtom);
 
-      // If no pageId in atom, try to extract from current path
-      if (!currentPageId && currentPath) {
-        if (isPermalink(currentPath)) {
-          currentPageId = removeHeadingSlash(currentPath);
-          // Update the atom with the extracted pageId
-          set(currentPageIdAtom, currentPageId);
-        }
+      // Sync pageId with current path - single atomic update
+      if (currentPageId !== targetPageId) {
+        currentPageId = targetPageId || undefined;
+        set(currentPageIdAtom, currentPageId);
       }
 
-      // Get URL parameter for specific revisionId
-      let revisionId: string | undefined;
-      if (isClient()) {
-        const urlParams = new URLSearchParams(window.location.search);
-        const requestRevisionId = urlParams.get('revisionId');
-        revisionId = requestRevisionId != null ? requestRevisionId : undefined;
-      }
+      // Get URL parameter for specific revisionId - only when needed
+      const revisionId = isClient() && window.location.search
+        ? new URLSearchParams(window.location.search).get('revisionId') || undefined
+        : undefined;
 
       setIsLoading(true);
       setError(null);
 
       try {
-        // Build API parameters - prefer pageId if available, fallback to path
-        const apiParams: { pageId?: string; path?: string; shareLinkId?: string; revisionId?: string } = { shareLinkId };
+        // Build API parameters efficiently
+        const apiParams: Record<string, string> = {};
 
+        if (shareLinkId) apiParams.shareLinkId = shareLinkId;
+        if (revisionId) apiParams.revisionId = revisionId;
+
+        // Use pageId when available, fallback to path
         if (currentPageId) {
           apiParams.pageId = currentPageId;
         }
         else if (currentPath) {
-          // For path-based navigation when pageId is not available
-          // Use the provided/decoded path to avoid double encoding
           apiParams.path = currentPath;
         }
         else {
-          // No pageId and no path, cannot proceed
-          return null;
-        }
-
-        if (revisionId) {
-          apiParams.revisionId = revisionId;
+          return null; // No valid identifier
         }
 
         const response = await apiv3Get<{ page: IPagePopulatedToShowRevision }>(
@@ -100,46 +74,41 @@ export const useFetchCurrentPage = (): {
         );
 
         const newData = response.data.page;
+
+        // Batch atom updates to minimize re-renders
         set(currentPageDataAtom, newData);
+        set(pageNotFoundAtom, false);
 
-        // Update pageId atom if we got the page data (important for path-based navigation)
-        if (newData?._id && newData._id !== currentPageId) {
-          set(currentPageIdAtom, newData._id);
+        // Update pageId atom if data differs from current
+        if (newData?._id !== currentPageId) {
+          set(currentPageIdAtom, newData?._id);
         }
 
-        // Reset routing state when page is successfully fetched
-        set(pageNotFoundAtom, false);
-
         return newData;
       }
       catch (err) {
-        // Handle page not found errors for same-route navigation
-        if (err instanceof Error) {
-          const errorMessage = err.message.toLowerCase();
-
-          // Check if it's a 404 or forbidden error
-          if (errorMessage.includes('not found') || errorMessage.includes('404')) {
-            set(pageNotFoundAtom, true);
-            set(pageNotCreatableAtom, false); // Will be determined by path analysis
-            set(currentPageDataAtom, undefined);
-
-            // For same route, we need to determine if page is creatable
-            // This should match the logic in injectPageDataForInitial
-            if (currentPath) {
-              const { pagePathUtils } = await import('@growi/core/dist/utils');
-              const isCreatable = pagePathUtils.isCreatablePage(currentPath);
-              set(pageNotCreatableAtom, !isCreatable);
-            }
-
-            return null;
+        const errorMsg = err instanceof Error ? err.message.toLowerCase() : '';
+
+        // Handle specific error types with batch updates
+        if (errorMsg.includes('not found') || errorMsg.includes('404')) {
+          // Batch update for 404 errors
+          set(pageNotFoundAtom, true);
+          set(currentPageDataAtom, undefined);
+
+          // Determine if page is creatable
+          if (currentPath) {
+            const { pagePathUtils } = await import('@growi/core/dist/utils');
+            set(pageNotCreatableAtom, !pagePathUtils.isCreatablePage(currentPath));
           }
+          return null;
+        }
 
-          if (errorMessage.includes('forbidden') || errorMessage.includes('403')) {
-            set(pageNotFoundAtom, false);
-            set(pageNotCreatableAtom, true); // Forbidden means page exists but not accessible
-            set(currentPageDataAtom, undefined);
-            return null;
-          }
+        if (errorMsg.includes('forbidden') || errorMsg.includes('403')) {
+          // Batch update for 403 errors
+          set(pageNotFoundAtom, false);
+          set(pageNotCreatableAtom, true);
+          set(currentPageDataAtom, undefined);
+          return null;
         }
 
         setError(new Error('Failed to fetch current page'));