refactor

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

@@ -1,21 +1,21 @@
 import type { ReactNode, JSX } from 'react';
 import React, { useEffect } from 'react';
 
+
 import EventEmitter from 'events';
 
 import { isIPageInfo } from '@growi/core';
 import type {
   IDataWithMeta,
 } from '@growi/core';
-import {
-  isClient,
-} from '@growi/core/dist/utils';
+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';
 import dynamic from 'next/dynamic';
 import Head from 'next/head';
-import { useRouter } from 'next/router';
 import superjson from 'superjson';
 
 import { BasicLayout } from '~/components/Layout/BasicLayout';
@@ -24,7 +24,7 @@ import { DrawioViewerScript } from '~/components/Script/DrawioViewerScript';
 import { useEditorModeClassName } from '~/services/layout/use-editor-mode-class-name';
 import { useHydrateSidebarAtoms } from '~/states/hydrate/sidebar';
 import {
-  useCurrentPageData, useFetchCurrentPage, useCurrentPageId, useCurrentPagePath, usePageNotFound,
+  useCurrentPageData, useCurrentPageId, useCurrentPagePath, usePageNotFound,
 } from '~/states/page';
 import { useHydratePageAtoms } from '~/states/page/hydrate';
 import {
@@ -41,6 +41,7 @@ import { useRedirectFrom } from '~/stores/page-redirect';
 import { useSetupGlobalSocket, useSetupGlobalSocketForPage } from '~/stores/websocket';
 import { useSWRMUTxCurrentPageYjsData } from '~/stores/yjs';
 
+import { useSameRouteNavigation, useShallowRouting } from './[[...path]]/hooks';
 import { getServerSidePropsForInitial, getServerSidePropsForSameRoute } from './[[...path]]/server-side-props';
 import type {
   Props, InitialProps, SameRouteEachProps, IPageToShowRevisionWithMeta,
@@ -113,6 +114,13 @@ 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;
 };
@@ -123,8 +131,6 @@ const Page: NextPageWithLayout<Props> = (props: Props) => {
     window.globalEmitter = new EventEmitter();
   }
 
-  const router = useRouter();
-
   useRedirectFrom(props.redirectFrom ?? null);
   useIsSharedUser(false); // this page can't be routed for '/share'
   useIsSearchPage(false);
@@ -134,13 +140,12 @@ const Page: NextPageWithLayout<Props> = (props: Props) => {
   useHydratePageAtoms(pageData);
 
   const [currentPage] = useCurrentPageData();
-  const [pageId, setCurrentPageId] = useCurrentPageId();
+  const [pageId] = useCurrentPageId();
   const [currentPagePath] = useCurrentPagePath();
   const [isNotFound] = usePageNotFound();
   const [rendererConfig] = useRendererConfig();
   const [disableLinkSharing] = useDisableLinkSharing();
 
-  const { fetchCurrentPage } = useFetchCurrentPage();
   const { trigger: mutateCurrentPageYjsDataFromApi } = useSWRMUTxCurrentPageYjsData();
 
   const { mutate: mutateEditingMarkdown } = useEditingMarkdown();
@@ -148,48 +153,24 @@ const Page: NextPageWithLayout<Props> = (props: Props) => {
   useSetupGlobalSocket();
   useSetupGlobalSocketForPage(pageId);
 
-  // Handle same-route navigation: fetch page data when needed
-  useEffect(() => {
-    // Skip if we have initial props with complete data
-    if (isInitialProps(props) && !props.skipSSR) {
-      return;
-    }
+  // Use custom hooks for navigation and routing
+  useSameRouteNavigation(props, extractPageIdFromPathname, isInitialProps);
+  useShallowRouting(props);
 
-    // For same-route or when skipSSR is true, fetch page data client-side
-    if (pageId != null) {
-      const mutatePageData = async() => {
-        setCurrentPageId(pageId);
-        const pageData = await fetchCurrentPage();
-        mutateEditingMarkdown(pageData?.revision?.body);
-      };
-
-      mutatePageData();
-    }
-  }, [pageId, fetchCurrentPage, mutateEditingMarkdown, props, setCurrentPageId]);
-
-  // Load current yjs data
+  // Load current yjs data - optimize to avoid unnecessary calls
   useEffect(() => {
     if (pageId != null && currentPage?.revision?._id != null && !isNotFound) {
       mutateCurrentPageYjsDataFromApi();
     }
-  }, [currentPage, mutateCurrentPageYjsDataFromApi, isNotFound, currentPage?.revision?._id, pageId]);
-
-  // sync pathname by Shallow Routing https://nextjs.org/docs/routing/shallow-routing
-  useEffect(() => {
-    const decodedURI = decodeURI(window.location.pathname);
-    if (isClient() && decodedURI !== props.currentPathname) {
-      const { search, hash } = window.location;
-      router.replace(`${props.currentPathname}${search}${hash}`, undefined, { shallow: true });
-    }
-  }, [props.currentPathname, router]);
+  }, [currentPage?.revision?._id, mutateCurrentPageYjsDataFromApi, isNotFound, pageId]);
 
-  // initialize mutateEditingMarkdown only once per page
-  // need to include useCurrentPathname not useCurrentPagePath
+  // Initialize mutateEditingMarkdown only once per page change
+  // Use currentPagePath not props.currentPathname to avoid unnecessary updates
   useEffect(() => {
-    if (props.currentPathname != null) {
+    if (currentPagePath != null) {
       mutateEditingMarkdown(currentPage?.revision?.body);
     }
-  }, [mutateEditingMarkdown, currentPage?.revision?.body, props.currentPathname]);
+  }, [mutateEditingMarkdown, currentPage?.revision?.body, currentPagePath]);
 
   // 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

