WIP: refactor

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

@@ -14,6 +14,7 @@ import type {
 } 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';
@@ -118,6 +119,31 @@ const isInitialProps = (props: Props): props is (InitialProps & SameRouteEachPro
 };
 
 const Page: NextPageWithLayout<Props> = (props: Props) => {
+  const router = useRouter();
+
+  // DEBUG: Log props changes to track browser back behavior
+  console.debug('Page component render:', {
+    currentPathname: props.currentPathname,
+    routerAsPath: router.asPath,
+    timestamp: new Date().toISOString(),
+  });
+
+  // DEBUG: Monitor router changes
+  useEffect(() => {
+    const handleRouteChange = (url: string) => {
+      console.debug('Router route change:', {
+        url,
+        propsCurrentPathname: props.currentPathname,
+        timestamp: new Date().toISOString(),
+      });
+    };
+
+    router.events.on('routeChangeComplete', handleRouteChange);
+    return () => {
+      router.events.off('routeChangeComplete', handleRouteChange);
+    };
+  }, [router.events, props.currentPathname]);
+
   // register global EventEmitter
   if (isClient() && window.globalEmitter == null) {
     window.globalEmitter = new EventEmitter();

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

@@ -8,4 +8,4 @@
 
 export { useSameRouteNavigation } from '../use-same-route-navigation';
 export { useShallowRouting } from '../use-shallow-routing';
-export { extractPageIdFromPathname, createNavigationState, isSamePage } from '../navigation-utils';
+export { extractPageIdFromPathname } from '../navigation-utils';

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

@@ -1,11 +1,6 @@
 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
@@ -13,46 +8,3 @@ import { removeHeadingSlash } from '@growi/core/dist/utils/path-utils';
 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]]/types.ts

