fix: page validation logic (#3626)

apps/core/src/components/page-detail-editor.tsx

@@ -76,7 +76,6 @@ const EditorWrapper = memo(function EditorWrapper({
             '--affine-font-family': value,
           } as CSSProperties
         }
-        key={`${workspace.id}-${pageId}`}
         mode={isPublic ? 'page' : currentMode}
         page={page}
         onInit={useCallback(

apps/core/src/components/root-app-sidebar/index.tsx

@@ -115,7 +115,7 @@ export const RootAppSidebar = ({
 
   const [sidebarOpen, setSidebarOpen] = useAtom(appSidebarOpenAtom);
   useEffect(() => {
-    if (isDesktop && typeof sidebarOpen === 'boolean') {
+    if (isDesktop) {
       window.apis?.ui.handleSidebarVisibilityChange(sidebarOpen).catch(err => {
         console.error(err);
       });

apps/core/src/hooks/use-navigate-helper.ts

@@ -1,9 +1,4 @@
 import type { WorkspaceSubPath } from '@affine/env/workspace';
-import {
-  currentPageIdAtom,
-  currentWorkspaceIdAtom,
-} from '@toeverything/infra/atom';
-import { useSetAtom } from 'jotai';
 import { useCallback } from 'react';
 // eslint-disable-next-line @typescript-eslint/no-restricted-imports
 import { useLocation, useNavigate } from 'react-router-dom';
@@ -16,8 +11,6 @@ export enum RouteLogic {
 export function useNavigateHelper() {
   const location = useLocation();
   const navigate = useNavigate();
-  const setWorkspaceId = useSetAtom(currentWorkspaceIdAtom);
-  const setCurrentPageId = useSetAtom(currentPageIdAtom);
 
   const jumpToPage = useCallback(
     (
@@ -25,13 +18,11 @@ export function useNavigateHelper() {
       pageId: string,
       logic: RouteLogic = RouteLogic.PUSH
     ) => {
-      setWorkspaceId(workspaceId);
-      setCurrentPageId(pageId);
       return navigate(`/workspace/${workspaceId}/${pageId}`, {
         replace: logic === RouteLogic.REPLACE,
       });
     },
-    [navigate, setCurrentPageId, setWorkspaceId]
+    [navigate]
   );
   const jumpToPublicWorkspacePage = useCallback(
     (
@@ -39,13 +30,11 @@ export function useNavigateHelper() {
       pageId: string,
       logic: RouteLogic = RouteLogic.PUSH
     ) => {
-      setWorkspaceId(workspaceId);
-      setCurrentPageId(pageId);
       return navigate(`/public-workspace/${workspaceId}/${pageId}`, {
         replace: logic === RouteLogic.REPLACE,
       });
     },
-    [navigate, setCurrentPageId, setWorkspaceId]
+    [navigate]
   );
   const jumpToSubPath = useCallback(
     (
@@ -53,18 +42,14 @@ export function useNavigateHelper() {
       subPath: WorkspaceSubPath,
       logic: RouteLogic = RouteLogic.PUSH
     ) => {
-      setWorkspaceId(workspaceId);
-      setCurrentPageId(null);
       return navigate(`/workspace/${workspaceId}/${subPath}`, {
         replace: logic === RouteLogic.REPLACE,
       });
     },
-    [navigate, setCurrentPageId, setWorkspaceId]
+    [navigate]
   );
   const openPage = useCallback(
     (workspaceId: string, pageId: string) => {
-      setWorkspaceId(workspaceId);
-      setCurrentPageId(pageId);
       const isPublicWorkspace =
         location.pathname.indexOf('/public-workspace') === 0;
       if (isPublicWorkspace) {
@@ -73,35 +58,25 @@ export function useNavigateHelper() {
         return jumpToPage(workspaceId, pageId);
       }
     },
-    [
+    [jumpToPage, jumpToPublicWorkspacePage, location.pathname]
-      jumpToPage,
-      jumpToPublicWorkspacePage,
-      location.pathname,
-      setCurrentPageId,
-      setWorkspaceId,
-    ]
   );
 
   const jumpToIndex = useCallback(
     (logic: RouteLogic = RouteLogic.PUSH) => {
-      setWorkspaceId(null);
-      setCurrentPageId(null);
       return navigate('/', {
         replace: logic === RouteLogic.REPLACE,
       });
     },
-    [navigate, setCurrentPageId, setWorkspaceId]
+    [navigate]
   );
 
   const jumpTo404 = useCallback(
     (logic: RouteLogic = RouteLogic.PUSH) => {
-      setWorkspaceId(null);
-      setCurrentPageId(null);
       return navigate('/404', {
         replace: logic === RouteLogic.REPLACE,
       });
     },
-    [navigate, setCurrentPageId, setWorkspaceId]
+    [navigate]
   );
 
   return {

apps/core/src/layouts/workspace-layout.tsx

@@ -89,15 +89,6 @@ export const QuickSearch = () => {
   );
 };
 
-declare global {
-  // eslint-disable-next-line no-var
-  var HALTING_PROBLEM_TIMEOUT: number;
-}
-
-if (globalThis.HALTING_PROBLEM_TIMEOUT === undefined) {
-  globalThis.HALTING_PROBLEM_TIMEOUT = 1000;
-}
-
 const showList: IslandItemNames[] = environment.isDesktop
   ? ['whatNew', 'contact', 'guide']
   : ['whatNew', 'contact'];

apps/core/src/pages/workspace/detail-page.tsx

@@ -7,12 +7,16 @@ import { WorkspaceSubPath } from '@affine/env/workspace';
 import type { EditorContainer } from '@blocksuite/editor';
 import { assertExists } from '@blocksuite/global/utils';
 import type { Page } from '@blocksuite/store';
-import { currentPageIdAtom, rootStore } from '@toeverything/infra/atom';
+import {
+  currentPageIdAtom,
+  currentWorkspaceAtom,
+  currentWorkspaceIdAtom,
+  rootStore,
+} from '@toeverything/infra/atom';
 import { useAtomValue } from 'jotai';
-import { useAtom } from 'jotai/react';
+import { type ReactElement, useCallback } from 'react';
-import { type ReactElement, useCallback, useEffect } from 'react';
 import type { LoaderFunction } from 'react-router-dom';
-import { useLocation, useParams } from 'react-router-dom';
+import { redirect } from 'react-router-dom';
 
 import { getUIAdapter } from '../../adapters/workspace';
 import { useCurrentWorkspace } from '../../hooks/current/use-current-workspace';
@@ -32,7 +36,7 @@ const DetailPageImpl = (): ReactElement => {
         return openPage(blockSuiteWorkspace.id, pageId);
       });
       const disposeTagClick = editor.slots.tagClicked.on(async ({ tagId }) => {
-        await jumpToSubPath(currentWorkspace.id, WorkspaceSubPath.ALL);
+        jumpToSubPath(currentWorkspace.id, WorkspaceSubPath.ALL);
         collectionManager.backToAll();
         collectionManager.setTemporaryFilter([createTagFilter(tagId)]);
       });
@@ -69,61 +73,39 @@ const DetailPageImpl = (): ReactElement => {
 };
 
 export const DetailPage = (): ReactElement => {
-  const { workspaceId, pageId } = useParams();
-  const location = useLocation();
-  const { jumpTo404 } = useNavigateHelper();
   const [currentWorkspace] = useCurrentWorkspace();
-  const [currentPageId, setCurrentPageId] = useAtom(currentPageIdAtom);
+  const currentPageId = useAtomValue(currentPageIdAtom);
   const page = currentPageId
     ? currentWorkspace.blockSuiteWorkspace.getPage(currentPageId)
     : null;
 
-  //#region check if page is valid
-  useEffect(() => {
-    // if the workspace changed, ignore the page check
-    if (currentWorkspace.id !== workspaceId) {
-      return;
-    }
-    if (typeof pageId === 'string' && currentPageId) {
-      if (currentPageId !== pageId) {
-        setCurrentPageId(pageId);
-      } else {
-        const page =
-          currentWorkspace.blockSuiteWorkspace.getPage(currentPageId);
-        if (!page) {
-          jumpTo404();
-        } else {
-          // fixme: cleanup jumpOnce in the right time
-          if (page.meta.jumpOnce) {
-            currentWorkspace.blockSuiteWorkspace.setPageMeta(currentPageId, {
-              jumpOnce: false,
-            });
-          }
-        }
-      }
-    }
-  }, [
-    currentPageId,
-    currentWorkspace.blockSuiteWorkspace,
-    currentWorkspace.id,
-    jumpTo404,
-    location.pathname,
-    pageId,
-    setCurrentPageId,
-    workspaceId,
-  ]);
-  //#endregion
-
   if (!currentPageId || !page) {
     return <PageDetailSkeleton key="current-page-is-null" />;
   }
   return <DetailPageImpl />;
 };
 
-export const loader: LoaderFunction = args => {
+export const loader: LoaderFunction = async args => {
+  if (args.params.workspaceId) {
+    localStorage.setItem('last_workspace_id', args.params.workspaceId);
+    rootStore.set(currentWorkspaceIdAtom, args.params.workspaceId);
+  }
   if (args.params.pageId) {
-    localStorage.setItem('last_page_id', args.params.pageId);
+    const pageId = args.params.pageId;
-    rootStore.set(currentPageIdAtom, args.params.pageId);
+    localStorage.setItem('last_page_id', pageId);
+    const currentWorkspace = await rootStore.get(currentWorkspaceAtom);
+    const page = currentWorkspace.getPage(pageId);
+    if (!page) {
+      return redirect('/404');
+    }
+    if (page.meta.jumpOnce) {
+      currentWorkspace.setPageMeta(page.id, {
+        jumpOnce: false,
+      });
+    }
+    rootStore.set(currentPageIdAtom, pageId);
+  } else {
+    return redirect('/404');
   }
   return null;
 };

apps/core/src/pages/workspace/index.tsx

@@ -1,5 +1,9 @@
 import { rootWorkspacesMetadataAtom } from '@affine/workspace/atom';
-import { currentWorkspaceIdAtom, rootStore } from '@toeverything/infra/atom';
+import {
+  currentPageIdAtom,
+  currentWorkspaceIdAtom,
+  rootStore,
+} from '@toeverything/infra/atom';
 import type { ReactElement } from 'react';
 import { type LoaderFunction, Outlet, redirect } from 'react-router-dom';
 
@@ -14,6 +18,9 @@ export const loader: LoaderFunction = async args => {
     localStorage.setItem('last_workspace_id', args.params.workspaceId);
     rootStore.set(currentWorkspaceIdAtom, args.params.workspaceId);
   }
+  if (!args.params.pageId) {
+    rootStore.set(currentPageIdAtom, null);
+  }
   return null;
 };
 

packages/component/src/components/block-suite-editor/index.tsx

@@ -95,7 +95,13 @@ const BlockSuiteEditorImpl = (props: EditorProps): ReactElement => {
     if (!container) {
       return;
     }
-    if (page.awarenessStore.getFlag('enable_block_hub')) {
+    container.appendChild(editor);
+    return () => {
+      container.removeChild(editor);
+    };
+  }, [editor]);
+
+  useEffect(() => {
     editor
       .createBlockHub()
       .then(blockHub => {
@@ -110,17 +116,13 @@ const BlockSuiteEditorImpl = (props: EditorProps): ReactElement => {
       .catch(err => {
         console.error(err);
       });
-    }
-
-    container.appendChild(editor);
     return () => {
       if (setBlockHub) {
         setBlockHub(null);
       }
       blockHubRef.current?.remove();
-      container.removeChild(editor);
     };
-  }, [editor, setBlockHub, page]);
+  }, [editor, page.awarenessStore, setBlockHub]);
 
   // issue: https://github.com/toeverything/AFFiNE/issues/2004
   const className = `editor-wrapper ${editor.mode}-mode ${

tests/affine-local/e2e/blocksuite/block-hub.spec.ts

@@ -1,13 +1,10 @@
 import { test } from '@affine-test/kit/playwright';
 import { checkBlockHub } from '@affine-test/kit/utils/editor';
 import { openHomePage } from '@affine-test/kit/utils/load-page';
-import { newPage, waitEditorLoad } from '@affine-test/kit/utils/page-logic';
+import { waitEditorLoad } from '@affine-test/kit/utils/page-logic';
 
 test('block-hub should work', async ({ page }) => {
   await openHomePage(page);
   await waitEditorLoad(page);
   await checkBlockHub(page);
-  await newPage(page);
-  await page.waitForTimeout(500);
-  await checkBlockHub(page);
 });