fix: remove useEffect on router sync with atoms (#2241)

2026-02-14 21:27:20 +00:00 · 2023-05-12 05:37:42 +08:00
parent 5008958e84
commit ec66b229fe
12 changed files with 206 additions and 192 deletions
--- a/apps/web/src/hooks/root/use-on-transform-workspace.ts
+++ b/apps/web/src/hooks/root/use-on-transform-workspace.ts
@@ -6,6 +6,7 @@ import {
  SignMethod,
  storageChangeSlot,
 } from '@affine/workspace/affine/login';
+import { rootCurrentWorkspaceIdAtom } from '@affine/workspace/atom';
 import type { WorkspaceRegistry } from '@affine/workspace/type';
 import { WorkspaceFlavour } from '@affine/workspace/type';
 import { useSetAtom } from 'jotai';
@@ -17,6 +18,7 @@ import { useTransformWorkspace } from '../use-transform-workspace';
 export function useOnTransformWorkspace() {
  const transformWorkspace = useTransformWorkspace();
  const setUser = useSetAtom(currentAffineUserAtom);
+  const setWorkspaceId = useSetAtom(rootCurrentWorkspaceIdAtom);
  return useCallback(
    async <From extends WorkspaceFlavour, To extends WorkspaceFlavour>(
      from: From,
@@ -43,8 +45,9 @@ export function useOnTransformWorkspace() {
          },
        })
      );
+      setWorkspaceId(workspaceId);
    },
-    [setUser, transformWorkspace]
+    [setUser, setWorkspaceId, transformWorkspace]
  );
 }

--- a/apps/web/src/hooks/use-router-and-workspace-with-page-id-defense.ts
+++ b/apps/web/src/hooks/use-router-and-workspace-with-page-id-defense.ts
@@ -2,7 +2,7 @@ import { DebugLogger } from '@affine/debug';
 import { rootCurrentPageIdAtom } from '@affine/workspace/atom';
 import { useAtom, useAtomValue } from 'jotai';
 import type { NextRouter } from 'next/router';
-import { useEffect, useRef } from 'react';
+import { useRef } from 'react';

 import { rootCurrentWorkspaceAtom } from '../atoms/root';
 export const HALT_PROBLEM_TIMEOUT = 1000;
@@ -12,42 +12,12 @@ const logger = new DebugLogger('useRouterWithWorkspaceIdDefense');
 export function useRouterAndWorkspaceWithPageIdDefense(router: NextRouter) {
  const currentWorkspace = useAtomValue(rootCurrentWorkspaceAtom);
  const [currentPageId, setCurrentPageId] = useAtom(rootCurrentPageIdAtom);
-  const fallbackModeRef = useRef(false);
-  useEffect(() => {
-    if (!router.isReady) {
-      return;
-    }
-    const { workspaceId, pageId } = router.query;
-    if (typeof pageId !== 'string') {
-      logger.warn('pageId is not a string', pageId);
-      return;
-    }
-    if (typeof workspaceId !== 'string') {
-      logger.warn('workspaceId is not a string', workspaceId);
-      return;
-    }
-    if (currentWorkspace?.id !== workspaceId) {
-      logger.warn('workspaceId is not currentWorkspace', workspaceId);
-      return;
-    }
-    if (currentPageId !== pageId && !fallbackModeRef.current) {
-      logger.info('set pageId', pageId, 'for workspace', workspaceId);
-      setCurrentPageId(pageId);
-      void router.push({
-        pathname: '/workspace/[workspaceId]/[pageId]',
-        query: {
-          ...router.query,
-          workspaceId,
-          pageId,
-        },
-      });
-    }
-  }, [currentPageId, currentWorkspace.id, router, setCurrentPageId]);
-  useEffect(() => {
-    if (fallbackModeRef.current) {
-      return;
-    }
-    const id = setTimeout(() => {
+  const timeoutRef = useRef<unknown | null>(null);
+  if (!router.isReady) {
+    return;
+  }
+  if (!timeoutRef.current) {
+    timeoutRef.current = setTimeout(() => {
      if (currentPageId) {
        const page =
          currentWorkspace.blockSuiteWorkspace.getPage(currentPageId);
@@ -70,19 +40,34 @@ export function useRouterAndWorkspaceWithPageIdDefense(router: NextRouter) {
                pageId: firstOne.id,
              },
            });
-            fallbackModeRef.current = true;
          }
        }
      }
    }, HALT_PROBLEM_TIMEOUT);
-    return () => {
-      clearTimeout(id);
-    };
-  }, [
-    currentPageId,
-    currentWorkspace.blockSuiteWorkspace,
-    currentWorkspace.id,
-    router,
-    setCurrentPageId,
-  ]);
+  }
+  const { workspaceId, pageId } = router.query;
+  if (typeof pageId !== 'string') {
+    console.warn('pageId is not a string', pageId);
+    return;
+  }
+  if (typeof workspaceId !== 'string') {
+    console.warn('workspaceId is not a string', workspaceId);
+    return;
+  }
+  if (currentWorkspace?.id !== workspaceId) {
+    console.warn('workspaceId is not currentWorkspace', workspaceId);
+    return;
+  }
+  if (currentPageId !== pageId) {
+    console.log('set current page id', pageId);
+    setCurrentPageId(pageId);
+    void router.push({
+      pathname: '/workspace/[workspaceId]/[pageId]',
+      query: {
+        ...router.query,
+        workspaceId,
+        pageId,
+      },
+    });
+  }
 }