@@ -0,0 +1,9 @@
+/**
+ * Custom hooks 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.
+ */
+
+export { useSameRouteNavigation } from '../use-same-route-navigation';
+export { useShallowRouting } from '../use-shallow-routing';

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

@@ -0,0 +1,78 @@
+import { useEffect } from 'react';
+
+import {
+  useCurrentPageData, useFetchCurrentPage, useCurrentPageId,
+} from '../../states/page';
+import { useEditingMarkdown } from '../../stores/editor';
+
+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
+ */
+export const useSameRouteNavigation = (
+    props: Props,
+    extractPageIdFromPathname: (pathname: string) => string | null,
+    isInitialProps: (props: Props) => props is (InitialProps & SameRouteEachProps),
+): void => {
+  const [currentPage] = useCurrentPageData();
+  const [pageId, setCurrentPageId] = useCurrentPageId();
+  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);
+    const skipSSR = isInitialProps(props) ? props.skipSSR : false;
+
+    // Skip if we have initial props with complete data
+    if (isInitialProps(props) && !skipSSR) {
+      return;
+    }
+
+    // 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;
+
+    // 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)
+      ))
+    );
+
+    if (needsFetch) {
+      const mutatePageData = async() => {
+        // Clear old data first to prevent using stale pageId
+        if (isPageIdMismatch || isPathMismatch) {
+          setCurrentPageId(undefined);
+        }
+
+        // Update pageId if we have a new one from permalink
+        if (newPageId != null) {
+          setCurrentPageId(newPageId);
+        }
+
+        // fetchCurrentPage will handle both pageId and path-based navigation
+        const pageData = await fetchCurrentPage(currentPathname);
+        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]);
+};

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

@@ -0,0 +1,25 @@
+import { useEffect } from 'react';
+
+import { isClient } from '@growi/core/dist/utils';
+import { useRouter } from 'next/router';
+
+import type { Props } from './types';
+
+/**
+ * Custom hook for syncing pathname by Shallow Routing
+ * Optimized to avoid unnecessary updates and handle URL synchronization
+ */
+export const useShallowRouting = (props: Props): void => {
+  const router = useRouter();
+
+  // Sync pathname by Shallow Routing - optimize to avoid unnecessary updates
+  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 });
+      }
+    }
+  }, [props.currentPathname, router]);
+};

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

@@ -2,6 +2,8 @@ import { useCallback, useState } from 'react';
 
 import type { IPagePopulatedToShowRevision } 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 { useAtomCallback } from 'jotai/utils';
 
 import { apiv3Get } from '~/client/util/apiv3-client';
@@ -17,7 +19,7 @@ import {
  * Replaces the complex useSWRMUTxCurrentPage with cleaner state management
  */
 export const useFetchCurrentPage = (): {
-  fetchCurrentPage: () => Promise<IPagePopulatedToShowRevision | null>,
+  fetchCurrentPage: (currentPathname?: string) => Promise<IPagePopulatedToShowRevision | null>,
   isLoading: boolean,
   error: Error | null,
 } => {
@@ -27,10 +29,38 @@ export const useFetchCurrentPage = (): {
   const [error, setError] = useState<Error | null>(null);
 
   const fetchCurrentPage = useAtomCallback(
-    useCallback(async(get, set) => {
-      const currentPageId = get(currentPageIdAtom);
+    useCallback(async(get, set, currentPathname?: string) => {
+      let currentPageId = get(currentPageIdAtom);
 
-      if (!currentPageId) return null;
+      // 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;
+
+        // 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);
+        }
+      }
+
+      // 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);
+        }
+      }
 
       // Get URL parameter for specific revisionId
       let revisionId: string | undefined;
@@ -44,14 +74,39 @@ export const useFetchCurrentPage = (): {
       setError(null);
 
       try {
+        // Build API parameters - prefer pageId if available, fallback to path
+        const apiParams: { pageId?: string; path?: string; shareLinkId?: string; revisionId?: string } = { shareLinkId };
+
+        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;
+        }
+
         const response = await apiv3Get<{ page: IPagePopulatedToShowRevision }>(
           '/page',
-          { pageId: currentPageId, shareLinkId, revisionId },
+          apiParams,
         );
 
         const newData = response.data.page;
         set(currentPageDataAtom, newData);
 
+        // Update pageId atom if we got the page data (important for path-based navigation)
+        if (newData?._id && newData._id !== currentPageId) {
+          set(currentPageIdAtom, newData._id);
+        }
+
         // Reset routing state when page is successfully fetched
         set(pageNotFoundAtom, false);
 
@@ -70,8 +125,7 @@ export const useFetchCurrentPage = (): {
 
             // For same route, we need to determine if page is creatable
             // This should match the logic in injectPageDataForInitial
-            if (isClient()) {
-              const currentPath = window.location.pathname;
+            if (currentPath) {
               const { pagePathUtils } = await import('@growi/core/dist/utils');
               const isCreatable = pagePathUtils.isCreatablePage(currentPath);
               set(pageNotCreatableAtom, !isCreatable);