Yuki Takei 7 месяцев назад
Родитель
Сommit
653effede5

+ 30 - 22
.serena/memories/page-transition-and-rendering-flow.md

@@ -1,50 +1,58 @@
-### ページ遷移とレンダリングフローの分析(リファクタリング後)
+# ページ遷移とレンダリングのデータフロー
 
 
-このドキュメントは、リファクタリング後のページ遷移から `PageView` コンポーネントのレンダリングまでのデータフローを、Jotai atom の役割に焦点を当てて概説します。
+このドキュメントは、GROWIのページ遷移からレンダリングまでのデータフローを解説します。
 
 
-#### 登場人物
+## 登場人物
 
 
-1.  **`[[...path]].page.tsx`**: Next.js の動的ルーティングを担うメインコンポーネント。
-2.  **`useSameRouteNavigation.ts`**: クライアントサイドでのパス変更を検知し、データ取得をトリガーするフック。
-3.  **`useFetchCurrentPage.ts`**: データ取得と関連する Jotai atom の更新を一元管理する、本リファクタリングの心臓部
+1.  **`[[...path]].page.tsx`**: Next.js の動的ルーティングを担うメインコンポーネント。サーバーサイドとクライアントサイドの両方で動作します。
+2.  **`useSameRouteNavigation.ts`**: クライアントサイドでのパス変更を検知し、データ取得を**トリガー**するフック。
+3.  **`useFetchCurrentPage.ts`**: データ取得と関連する Jotai atom の更新を一元管理するフック。データ取得が本当に必要かどうかの最終判断も担います
 4.  **`useShallowRouting.ts`**: サーバーサイドで正規化されたパスとブラウザのURLを同期させるフック。
 4.  **`useShallowRouting.ts`**: サーバーサイドで正規化されたパスとブラウザのURLを同期させるフック。
-5.  **`server-side-props.ts`**: サーバーサイドレンダリング(SSR)時にページデータを取得し、`props` としてページコンポーネントに渡す。
+5.  **`server-side-props.ts`**: サーバーサイドレンダリング(SSR)時にページデータを取得し、`props` としてページコンポーネントに渡します。
 
 
 ---
 ---
 
 
-#### フロー1: サーバーサイドレンダリング(初回アクセス時)
+## フロー1: サーバーサイドレンダリング(初回アクセス時)
 
 
-1.  **リクエスト受信**: ユーザーがURL(例: `/user/sotarok/memo`)に直接アクセスします。
+ユーザーがURLに直接アクセスするか、ページをリロードした際のフローです。
+
+1.  **リクエスト受信**: サーバーがユーザーからのリクエスト(例: `/user/username/memo`)を受け取ります。
 2.  **`getServerSideProps` の実行**:
 2.  **`getServerSideProps` の実行**:
     - `server-side-props.ts` の `getServerSidePropsForInitial` が実行されます。
     - `server-side-props.ts` の `getServerSidePropsForInitial` が実行されます。
-    - `retrievePageData` が呼び出され、パスの正規化(例: `/user/sotarok` → `/user/sotarok/`)が行われ、APIからページデータを取得します。
+    - `retrievePageData` が呼び出され、パスの正規化(例: `/user/username` → `/user/username/`)が行われ、APIからページデータを取得します。
     - 取得したデータと、正規化後のパス (`currentPathname`) を `props` として `[[...path]].page.tsx` に渡します。
     - 取得したデータと、正規化後のパス (`currentPathname`) を `props` として `[[...path]].page.tsx` に渡します。
-3.  **コンポーネントのレンダリング**:
-    - `[[...path]].page.tsx` は `props` を受け取り、`PageView` などのコンポーネントをレンダリングします。
-    - 同時に **`useShallowRouting`** が実行されます。
-4.  **URLの正規化**:
-    - `useShallowRouting` は、ブラウザのURL (`/user/sotarok/memo`) と `props.currentPathname` (`/user/sotarok/memo/`) を比較します。
+3.  **コンポーネントのレンダリングとJotai Atomの初期化**:
+    - `[[...path]].page.tsx` は `props` を受け取り、そのデータで `currentPageDataAtom` などのJotai Atomを初期化します。
+    - `PageView` などのコンポーネントがサーバーサイドでレンダリングされます。
+4.  **クライアントサイドでのハイドレーションとURL正規化**:
+    - レンダリングされたHTMLがブラウザに送信され、Reactがハイドレーションを行います。
+    - **`useShallowRouting`** が実行され、ブラウザのURL (`/user/username/memo`) と `props.currentPathname` (`/user/username/memo/`) を比較します。
     - 差異がある場合、`router.replace` を `shallow: true` で実行し、ブラウザのURLをサーバーが認識している正規化後のパスに静かに更新します。
     - 差異がある場合、`router.replace` を `shallow: true` で実行し、ブラウザのURLをサーバーが認識している正規化後のパスに静かに更新します。
 
 
 ---
 ---
 
 