--- a/apps/web/src/hooks/use-router-with-workspace-id-defense.ts
+++ b/apps/web/src/hooks/use-router-with-workspace-id-defense.ts
@@ -6,7 +6,7 @@ import {
 } from '@affine/workspace/atom';
 import { useAtom, useAtomValue, useSetAtom } from 'jotai';
 import type { NextRouter } from 'next/router';
-import { useEffect } from 'react';
+import { useMemo } from 'react';

 const logger = new DebugLogger('useRouterWithWorkspaceIdDefense');

@@ -16,38 +16,32 @@ export function useRouterWithWorkspaceIdDefense(router: NextRouter) {
    rootCurrentWorkspaceIdAtom
  );
  const setCurrentPageId = useSetAtom(rootCurrentPageIdAtom);
-  useEffect(() => {
-    if (!router.isReady) {
-      return;
+  const exist = useMemo(
+    () => metadata.find(m => m.id === currentWorkspaceId),
+    [currentWorkspaceId, metadata]
+  );
+  if (!router.isReady) {
+    return;
+  }
+  if (!currentWorkspaceId) {
+    return;
+  }
+  if (!exist) {
+    console.warn('workspace not exist, redirect to first one');
+    // clean up
+    setCurrentWorkspaceId(null);
+    setCurrentPageId(null);
+    const firstOne = metadata.at(0);
+    if (!firstOne) {
+      throw new Error('no workspace');
    }
-    if (!currentWorkspaceId) {
-      return;
-    }
-    const exist = metadata.find(m => m.id === currentWorkspaceId);
-    if (!exist) {
-      console.warn('workspace not exist, redirect to first one');
-      // clean up
-      setCurrentWorkspaceId(null);
-      setCurrentPageId(null);
-      const firstOne = metadata.at(0);
-      if (!firstOne) {
-        throw new Error('no workspace');
-      }
-      logger.debug('redirect to', firstOne.id);
-      void router.push({
-        pathname: '/workspace/[workspaceId]/all',
-        query: {
-          ...router.query,
-          workspaceId: firstOne.id,
-        },
-      });
-    }
-  }, [
-    currentWorkspaceId,
-    metadata,
-    router,
-    router.isReady,
-    setCurrentPageId,
-    setCurrentWorkspaceId,
-  ]);
+    logger.debug('redirect to', firstOne.id);
+    void router.push({
+      pathname: '/workspace/[workspaceId]/all',
+      query: {
+        ...router.query,
+        workspaceId: firstOne.id,
+      },
+    });
+  }
 }
--- a/apps/web/src/hooks/use-sync-router-with-current-page-id.ts
+++ b/apps/web/src/hooks/use-sync-router-with-current-page-id.ts
@@ -1,21 +1,21 @@
 import { rootCurrentPageIdAtom } from '@affine/workspace/atom';
-import { useSetAtom } from 'jotai';
+import { useAtom } from 'jotai';
 import type { NextRouter } from 'next/router';
-import { useEffect } from 'react';

 export function useSyncRouterWithCurrentPageId(router: NextRouter) {
-  const setCurrentPageId = useSetAtom(rootCurrentPageIdAtom);
-  useEffect(() => {
-    if (!router.isReady) {
-      return;
-    }
-    const pageId = router.query.pageId;
-    if (typeof pageId === 'string') {
-      console.log('set page id', pageId);
-      setCurrentPageId(pageId);
-    } else if (pageId === undefined) {
-      console.log('cleanup page');
-      setCurrentPageId(null);
-    }
-  }, [router.isReady, router.query.pageId, setCurrentPageId]);
+  const [currentPageId, setCurrentPageId] = useAtom(rootCurrentPageIdAtom);
+  if (!router.isReady) {
+    return;
+  }
+  const pageId = router.query.pageId;
+  if (currentPageId === pageId) {
+    return;
+  }
+  if (typeof pageId === 'string') {
+    console.log('set page id', pageId);
+    setCurrentPageId(pageId);
+  } else if (pageId === undefined) {
+    console.log('cleanup page');
+    setCurrentPageId(null);
+  }
 }
--- a/apps/web/src/hooks/use-sync-router-with-current-workspace-id.ts
+++ b/apps/web/src/hooks/use-sync-router-with-current-workspace-id.ts
@@ -5,7 +5,6 @@ import {
 } from '@affine/workspace/atom';
 import { useAtom, useAtomValue } from 'jotai';
 import type { NextRouter } from 'next/router';
-import { useEffect } from 'react';

 const logger = new DebugLogger('useSyncRouterWithCurrentWorkspaceId');

