From 62a60756754e1085c185e4b606a98e6f27de7df5 Mon Sep 17 00:00:00 2001 From: pengx17 Date: Sat, 23 Mar 2024 12:15:05 +0000 Subject: [PATCH] fix(core): do not ensure properties on read (#6263) --- .../page-properties-manager.ts | 66 ++++++++--------- .../modules/workspace/properties/adapter.ts | 73 +++++-------------- .../modules/workspace/properties/schema.ts | 6 +- 3 files changed, 51 insertions(+), 94 deletions(-) diff --git a/packages/frontend/core/src/components/affine/page-properties/page-properties-manager.ts b/packages/frontend/core/src/components/affine/page-properties/page-properties-manager.ts index a856559a96..060468261b 100644 --- a/packages/frontend/core/src/components/affine/page-properties/page-properties-manager.ts +++ b/packages/frontend/core/src/components/affine/page-properties/page-properties-manager.ts @@ -1,8 +1,8 @@ +/* eslint-disable @typescript-eslint/no-non-null-assertion */ import type { WorkspacePropertiesAdapter } from '@affine/core/modules/workspace'; import type { PageInfoCustomProperty, PageInfoCustomPropertyMeta, - TagOption, } from '@affine/core/modules/workspace/properties/schema'; import { PagePropertyType } from '@affine/core/modules/workspace/properties/schema'; import { DebugLogger } from '@affine/debug'; @@ -41,20 +41,16 @@ export const newPropertyTypes: PagePropertyType[] = [ export class PagePropertiesMetaManager { constructor(private readonly adapter: WorkspacePropertiesAdapter) {} - get tagOptions() { - return this.adapter.tagOptions; - } - get propertiesSchema() { - return this.adapter.schema.pageProperties; + return this.adapter.schema?.pageProperties ?? {}; } get systemPropertiesSchema() { - return this.adapter.schema.pageProperties.system; + return this.adapter.schema?.pageProperties.system ?? {}; } get customPropertiesSchema() { - return this.adapter.schema.pageProperties.custom; + return this.adapter.schema?.pageProperties.custom ?? {}; } getOrderedPropertiesSchema() { @@ -126,9 +122,11 @@ export class PagePropertiesMetaManager { const mapping = new Map>(); for (const page of this.adapter.workspace.docCollection.docs.values()) { const properties = this.adapter.getPageProperties(page.id); - for (const id of Object.keys(properties.custom)) { - if (!mapping.has(id)) mapping.set(id, new Set()); - mapping.get(id)?.add(page.id); + if (properties) { + for (const id of Object.keys(properties.custom)) { + if (!mapping.has(id)) mapping.set(id, new Set()); + mapping.get(id)?.add(page.id); + } } } return mapping; @@ -145,13 +143,14 @@ export class PagePropertiesManager { private readonly adapter: WorkspacePropertiesAdapter, public readonly pageId: string ) { - this.adapter.ensurePageProperties(this.pageId); this.metaManager = new PagePropertiesMetaManager(this.adapter); + this.ensureRequiredProperties(); } // prevent infinite loop private ensuring = false; ensureRequiredProperties() { + this.adapter.ensurePageProperties(this.pageId); if (this.ensuring) return; this.ensuring = true; this.transact(() => { @@ -184,12 +183,7 @@ export class PagePropertiesManager { return this.intrinsicMeta?.createDate; } - get pageTags() { - return this.adapter.getPageTags(this.pageId); - } - get properties() { - this.ensureRequiredProperties(); return this.adapter.getPageProperties(this.pageId); } @@ -197,23 +191,17 @@ export class PagePropertiesManager { return !!this.page?.readonly; } - addPageTag(pageId: string, tag: TagOption | string) { - this.adapter.addPageTag(pageId, tag); - } - - removePageTag(pageId: string, tag: TagOption | string) { - this.adapter.removePageTag(pageId, tag); - } - /** * get custom properties (filter out properties that are not in schema) */ getCustomProperties(): Record { - return Object.fromEntries( - Object.entries(this.properties.custom).filter(([id]) => - this.metaManager.checkPropertyExists(id) - ) - ); + return this.properties + ? Object.fromEntries( + Object.entries(this.properties.custom).filter(([id]) => + this.metaManager.checkPropertyExists(id) + ) + ) + : {}; } getOrderedCustomProperties() { @@ -231,10 +219,11 @@ export class PagePropertiesManager { } getCustomProperty(id: string) { - return this.properties.custom[id]; + return this.properties?.custom[id]; } addCustomProperty(id: string, value?: any) { + this.ensureRequiredProperties(); if (!this.metaManager.checkPropertyExists(id)) { logger.warn(`property ${id} not found`); return; @@ -246,11 +235,11 @@ export class PagePropertiesManager { } const newOrder = generateKeyBetween(this.largestOrder(), null); - if (this.properties.custom[id]) { + if (this.properties!.custom[id]) { logger.warn(`custom property ${id} already exists`); } - this.properties.custom[id] = { + this.properties!.custom[id] = { id, value, order: newOrder, @@ -259,6 +248,7 @@ export class PagePropertiesManager { } moveCustomProperty(from: number, to: number) { + this.ensureRequiredProperties(); // move from -> to means change from's order to a new order between to and to -1/+1 const properties = this.getOrderedCustomProperties(); const fromProperty = properties[from]; @@ -269,19 +259,21 @@ export class PagePropertiesManager { ? [toProperty.order, toNextProperty?.order ?? null] : [toNextProperty?.order ?? null, toProperty.order]; const newOrder = generateKeyBetween(...args); - this.properties.custom[fromProperty.id].order = newOrder; + this.properties!.custom[fromProperty.id].order = newOrder; } hasCustomProperty(id: string) { - return !!this.properties.custom[id]; + return !!this.properties?.custom[id]; } removeCustomProperty(id: string) { - delete this.properties.custom[id]; + this.ensureRequiredProperties(); + delete this.properties!.custom[id]; } updateCustomProperty(id: string, opt: Partial) { - if (!this.properties.custom[id]) { + this.ensureRequiredProperties(); + if (!this.properties?.custom[id]) { logger.warn(`custom property ${id} not found`); return; } diff --git a/packages/frontend/core/src/modules/workspace/properties/adapter.ts b/packages/frontend/core/src/modules/workspace/properties/adapter.ts index 53524870fd..c2cba0174f 100644 --- a/packages/frontend/core/src/modules/workspace/properties/adapter.ts +++ b/packages/frontend/core/src/modules/workspace/properties/adapter.ts @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/no-non-null-assertion */ // the adapter is to bridge the workspace rootdoc & native js bindings import { createYProxy, type Y } from '@blocksuite/store'; @@ -7,7 +8,6 @@ import { defaultsDeep } from 'lodash-es'; import { PagePropertyType, PageSystemPropertyId, - type TagOption, type WorkspaceAffineProperties, type WorkspaceFavoriteItem, } from './schema'; @@ -28,17 +28,21 @@ export class WorkspacePropertiesAdapter { public readonly proxy: WorkspaceAffineProperties; public readonly properties: Y.Map; + private ensuredRoot = false; + private ensuredPages = {} as Record; + constructor(public readonly workspace: Workspace) { // check if properties exists, if not, create one const rootDoc = workspace.docCollection.doc; this.properties = rootDoc.getMap(AFFINE_PROPERTIES_ID); this.proxy = createYProxy(this.properties); - - // fixme: deal with migration issue? - this.ensureRootProperties(); } private ensureRootProperties() { + if (this.ensuredRoot) { + return; + } + this.ensuredRoot = true; // todo: deal with schema change issue // fixme: may not to be called every time defaultsDeep(this.proxy, { @@ -70,6 +74,11 @@ export class WorkspacePropertiesAdapter { } ensurePageProperties(pageId: string) { + this.ensureRootProperties(); + if (this.ensuredPages[pageId]) { + return; + } + this.ensuredPages[pageId] = true; // fixme: may not to be called every time defaultsDeep(this.proxy.pageProperties, { [pageId]: { @@ -108,65 +117,21 @@ export class WorkspacePropertiesAdapter { // ====== utilities ====== getPageProperties(pageId: string) { - this.ensurePageProperties(pageId); - return this.pageProperties[pageId]; + return this.pageProperties?.[pageId] ?? null; } isFavorite(id: string, type: WorkspaceFavoriteItem['type']) { - return this.favorites[id]?.type === type; + return this.favorites?.[id]?.type === type; } getJournalPageDateString(id: string) { - return this.pageProperties[id]?.system[PageSystemPropertyId.Journal]?.value; + return this.pageProperties?.[id]?.system[PageSystemPropertyId.Journal] + ?.value; } setJournalPageDateString(id: string, date: string) { this.ensurePageProperties(id); - const pageProperties = this.pageProperties[id]; - pageProperties.system[PageSystemPropertyId.Journal].value = date; - } - - get tagOptions() { - return this.schema.pageProperties.system[PageSystemPropertyId.Tags].options; - } - - // page tags could be reactive - getPageTags(pageId: string) { - const tags = - this.getPageProperties(pageId)?.system[PageSystemPropertyId.Tags].value ?? - []; - const optionsMap = Object.fromEntries(this.tagOptions.map(o => [o.id, o])); - return tags.map(tag => optionsMap[tag]).filter((t): t is TagOption => !!t); - } - - addPageTag(pageId: string, tag: TagOption | string) { - this.ensurePageProperties(pageId); - const tags = this.getPageTags(pageId); - const tagId = typeof tag === 'string' ? tag : tag.id; - if (tags.some(t => t.id === tagId)) { - return; - } - // add tag option if not exist - if (!this.tagOptions.some(t => t.id === tagId)) { - if (typeof tag === 'string') { - throw new Error(`Tag ${tag} does not exist`); - } else { - this.tagOptions.push(tag); - } - } - const pageProperties = this.pageProperties[pageId]; - pageProperties.system[PageSystemPropertyId.Tags].value.push(tagId); - } - - removePageTag(pageId: string, tag: TagOption | string) { - this.ensurePageProperties(pageId); - const tags = this.getPageTags(pageId); - const tagId = typeof tag === 'string' ? tag : tag.id; - const index = tags.findIndex(t => t.id === tagId); - if (index === -1) { - return; - } - const pageProperties = this.pageProperties[pageId]; - pageProperties.system[PageSystemPropertyId.Tags].value.splice(index, 1); + const pageProperties = this.pageProperties?.[id]; + pageProperties!.system[PageSystemPropertyId.Journal].value = date; } } diff --git a/packages/frontend/core/src/modules/workspace/properties/schema.ts b/packages/frontend/core/src/modules/workspace/properties/schema.ts index 4eaa6a1c1a..f07671b859 100644 --- a/packages/frontend/core/src/modules/workspace/properties/schema.ts +++ b/packages/frontend/core/src/modules/workspace/properties/schema.ts @@ -100,9 +100,9 @@ const WorkspacePagePropertiesSchema = z.object({ }); export const WorkspaceAffinePropertiesSchema = z.object({ - schema: WorkspaceAffinePropertiesSchemaSchema, - favorites: z.record(WorkspaceFavoriteItemSchema), - pageProperties: z.record(WorkspacePagePropertiesSchema), + schema: WorkspaceAffinePropertiesSchemaSchema.optional(), + favorites: z.record(WorkspaceFavoriteItemSchema).optional(), + pageProperties: z.record(WorkspacePagePropertiesSchema).optional(), }); export type PageInfoCustomPropertyMeta = z.infer<