-#### フロー2: クライアントサイドナビゲーション(`<Link>` クリック時)
+## フロー2: クライアントサイドナビゲーション(`<Link>` クリック時)
+
+アプリケーション内でページ間を移動する際のフローです。
 
 
 1.  **ナビゲーション開始**:
 1.  **ナビゲーション開始**:
     - ユーザーが `<Link href="/new/page">` をクリックします。
     - ユーザーが `<Link href="/new/page">` をクリックします。
     - Next.js の `useRouter` がURLの変更を検出し、`[[...path]].page.tsx` が再評価されます。
     - Next.js の `useRouter` がURLの変更を検出し、`[[...path]].page.tsx` が再評価されます。
 2.  **`useSameRouteNavigation` によるトリガー**:
 2.  **`useSameRouteNavigation` によるトリガー**:
     - このフックの `useEffect` が `router.asPath` の変更 (`/new/page`) を検知します。
     - このフックの `useEffect` が `router.asPath` の変更 (`/new/page`) を検知します。
-    - **`fetchCurrentPage({ path: '/new/page' })`** を呼び出します。このフックの役割は、データ取得の「トリガー」に専念します。
-3.  **`useFetchCurrentPage` によるデータ取得と状態更新**:
+    - **`fetchCurrentPage({ path: '/new/page' })`** を呼び出します。このフックは常にデータ取得を試みます。
+3.  **`useFetchCurrentPage` によるデータ取得の判断と実行**:
     - `fetchCurrentPage` 関数が実行されます。
     - `fetchCurrentPage` 関数が実行されます。
-    - **3a. 読み込み状態開始**: `pageLoadingAtom` を `true` に設定。
-    - **3b. パス解析**: 引数の `path` をデコードし、パーマリンクかどうかを判定します。
-    - **3c. APIパラメータ構築**: パスがパーマリンクなら `pageId`、通常パスなら `path` を使ってAPIリクエストのパラメータを組み立てます。
-    - **3d. API通信**: `apiv3Get('/page', ...)` を実行します。
+    - **3a. 重複取得の防止(ガード節)**:
+        - これから取得しようとしているパス(またはパーマリンクから抽出したページID)が、現在Jotaiで管理されているページのパスやIDと同じでないかチェックします。
+        - 同じであれば、APIを叩かずに処理を中断し、現在のページデータを返します。
+    - **3b. 読み込み状態開始**: `pageLoadingAtom` を `true` に設定します。
+    - **3c. API通信**: `apiv3Get('/page', ...)` を実行してサーバーから新しいページデータを取得します。
 4.  **アトミックな状態更新**:
 4.  **アトミックな状態更新**:
     - APIからデータが返ってきた後、関連する **すべてのatomを一度に更新** します。
     - APIからデータが返ってきた後、関連する **すべてのatomを一度に更新** します。
         - `currentPageDataAtom`, `currentPageIdAtom`, `pageNotFoundAtom`, `pageLoadingAtom` など。
         - `currentPageDataAtom`, `currentPageIdAtom`, `pageNotFoundAtom`, `pageLoadingAtom` など。
     - これにより、中間的な状態(`pageId`が`undefined`になるなど)が発生することなく、データが完全に揃った状態で一度だけ状態が更新されます。
     - これにより、中間的な状態(`pageId`が`undefined`になるなど)が発生することなく、データが完全に揃った状態で一度だけ状態が更新されます。
 5.  **`PageView` の最終レンダリング**:
 5.  **`PageView` の最終レンダリング**:
     - `currentPageDataAtom` の更新がトリガーとなり、`PageView` コンポーネントが新しいデータで再レンダリングされます。
     - `currentPageDataAtom` の更新がトリガーとなり、`PageView` コンポーネントが新しいデータで再レンダリングされます。