@@ -78,8 +78,6 @@ export type ExtendedInitialProps = InitialProps & SameRouteEachProps;
  * Lightweight validation for same-route navigation
  */
 export function isValidSameRouteProps(props: unknown): props is SameRouteEachProps {
-  logger.warn('isValidSameRouteProps');
-
   if (typeof props !== 'object' || props === null) {
     logger.warn('isValidSameRouteProps: props is not an object or is null');
     return false;
@@ -117,8 +115,6 @@ export function isValidSameRouteProps(props: unknown): props is SameRouteEachPro
  * First validates SameRouteEachProps, then checks InitialProps-specific properties
  */
 export function isValidInitialAndSameRouteProps(props: unknown): props is InitialProps & SameRouteEachProps {
-  logger.warn('isValidInitialAndSameRouteProps');
-
   // First, validate SameRouteEachProps
   if (!isValidSameRouteProps(props)) {
     logger.warn('isValidInitialAndSameRouteProps: SameRouteEachProps validation failed');

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

@@ -1,11 +1,13 @@
-import { useEffect, useMemo } from 'react';
+import { useEffect, useMemo, useRef } from 'react';
+
+import { useRouter } from 'next/router';
 
 import {
   useCurrentPageData, useFetchCurrentPage, useCurrentPageId,
 } from '../../states/page';
 import { useEditingMarkdown } from '../../stores/editor';
 
-import { extractPageIdFromPathname, createNavigationState } from './navigation-utils';
+import { extractPageIdFromPathname } from './navigation-utils';
 import type { Props, InitialProps, SameRouteEachProps } from './types';
 
 /**
@@ -17,55 +19,132 @@ export const useSameRouteNavigation = (
     _extractPageIdFromPathname: (pathname: string) => string | null, // Legacy parameter for backward compatibility
     isInitialProps: (props: Props) => props is (InitialProps & SameRouteEachProps),
 ): void => {
+  const router = useRouter();
   const [currentPage] = useCurrentPageData();
   const [pageId, setCurrentPageId] = useCurrentPageId();
   const { fetchCurrentPage } = useFetchCurrentPage();
   const { mutate: mutateEditingMarkdown } = useEditingMarkdown();
 
-  // Memoize navigation state using centralized logic
-  const navigationState = useMemo(() => {
-    const targetPathname = props.currentPathname;
-    const targetPageId = extractPageIdFromPathname(targetPathname);
+  // Track the last processed pathname to prevent unnecessary operations
+  const lastProcessedPathnameRef = useRef<string | null>(null);
+  const isFetchingRef = useRef<boolean>(false);
+
+  // Process pathname changes - monitor both props.currentPathname and router.asPath
+  useEffect(() => {
+    // Use router.asPath for browser back/forward compatibility, fallback to props.currentPathname
+    const targetPathname = router.asPath || props.currentPathname;
+
+    // Always log when useEffect is triggered
+    console.debug('useSameRouteNavigation useEffect triggered:', {
+      targetPathname,
+      lastProcessed: lastProcessedPathnameRef.current,
+      timestamp: new Date().toISOString(),
+    });
+
+    // Skip if we already processed this pathname
+    if (lastProcessedPathnameRef.current === targetPathname) {
+      console.debug('Skipping - already processed:', targetPathname);
+      return;
+    }
+
+    // Skip if we have initial data and don't need to refetch
     const skipSSR = isInitialProps(props) ? props.skipSSR : false;
     const hasInitialData = isInitialProps(props) && !skipSSR;
 
-    return createNavigationState({
-      pageId,
-      currentPagePath: currentPage?.path,
+    if (hasInitialData) {
+      console.debug('Skipping fetch - has initial data:', targetPathname);
+      lastProcessedPathnameRef.current = targetPathname;
+      return;
+    }
+
+    // Check if we need to fetch data
+    const targetPageId = extractPageIdFromPathname(targetPathname);
+    const currentPagePath = currentPage?.path;
+
+    // Always fetch if:
+    // 1. No current page data
+    // 2. Different page ID (only if both are defined)
+    // 3. Different path
+    const shouldFetch = (
+      !currentPagePath // No current page
+      || (targetPageId != null && pageId != null && pageId !== targetPageId) // Different page ID (strict comparison)
+      || (currentPagePath !== targetPathname) // Different path
+    );
+
+    console.debug('shouldFetch decision:', {
+      currentPagePath,
       targetPathname,
       targetPageId,
-      hasInitialData,
-      skipSSR,
+      pageId,
+      condition1_noCurrentPage: !currentPagePath,
+      condition2_differentPageId: (targetPageId != null && pageId != null && pageId !== targetPageId),
+      condition3_differentPath: (currentPagePath !== targetPathname),
+      finalDecision: shouldFetch,
     });
-  }, [props, isInitialProps, pageId, currentPage?.path]);
 
-  // Handle same-route navigation with optimized batch operations
-  useEffect(() => {
-    if (!navigationState.shouldFetch) return;
+    if (!shouldFetch) {
+      console.debug('No fetch needed for:', targetPathname);
+      lastProcessedPathnameRef.current = targetPathname;
+      return;
+    }
 
-    // Batch state updates for optimal performance
-    const updatePageState = async(): Promise<void> => {
-      const targetPageId = extractPageIdFromPathname(props.currentPathname);
+    // Prevent concurrent fetches
+    if (isFetchingRef.current) {
+      console.debug('Fetch already in progress, skipping:', targetPathname);
+      return;
+    }
 
-      // Clear stale pageId atomically when needed
-      if (navigationState.isPageIdMismatch || navigationState.isPathMismatch) {
+    console.debug('Starting fetch for:', targetPathname, {
+      currentPagePath,
+      targetPageId,
+      pageId,
+    });
+
+    isFetchingRef.current = true;
+
+    const updatePageState = async(): Promise<void> => {
+      try {
+        // Clear current state to ensure clean transition
         setCurrentPageId(undefined);
-      }
 
-      // Set new pageId if available
-      if (targetPageId) {
-        setCurrentPageId(targetPageId);
-      }
+        // Set new pageId if available
+        if (targetPageId) {
+          setCurrentPageId(targetPageId);
+        }
+
+        // Fetch page data
+        const pageData = await fetchCurrentPage(targetPathname);
 
-      // Fetch and update page data
-      const pageData = await fetchCurrentPage(props.currentPathname);
-      if (pageData?.revision?.body !== undefined) {
-        mutateEditingMarkdown(pageData.revision.body);
+        console.debug('Page data fetched for:', targetPathname, !!pageData);
+
+        // Update editing markdown if we have body content
+        if (pageData?.revision?.body !== undefined) {
+          mutateEditingMarkdown(pageData.revision.body);
+        }
+
+        // Mark as processed
+        lastProcessedPathnameRef.current = targetPathname;
+      }
+      catch (error) {
+        console.error('Error fetching page data for:', targetPathname, error);
+      }
+      finally {
+        isFetchingRef.current = false;
       }
     };
 
     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]);
+  }, [
+    props.currentPathname, // Always trigger on pathname change
+    router.asPath, // Also trigger on browser back/forward navigation
+  ]);
+
+  // Cleanup on unmount
+  useEffect(() => {
+    return () => {
+      isFetchingRef.current = false;
+    };
+  }, []);
 };

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

@@ -2,8 +2,7 @@ import { useCallback, useState } from 'react';
 
 import type { IPagePopulatedToShowRevision } from '@growi/core';
 import { isClient } from '@growi/core/dist/utils';
-import { isCreatablePage, isPermalink } from '@growi/core/dist/utils/page-path-utils';
-import { removeHeadingSlash } from '@growi/core/dist/utils/path-utils';
+import { isCreatablePage } from '@growi/core/dist/utils/page-path-utils';
 import { useAtomCallback } from 'jotai/utils';
 
 import { apiv3Get } from '~/client/util/apiv3-client';
@@ -33,14 +32,14 @@ export const useFetchCurrentPage = (): {
       const currentPath = currentPathname || (isClient() ? decodeURIComponent(window.location.pathname) : '');
 
       // Determine target pageId from current path
-      const targetPageId = isPermalink(currentPath) ? removeHeadingSlash(currentPath) : null;
-      let currentPageId = get(currentPageIdAtom);
-
-      // Sync pageId with current path - single atomic update
-      if (currentPageId !== targetPageId) {
-        currentPageId = targetPageId || undefined;
-        set(currentPageIdAtom, currentPageId);
-      }
+      // const targetPageId = isPermalink(currentPath) ? removeHeadingSlash(currentPath) : null;
+      const currentPageId = get(currentPageIdAtom);
+
+      // NOTE: PageId sync is now handled by useSameRouteNavigation to prevent conflicts
+      // if (currentPageId !== targetPageId) {
+      //   currentPageId = targetPageId || undefined;
+      //   set(currentPageIdAtom, currentPageId);
+      // }
 
       // Get URL parameter for specific revisionId - only when needed
       const revisionId = isClient() && window.location.search