From d859c4252b880aaf0779c7a6ac7973e786a43422 Mon Sep 17 00:00:00 2001 From: Saul-Mirone Date: Mon, 5 May 2025 09:24:09 +0000 Subject: [PATCH] refactor(editor): move history from doc to store (#12131) ## Summary by CodeRabbit - **New Features** - Undo/redo history management is now centralized in the workspace, providing more consistent and robust undo/redo behavior. - History update events are emitted at the workspace level, enabling better tracking of changes. - **Bug Fixes** - Improved reliability of undo/redo actions by shifting history management from documents to the workspace. - **Documentation** - Updated and clarified documentation for history-related APIs, including improved examples and clearer descriptions. --- .../embed-synced-doc-block/configs/toolbar.ts | 4 +- .../affine/shared/src/utils/model/doc.ts | 2 +- .../api/@blocksuite/store/classes/Store.md | 160 +++++++----------- .../store/interfaces/StoreSlots.md | 2 +- blocksuite/framework/store/src/model/doc.ts | 18 -- .../framework/store/src/model/store/store.ts | 97 ++++++++--- .../framework/store/src/test/test-doc.ts | 98 ----------- .../playground/apps/starter/data/database.ts | 2 +- .../playground/apps/starter/data/embed.ts | 2 +- .../playground/apps/starter/data/empty.ts | 2 +- .../apps/starter/utils/collection.ts | 1 - .../core/src/modules/workspace/impls/doc.ts | 94 +--------- 12 files changed, 145 insertions(+), 337 deletions(-) diff --git a/blocksuite/affine/blocks/embed-doc/src/embed-synced-doc-block/configs/toolbar.ts b/blocksuite/affine/blocks/embed-doc/src/embed-synced-doc-block/configs/toolbar.ts index e02b0f80d8..c5ebea0918 100644 --- a/blocksuite/affine/blocks/embed-doc/src/embed-synced-doc-block/configs/toolbar.ts +++ b/blocksuite/affine/blocks/embed-doc/src/embed-synced-doc-block/configs/toolbar.ts @@ -310,7 +310,7 @@ const builtinSurfaceToolbarConfig = { note.props.displayMode !== NoteDisplayMode.EdgelessOnly ); - ctx.doc.captureSync(); + ctx.store.captureSync(); ctx.chain .pipe(duplicateSelectedModelsCommand, { selectedModels: [model], @@ -350,7 +350,7 @@ const builtinSurfaceToolbarConfig = { } if (contentModels.length === 0) return; - ctx.doc.captureSync(); + ctx.store.captureSync(); ctx.chain .pipe(draftSelectedModelsCommand, { selectedModels: contentModels, diff --git a/blocksuite/affine/shared/src/utils/model/doc.ts b/blocksuite/affine/shared/src/utils/model/doc.ts index 487438aa59..d1882a9918 100644 --- a/blocksuite/affine/shared/src/utils/model/doc.ts +++ b/blocksuite/affine/shared/src/utils/model/doc.ts @@ -32,7 +32,7 @@ export function createDefaultDoc( store.addBlock('affine:paragraph', {}, noteId); // To make sure the content of new doc would not be clear // By undo operation for the first time - doc.resetHistory(); + store.resetHistory(); return store; } diff --git a/blocksuite/docs/api/@blocksuite/store/classes/Store.md b/blocksuite/docs/api/@blocksuite/store/classes/Store.md index 1745c1b89b..06a94561e6 100644 --- a/blocksuite/docs/api/@blocksuite/store/classes/Store.md +++ b/blocksuite/docs/api/@blocksuite/store/classes/Store.md @@ -721,36 +721,6 @@ Check if the store can undo *** -### captureSync - -#### Get Signature - -> **get** **captureSync**(): () => `void` - -Force the following history to be captured into a new stack. - -##### Example - -```ts -op1(); -op2(); -store.captureSync(); -op3(); - -store.undo(); // undo op3 -store.undo(); // undo op1, op2 -``` - -##### Returns - -> (): `void` - -###### Returns - -`void` - -*** - ### history #### Get Signature @@ -765,51 +735,75 @@ Get the Y.UndoManager instance for current store. *** -### redo +### captureSync() -#### Get Signature +> **captureSync**(): `void` -> **get** **redo**(): () => `void` +Force the following history to be captured into a new stack. + +#### Returns + +`void` + +#### Example + +```ts +op1(); +op2(); +store.captureSync(); +op3(); + +store.undo(); // undo op3 +store.undo(); // undo op1, op2 +``` + +*** + +### redo() + +> **redo**(): `void` Redo the last undone transaction. -##### Returns - -> (): `void` - -###### Returns +#### Returns `void` *** -### resetHistory +### resetHistory() -#### Get Signature - -> **get** **resetHistory**(): () => `void` +> **resetHistory**(): `void` Reset the history of the store. -##### Returns - -> (): `void` - -###### Returns +#### Returns `void` *** -### transact +### transact() -#### Get Signature - -> **get** **transact**(): (`fn`, `shouldTransact?`) => `void` +> **transact**(`fn`, `shouldTransact`): `void` Execute a transaction. -##### Example +#### Parameters + +##### fn + +() => `void` + +##### shouldTransact + +`boolean` = `...` + +#### Returns + +`void` + +#### Example ```ts store.transact(() => { @@ -818,53 +812,37 @@ store.transact(() => { }); ``` -##### Returns - -> (`fn`, `shouldTransact?`): `void` - -###### Parameters - -###### fn - -() => `void` - -###### shouldTransact? - -`boolean` - -###### Returns - -`void` - *** -### undo +### undo() -#### Get Signature - -> **get** **undo**(): () => `void` +> **undo**(): `void` Undo the last transaction. -##### Returns - -> (): `void` - -###### Returns +#### Returns `void` *** -### withoutTransact +### withoutTransact() -#### Get Signature - -> **get** **withoutTransact**(): (`fn`) => `void` +> **withoutTransact**(`fn`): `void` Execute a transaction without capturing the history. -##### Example +#### Parameters + +##### fn + +() => `void` + +#### Returns + +`void` + +#### Example ```ts store.withoutTransact(() => { @@ -873,20 +851,6 @@ store.withoutTransact(() => { }); ``` -##### Returns - -> (`fn`): `void` - -###### Parameters - -###### fn - -() => `void` - -###### Returns - -`void` - ## Store Lifecycle ### disposableGroup diff --git a/blocksuite/docs/api/@blocksuite/store/interfaces/StoreSlots.md b/blocksuite/docs/api/@blocksuite/store/interfaces/StoreSlots.md index 34147b814f..5a6a74c03e 100644 --- a/blocksuite/docs/api/@blocksuite/store/interfaces/StoreSlots.md +++ b/blocksuite/docs/api/@blocksuite/store/interfaces/StoreSlots.md @@ -36,7 +36,7 @@ The payload can have three types: > **historyUpdated**: `Subject`\<`void`\> -This fires when the doc history is updated. +This fires when the history is updated. *** diff --git a/blocksuite/framework/store/src/model/doc.ts b/blocksuite/framework/store/src/model/doc.ts index b274fed7d3..33ad2cdb55 100644 --- a/blocksuite/framework/store/src/model/doc.ts +++ b/blocksuite/framework/store/src/model/doc.ts @@ -9,10 +9,6 @@ import type { Workspace } from './workspace.js'; import type { DocMeta } from './workspace-meta.js'; export type GetBlocksOptions = Omit; -export type CreateBlocksOptions = GetBlocksOptions & { - id?: string; -}; -export type YBlocks = Y.Map; export interface Doc { readonly id: string; @@ -24,10 +20,6 @@ export interface Doc { dispose(): void; slots: { - /** - * This fires when the doc history is updated. - */ - historyUpdated: Subject; /** * @internal * This fires when the doc yBlock is updated. @@ -39,16 +31,6 @@ export interface Doc { }>; }; - get history(): Y.UndoManager; - get canRedo(): boolean; - get canUndo(): boolean; - undo(): void; - redo(): void; - resetHistory(): void; - transact(fn: () => void, shouldTransact?: boolean): void; - withoutTransact(fn: () => void): void; - - captureSync(): void; clear(): void; getStore(options?: GetBlocksOptions): Store; clearQuery(query: Query, readonly?: boolean): void; diff --git a/blocksuite/framework/store/src/model/store/store.ts b/blocksuite/framework/store/src/model/store/store.ts index 27d17a12b3..c790761415 100644 --- a/blocksuite/framework/store/src/model/store/store.ts +++ b/blocksuite/framework/store/src/model/store/store.ts @@ -3,6 +3,7 @@ import { DisposableGroup } from '@blocksuite/global/disposable'; import { BlockSuiteError, ErrorCode } from '@blocksuite/global/exceptions'; import { computed, signal } from '@preact/signals-core'; import { Subject } from 'rxjs'; +import * as Y from 'yjs'; import type { ExtensionType } from '../../extension/extension.js'; import { @@ -160,6 +161,10 @@ export type StoreSlots = Doc['slots'] & { * */ blockUpdated: Subject; + /** + * This fires when the history is updated. + */ + historyUpdated: Subject; }; const internalExtensions = [StoreSelectionExtension]; @@ -186,6 +191,8 @@ export class Store { private readonly _provider: ServiceProvider; + private _shouldTransact = true; + private readonly _runQuery = (block: Block) => { runQuery(this._query, block); }; @@ -209,6 +216,10 @@ export class Store { private readonly _schema: Schema; + private readonly _canRedo = signal(false); + + private readonly _canUndo = signal(false); + /** * Get the id of the store. * @@ -284,7 +295,7 @@ export class Store { if (this.readonly) { return false; } - return this._doc.canRedo; + return this._canRedo.peek(); } /** @@ -296,7 +307,7 @@ export class Store { if (this.readonly) { return false; } - return this._doc.canUndo; + return this._canUndo.peek(); } /** @@ -304,13 +315,12 @@ export class Store { * * @category History */ - get undo() { + undo() { if (this.readonly) { - return () => { - console.error('cannot undo in readonly mode'); - }; + console.error('cannot undo in readonly mode'); + return; } - return this._doc.undo.bind(this._doc); + this._history.undo(); } /** @@ -318,13 +328,12 @@ export class Store { * * @category History */ - get redo() { + redo() { if (this.readonly) { - return () => { - console.error('cannot undo in readonly mode'); - }; + console.error('cannot undo in readonly mode'); + return; } - return this._doc.redo.bind(this._doc); + this._history.redo(); } /** @@ -332,8 +341,8 @@ export class Store { * * @category History */ - get resetHistory() { - return this._doc.resetHistory.bind(this._doc); + resetHistory() { + return this._history.clear(); } /** @@ -349,8 +358,21 @@ export class Store { * * @category History */ - get transact() { - return this._doc.transact.bind(this._doc); + transact(fn: () => void, shouldTransact: boolean = this._shouldTransact) { + const spaceDoc = this.doc.spaceDoc; + spaceDoc.transact( + () => { + try { + fn(); + } catch (e) { + console.error( + `An error occurred while Y.doc ${spaceDoc.guid} transacting:` + ); + console.error(e); + } + }, + shouldTransact ? this.rootDoc.clientID : null + ); } /** @@ -366,8 +388,10 @@ export class Store { * * @category History */ - get withoutTransact() { - return this._doc.withoutTransact.bind(this._doc); + withoutTransact(fn: () => void) { + this._shouldTransact = false; + fn(); + this._shouldTransact = true; } /** @@ -386,8 +410,8 @@ export class Store { * * @category History */ - get captureSync() { - return this._doc.captureSync.bind(this._doc); + captureSync() { + this._history.stopCapturing(); } /** @@ -403,7 +427,7 @@ export class Store { * @category History */ get history() { - return this._doc.history; + return this._history; } /** @@ -516,6 +540,8 @@ export class Store { private _isDisposed = false; + private readonly _history!: Y.UndoManager; + /** * @internal * In most cases, you don't need to use the constructor directly. @@ -528,7 +554,7 @@ export class Store { rootAdded: new Subject(), rootDeleted: new Subject(), blockUpdated: new Subject(), - historyUpdated: this._doc.slots.historyUpdated, + historyUpdated: new Subject(), yBlockUpdated: this._doc.slots.yBlockUpdated, }; this._schema = new Schema(); @@ -565,9 +591,35 @@ export class Store { this._onBlockAdded(id, false, true); }); + this._history = new Y.UndoManager([this._yBlocks], { + trackedOrigins: new Set([this.doc.spaceDoc.clientID]), + }); + + this._updateCanUndoRedoSignals(); + this._history.on('stack-cleared', this._historyObserver); + this._history.on('stack-item-added', this._historyObserver); + this._history.on('stack-item-popped', this._historyObserver); + this._history.on('stack-item-updated', this._historyObserver); + this._subscribeToSlots(); } + private readonly _updateCanUndoRedoSignals = () => { + const canRedo = this._history.canRedo(); + const canUndo = this._history.canUndo(); + if (this._canRedo.peek() !== canRedo) { + this._canRedo.value = canRedo; + } + if (this._canUndo.peek() !== canUndo) { + this._canUndo.value = canUndo; + } + }; + + private readonly _historyObserver = () => { + this._updateCanUndoRedoSignals(); + this.slots.historyUpdated.next(); + }; + private readonly _subscribeToSlots = () => { this.disposableGroup.add( this._doc.slots.yBlockUpdated.subscribe(({ type, id, isLocal }) => { @@ -1204,6 +1256,7 @@ export class Store { this.slots.rootAdded.complete(); this.slots.rootDeleted.complete(); this.slots.blockUpdated.complete(); + this.slots.historyUpdated.complete(); this.disposableGroup.dispose(); this._isDisposed = true; } diff --git a/blocksuite/framework/store/src/test/test-doc.ts b/blocksuite/framework/store/src/test/test-doc.ts index b1d7339c21..415b8bcffd 100644 --- a/blocksuite/framework/store/src/test/test-doc.ts +++ b/blocksuite/framework/store/src/test/test-doc.ts @@ -1,4 +1,3 @@ -import { signal } from '@preact/signals-core'; import { Subject } from 'rxjs'; import * as Y from 'yjs'; @@ -17,10 +16,6 @@ type DocOptions = { }; export class TestDoc implements Doc { - private readonly _canRedo$ = signal(false); - - private readonly _canUndo$ = signal(false); - private readonly _collection: Workspace; private readonly _storeMap = new Map(); @@ -30,13 +25,6 @@ export class TestDoc implements Doc { events.forEach(event => this._handleYEvent(event)); }; - private _history!: Y.UndoManager; - - private readonly _historyObserver = () => { - this._updateCanUndoRedoSignals(); - this.slots.historyUpdated.next(); - }; - private readonly _initSubDoc = () => { let subDoc = this.rootDoc.getMap('spaces').get(this.id); if (!subDoc) { @@ -77,19 +65,6 @@ export class TestDoc implements Doc { /** Indicate whether the block tree is ready */ private _ready = false; - private _shouldTransact = true; - - private readonly _updateCanUndoRedoSignals = () => { - const canRedo = this._history.canRedo(); - const canUndo = this._history.canUndo(); - if (this._canRedo$.peek() !== canRedo) { - this._canRedo$.value = canRedo; - } - if (this._canUndo$.peek() !== canUndo) { - this._canUndo$.value = canUndo; - } - }; - protected readonly _yBlocks: Y.Map; /** @@ -105,7 +80,6 @@ export class TestDoc implements Doc { readonly rootDoc: Y.Doc; readonly slots = { - historyUpdated: new Subject(), yBlockUpdated: new Subject< | { type: 'add'; @@ -124,30 +98,10 @@ export class TestDoc implements Doc { return this.workspace.blobSync; } - get canRedo() { - return this._canRedo$.peek(); - } - - get canRedo$() { - return this._canRedo$; - } - - get canUndo() { - return this._canUndo$.peek(); - } - - get canUndo$() { - return this._canUndo$; - } - get workspace() { return this._collection; } - get history() { - return this._history; - } - get isEmpty() { return this._yBlocks.size === 0; } @@ -227,19 +181,6 @@ export class TestDoc implements Doc { private _initYBlocks() { const { _yBlocks } = this; _yBlocks.observeDeep(this._handleYEvents); - this._history = new Y.UndoManager([_yBlocks], { - trackedOrigins: new Set([this._ySpaceDoc.clientID]), - }); - - this._history.on('stack-cleared', this._historyObserver); - this._history.on('stack-item-added', this._historyObserver); - this._history.on('stack-item-popped', this._historyObserver); - this._history.on('stack-item-updated', this._historyObserver); - } - - /** Capture current operations to undo stack synchronously. */ - captureSync() { - this._history.stopCapturing(); } clear() { @@ -258,8 +199,6 @@ export class TestDoc implements Doc { } dispose() { - this.slots.historyUpdated.complete(); - if (this.ready) { this._yBlocks.unobserveDeep(this._handleYEvents); this._yBlocks.clear(); @@ -337,45 +276,8 @@ export class TestDoc implements Doc { return this; } - redo() { - this._history.redo(); - } - - undo() { - this._history.undo(); - } - remove() { this._destroy(); this.rootDoc.getMap('spaces').delete(this.id); } - - resetHistory() { - this._history.clear(); - } - - /** - * If `shouldTransact` is `false`, the transaction will not be push to the history stack. - */ - transact(fn: () => void, shouldTransact: boolean = this._shouldTransact) { - this._ySpaceDoc.transact( - () => { - try { - fn(); - } catch (e) { - console.error( - `An error occurred while Y.doc ${this._ySpaceDoc.guid} transacting:` - ); - console.error(e); - } - }, - shouldTransact ? this.rootDoc.clientID : null - ); - } - - withoutTransact(callback: () => void) { - this._shouldTransact = false; - callback(); - this._shouldTransact = true; - } } diff --git a/blocksuite/playground/apps/starter/data/database.ts b/blocksuite/playground/apps/starter/data/database.ts index a7f926571f..9d174ce12c 100644 --- a/blocksuite/playground/apps/starter/data/database.ts +++ b/blocksuite/playground/apps/starter/data/database.ts @@ -112,7 +112,7 @@ export const database: InitFn = (collection: Workspace, id: string) => { store.addBlock('affine:paragraph', {}, noteId); datasource.viewManager.viewAdd(viewPresets.kanbanViewMeta.type); - doc.resetHistory(); + store.resetHistory(); }; // Add database block inside note block addDatabase('Database 1', false); diff --git a/blocksuite/playground/apps/starter/data/embed.ts b/blocksuite/playground/apps/starter/data/embed.ts index 50fd985d4a..39a0236fcf 100644 --- a/blocksuite/playground/apps/starter/data/embed.ts +++ b/blocksuite/playground/apps/starter/data/embed.ts @@ -47,7 +47,7 @@ export const embed: InitFn = (collection: Workspace, id: string) => { store.addBlock('affine:paragraph', {}, noteId); }); - doc.resetHistory(); + store.resetHistory(); }; embed.id = 'embed'; diff --git a/blocksuite/playground/apps/starter/data/empty.ts b/blocksuite/playground/apps/starter/data/empty.ts index fdb45379d3..589ffa9a29 100644 --- a/blocksuite/playground/apps/starter/data/empty.ts +++ b/blocksuite/playground/apps/starter/data/empty.ts @@ -21,7 +21,7 @@ export const empty: InitFn = (collection: Workspace, id: string) => { store.addBlock('affine:paragraph', {}, noteId); }); - doc.resetHistory(); + store.resetHistory(); }; empty.id = 'empty'; diff --git a/blocksuite/playground/apps/starter/utils/collection.ts b/blocksuite/playground/apps/starter/utils/collection.ts index 4fed772180..f31a37c851 100644 --- a/blocksuite/playground/apps/starter/utils/collection.ts +++ b/blocksuite/playground/apps/starter/utils/collection.ts @@ -95,6 +95,5 @@ export async function initStarterDocCollection(collection: TestWorkspace) { if (!doc?.loaded) { doc?.load(); } - doc?.resetHistory(); } } diff --git a/packages/frontend/core/src/modules/workspace/impls/doc.ts b/packages/frontend/core/src/modules/workspace/impls/doc.ts index f36ae4e3f6..ba4711ce7b 100644 --- a/packages/frontend/core/src/modules/workspace/impls/doc.ts +++ b/packages/frontend/core/src/modules/workspace/impls/doc.ts @@ -9,7 +9,6 @@ import { type Workspace, type YBlock, } from '@blocksuite/affine/store'; -import { signal } from '@preact/signals-core'; import { Subject } from 'rxjs'; import { Awareness } from 'y-protocols/awareness.js'; import * as Y from 'yjs'; @@ -21,10 +20,6 @@ type DocOptions = { }; export class DocImpl implements Doc { - private readonly _canRedo = signal(false); - - private readonly _canUndo = signal(false); - private readonly _collection: Workspace; private readonly _storeMap = new Map(); @@ -34,13 +29,6 @@ export class DocImpl implements Doc { events.forEach(event => this._handleYEvent(event)); }; - private _history!: Y.UndoManager; - - private readonly _historyObserver = () => { - this._updateCanUndoRedoSignals(); - this.slots.historyUpdated.next(); - }; - private readonly _initSubDoc = () => { { // This is a piece of old version compatible code. The old version relies on the subdoc instance on `spaces`. @@ -55,9 +43,7 @@ export class DocImpl implements Doc { } } - const spaceDoc = new Y.Doc({ - guid: this.id, - }); + const spaceDoc = new Y.Doc({ guid: this.id }); spaceDoc.clientID = this.rootDoc.clientID; this._loaded = false; @@ -72,19 +58,6 @@ export class DocImpl implements Doc { /** Indicate whether the block tree is ready */ private _ready = false; - private _shouldTransact = true; - - private readonly _updateCanUndoRedoSignals = () => { - const canRedo = this._history.canRedo(); - const canUndo = this._history.canUndo(); - if (this._canRedo.peek() !== canRedo) { - this._canRedo.value = canRedo; - } - if (this._canUndo.peek() !== canUndo) { - this._canUndo.value = canUndo; - } - }; - protected readonly _yBlocks: Y.Map; /** @@ -102,8 +75,6 @@ export class DocImpl implements Doc { readonly rootDoc: Y.Doc; readonly slots = { - // eslint-disable-next-line rxjs/finnish - historyUpdated: new Subject(), // eslint-disable-next-line rxjs/finnish yBlockUpdated: new Subject< | { @@ -123,22 +94,10 @@ export class DocImpl implements Doc { return this.workspace.blobSync; } - get canRedo() { - return this._canRedo.peek(); - } - - get canUndo() { - return this._canUndo.peek(); - } - get workspace() { return this._collection; } - get history() { - return this._history; - } - get isEmpty() { return this._yBlocks.size === 0; } @@ -217,19 +176,6 @@ export class DocImpl implements Doc { private _initYBlocks() { const { _yBlocks } = this; _yBlocks.observeDeep(this._handleYEvents); - this._history = new Y.UndoManager([_yBlocks], { - trackedOrigins: new Set([this._ySpaceDoc.clientID]), - }); - - this._history.on('stack-cleared', this._historyObserver); - this._history.on('stack-item-added', this._historyObserver); - this._history.on('stack-item-popped', this._historyObserver); - this._history.on('stack-item-updated', this._historyObserver); - } - - /** Capture current operations to undo stack synchronously. */ - captureSync() { - this._history.stopCapturing(); } clear() { @@ -250,7 +196,6 @@ export class DocImpl implements Doc { dispose() { this._destroy(); - this.slots.historyUpdated.unsubscribe(); if (this.ready) { this._yBlocks.unobserveDeep(this._handleYEvents); @@ -335,45 +280,8 @@ export class DocImpl implements Doc { return this; } - redo() { - this._history.redo(); - } - - undo() { - this._history.undo(); - } - remove() { this._destroy(); this.rootDoc.getMap('spaces').delete(this.id); } - - resetHistory() { - this._history.clear(); - } - - /** - * If `shouldTransact` is `false`, the transaction will not be push to the history stack. - */ - transact(fn: () => void, shouldTransact: boolean = this._shouldTransact) { - this._ySpaceDoc.transact( - () => { - try { - fn(); - } catch (e) { - console.error( - `An error occurred while Y.doc ${this._ySpaceDoc.guid} transacting:` - ); - console.error(e); - } - }, - shouldTransact ? this.rootDoc.clientID : null - ); - } - - withoutTransact(callback: () => void) { - this._shouldTransact = false; - callback(); - this._shouldTransact = true; - } }