@@ -14,34 +13,49 @@ export function useSyncRouterWithCurrentWorkspaceId(router: NextRouter) {
    rootCurrentWorkspaceIdAtom
  );
  const metadata = useAtomValue(rootWorkspacesMetadataAtom);
-  useEffect(() => {
-    if (!router.isReady) {
-      return;
-    }
-    const workspaceId = router.query.workspaceId;
-    if (typeof workspaceId !== 'string') {
-      return;
-    }
-    if (currentWorkspaceId) {
-      if (currentWorkspaceId !== workspaceId) {
-        const target = metadata.find(workspace => workspace.id === workspaceId);
-        if (!target) {
-          logger.debug('workspace not exist, redirect to current one');
-          // workspaceId is invalid, redirect to currentWorkspaceId
-          void router.push({
-            pathname: router.pathname,
-            query: {
-              ...router.query,
-              workspaceId: currentWorkspaceId,
-            },
-          });
-        }
+  if (!router.isReady) {
+    return;
+  }
+  const workspaceId = router.query.workspaceId;
+  if (typeof workspaceId !== 'string') {
+    return;
+  }
+  if (currentWorkspaceId === workspaceId) {
+    return;
+  }
+  if (currentWorkspaceId) {
+    if (currentWorkspaceId !== workspaceId) {
+      const target = metadata.find(workspace => workspace.id === workspaceId);
+      logger.debug('workspace not exist, redirect to current one');
+      if (!target) {
+        // workspaceId is invalid, redirect to currentWorkspaceId
+        void router.push({
+          pathname: router.pathname,
+          query: {
+            ...router.query,
+            workspaceId: currentWorkspaceId,
+          },
+        });
      }
-      return;
    }
-    const targetWorkspace = metadata.find(
-      workspace => workspace.id === workspaceId
-    );
+    return;
+  }
+  const targetWorkspace = metadata.find(
+    workspace => workspace.id === workspaceId
+  );
+  if (targetWorkspace) {
+    console.log('set workspace id', workspaceId);
+    setCurrentWorkspaceId(targetWorkspace.id);
+    logger.debug('redirect to', targetWorkspace.id);
+    void router.push({
+      pathname: router.pathname,
+      query: {
+        ...router.query,
+        workspaceId: targetWorkspace.id,
+      },
+    });
+  } else {
+    const targetWorkspace = metadata.at(0);
    if (targetWorkspace) {
      console.log('set workspace id', workspaceId);
      setCurrentWorkspaceId(targetWorkspace.id);
@@ -53,20 +67,6 @@ export function useSyncRouterWithCurrentWorkspaceId(router: NextRouter) {
          workspaceId: targetWorkspace.id,
        },
      });
-    } else {
-      const targetWorkspace = metadata.at(0);
-      if (targetWorkspace) {
-        console.log('set workspace id', workspaceId);
-        setCurrentWorkspaceId(targetWorkspace.id);
-        logger.debug('redirect to', targetWorkspace.id);
-        void router.push({
-          pathname: router.pathname,
-          query: {
-            ...router.query,
-            workspaceId: targetWorkspace.id,
-          },
-        });
-      }
    }
-  }, [currentWorkspaceId, metadata, router, setCurrentWorkspaceId]);
+  }
 }
--- a/apps/web/src/hooks/use-transform-workspace.ts
+++ b/apps/web/src/hooks/use-transform-workspace.ts
@@ -1,7 +1,4 @@
-import {
-  rootCurrentWorkspaceIdAtom,
-  rootWorkspacesMetadataAtom,
-} from '@affine/workspace/atom';
+import { rootWorkspacesMetadataAtom } from '@affine/workspace/atom';
 import type { WorkspaceFlavour } from '@affine/workspace/type';
 import type { WorkspaceRegistry } from '@affine/workspace/type';
 import { useSetAtom } from 'jotai';
@@ -15,7 +12,6 @@ import { WorkspaceAdapters } from '../plugins';
 * The logic here is to delete the old workspace and create a new one.
 */
 export function useTransformWorkspace() {
-  const setCurrentWorkspaceId = useSetAtom(rootCurrentWorkspaceIdAtom);
  const set = useSetAtom(rootWorkspacesMetadataAtom);
  return useCallback(
    async <From extends WorkspaceFlavour, To extends WorkspaceFlavour>(
@@ -23,10 +19,11 @@ export function useTransformWorkspace() {
      to: To,
      workspace: WorkspaceRegistry[From]
    ): Promise<string> => {
-      await WorkspaceAdapters[from].CRUD.delete(workspace as any);
+      // create first, then delete, in case of failure
      const newId = await WorkspaceAdapters[to].CRUD.create(
        workspace.blockSuiteWorkspace
      );
+      await WorkspaceAdapters[from].CRUD.delete(workspace as any);
      set(workspaces => {
        const idx = workspaces.findIndex(ws => ws.id === workspace.id);
        workspaces.splice(idx, 1, {
@@ -35,9 +32,8 @@ export function useTransformWorkspace() {
        });
        return [...workspaces];
      });
-      setCurrentWorkspaceId(newId);
      return newId;
    },
-    [set, setCurrentWorkspaceId]
+    [set]
  );
 }