+6.  **副作用の実行**:
+    - `useSameRouteNavigation` 内で `fetchCurrentPage` が完了した後、`mutateEditingMarkdown` が呼び出され、エディタの状態が更新されます。

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

@@ -2,31 +2,28 @@ import { useEffect } from 'react';
 
 
 import { useRouter } from 'next/router';
 import { useRouter } from 'next/router';
 
 
-import { useCurrentPageData, useFetchCurrentPage } from '~/states/page';
+import { useFetchCurrentPage } from '~/states/page';
 import { useEditingMarkdown } from '~/stores/editor';
 import { useEditingMarkdown } from '~/stores/editor';
 
 
+/**
+ * This hook is a trigger to fetch page data on client-side navigation.
+ * It detects changes in `router.asPath` and calls `fetchCurrentPage`.
+ * The responsibility for determining whether to actually fetch data
+ * is delegated to `useFetchCurrentPage`.
+ */
 export const useSameRouteNavigation = (): void => {
 export const useSameRouteNavigation = (): void => {
   const router = useRouter();
   const router = useRouter();
-  const [currentPage] = useCurrentPageData();
   const { fetchCurrentPage } = useFetchCurrentPage();
   const { fetchCurrentPage } = useFetchCurrentPage();
   const { mutate: mutateEditingMarkdown } = useEditingMarkdown();
   const { mutate: mutateEditingMarkdown } = useEditingMarkdown();
 
 
   // useEffect to trigger data fetching when the path changes
   // useEffect to trigger data fetching when the path changes
   useEffect(() => {
   useEffect(() => {
-    const targetPath = router.asPath;
-    const currentPagePath = currentPage?.path;
-
-    // Do nothing if the target path is the same as the currently loaded page's path
-    if (targetPath === currentPagePath) {
-      return;
-    }
-
     const fetch = async() => {
     const fetch = async() => {
-      const pageData = await fetchCurrentPage({ path: targetPath });
+      const pageData = await fetchCurrentPage({ path: router.asPath });
       if (pageData?.revision?.body != null) {
       if (pageData?.revision?.body != null) {
         mutateEditingMarkdown(pageData.revision.body);
         mutateEditingMarkdown(pageData.revision.body);
       }
       }
     };
     };
     fetch();
     fetch();
-  }, [router.asPath, currentPage?.path, fetchCurrentPage, mutateEditingMarkdown]);
+  }, [router.asPath, fetchCurrentPage, mutateEditingMarkdown]);
 };
 };

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

