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

apps/web/src/blocksuite/providers/affine/index.ts

@@ -30,6 +30,7 @@ export const createAffineDownloadProvider = (
           new Uint8Array(hashMap.get(id) as ArrayBuffer)
         );
         connected = true;
+        callbacks.forEach(cb => cb());
         return;
       }
       affineApis
@@ -41,6 +42,8 @@ export const createAffineDownloadProvider = (
             blockSuiteWorkspace.doc,
             new Uint8Array(binary)
           );
+          connected = true;
+          callbacks.forEach(cb => cb());
         })
         .catch(e => {
           providerLogger.error('downloadWorkspace', e);

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]
   );
 }
 

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);
+  const timeoutRef = useRef<unknown | null>(null);
-  useEffect(() => {
+  if (!router.isReady) {
-    if (!router.isReady) {
+    return;
-      return;
+  }
-    }
+  if (!timeoutRef.current) {
-    const { workspaceId, pageId } = router.query;
+    timeoutRef.current = setTimeout(() => {
-    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(() => {
       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);
+  const { workspaceId, pageId } = router.query;
-    };
+  if (typeof pageId !== 'string') {
-  }, [
+    console.warn('pageId is not a string', pageId);
-    currentPageId,
+    return;
-    currentWorkspace.blockSuiteWorkspace,
+  }
-    currentWorkspace.id,
+  if (typeof workspaceId !== 'string') {
-    router,
+    console.warn('workspaceId is not a string', workspaceId);
-    setCurrentPageId,
+    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,
+      },
+    });
+  }
 }

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(() => {
+  const exist = useMemo(
-    if (!router.isReady) {
+    () => metadata.find(m => m.id === currentWorkspaceId),
-      return;
+    [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) {
+    logger.debug('redirect to', firstOne.id);
-      return;
+    void router.push({
-    }
+      pathname: '/workspace/[workspaceId]/all',
-    const exist = metadata.find(m => m.id === currentWorkspaceId);
+      query: {
-    if (!exist) {
+        ...router.query,
-      console.warn('workspace not exist, redirect to first one');
+        workspaceId: firstOne.id,
-      // 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,
-  ]);
 }

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);
+  const [currentPageId, setCurrentPageId] = useAtom(rootCurrentPageIdAtom);
-  useEffect(() => {
+  if (!router.isReady) {
-    if (!router.isReady) {
+    return;
-      return;
+  }
-    }
+  const pageId = router.query.pageId;
-    const pageId = router.query.pageId;
+  if (currentPageId === pageId) {
-    if (typeof pageId === 'string') {
+    return;
-      console.log('set page id', pageId);
+  }
-      setCurrentPageId(pageId);
+  if (typeof pageId === 'string') {
-    } else if (pageId === undefined) {
+    console.log('set page id', pageId);
-      console.log('cleanup page');
+    setCurrentPageId(pageId);
-      setCurrentPageId(null);
+  } else if (pageId === undefined) {
-    }
+    console.log('cleanup page');
-  }, [router.isReady, router.query.pageId, setCurrentPageId]);
+    setCurrentPageId(null);
+  }
 }

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) {
-    if (!router.isReady) {
+    return;
-      return;
+  }
-    }
+  const workspaceId = router.query.workspaceId;
-    const workspaceId = router.query.workspaceId;
+  if (typeof workspaceId !== 'string') {
-    if (typeof workspaceId !== 'string') {
+    return;
-      return;
+  }
-    }
+  if (currentWorkspaceId === workspaceId) {
-    if (currentWorkspaceId) {
+    return;
-      if (currentWorkspaceId !== workspaceId) {
+  }
-        const target = metadata.find(workspace => workspace.id === workspaceId);
+  if (currentWorkspaceId) {
-        if (!target) {
+    if (currentWorkspaceId !== workspaceId) {
-          logger.debug('workspace not exist, redirect to current one');
+      const target = metadata.find(workspace => workspace.id === workspaceId);
-          // workspaceId is invalid, redirect to currentWorkspaceId
+      logger.debug('workspace not exist, redirect to current one');
-          void router.push({
+      if (!target) {
-            pathname: router.pathname,
+        // workspaceId is invalid, redirect to currentWorkspaceId
-            query: {
+        void router.push({
-              ...router.query,
+          pathname: router.pathname,
-              workspaceId: currentWorkspaceId,
+          query: {
-            },
+            ...router.query,
-          });
+            workspaceId: currentWorkspaceId,
-        }
+          },
+        });
       }
-      return;
     }
-    const targetWorkspace = metadata.find(
+    return;
-      workspace => workspace.id === workspaceId
+  }
-    );
+  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]);
+  }
 }

apps/web/src/hooks/use-transform-workspace.ts

@@ -1,7 +1,4 @@
-import {
+import { rootWorkspacesMetadataAtom } from '@affine/workspace/atom';
-  rootCurrentWorkspaceIdAtom,
-  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]
   );
 }

