From 8d117123d70a9987b9049657f58a81a5853a4018 Mon Sep 17 00:00:00 2001 From: Himself65 Date: Fri, 12 May 2023 05:37:42 +0800 Subject: [PATCH] fix: remove `useEffect` on router sync with atoms (#2241) --- .../src/blocksuite/providers/affine/index.ts | 3 + .../hooks/root/use-on-transform-workspace.ts | 5 +- ...uter-and-workspace-with-page-id-defense.ts | 81 +++++++--------- .../use-router-with-workspace-id-defense.ts | 62 ++++++------- .../use-sync-router-with-current-page-id.ts | 32 +++---- ...e-sync-router-with-current-workspace-id.ts | 84 ++++++++--------- apps/web/src/hooks/use-transform-workspace.ts | 12 +-- .../pages/workspace/[workspaceId]/setting.tsx | 93 ++++++++++--------- apps/web/src/plugins/affine/index.tsx | 23 +++++ packages/workspace/package.json | 1 + .../src/affine/__tests__/api.spec.ts | 1 + yarn.lock | 1 + 12 files changed, 206 insertions(+), 192 deletions(-) diff --git a/apps/web/src/blocksuite/providers/affine/index.ts b/apps/web/src/blocksuite/providers/affine/index.ts index 7c87ef6164..d4edad15d6 100644 --- a/apps/web/src/blocksuite/providers/affine/index.ts +++ b/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); diff --git a/apps/web/src/hooks/root/use-on-transform-workspace.ts b/apps/web/src/hooks/root/use-on-transform-workspace.ts index 9ca4b36f61..025c5ad365 100644 --- 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: From, @@ -43,8 +45,9 @@ export function useOnTransformWorkspace() { }, }) ); + setWorkspaceId(workspaceId); }, - [setUser, transformWorkspace] + [setUser, setWorkspaceId, transformWorkspace] ); } diff --git 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 index 787b3e2bda..d29f3e6470 100644 --- 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(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, + }, + }); + } } diff --git a/apps/web/src/hooks/use-router-with-workspace-id-defense.ts b/apps/web/src/hooks/use-router-with-workspace-id-defense.ts index 7c7627213f..f66f19c040 100644 --- 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, + }, + }); + } } diff --git 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 index 92b77a65d4..8cf319281c 100644 --- 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); + } } diff --git 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 index 5a622742f1..052bd16eca 100644 --- 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]); + } } diff --git a/apps/web/src/hooks/use-transform-workspace.ts b/apps/web/src/hooks/use-transform-workspace.ts index 141c3eaf4b..3c1bce8217 100644 --- 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 ( @@ -23,10 +19,11 @@ export function useTransformWorkspace() { to: To, workspace: WorkspaceRegistry[From] ): Promise => { - 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] ); } diff --git a/apps/web/src/pages/workspace/[workspaceId]/setting.tsx b/apps/web/src/pages/workspace/[workspaceId]/setting.tsx index 48feb29023..c770c2d689 100644 --- a/apps/web/src/pages/workspace/[workspaceId]/setting.tsx +++ b/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.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(() => { - 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]); + useTabRouterSync(router, currentTab, setCurrentTab); const helper = useAppHelper(); diff --git a/apps/web/src/plugins/affine/index.tsx b/apps/web/src/plugins/affine/index.tsx index 984f96c7cf..ec1dd31ebd 100644 --- a/apps/web/src/plugins/affine/index.tsx +++ b/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 = { ); } } + { + 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(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 diff --git a/packages/workspace/package.json b/packages/workspace/package.json index 83b4f8d829..ff1456c62d 100644 --- a/packages/workspace/package.json +++ b/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", diff --git a/packages/workspace/src/affine/__tests__/api.spec.ts b/packages/workspace/src/affine/__tests__/api.spec.ts index 40f2681986..be45a4ca14 100644 --- a/packages/workspace/src/affine/__tests__/api.spec.ts +++ b/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'; diff --git a/yarn.lock b/yarn.lock index a76b4a6e92..11e88b803a 100644 --- a/yarn.lock +++ b/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