@@ -36,14 +36,38 @@ export const useFetchCurrentPage = (): {
   const error = useAtomValue(pageErrorAtom);
   const error = useAtomValue(pageErrorAtom);
 
 
   const fetchCurrentPage = useAtomCallback(
   const fetchCurrentPage = useAtomCallback(
-    useCallback(async(get, set, args?: FetchPageArgs) => {
+    useCallback(async(get, set, args?: FetchPageArgs): Promise<IPagePopulatedToShowRevision | null> => {
+      const currentPageId = get(currentPageIdAtom);
+      const currentPageData = get(currentPageDataAtom);
+
+      // Process path first to handle permalinks
+      let decodedPath: string | undefined;
+      if (args?.path != null) {
+        try {
+          decodedPath = decodeURIComponent(args.path);
+        }
+        catch (e) {
+          decodedPath = args.path;
+        }
+      }
+
+      // Guard clause to prevent unnecessary fetching
+      if (args?.pageId != null && args.pageId === currentPageId) {
+        return currentPageData ?? null;
+      }
+      if (decodedPath != null) {
+        if (isPermalink(decodedPath) && removeHeadingSlash(decodedPath) === currentPageId) {
+          return currentPageData ?? null;
+        }
+        if (decodedPath === currentPageData?.path) {
+          return currentPageData ?? null;
+        }
+      }
+
       set(pageLoadingAtom, true);
       set(pageLoadingAtom, true);
       set(pageErrorAtom, null);
       set(pageErrorAtom, null);
 
 
-      const currentPageId = get(currentPageIdAtom);
-
       // determine parameters
       // determine parameters
-      const path = args?.path;
       const pageId = args?.pageId;
       const pageId = args?.pageId;
       const revisionId = args?.revisionId ?? (isClient() ? new URLSearchParams(window.location.search).get('revisionId') : undefined);
       const revisionId = args?.revisionId ?? (isClient() ? new URLSearchParams(window.location.search).get('revisionId') : undefined);
 
 
@@ -56,17 +80,6 @@ export const useFetchCurrentPage = (): {
         params.revisionId = revisionId;
         params.revisionId = revisionId;
       }
       }
 
 
-      // Process path first to handle permalinks
-      let decodedPath: string | undefined;
-      if (path != null) {
-        try {
-          decodedPath = decodeURIComponent(path);
-        }
-        catch (e) {
-          decodedPath = path;
-        }
-      }
-
       // priority: pageId > permalink > path
       // priority: pageId > permalink > path
       if (pageId != null) {
       if (pageId != null) {
         params.pageId = pageId;
         params.pageId = pageId;
@@ -92,6 +105,7 @@ export const useFetchCurrentPage = (): {
       else {
       else {
         // TODO: https://github.com/weseek/growi/pull/9118
         // TODO: https://github.com/weseek/growi/pull/9118
         // throw new Error('Either path or pageId must be provided when not in a browser environment');
         // throw new Error('Either path or pageId must be provided when not in a browser environment');
+        set(pageLoadingAtom, false);
         return null;
         return null;
       }
       }
 
 
@@ -99,48 +113,31 @@ export const useFetchCurrentPage = (): {
         const { data } = await apiv3Get<{ page: IPagePopulatedToShowRevision }>('/page', params);
         const { data } = await apiv3Get<{ page: IPagePopulatedToShowRevision }>('/page', params);
         const { page: newData } = data;
         const { page: newData } = data;
 
 
-        // Batch atom updates to minimize re-renders
         set(currentPageDataAtom, newData);
         set(currentPageDataAtom, newData);
+        set(currentPageIdAtom, newData._id);
         set(pageNotFoundAtom, false);
         set(pageNotFoundAtom, false);
-
-        // Update pageId atom if data differs from current
-        if (newData?._id !== currentPageId) {
-          set(currentPageIdAtom, newData?._id);
-        }
+        set(pageNotCreatableAtom, false);
 
 
         return newData;
         return newData;
       }
       }
       catch (err) {
       catch (err) {
-        const error = err instanceof Error ? err : new Error('Unknown error occurred');
-        const errorMsg = error.message.toLowerCase();
+        set(pageErrorAtom, err as Error);
 
 
-        // Handle specific error types with batch updates
-        if (errorMsg.includes('not found') || errorMsg.includes('404')) {
+        const apiError = err as any; // eslint-disable-line @typescript-eslint/no-explicit-any
+        if (apiError.response?.status === 404) {
           set(pageNotFoundAtom, true);
           set(pageNotFoundAtom, true);
-          set(currentPageDataAtom, undefined);
-
-          if (path != null) {
-            set(pageNotCreatableAtom, !isCreatablePage(path));
+          if (params.path != null) {
+            set(pageNotCreatableAtom, !isCreatablePage(params.path));
           }
           }
-          return null;
-        }
-
-        if (errorMsg.includes('forbidden') || errorMsg.includes('403')) {
-          set(pageNotFoundAtom, false);
-          set(pageNotCreatableAtom, true);
-          set(currentPageDataAtom, undefined);
-          return null;
         }
         }
-
-        set(pageErrorAtom, new Error(`Failed to fetch current page: ${error.message}`));
-        return null;
       }
       }
       finally {
       finally {
         set(pageLoadingAtom, false);
         set(pageLoadingAtom, false);
       }
       }
+
+      return null;
     }, [shareLinkId]),
     }, [shareLinkId]),
   );
   );
 
 
   return { fetchCurrentPage, isLoading, error };
   return { fetchCurrentPage, isLoading, error };
-
 };
 };

+ 2 - 2
packages/core/src/utils/page-path-utils/index.spec.ts

@@ -188,8 +188,8 @@ describe.concurrent('isCreatablePage test', () => {
 
 
   describe.concurrent('Test getUsernameByPath', () => {
   describe.concurrent('Test getUsernameByPath', () => {
     test.concurrent('found', () => {
     test.concurrent('found', () => {
-      const username = getUsernameByPath('/user/sotarok');
-      expect(username).toBe('sotarok');
+      const username = getUsernameByPath('/user/username');
+      expect(username).toBe('yuki');
     });
     });
 
 
     test.concurrent('found with slash', () => {
     test.concurrent('found with slash', () => {