apps/web/src/pages/workspace/[workspaceId]/setting.tsx

@@ -11,6 +11,7 @@ import { assertExists } from '@blocksuite/store';
 import { useAtom, useAtomValue } from 'jotai';
 import { atomWithStorage } from 'jotai/utils';
 import Head from 'next/head';
+import type { NextRouter } from 'next/router';
 import { useRouter } from 'next/router';
 import React, { useCallback, useEffect } from 'react';
 
@@ -31,6 +32,53 @@ const settingPanelAtom = atomWithStorage<SettingPanel>(
   settingPanel.General
 );
 
+function useTabRouterSync(
+  router: NextRouter,
+  currentTab: SettingPanel,
+  setCurrentTab: (tab: SettingPanel) => void
+) {
+  if (!router.isReady) {
+    return;
+  }
+  const queryCurrentTab =
+    typeof router.query.currentTab === 'string'
+      ? router.query.currentTab
+      : null;
+  if (
+    queryCurrentTab !== null &&
+    settingPanelValues.indexOf(queryCurrentTab as SettingPanel) === -1
+  ) {
+    setCurrentTab(settingPanel.General);
+    void router.replace({
+      pathname: router.pathname,
+      query: {
+        ...router.query,
+        currentTab: settingPanel.General,
+      },
+    });
+    return;
+  } else if (settingPanelValues.indexOf(currentTab as SettingPanel) === -1) {
+    setCurrentTab(settingPanel.General);
+    void router.replace({
+      pathname: router.pathname,
+      query: {
+        ...router.query,
+        currentTab: settingPanel.General,
+      },
+    });
+    return;
+  } else if (queryCurrentTab !== currentTab) {
+    void router.replace({
+      pathname: router.pathname,
+      query: {
+        ...router.query,
+        currentTab: currentTab,
+      },
+    });
+    return;
+  }
+}
+
 const SettingPage: NextPageWithLayout = () => {
   const router = useRouter();
   const workspaceIds = useAtomValue(rootWorkspacesMetadataAtom);
@@ -42,7 +90,7 @@ const SettingPage: NextPageWithLayout = () => {
   const onChangeTab = useCallback(
     (tab: SettingPanel) => {
       setCurrentTab(tab as SettingPanel);
-      router.push({
+      void router.push({
         pathname: router.pathname,
         query: {
           ...router.query,
@@ -52,48 +100,7 @@ const SettingPage: NextPageWithLayout = () => {
     },
     [router, setCurrentTab]
   );
-  useEffect(() => {
+  useTabRouterSync(router, currentTab, setCurrentTab);
-    if (!router.isReady) {
-      return;
-    }
-    const queryCurrentTab =
-      typeof router.query.currentTab === 'string'
-        ? router.query.currentTab
-        : null;
-    if (
-      queryCurrentTab !== null &&
-      settingPanelValues.indexOf(queryCurrentTab as SettingPanel) === -1
-    ) {
-      setCurrentTab(settingPanel.General);
-      router.replace({
-        pathname: router.pathname,
-        query: {
-          ...router.query,
-          currentTab: settingPanel.General,
-        },
-      });
-      return;
-    } else if (settingPanelValues.indexOf(currentTab as SettingPanel) === -1) {
-      setCurrentTab(settingPanel.General);
-      router.replace({
-        pathname: router.pathname,
-        query: {
-          ...router.query,
-          currentTab: settingPanel.General,
-        },
-      });
-      return;
-    } else if (queryCurrentTab !== currentTab) {
-      router.replace({
-        pathname: router.pathname,
-        query: {
-          ...router.query,
-          currentTab: currentTab,
-        },
-      });
-      return;
-    }
-  }, [currentTab, router, setCurrentTab]);
 
   const helper = useAppHelper();
 

apps/web/src/plugins/affine/index.tsx

@@ -11,6 +11,7 @@ import {
   SignMethod,
 } from '@affine/workspace/affine/login';
 import { rootStore, rootWorkspacesMetadataAtom } from '@affine/workspace/atom';
+import { createIndexedDBBackgroundProvider } from '@affine/workspace/providers';
 import type { AffineLegacyCloudWorkspace } from '@affine/workspace/type';
 import {
   LoadPriority,
@@ -28,6 +29,7 @@ import { mutate } from 'swr';
 import { z } from 'zod';
 
 import { createAffineProviders } from '../../blocksuite';
+import { createAffineDownloadProvider } from '../../blocksuite/providers/affine';
 import { PageNotFoundError } from '../../components/affine/affine-error-eoundary';
 import { WorkspaceSettingDetail } from '../../components/affine/workspace-setting-detail';
 import { BlockSuitePageList } from '../../components/blocksuite/block-suite-page-list';
@@ -148,6 +150,27 @@ export const AffinePlugin: WorkspaceAdapter<WorkspaceFlavour.AFFINE> = {
           );
         }
       }
+      {
+        const bs = createEmptyBlockSuiteWorkspace(id, WorkspaceFlavour.AFFINE, {
+          workspaceApis: affineApis,
+        });
+        // fixme:
+        //  force to download workspace binary
+        //  to make sure the workspace is synced
+        const provider = createAffineDownloadProvider(bs);
+        const indexedDBProvider = createIndexedDBBackgroundProvider(bs);
+        await new Promise<void>(resolve => {
+          indexedDBProvider.callbacks.add(() => {
+            resolve();
+          });
+          provider.callbacks.add(() => {
+            indexedDBProvider.connect();
+          });
+          provider.connect();
+        });
+        provider.disconnect();
+        indexedDBProvider.disconnect();
+      }
 
       await mutate(matcher => matcher === QueryKey.getWorkspaces);
       // refresh the local storage

packages/workspace/package.json

@@ -22,6 +22,7 @@
     "@affine/component": "workspace:*",
     "@affine/debug": "workspace:*",
     "@affine/env": "workspace:*",
+    "@toeverything/hooks": "workspace:*",
     "@toeverything/y-indexeddb": "workspace:*",
     "firebase": "^9.21.0",
     "jotai": "^2.1.0",

packages/workspace/src/affine/__tests__/api.spec.ts

@@ -17,6 +17,7 @@ import { assertExists } from '@blocksuite/global/utils';
 import type { Page, PageMeta } from '@blocksuite/store';
 import { Workspace } from '@blocksuite/store';
 import { faker } from '@faker-js/faker';
+import type {} from '@toeverything/hooks/use-block-suite-page-meta';
 import { beforeEach, describe, expect, test, vi } from 'vitest';
 import { WebSocket } from 'ws';
 import { applyUpdate } from 'yjs';

yarn.lock

@@ -371,6 +371,7 @@ __metadata:
     "@affine/component": "workspace:*"
     "@affine/debug": "workspace:*"
     "@affine/env": "workspace:*"
+    "@toeverything/hooks": "workspace:*"
     "@toeverything/y-indexeddb": "workspace:*"
     "@types/ws": ^8.5.4
     firebase: ^9.21.0