From 8b5d9973222fe81fb8c9d6baee229ecfad8244f0 Mon Sep 17 00:00:00 2001 From: Alex Yang Date: Sun, 9 Jul 2023 13:01:09 +0800 Subject: [PATCH] refactor(hooks): reduce null types (#3111) --- .../delete-leave-workspace/delete/index.tsx | 2 +- .../WorkspaceSelector/workspace-selector.tsx | 4 +-- .../src/components/root-app-sidebar/index.tsx | 2 +- packages/hooks/src/__tests__/index.spec.ts | 14 +------- .../src/use-block-suite-page-references.ts | 36 ++++++++++--------- .../src/use-block-suite-workspace-helper.ts | 2 -- .../src/use-block-suite-workspace-name.ts | 26 ++++---------- .../src/use-block-suite-workspace-page.ts | 3 +- 8 files changed, 31 insertions(+), 58 deletions(-) diff --git a/apps/web/src/components/affine/new-workspace-setting-detail/delete-leave-workspace/delete/index.tsx b/apps/web/src/components/affine/new-workspace-setting-detail/delete-leave-workspace/delete/index.tsx index bfbd1ffb74..414bc1337f 100644 --- a/apps/web/src/components/affine/new-workspace-setting-detail/delete-leave-workspace/delete/index.tsx +++ b/apps/web/src/components/affine/new-workspace-setting-detail/delete-leave-workspace/delete/index.tsx @@ -30,7 +30,7 @@ export const WorkspaceDeleteModal = ({ onDeleteWorkspace, }: WorkspaceDeleteProps) => { const [workspaceName] = useBlockSuiteWorkspaceName( - workspace.blockSuiteWorkspace ?? null + workspace.blockSuiteWorkspace ); const [deleteStr, setDeleteStr] = useState(''); const allowDelete = deleteStr === workspaceName; diff --git a/apps/web/src/components/pure/workspace-slider-bar/WorkspaceSelector/workspace-selector.tsx b/apps/web/src/components/pure/workspace-slider-bar/WorkspaceSelector/workspace-selector.tsx index b5f6ce64ca..9be7c01dad 100644 --- a/apps/web/src/components/pure/workspace-slider-bar/WorkspaceSelector/workspace-selector.tsx +++ b/apps/web/src/components/pure/workspace-slider-bar/WorkspaceSelector/workspace-selector.tsx @@ -15,7 +15,7 @@ import { } from './styles'; export type WorkspaceSelectorProps = { - currentWorkspace: AllWorkspace | null; + currentWorkspace: AllWorkspace; onClick: () => void; }; @@ -28,7 +28,7 @@ export const WorkspaceSelector: React.FC = ({ onClick, }) => { const [name] = useBlockSuiteWorkspaceName( - currentWorkspace?.blockSuiteWorkspace ?? null + currentWorkspace?.blockSuiteWorkspace ); const [workspace] = useCurrentWorkspace(); diff --git a/apps/web/src/components/root-app-sidebar/index.tsx b/apps/web/src/components/root-app-sidebar/index.tsx index eebfdeb38b..10eee9af67 100644 --- a/apps/web/src/components/root-app-sidebar/index.tsx +++ b/apps/web/src/components/root-app-sidebar/index.tsx @@ -37,7 +37,7 @@ export type RootAppSidebarProps = { onOpenQuickSearchModal: () => void; onOpenSettingModal: () => void; onOpenWorkspaceListModal: () => void; - currentWorkspace: AllWorkspace | null; + currentWorkspace: AllWorkspace; openPage: (pageId: string) => void; createPage: () => Page; currentPath: string; diff --git a/packages/hooks/src/__tests__/index.spec.ts b/packages/hooks/src/__tests__/index.spec.ts index 09d2b9ec68..c4afb7a834 100644 --- a/packages/hooks/src/__tests__/index.spec.ts +++ b/packages/hooks/src/__tests__/index.spec.ts @@ -3,14 +3,13 @@ */ import 'fake-indexeddb/auto'; -import { UNTITLED_WORKSPACE_NAME } from '@affine/env/constant'; import { __unstableSchemas, AffineSchemas } from '@blocksuite/blocks/models'; import { assertExists } from '@blocksuite/global/utils'; import type { Page } from '@blocksuite/store'; import { Workspace as BlockSuiteWorkspace } from '@blocksuite/store'; import { renderHook } from '@testing-library/react'; import { useAtomValue } from 'jotai'; -import { describe, expect, test, vitest } from 'vitest'; +import { describe, expect, test } from 'vitest'; import { beforeEach } from 'vitest'; import { useBlockSuitePagePreview } from '../use-block-suite-page-preview'; @@ -52,17 +51,6 @@ describe('useBlockSuiteWorkspaceName', () => { workspaceNameHook.result.current[1]('test 3'); expect(blockSuiteWorkspace.meta.name).toBe('test 3'); }); - - test('null', () => { - const workspaceNameHook = renderHook(() => - useBlockSuiteWorkspaceName(null) - ); - vitest.spyOn(globalThis.console, 'warn'); - expect(workspaceNameHook.result.current[0]).toBe(UNTITLED_WORKSPACE_NAME); - workspaceNameHook.result.current[1]('test'); - expect(globalThis.console.warn).toHaveBeenCalledTimes(2); - expect(workspaceNameHook.result.current[0]).toBe(UNTITLED_WORKSPACE_NAME); - }); }); describe('useBlockSuiteWorkspacePageTitle', () => { diff --git a/packages/hooks/src/use-block-suite-page-references.ts b/packages/hooks/src/use-block-suite-page-references.ts index de2bdb7b5b..8a46cde154 100644 --- a/packages/hooks/src/use-block-suite-page-references.ts +++ b/packages/hooks/src/use-block-suite-page-references.ts @@ -1,9 +1,10 @@ +import { assertExists } from '@blocksuite/global/utils'; import type { Page, Workspace } from '@blocksuite/store'; -import { atom, useAtomValue } from 'jotai'; -import { atomFamily } from 'jotai/utils'; +import { type Atom, atom, useAtomValue } from 'jotai'; import { useBlockSuiteWorkspacePage } from './use-block-suite-workspace-page'; +const weakMap = new WeakMap>(); function getPageReferences(page: Page): string[] { // todo: is there a way to use page indexer to get all references? return page @@ -13,26 +14,27 @@ function getPageReferences(page: Page): string[] { .filter(Boolean); } -const pageReferencesAtomFamily = atomFamily((page: Page | null) => { - if (page === null) { - return atom([]); - } - const baseAtom = atom(getPageReferences(page)); - baseAtom.onMount = set => { - const dispose = page.slots.yUpdated.on(() => { - set(getPageReferences(page)); - }); - return () => { - dispose.dispose(); +const getPageReferencesAtom = (page: Page) => { + if (!weakMap.has(page)) { + const baseAtom = atom(getPageReferences(page)); + baseAtom.onMount = set => { + const dispose = page.slots.yUpdated.on(() => { + set(getPageReferences(page)); + }); + return () => { + dispose.dispose(); + }; }; - }; - return baseAtom; -}); + weakMap.set(page, baseAtom); + } + return weakMap.get(page) as Atom; +}; export function useBlockSuitePageReferences( blockSuiteWorkspace: Workspace, pageId: string ): string[] { const page = useBlockSuiteWorkspacePage(blockSuiteWorkspace, pageId); - return useAtomValue(pageReferencesAtomFamily(page)); + assertExists(page); + return useAtomValue(getPageReferencesAtom(page)); } diff --git a/packages/hooks/src/use-block-suite-workspace-helper.ts b/packages/hooks/src/use-block-suite-workspace-helper.ts index 430df54004..3695884435 100644 --- a/packages/hooks/src/use-block-suite-workspace-helper.ts +++ b/packages/hooks/src/use-block-suite-workspace-helper.ts @@ -1,4 +1,3 @@ -import { assertExists } from '@blocksuite/global/utils'; import type { Page, Workspace } from '@blocksuite/store'; import { useMemo } from 'react'; @@ -6,7 +5,6 @@ export function useBlockSuiteWorkspaceHelper(blockSuiteWorkspace: Workspace) { return useMemo( () => ({ createPage: (pageId?: string): Page => { - assertExists(blockSuiteWorkspace); return blockSuiteWorkspace.createPage({ id: pageId }); }, }), diff --git a/packages/hooks/src/use-block-suite-workspace-name.ts b/packages/hooks/src/use-block-suite-workspace-name.ts index 8839fb5663..2f9365571d 100644 --- a/packages/hooks/src/use-block-suite-workspace-name.ts +++ b/packages/hooks/src/use-block-suite-workspace-name.ts @@ -1,28 +1,15 @@ import { UNTITLED_WORKSPACE_NAME } from '@affine/env/constant'; -import { assertExists } from '@blocksuite/global/utils'; import type { Workspace } from '@blocksuite/store'; import type { Atom, WritableAtom } from 'jotai'; import { atom, useAtom } from 'jotai'; -const weakMap = new WeakMap< - Workspace, - WritableAtom & Atom ->(); +type StringAtom = WritableAtom & Atom; -const emptyWorkspaceNameAtom = atom(UNTITLED_WORKSPACE_NAME, () => { - console.warn('you cannot set the name of an null workspace.'); - console.warn('this is a bug in the code.'); -}); +const weakMap = new WeakMap(); -export function useBlockSuiteWorkspaceName( - blockSuiteWorkspace: Workspace | null -) { - let nameAtom: - | (WritableAtom & Atom) - | undefined; - if (!blockSuiteWorkspace) { - nameAtom = emptyWorkspaceNameAtom; - } else if (!weakMap.has(blockSuiteWorkspace)) { +export function useBlockSuiteWorkspaceName(blockSuiteWorkspace: Workspace) { + let nameAtom: StringAtom; + if (!weakMap.has(blockSuiteWorkspace)) { const baseAtom = atom( blockSuiteWorkspace.meta.name ?? UNTITLED_WORKSPACE_NAME ); @@ -44,8 +31,7 @@ export function useBlockSuiteWorkspaceName( weakMap.set(blockSuiteWorkspace, writableAtom); nameAtom = writableAtom; } else { - nameAtom = weakMap.get(blockSuiteWorkspace); - assertExists(nameAtom); + nameAtom = weakMap.get(blockSuiteWorkspace) as StringAtom; } return useAtom(nameAtom); } diff --git a/packages/hooks/src/use-block-suite-workspace-page.ts b/packages/hooks/src/use-block-suite-workspace-page.ts index 68461726ab..368bf906fb 100644 --- a/packages/hooks/src/use-block-suite-workspace-page.ts +++ b/packages/hooks/src/use-block-suite-workspace-page.ts @@ -51,6 +51,5 @@ export function useBlockSuiteWorkspacePage( ): Page | null { const pageAtom = getAtom(blockSuiteWorkspace, pageId); assertExists(pageAtom); - const page = useAtomValue(pageAtom); - return page; + return useAtomValue(pageAtom); }