From 8151e6f586149e4447149d25bd70868a5a4e8838 Mon Sep 17 00:00:00 2001 From: alex Date: Wed, 24 Apr 2024 19:26:10 +0100 Subject: [PATCH] Automatic undo/redo (#3364) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Our undo-redo system before this diff is based on commands. A command is: - A function that produces some data required to perform and undo a change - A function that actually performs the change, based on the data - Another function that undoes the change, based on the data - Optionally, a function to _redo_ the change, although in practice we never use this Each command that gets run is added to the undo/redo stack unless it says it shouldn't be. This diff replaces this system of commands with a new one where all changes to the store are automatically recorded in the undo/redo stack. You can imagine the new history manager like a tape recorder - it automatically records everything that happens to the store in a special diff, unless you "pause" the recording and ask it not to. Undo and redo rewind/fast-forward the tape to certain marks. As the command concept is gone, the things that were commands are now just functions that manipulate the store. One other change here is that the store's after-phase callbacks (and the after-phase side-effects as a result) are now batched up and called at the end of certain key operations. For example, `applyDiff` would previously call all the `afterCreate` callbacks before making any removals from the diff. Now, it (and anything else that uses `store.atomic(fn)` will defer firing any after callbacks until the end of an operation. before callbacks are still called part-way through operations. ## Design options Automatic recording is a fairly large big semantic change, particularly to the standalone `store.put`/`store.remove` etc. commands. We could instead make not-recording the default, and make recording opt-in instead. However, I think auto-record-by-default is the right choice for a few reasons: 1. Switching to a recording-based vs command-based undo-redo model is fundamentally a big semantic change. In the past, `store.put` etc. were always ignored. Now, regardless of whether we choose record-by-default or ignore-by-default, the behaviour of `store.put` is _context_ dependant. 2. Switching to ignore-by-default means that either our commands don't record undo/redo history any more (unless wrapped in `editor.history.record`, a far larger semantic change) or they have to always-record/all accept a history options bag. If we choose always-record, we can't use commands within `history.ignore` as they'll start recording again. If we choose the history options bag, we have to accept those options in 10s of methods - basically the entire `Editor` api surface. Overall, given that some breaking semantic change here is unavoidable, I think that record-by-default hits the right balance of tradeoffs. I think it's a better API going forward, whilst also not being too disruptive as the APIs it affects are very "deep" ones that we don't typically encourage people to use. ### Change Type - [x] `sdk` — Changes the tldraw SDK - [x] `improvement` — Improving existing features - [x] `galaxy brain` — Architectural changes ### Release Note #### Breaking changes ##### 1. History Options Previously, some (not all!) commands accepted a history options object with `squashing`, `ephemeral`, and `preserveRedoStack` flags. Squashing enabled/disabled a memory optimisation (storing individual commands vs squashing them together). Ephemeral stopped a command from affecting the undo/redo stack at all. Preserve redo stack stopped commands from wiping the redo stack. These flags were never available consistently - some commands had them and others didn't. In this version, most of these flags have been removed. `squashing` is gone entirely (everything squashes & does so much faster than before). There were a couple of commands that had a special default - for example, `updateInstanceState` used to default to being `ephemeral`. Those maintain the defaults, but the options look a little different now - `{ephemeral: true}` is now `{history: 'ignore'}` and `{preserveRedoStack: true}` is now `{history: 'record-preserveRedoStack'}`. If you were previously using these options in places where they've now been removed, you can use wrap them with `editor.history.ignore(fn)` or `editor.history.batch(fn, {history: 'record-preserveRedoStack'})`. For example, ```ts editor.nudgeShapes(..., { ephemeral: true }) ``` can now be written as ```ts editor.history.ignore(() => { editor.nudgeShapes(...) }) ``` ##### 2. Automatic recording Previously, only commands (e.g. `editor.updateShapes` and things that use it) were added to the undo/redo stack. Everything else (e.g. `editor.store.put`) wasn't. Now, _everything_ that touches the store is recorded in the undo/redo stack (unless it's part of `mergeRemoteChanges`). You can use `editor.history.ignore(fn)` as above if you want to make other changes to the store that aren't recorded - this is short for `editor.history.batch(fn, {history: 'ignore'})` When upgrading to this version of tldraw, you shouldn't need to change anything unless you're using `store.put`, `store.remove`, or `store.applyDiff` outside of `store.mergeRemoteChanges`. If you are, you can preserve the functionality of those not being recorded by wrapping them either in `mergeRemoteChanges` (if they're multiplayer-related) or `history.ignore` as appropriate. ##### 3. Side effects Before this diff, any changes in side-effects weren't captured by the undo-redo stack. This was actually the motivation for this change in the first place! But it's a pretty big change, and if you're using side effects we recommend you double-check how they interact with undo/redo before/after this change. To get the old behaviour back, wrap your side effects in `editor.history.ignore`. ##### 4. Mark options Previously, `editor.mark(id)` accepted two additional boolean parameters: `onUndo` and `onRedo`. If these were set to false, then when undoing or redoing we'd skip over that mark and keep going until we found one with those values set to true. We've removed those options - if you're using them, let us know and we'll figure out an alternative! --- .../context-toolbar/ContextToolbar.tsx | 4 +- .../CustomStylePanelExample.tsx | 4 +- .../user-presence/UserPresenceExample.tsx | 30 +- apps/vscode/editor/src/ChangeResponder.tsx | 4 +- packages/editor/api-report.md | 145 +- packages/editor/src/index.ts | 8 +- packages/editor/src/lib/TldrawEditor.tsx | 7 +- .../editor/src/lib/config/createTLStore.ts | 9 +- packages/editor/src/lib/editor/Editor.ts | 1564 +++++++---------- .../editor/managers/HistoryManager.test.ts | 309 ++-- .../src/lib/editor/managers/HistoryManager.ts | 445 ++--- .../lib/editor/managers/SideEffectManager.ts | 60 +- .../editor/src/lib/editor/types/emit-types.ts | 2 - .../src/lib/editor/types/history-types.ts | 59 +- packages/store/api-report.md | 36 +- packages/store/src/index.ts | 11 +- packages/store/src/lib/RecordType.ts | 18 +- packages/store/src/lib/RecordsDiff.ts | 107 ++ packages/store/src/lib/Store.ts | 272 ++- packages/store/src/lib/StoreQueries.ts | 3 +- packages/store/src/lib/StoreSchema.ts | 7 + .../store/src/lib/test/recordStore.test.ts | 270 ++- packages/tldraw/api-report.md | 6 +- .../lib/shapes/arrow/toolStates/Pointing.ts | 6 +- .../src/lib/shapes/draw/toolStates/Drawing.ts | 10 +- .../frame/components/FrameLabelInput.tsx | 34 +- .../src/lib/shapes/note/NoteShapeTool.test.ts | 1 - .../tools/HandTool/childStates/Pointing.ts | 5 +- .../tools/SelectTool/childStates/Brushing.ts | 4 +- .../childStates/Crop/children/Idle.ts | 14 +- .../Crop/children/TranslatingCrop.ts | 7 +- .../tools/SelectTool/childStates/Cropping.ts | 9 +- .../SelectTool/childStates/DraggingHandle.tsx | 12 +- .../lib/tools/SelectTool/childStates/Idle.ts | 5 +- .../childStates/PointingArrowLabel.ts | 14 +- .../childStates/PointingCropHandle.ts | 12 +- .../SelectTool/childStates/PointingHandle.ts | 10 +- .../childStates/PointingResizeHandle.ts | 8 +- .../childStates/PointingRotateHandle.ts | 13 +- .../tools/SelectTool/childStates/Resizing.ts | 10 +- .../tools/SelectTool/childStates/Rotating.ts | 16 +- .../childStates/ScribbleBrushing.ts | 4 +- .../SelectTool/childStates/Translating.ts | 3 +- .../tldraw/src/lib/tools/ZoomTool/ZoomTool.ts | 15 +- .../lib/ui/components/MobileStylePanel.tsx | 2 +- .../ui/components/PageMenu/PageItemInput.tsx | 17 +- .../StylePanel/DefaultStylePanel.tsx | 2 +- .../StylePanel/DefaultStylePanelContent.tsx | 20 +- .../StylePanel/DoubleDropdownPicker.tsx | 6 +- .../components/StylePanel/DropdownPicker.tsx | 4 +- .../primitives/TldrawUiButtonPicker.tsx | 10 +- .../components/primitives/TldrawUiInput.tsx | 5 +- .../components/primitives/TldrawUiSlider.tsx | 6 +- .../tldraw/src/lib/ui/context/actions.tsx | 15 +- .../ui/hooks/clipboard/pasteTldrawContent.ts | 4 +- packages/tldraw/src/lib/ui/hooks/usePrint.ts | 2 +- packages/tldraw/src/lib/ui/hooks/useTools.tsx | 10 +- .../tldraw/src/test/TldrawEditor.test.tsx | 10 +- .../src/test/commands/alignShapes.test.tsx | 2 +- .../test/commands/distributeShapes.test.ts | 2 +- .../test/commands/moveShapesToPage.test.ts | 3 +- .../tldraw/src/test/commands/squash.test.ts | 13 - .../src/test/commands/stackShapes.test.ts | 2 +- .../tldraw/src/test/commands/stretch.test.tsx | 2 +- packages/tldraw/src/test/testutils/pretty.ts | 129 ++ packages/tlschema/src/records/TLInstance.ts | 33 +- packages/tlschema/src/records/TLPageState.ts | 12 + packages/tlsync/src/lib/TLSyncClient.ts | 28 +- packages/tlsync/src/test/syncFuzz.test.ts | 82 +- 69 files changed, 2106 insertions(+), 1907 deletions(-) create mode 100644 packages/store/src/lib/RecordsDiff.ts delete mode 100644 packages/tldraw/src/test/commands/squash.test.ts create mode 100644 packages/tldraw/src/test/testutils/pretty.ts diff --git a/apps/examples/src/examples/context-toolbar/ContextToolbar.tsx b/apps/examples/src/examples/context-toolbar/ContextToolbar.tsx index 4a92f88ba..c28cb4763 100644 --- a/apps/examples/src/examples/context-toolbar/ContextToolbar.tsx +++ b/apps/examples/src/examples/context-toolbar/ContextToolbar.tsx @@ -77,9 +77,7 @@ const ContextToolbarComponent = track(() => { width: 32, background: isActive ? 'var(--color-muted-2)' : 'transparent', }} - onClick={() => - editor.setStyleForSelectedShapes(DefaultSizeStyle, value, { squashing: false }) - } + onClick={() => editor.setStyleForSelectedShapes(DefaultSizeStyle, value)} > diff --git a/apps/examples/src/examples/custom-style-panel/CustomStylePanelExample.tsx b/apps/examples/src/examples/custom-style-panel/CustomStylePanelExample.tsx index c8d27b5c3..f8df67102 100644 --- a/apps/examples/src/examples/custom-style-panel/CustomStylePanelExample.tsx +++ b/apps/examples/src/examples/custom-style-panel/CustomStylePanelExample.tsx @@ -25,7 +25,7 @@ function CustomStylePanel(props: TLUiStylePanelProps) { { - editor.setStyleForSelectedShapes(DefaultColorStyle, 'red', { squashing: true }) + editor.setStyleForSelectedShapes(DefaultColorStyle, 'red') }} > Red @@ -35,7 +35,7 @@ function CustomStylePanel(props: TLUiStylePanelProps) { { - editor.setStyleForSelectedShapes(DefaultColorStyle, 'green', { squashing: true }) + editor.setStyleForSelectedShapes(DefaultColorStyle, 'green') }} > Green diff --git a/apps/examples/src/examples/user-presence/UserPresenceExample.tsx b/apps/examples/src/examples/user-presence/UserPresenceExample.tsx index 070e6b7a5..1816c3e99 100644 --- a/apps/examples/src/examples/user-presence/UserPresenceExample.tsx +++ b/apps/examples/src/examples/user-presence/UserPresenceExample.tsx @@ -29,7 +29,9 @@ export default function UserPresenceExample() { chatMessage: CURSOR_CHAT_MESSAGE, }) - editor.store.put([peerPresence]) + editor.store.mergeRemoteChanges(() => { + editor.store.put([peerPresence]) + }) // [b] const raf = rRaf.current @@ -67,23 +69,29 @@ export default function UserPresenceExample() { ) } - editor.store.put([ - { - ...peerPresence, - cursor, - chatMessage, - lastActivityTimestamp: now, - }, - ]) + editor.store.mergeRemoteChanges(() => { + editor.store.put([ + { + ...peerPresence, + cursor, + chatMessage, + lastActivityTimestamp: now, + }, + ]) + }) rRaf.current = requestAnimationFrame(loop) } rRaf.current = requestAnimationFrame(loop) } else { - editor.store.put([{ ...peerPresence, lastActivityTimestamp: Date.now() }]) - rRaf.current = setInterval(() => { + editor.store.mergeRemoteChanges(() => { editor.store.put([{ ...peerPresence, lastActivityTimestamp: Date.now() }]) + }) + rRaf.current = setInterval(() => { + editor.store.mergeRemoteChanges(() => { + editor.store.put([{ ...peerPresence, lastActivityTimestamp: Date.now() }]) + }) }, 1000) } }} diff --git a/apps/vscode/editor/src/ChangeResponder.tsx b/apps/vscode/editor/src/ChangeResponder.tsx index 7cb77f6c7..a0bbb5cdd 100644 --- a/apps/vscode/editor/src/ChangeResponder.tsx +++ b/apps/vscode/editor/src/ChangeResponder.tsx @@ -57,11 +57,11 @@ export const ChangeResponder = () => { type: 'vscode:editor-loaded', }) - editor.on('change-history', handleChange) + const dispose = editor.store.listen(handleChange, { scope: 'document' }) return () => { handleChange() - editor.off('change-history', handleChange) + dispose() } }, [editor]) diff --git a/packages/editor/api-report.md b/packages/editor/api-report.md index a8039822c..99c6779e9 100644 --- a/packages/editor/api-report.md +++ b/packages/editor/api-report.md @@ -29,10 +29,12 @@ import { default as React_2 } from 'react'; import * as React_3 from 'react'; import { ReactElement } from 'react'; import { ReactNode } from 'react'; +import { RecordsDiff } from '@tldraw/store'; import { SerializedSchema } from '@tldraw/store'; import { SerializedStore } from '@tldraw/store'; import { ShapeProps } from '@tldraw/tlschema'; import { Signal } from '@tldraw/state'; +import { Store } from '@tldraw/store'; import { StoreSchema } from '@tldraw/store'; import { StoreSnapshot } from '@tldraw/store'; import { StyleProp } from '@tldraw/tlschema'; @@ -375,7 +377,7 @@ export function counterClockwiseAngleDist(a0: number, a1: number): number; export function createSessionStateSnapshotSignal(store: TLStore): Signal; // @public -export function createTLStore({ initialData, defaultName, ...rest }: TLStoreOptions): TLStore; +export function createTLStore({ initialData, defaultName, id, ...rest }: TLStoreOptions): TLStore; // @public (undocumented) export function createTLUser(opts?: { @@ -602,7 +604,7 @@ export class Editor extends EventEmitter { }): this; bail(): this; bailToMark(id: string): this; - batch(fn: () => void): this; + batch(fn: () => void, opts?: TLHistoryBatchOptions): this; bringForward(shapes: TLShape[] | TLShapeId[]): this; bringToFront(shapes: TLShape[] | TLShapeId[]): this; cancel(): this; @@ -810,7 +812,7 @@ export class Editor extends EventEmitter { getZoomLevel(): number; groupShapes(shapes: TLShape[] | TLShapeId[], groupId?: TLShapeId): this; hasAncestor(shape: TLShape | TLShapeId | undefined, ancestorId: TLShapeId): boolean; - readonly history: HistoryManager; + readonly history: HistoryManager; inputs: { buttons: Set; keys: Set; @@ -832,6 +834,7 @@ export class Editor extends EventEmitter { isPointing: boolean; }; interrupt(): this; + isAncestorSelected(shape: TLShape | TLShapeId): boolean; isIn(path: string): boolean; isInAny(...paths: string[]): boolean; isPointInShape(shape: TLShape | TLShapeId, point: VecLike, opts?: { @@ -845,9 +848,9 @@ export class Editor extends EventEmitter { isShapeOrAncestorLocked(shape?: TLShape): boolean; // (undocumented) isShapeOrAncestorLocked(id?: TLShapeId): boolean; - mark(markId?: string, onUndo?: boolean, onRedo?: boolean): this; + mark(markId?: string): this; moveShapesToPage(shapes: TLShape[] | TLShapeId[], pageId: TLPageId): this; - nudgeShapes(shapes: TLShape[] | TLShapeId[], offset: VecLike, historyOptions?: TLCommandHistoryOptions): this; + nudgeShapes(shapes: TLShape[] | TLShapeId[], offset: VecLike): this; packShapes(shapes: TLShape[] | TLShapeId[], gap: number): this; pageToScreen(point: VecLike): { x: number; @@ -876,7 +879,7 @@ export class Editor extends EventEmitter { registerExternalContentHandler(type: T, handler: ((info: T extends TLExternalContent['type'] ? TLExternalContent & { type: T; } : TLExternalContent) => void) | null): this; - renamePage(page: TLPage | TLPageId, name: string, historyOptions?: TLCommandHistoryOptions): this; + renamePage(page: TLPage | TLPageId, name: string): this; renderingBoundsMargin: number; reparentShapes(shapes: TLShape[] | TLShapeId[], parentId: TLParentId, insertIndex?: IndexKey): this; resetZoom(point?: Vec, animation?: TLAnimationOptions): this; @@ -896,7 +899,7 @@ export class Editor extends EventEmitter { sendToBack(shapes: TLShape[] | TLShapeId[]): this; setCamera(point: VecLike, animation?: TLAnimationOptions): this; setCroppingShape(shape: null | TLShape | TLShapeId): this; - setCurrentPage(page: TLPage | TLPageId, historyOptions?: TLCommandHistoryOptions): this; + setCurrentPage(page: TLPage | TLPageId): this; setCurrentTool(id: string, info?: {}): this; setCursor: (cursor: Partial) => this; setEditingShape(shape: null | TLShape | TLShapeId): this; @@ -904,11 +907,11 @@ export class Editor extends EventEmitter { setFocusedGroup(shape: null | TLGroupShape | TLShapeId): this; setHintingShapes(shapes: TLShape[] | TLShapeId[]): this; setHoveredShape(shape: null | TLShape | TLShapeId): this; - setOpacityForNextShapes(opacity: number, historyOptions?: TLCommandHistoryOptions): this; - setOpacityForSelectedShapes(opacity: number, historyOptions?: TLCommandHistoryOptions): this; - setSelectedShapes(shapes: TLShape[] | TLShapeId[], historyOptions?: TLCommandHistoryOptions): this; - setStyleForNextShapes(style: StyleProp, value: T, historyOptions?: TLCommandHistoryOptions): this; - setStyleForSelectedShapes>(style: S, value: StylePropValue, historyOptions?: TLCommandHistoryOptions): this; + setOpacityForNextShapes(opacity: number, historyOptions?: TLHistoryBatchOptions): this; + setOpacityForSelectedShapes(opacity: number): this; + setSelectedShapes(shapes: TLShape[] | TLShapeId[]): this; + setStyleForNextShapes(style: StyleProp, value: T, historyOptions?: TLHistoryBatchOptions): this; + setStyleForSelectedShapes>(style: S, value: StylePropValue): this; shapeUtils: { readonly [K in string]?: ShapeUtil; }; @@ -937,14 +940,16 @@ export class Editor extends EventEmitter { // (undocumented) ungroupShapes(ids: TLShape[]): this; updateAssets(assets: TLAssetPartial[]): this; - updateCurrentPageState(partial: Partial>, historyOptions?: TLCommandHistoryOptions): this; + updateCurrentPageState(partial: Partial>, historyOptions?: TLHistoryBatchOptions): this; + // (undocumented) + _updateCurrentPageState: (partial: Partial>, historyOptions?: TLHistoryBatchOptions) => void; updateDocumentSettings(settings: Partial): this; - updateInstanceState(partial: Partial>, historyOptions?: TLCommandHistoryOptions): this; - updatePage(partial: RequiredKeys, historyOptions?: TLCommandHistoryOptions): this; + updateInstanceState(partial: Partial>, historyOptions?: TLHistoryBatchOptions): this; + updatePage(partial: RequiredKeys): this; // @internal updateRenderingBounds(): this; - updateShape(partial: null | TLShapePartial | undefined, historyOptions?: TLCommandHistoryOptions): this; - updateShapes(partials: (null | TLShapePartial | undefined)[], historyOptions?: TLCommandHistoryOptions): this; + updateShape(partial: null | TLShapePartial | undefined): this; + updateShapes(partials: (null | TLShapePartial | undefined)[]): this; updateViewportScreenBounds(screenBounds: Box, center?: boolean): this; readonly user: UserPreferencesManager; visitDescendants(parent: TLPage | TLParentId | TLShape, visitor: (id: TLShapeId) => false | void): this; @@ -1208,6 +1213,55 @@ export function hardResetEditor(): void; // @internal (undocumented) export const HASH_PATTERN_ZOOM_NAMES: Record; +// @public (undocumented) +export class HistoryManager { + constructor(opts: { + annotateError?: (error: unknown) => void; + store: Store; + }); + // (undocumented) + bail: () => this; + // (undocumented) + bailToMark: (id: string) => this; + // (undocumented) + batch: (fn: () => void, opts?: TLHistoryBatchOptions) => this; + // (undocumented) + clear(): void; + // @internal (undocumented) + debug(): { + pendingDiff: { + diff: RecordsDiff; + isEmpty: boolean; + }; + redos: (NonNullable> | undefined)[]; + state: HistoryRecorderState; + undos: (NonNullable> | undefined)[]; + }; + // (undocumented) + readonly dispose: () => void; + // (undocumented) + getNumRedos(): number; + // (undocumented) + getNumUndos(): number; + // (undocumented) + ignore(fn: () => void): this; + // @internal (undocumented) + _isInBatch: boolean; + // (undocumented) + mark: (id?: string) => string; + // (undocumented) + onBatchComplete: () => void; + // (undocumented) + redo: () => this | undefined; + // @internal (undocumented) + stacks: Atom< { + redos: Stack>; + undos: Stack>; + }, unknown>; + // (undocumented) + undo: () => this; +} + // @public (undocumented) export const HIT_TEST_MARGIN = 8; @@ -1723,6 +1777,17 @@ export class SideEffectManager; + afterCreate?: TLAfterCreateHandler; + afterDelete?: TLAfterDeleteHandler; + beforeChange?: TLBeforeChangeHandler; + beforeCreate?: TLBeforeCreateHandler; + beforeDelete?: TLBeforeDeleteHandler; + }; + }): () => void; registerAfterChangeHandler(typeName: T, handler: TLAfterChangeHandler): () => void; @@ -2037,29 +2102,6 @@ export type TLCollaboratorHintProps = { zoom: number; }; -// @public (undocumented) -export type TLCommand = { - preservesRedoStack?: boolean; - data: Data; - name: Name; - type: 'command'; -}; - -// @public (undocumented) -export type TLCommandHandler = { - squash?: (prevData: Data, nextData: Data) => Data; - do: (data: Data) => void; - redo?: (data: Data) => void; - undo: (data: Data) => void; -}; - -// @public (undocumented) -export type TLCommandHistoryOptions = Partial<{ - preservesRedoStack: boolean; - squashing: boolean; - ephemeral: boolean; -}>; - // @public (undocumented) export type TLCompleteEvent = (info: TLCompleteEventInfo) => void; @@ -2193,17 +2235,6 @@ export type TLEventInfo = TLCancelEventInfo | TLClickEventInfo | TLCompleteEvent // @public (undocumented) export interface TLEventMap { - // (undocumented) - 'change-history': [{ - markId?: string; - reason: 'bail'; - } | { - reason: 'push' | 'redo' | 'undo'; - }]; - // (undocumented) - 'mark-history': [{ - id: string; - }]; // (undocumented) 'max-shapes': [{ count: number; @@ -2316,17 +2347,6 @@ export type TLHandlesProps = { children: ReactNode; }; -// @public (undocumented) -export type TLHistoryEntry = TLCommand | TLHistoryMark; - -// @public (undocumented) -export type TLHistoryMark = { - id: string; - onRedo: boolean; - onUndo: boolean; - type: 'STOP'; -}; - // @public (undocumented) export type TLInterruptEvent = (info: TLInterruptEventInfo) => void; @@ -2610,6 +2630,7 @@ export type TLStoreEventInfo = HistoryEntry; // @public (undocumented) export type TLStoreOptions = { defaultName?: string; + id?: string; initialData?: SerializedStore; } & ({ migrations?: readonly MigrationSequence[]; diff --git a/packages/editor/src/index.ts b/packages/editor/src/index.ts index 4577b1395..d89708f3e 100644 --- a/packages/editor/src/index.ts +++ b/packages/editor/src/index.ts @@ -17,7 +17,6 @@ export { type Atom, type Signal, } from '@tldraw/state' -export type { TLCommandHistoryOptions } from './lib/editor/types/history-types' // eslint-disable-next-line local/no-export-star export * from '@tldraw/store' // eslint-disable-next-line local/no-export-star @@ -131,6 +130,7 @@ export { type TLEditorOptions, type TLResizeShapeOptions, } from './lib/editor/Editor' +export { HistoryManager } from './lib/editor/managers/HistoryManager' export type { SideEffectManager, TLAfterChangeHandler, @@ -235,12 +235,6 @@ export { type TLExternalContent, type TLExternalContentSource, } from './lib/editor/types/external-content' -export { - type TLCommand, - type TLCommandHandler, - type TLHistoryEntry, - type TLHistoryMark, -} from './lib/editor/types/history-types' export { type RequiredKeys, type TLSvgOptions } from './lib/editor/types/misc-types' export { type TLResizeHandle, type TLSelectionHandle } from './lib/editor/types/selection-types' export { ContainerProvider, useContainer } from './lib/hooks/useContainer' diff --git a/packages/editor/src/lib/TldrawEditor.tsx b/packages/editor/src/lib/TldrawEditor.tsx index 32e47764d..87ea9b1fe 100644 --- a/packages/editor/src/lib/TldrawEditor.tsx +++ b/packages/editor/src/lib/TldrawEditor.tsx @@ -380,8 +380,11 @@ function useOnMount(onMount?: TLOnMountHandler) { const editor = useEditor() const onMountEvent = useEvent((editor: Editor) => { - const teardown = onMount?.(editor) - editor.emit('mount') + let teardown: (() => void) | void = undefined + editor.history.ignore(() => { + teardown = onMount?.(editor) + editor.emit('mount') + }) window.tldrawReady = true return teardown }) diff --git a/packages/editor/src/lib/config/createTLStore.ts b/packages/editor/src/lib/config/createTLStore.ts index b910b6561..3361a18a5 100644 --- a/packages/editor/src/lib/config/createTLStore.ts +++ b/packages/editor/src/lib/config/createTLStore.ts @@ -14,6 +14,7 @@ import { TLAnyShapeUtilConstructor, checkShapesAndAddCore } from './defaultShape export type TLStoreOptions = { initialData?: SerializedStore defaultName?: string + id?: string } & ( | { shapeUtils?: readonly TLAnyShapeUtilConstructor[]; migrations?: readonly MigrationSequence[] } | { schema?: StoreSchema } @@ -28,7 +29,12 @@ export type TLStoreEventInfo = HistoryEntry * @param opts - Options for creating the store. * * @public */ -export function createTLStore({ initialData, defaultName = '', ...rest }: TLStoreOptions): TLStore { +export function createTLStore({ + initialData, + defaultName = '', + id, + ...rest +}: TLStoreOptions): TLStore { const schema = 'schema' in rest && rest.schema ? // we have a schema @@ -42,6 +48,7 @@ export function createTLStore({ initialData, defaultName = '', ...rest }: TLStor }) return new Store({ + id, schema, initialData, props: { diff --git a/packages/editor/src/lib/editor/Editor.ts b/packages/editor/src/lib/editor/Editor.ts index c74be045e..88e44208f 100644 --- a/packages/editor/src/lib/editor/Editor.ts +++ b/packages/editor/src/lib/editor/Editor.ts @@ -52,7 +52,6 @@ import { getIndicesBetween, getOwnProperty, hasOwnProperty, - objectMapValues, sortById, sortByIndex, structuredClone, @@ -129,7 +128,7 @@ import { TLWheelEventInfo, } from './types/event-types' import { TLExternalAssetContent, TLExternalContent } from './types/external-content' -import { TLCommandHistoryOptions } from './types/history-types' +import { TLHistoryBatchOptions } from './types/history-types' import { OptionalKeys, RequiredKeys, TLSvgOptions } from './types/misc-types' import { TLResizeHandle } from './types/selection-types' @@ -198,6 +197,13 @@ export class Editor extends EventEmitter { super() this.store = store + this.history = new HistoryManager({ + store, + annotateError: (error) => { + this.annotateError(error, { origin: 'history.batch', willCrashApp: true }) + this.crash(error) + }, + }) this.snaps = new SnapManager(this) @@ -421,168 +427,204 @@ export class Editor extends EventEmitter { this.sideEffects = new SideEffectManager(this) - this.sideEffects.registerBatchCompleteHandler(() => { - for (const parentId of invalidParents) { - invalidParents.delete(parentId) - const parent = this.getShape(parentId) - if (!parent) continue + this.disposables.add( + this.sideEffects.registerBatchCompleteHandler(() => { + for (const parentId of invalidParents) { + invalidParents.delete(parentId) + const parent = this.getShape(parentId) + if (!parent) continue - const util = this.getShapeUtil(parent) - const changes = util.onChildrenChange?.(parent) + const util = this.getShapeUtil(parent) + const changes = util.onChildrenChange?.(parent) - if (changes?.length) { - this.updateShapes(changes, { squashing: true }) + if (changes?.length) { + this.updateShapes(changes) + } } - } - this.emit('update') - }) + this.emit('update') + }) + ) - this.sideEffects.registerBeforeDeleteHandler('shape', (record) => { - // if the deleted shape has a parent shape make sure we call it's onChildrenChange callback - if (record.parentId && isShapeId(record.parentId)) { - invalidParents.add(record.parentId) - } - // clean up any arrows bound to this shape - const bindings = this._getArrowBindingsIndex().get()[record.id] - if (bindings?.length) { - for (const { arrowId, handleId } of bindings) { - const arrow = this.getShape(arrowId) - if (!arrow) continue - unbindArrowTerminal(arrow, handleId) - } - } - const deletedIds = new Set([record.id]) - const updates = compact( - this.getPageStates().map((pageState) => { - return cleanupInstancePageState(pageState, deletedIds) - }) - ) - - if (updates.length) { - this.store.put(updates) - } - }) - - this.sideEffects.registerBeforeDeleteHandler('page', (record) => { - // page was deleted, need to check whether it's the current page and select another one if so - if (this.getInstanceState().currentPageId !== record.id) return - - const backupPageId = this.getPages().find((p) => p.id !== record.id)?.id - if (!backupPageId) return - this.store.put([{ ...this.getInstanceState(), currentPageId: backupPageId }]) - - // delete the camera and state for the page if necessary - const cameraId = CameraRecordType.createId(record.id) - const instance_PageStateId = InstancePageStateRecordType.createId(record.id) - this.store.remove([cameraId, instance_PageStateId]) - }) - - this.sideEffects.registerAfterChangeHandler('shape', (prev, next) => { - if (this.isShapeOfType(next, 'arrow')) { - arrowDidUpdate(next) - } - - // if the shape's parent changed and it is bound to an arrow, update the arrow's parent - if (prev.parentId !== next.parentId) { - const reparentBoundArrows = (id: TLShapeId) => { - const boundArrows = this._getArrowBindingsIndex().get()[id] - if (boundArrows?.length) { - for (const arrow of boundArrows) { - reparentArrow(arrow.arrowId) + this.disposables.add( + this.sideEffects.register({ + shape: { + afterCreate: (record) => { + if (this.isShapeOfType(record, 'arrow')) { + arrowDidUpdate(record) } - } - } - reparentBoundArrows(next.id) - this.visitDescendants(next.id, reparentBoundArrows) - } - - // if this shape moved to a new page, clean up any previous page's instance state - if (prev.parentId !== next.parentId && isPageId(next.parentId)) { - const allMovingIds = new Set([prev.id]) - this.visitDescendants(prev.id, (id) => { - allMovingIds.add(id) - }) - - for (const instancePageState of this.getPageStates()) { - if (instancePageState.pageId === next.parentId) continue - const nextPageState = cleanupInstancePageState(instancePageState, allMovingIds) - - if (nextPageState) { - this.store.put([nextPageState]) - } - } - } - - if (prev.parentId && isShapeId(prev.parentId)) { - invalidParents.add(prev.parentId) - } - - if (next.parentId !== prev.parentId && isShapeId(next.parentId)) { - invalidParents.add(next.parentId) - } - }) - - this.sideEffects.registerAfterChangeHandler('instance_page_state', (prev, next) => { - if (prev?.selectedShapeIds !== next?.selectedShapeIds) { - // ensure that descendants and ancestors are not selected at the same time - const filtered = next.selectedShapeIds.filter((id) => { - let parentId = this.getShape(id)?.parentId - while (isShapeId(parentId)) { - if (next.selectedShapeIds.includes(parentId)) { - return false + }, + afterChange: (prev, next) => { + if (this.isShapeOfType(next, 'arrow')) { + arrowDidUpdate(next) } - parentId = this.getShape(parentId)?.parentId - } - return true - }) - let nextFocusedGroupId: null | TLShapeId = null + // if the shape's parent changed and it is bound to an arrow, update the arrow's parent + if (prev.parentId !== next.parentId) { + const reparentBoundArrows = (id: TLShapeId) => { + const boundArrows = this._getArrowBindingsIndex().get()[id] + if (boundArrows?.length) { + for (const arrow of boundArrows) { + reparentArrow(arrow.arrowId) + } + } + } + reparentBoundArrows(next.id) + this.visitDescendants(next.id, reparentBoundArrows) + } - if (filtered.length > 0) { - const commonGroupAncestor = this.findCommonAncestor( - compact(filtered.map((id) => this.getShape(id))), - (shape) => this.isShapeOfType(shape, 'group') - ) + // if this shape moved to a new page, clean up any previous page's instance state + if (prev.parentId !== next.parentId && isPageId(next.parentId)) { + const allMovingIds = new Set([prev.id]) + this.visitDescendants(prev.id, (id) => { + allMovingIds.add(id) + }) - if (commonGroupAncestor) { - nextFocusedGroupId = commonGroupAncestor - } - } else { - if (next?.focusedGroupId) { - nextFocusedGroupId = next.focusedGroupId - } - } + for (const instancePageState of this.getPageStates()) { + if (instancePageState.pageId === next.parentId) continue + const nextPageState = cleanupInstancePageState(instancePageState, allMovingIds) - if ( - filtered.length !== next.selectedShapeIds.length || - nextFocusedGroupId !== next.focusedGroupId - ) { - this.store.put([ - { ...next, selectedShapeIds: filtered, focusedGroupId: nextFocusedGroupId ?? null }, - ]) - } - } - }) + if (nextPageState) { + this.store.put([nextPageState]) + } + } + } - this.sideEffects.registerAfterCreateHandler('shape', (record) => { - if (this.isShapeOfType(record, 'arrow')) { - arrowDidUpdate(record) - } - }) + if (prev.parentId && isShapeId(prev.parentId)) { + invalidParents.add(prev.parentId) + } - this.sideEffects.registerAfterCreateHandler('page', (record) => { - const cameraId = CameraRecordType.createId(record.id) - const _pageStateId = InstancePageStateRecordType.createId(record.id) - if (!this.store.has(cameraId)) { - this.store.put([CameraRecordType.create({ id: cameraId })]) - } - if (!this.store.has(_pageStateId)) { - this.store.put([ - InstancePageStateRecordType.create({ id: _pageStateId, pageId: record.id }), - ]) - } - }) + if (next.parentId !== prev.parentId && isShapeId(next.parentId)) { + invalidParents.add(next.parentId) + } + }, + beforeDelete: (record) => { + // if the deleted shape has a parent shape make sure we call it's onChildrenChange callback + if (record.parentId && isShapeId(record.parentId)) { + invalidParents.add(record.parentId) + } + // clean up any arrows bound to this shape + const bindings = this._getArrowBindingsIndex().get()[record.id] + if (bindings?.length) { + for (const { arrowId, handleId } of bindings) { + const arrow = this.getShape(arrowId) + if (!arrow) continue + unbindArrowTerminal(arrow, handleId) + } + } + const deletedIds = new Set([record.id]) + const updates = compact( + this.getPageStates().map((pageState) => { + return cleanupInstancePageState(pageState, deletedIds) + }) + ) + + if (updates.length) { + this.store.put(updates) + } + }, + }, + page: { + afterCreate: (record) => { + const cameraId = CameraRecordType.createId(record.id) + const _pageStateId = InstancePageStateRecordType.createId(record.id) + if (!this.store.has(cameraId)) { + this.store.put([CameraRecordType.create({ id: cameraId })]) + } + if (!this.store.has(_pageStateId)) { + this.store.put([ + InstancePageStateRecordType.create({ id: _pageStateId, pageId: record.id }), + ]) + } + }, + afterDelete: (record, source) => { + // page was deleted, need to check whether it's the current page and select another one if so + if (this.getInstanceState()?.currentPageId === record.id) { + const backupPageId = this.getPages().find((p) => p.id !== record.id)?.id + if (backupPageId) { + this.store.put([{ ...this.getInstanceState(), currentPageId: backupPageId }]) + } else if (source === 'user') { + // fall back to ensureStoreIsUsable: + this.store.ensureStoreIsUsable() + } + } + + // delete the camera and state for the page if necessary + const cameraId = CameraRecordType.createId(record.id) + const instance_PageStateId = InstancePageStateRecordType.createId(record.id) + this.store.remove([cameraId, instance_PageStateId]) + }, + }, + instance: { + afterChange: (prev, next, source) => { + // instance should never be updated to a page that no longer exists (this can + // happen when undoing a change that involves switching to a page that has since + // been deleted by another user) + if (!this.store.has(next.currentPageId)) { + const backupPageId = this.store.has(prev.currentPageId) + ? prev.currentPageId + : this.getPages()[0]?.id + if (backupPageId) { + this.store.update(next.id, (instance) => ({ + ...instance, + currentPageId: backupPageId, + })) + } else if (source === 'user') { + // fall back to ensureStoreIsUsable: + this.store.ensureStoreIsUsable() + } + } + }, + }, + instance_page_state: { + afterChange: (prev, next) => { + if (prev?.selectedShapeIds !== next?.selectedShapeIds) { + // ensure that descendants and ancestors are not selected at the same time + const filtered = next.selectedShapeIds.filter((id) => { + let parentId = this.getShape(id)?.parentId + while (isShapeId(parentId)) { + if (next.selectedShapeIds.includes(parentId)) { + return false + } + parentId = this.getShape(parentId)?.parentId + } + return true + }) + + let nextFocusedGroupId: null | TLShapeId = null + + if (filtered.length > 0) { + const commonGroupAncestor = this.findCommonAncestor( + compact(filtered.map((id) => this.getShape(id))), + (shape) => this.isShapeOfType(shape, 'group') + ) + + if (commonGroupAncestor) { + nextFocusedGroupId = commonGroupAncestor + } + } else { + if (next?.focusedGroupId) { + nextFocusedGroupId = next.focusedGroupId + } + } + + if ( + filtered.length !== next.selectedShapeIds.length || + nextFocusedGroupId !== next.focusedGroupId + ) { + this.store.put([ + { + ...next, + selectedShapeIds: filtered, + focusedGroupId: nextFocusedGroupId ?? null, + }, + ]) + } + } + }, + }, + }) + ) this._currentPageShapeIds = deriveShapeIdsInCurrentPage(this.store, () => this.getCurrentPageId() @@ -594,18 +636,18 @@ export class Editor extends EventEmitter { this.emit('change', changes) }) ) + this.disposables.add(this.history.dispose) - this.store.ensureStoreIsUsable() + this.history.ignore(() => { + this.store.ensureStoreIsUsable() - // clear ephemeral state - this._setInstancePageState( - { + // clear ephemeral state + this._updateCurrentPageState({ editingShapeId: null, hoveredShapeId: null, erasingShapeIds: [], - }, - { ephemeral: true } - ) + }) + }) if (initialState && this.root.children[initialState] === undefined) { throw Error(`No state found for initialState "${initialState}".`) @@ -757,14 +799,7 @@ export class Editor extends EventEmitter { * * @readonly */ - readonly history = new HistoryManager( - this, - // () => this._complete(), - (error) => { - this.annotateError(error, { origin: 'history.batch', willCrashApp: true }) - this.crash(error) - } - ) + readonly history: HistoryManager /** * Undo to the last mark. @@ -827,13 +862,11 @@ export class Editor extends EventEmitter { * ``` * * @param markId - The mark's id, usually the reason for adding the mark. - * @param onUndo - Whether to stop at the mark when undoing. - * @param onRedo - Whether to stop at the mark when redoing. * * @public */ - mark(markId?: string, onUndo?: boolean, onRedo?: boolean): this { - this.history.mark(markId, onUndo, onRedo) + mark(markId?: string): this { + this.history.mark(markId) return this } @@ -872,8 +905,8 @@ export class Editor extends EventEmitter { * * @public */ - batch(fn: () => void): this { - this.history.batch(fn) + batch(fn: () => void, opts?: TLHistoryBatchOptions): this { + this.history.batch(fn, opts) return this } @@ -1155,7 +1188,9 @@ export class Editor extends EventEmitter { * @public **/ updateDocumentSettings(settings: Partial): this { - this.store.put([{ ...this.getDocumentSettings(), ...settings }]) + this.history.ignore(() => { + this.store.put([{ ...this.getDocumentSettings(), ...settings }]) + }) return this } @@ -1174,22 +1209,21 @@ export class Editor extends EventEmitter { * Update the instance's state. * * @param partial - A partial object to update the instance state with. - * @param historyOptions - The history options for the change. * * @public */ updateInstanceState( partial: Partial>, - historyOptions?: TLCommandHistoryOptions + historyOptions?: TLHistoryBatchOptions ): this { - this._updateInstanceState(partial, { ephemeral: true, squashing: true, ...historyOptions }) + this._updateInstanceState(partial, { history: 'ignore', ...historyOptions }) if (partial.isChangingStyle !== undefined) { clearTimeout(this._isChangingStyleTimeout) if (partial.isChangingStyle === true) { // If we've set to true, set a new reset timeout to change the value back to false after 2 seconds this._isChangingStyleTimeout = setTimeout(() => { - this.updateInstanceState({ isChangingStyle: false }, { ephemeral: true }) + this._updateInstanceState({ isChangingStyle: false }, { history: 'ignore' }) }, 2000) } } @@ -1198,34 +1232,19 @@ export class Editor extends EventEmitter { } /** @internal */ - private _updateInstanceState = this.history.createCommand( - 'updateInstanceState', - ( - partial: Partial>, - historyOptions?: TLCommandHistoryOptions - ) => { - const prev = this.store.get(this.getInstanceState().id)! - const next = { ...prev, ...partial } - - return { - data: { prev, next }, - ephemeral: false, - squashing: false, - ...historyOptions, - } - }, - { - do: ({ next }) => { - this.store.put([next]) - }, - undo: ({ prev }) => { - this.store.put([prev]) - }, - squash({ prev }, { next }) { - return { prev, next } - }, - } - ) + private _updateInstanceState = ( + partial: Partial>, + opts?: TLHistoryBatchOptions + ) => { + this.batch(() => { + this.store.put([ + { + ...this.getInstanceState(), + ...partial, + }, + ]) + }, opts) + } /** @internal */ private _isChangingStyleTimeout = -1 as any @@ -1327,10 +1346,7 @@ export class Editor extends EventEmitter { * @public */ setCursor = (cursor: Partial): this => { - this.updateInstanceState( - { cursor: { ...this.getInstanceState().cursor, ...cursor } }, - { ephemeral: true } - ) + this.updateInstanceState({ cursor: { ...this.getInstanceState().cursor, ...cursor } }) return this } @@ -1382,31 +1398,22 @@ export class Editor extends EventEmitter { partial: Partial< Omit >, - historyOptions?: TLCommandHistoryOptions + historyOptions?: TLHistoryBatchOptions ): this { - this._setInstancePageState(partial, historyOptions) + this._updateCurrentPageState(partial, historyOptions) return this } - - /** @internal */ - private _setInstancePageState = this.history.createCommand( - 'setInstancePageState', - ( - partial: Partial>, - historyOptions?: TLCommandHistoryOptions - ) => { - const prev = this.store.get(partial.id ?? this.getCurrentPageState().id)! - return { data: { prev, partial }, ...historyOptions } - }, - { - do: ({ prev, partial }) => { - this.store.update(prev.id, (state) => ({ ...state, ...partial })) - }, - undo: ({ prev }) => { - this.store.update(prev.id, () => prev) - }, - } - ) + _updateCurrentPageState = ( + partial: Partial>, + historyOptions?: TLHistoryBatchOptions + ) => { + this.batch(() => { + this.store.update(partial.id ?? this.getCurrentPageState().id, (state) => ({ + ...state, + ...partial, + })) + }, historyOptions) + } /** * The current selected ids. @@ -1438,54 +1445,35 @@ export class Editor extends EventEmitter { * ``` * * @param ids - The ids to select. - * @param historyOptions - The history options for the change. * * @public */ - setSelectedShapes( - shapes: TLShapeId[] | TLShape[], - historyOptions?: TLCommandHistoryOptions - ): this { - const ids = shapes.map((shape) => (typeof shape === 'string' ? shape : shape.id)) - this._setSelectedShapes(ids, historyOptions) - return this - } - - /** @internal */ - private _setSelectedShapes = this.history.createCommand( - 'setSelectedShapes', - (ids: TLShapeId[], historyOptions?: TLCommandHistoryOptions) => { + setSelectedShapes(shapes: TLShapeId[] | TLShape[]): this { + return this.batch(() => { + const ids = shapes.map((shape) => (typeof shape === 'string' ? shape : shape.id)) const { selectedShapeIds: prevSelectedShapeIds } = this.getCurrentPageState() const prevSet = new Set(prevSelectedShapeIds) if (ids.length === prevSet.size && ids.every((id) => prevSet.has(id))) return null - return { - data: { selectedShapeIds: ids, prevSelectedShapeIds }, - preservesRedoStack: true, - ...historyOptions, - } - }, - { - do: ({ selectedShapeIds }) => { - this.store.put([{ ...this.getCurrentPageState(), selectedShapeIds }]) - }, - undo: ({ prevSelectedShapeIds }) => { - this.store.put([ - { - ...this.getCurrentPageState(), - selectedShapeIds: prevSelectedShapeIds, - }, - ]) - }, - squash({ prevSelectedShapeIds }, { selectedShapeIds }) { - return { - selectedShapeIds, - prevSelectedShapeIds, - } - }, - } - ) + this.store.put([{ ...this.getCurrentPageState(), selectedShapeIds: ids }]) + }) + } + + /** + * Determine whether or not any of a shape's ancestors are selected. + * + * @param id - The id of the shape to check. + * + * @public + */ + isAncestorSelected(shape: TLShape | TLShapeId): boolean { + const id = typeof shape === 'string' ? shape : shape?.id ?? null + const _shape = this.getShape(id) + if (!_shape) return false + const selectedShapeIds = this.getSelectedShapeIds() + return !!this.findShapeAncestor(_shape, (parent) => selectedShapeIds.includes(parent.id)) + } /** * Select one or more shapes. @@ -1736,37 +1724,14 @@ export class Editor extends EventEmitter { } if (id === this.getFocusedGroupId()) return this - this._setFocusedGroupId(id) - return this - } - /** @internal */ - private _setFocusedGroupId = this.history.createCommand( - 'setFocusedGroupId', - (next: TLShapeId | null) => { - const prev = this.getCurrentPageState().focusedGroupId - if (prev === next) return - return { - data: { - prev, - next, - }, - preservesRedoStack: true, - squashing: true, - } - }, - { - do: ({ next }) => { - this.store.update(this.getCurrentPageState().id, (s) => ({ ...s, focusedGroupId: next })) + return this.batch( + () => { + this.store.update(this.getCurrentPageState().id, (s) => ({ ...s, focusedGroupId: id })) }, - undo: ({ prev }) => { - this.store.update(this.getCurrentPageState().id, (s) => ({ ...s, focusedGroupId: prev })) - }, - squash({ prev }, { next }) { - return { prev, next } - }, - } - ) + { history: 'record-preserveRedoStack' } + ) + } /** * Exit the current focused group, moving up to the next parent group if there is one. @@ -1831,13 +1796,13 @@ export class Editor extends EventEmitter { if (id) { const shape = this.getShape(id) if (shape && this.getShapeUtil(shape).canEdit(shape)) { - this._setInstancePageState({ editingShapeId: id }) + this._updateCurrentPageState({ editingShapeId: id }) return this } } // Either we just set the editing id to null, or the shape was missing or not editable - this._setInstancePageState({ editingShapeId: null }) + this._updateCurrentPageState({ editingShapeId: null }) } return this } @@ -1879,7 +1844,7 @@ export class Editor extends EventEmitter { setHoveredShape(shape: TLShapeId | TLShape | null): this { const id = typeof shape === 'string' ? shape : shape?.id ?? null if (id === this.getHoveredShapeId()) return this - this.updateCurrentPageState({ hoveredShapeId: id }, { ephemeral: true }) + this.updateCurrentPageState({ hoveredShapeId: id }) return this } @@ -1922,7 +1887,7 @@ export class Editor extends EventEmitter { ? (shapes as TLShapeId[]) : (shapes as TLShape[]).map((shape) => shape.id) // always ephemeral - this.updateCurrentPageState({ hintingShapeIds: dedupe(ids) }, { ephemeral: true }) + this.updateCurrentPageState({ hintingShapeIds: dedupe(ids) }, { history: 'ignore' }) return this } @@ -1967,20 +1932,22 @@ export class Editor extends EventEmitter { : (shapes as TLShape[]).map((shape) => shape.id) ids.sort() // sort the incoming ids const erasingShapeIds = this.getErasingShapeIds() - if (ids.length === erasingShapeIds.length) { - // if the new ids are the same length as the current ids, they might be the same. - // presuming the current ids are also sorted, check each item to see if it's the same; - // if we find any unequal, then we know the new ids are different. - for (let i = 0; i < ids.length; i++) { - if (ids[i] !== erasingShapeIds[i]) { - this._setInstancePageState({ erasingShapeIds: ids }, { ephemeral: true }) - break + this.history.ignore(() => { + if (ids.length === erasingShapeIds.length) { + // if the new ids are the same length as the current ids, they might be the same. + // presuming the current ids are also sorted, check each item to see if it's the same; + // if we find any unequal, then we know the new ids are different. + for (let i = 0; i < ids.length; i++) { + if (ids[i] !== erasingShapeIds[i]) { + this._updateCurrentPageState({ erasingShapeIds: ids }) + break + } } + } else { + // if the ids are a different length, then we know they're different. + this._updateCurrentPageState({ erasingShapeIds: ids }) } - } else { - // if the ids are a different length, then we know they're different. - this._setInstancePageState({ erasingShapeIds: ids }, { ephemeral: true }) - } + }) return this } @@ -2738,25 +2705,16 @@ export class Editor extends EventEmitter { if (_willSetInitialBounds) { // If we have just received the initial bounds, don't center the camera. this._willSetInitialBounds = false - this.updateInstanceState( - { screenBounds: screenBounds.toJson(), insets }, - { squashing: true, ephemeral: true } - ) + this.updateInstanceState({ screenBounds: screenBounds.toJson(), insets }) } else { if (center && !this.getInstanceState().followingUserId) { // Get the page center before the change, make the change, and restore it const before = this.getViewportPageCenter() - this.updateInstanceState( - { screenBounds: screenBounds.toJson(), insets }, - { squashing: true, ephemeral: true } - ) + this.updateInstanceState({ screenBounds: screenBounds.toJson(), insets }) this.centerOnPoint(before) } else { // Otherwise, - this.updateInstanceState( - { screenBounds: screenBounds.toJson(), insets }, - { squashing: true, ephemeral: true } - ) + this.updateInstanceState({ screenBounds: screenBounds.toJson(), insets }) } } } @@ -2943,7 +2901,7 @@ export class Editor extends EventEmitter { transact(() => { this.stopFollowingUser() - this.updateInstanceState({ followingUserId: userId }, { ephemeral: true }) + this.updateInstanceState({ followingUserId: userId }) }) const cancel = () => { @@ -3048,7 +3006,7 @@ export class Editor extends EventEmitter { * @public */ stopFollowingUser(): this { - this.updateInstanceState({ followingUserId: null }, { ephemeral: true }) + this.updateInstanceState({ followingUserId: null }) this.emit('stop-following') return this } @@ -3349,70 +3307,24 @@ export class Editor extends EventEmitter { * ``` * * @param page - The page (or page id) to set as the current page. - * @param historyOptions - The history options for the change. * * @public */ - setCurrentPage(page: TLPageId | TLPage, historyOptions?: TLCommandHistoryOptions): this { + setCurrentPage(page: TLPageId | TLPage): this { const pageId = typeof page === 'string' ? page : page.id - this._setCurrentPageId(pageId, historyOptions) - return this - } - /** @internal */ - private _setCurrentPageId = this.history.createCommand( - 'setCurrentPage', - (pageId: TLPageId, historyOptions?: TLCommandHistoryOptions) => { - if (!this.store.has(pageId)) { - console.error("Tried to set the current page id to a page that doesn't exist.") - return - } - this.stopFollowingUser() - - return { - data: { toId: pageId, fromId: this.getCurrentPageId() }, - squashing: true, - preservesRedoStack: true, - ...historyOptions, - } - }, - { - do: ({ toId }) => { - if (!this.store.has(toId)) { - // in multiplayer contexts this page might have been deleted - return - } - if (!this.getPageStates().find((p) => p.pageId === toId)) { - const camera = CameraRecordType.create({ - id: CameraRecordType.createId(toId), - }) - this.store.put([ - camera, - InstancePageStateRecordType.create({ - id: InstancePageStateRecordType.createId(toId), - pageId: toId, - }), - ]) - } - - this.store.put([{ ...this.getInstanceState(), currentPageId: toId }]) - - this.updateRenderingBounds() - }, - undo: ({ fromId }) => { - if (!this.store.has(fromId)) { - // in multiplayer contexts this page might have been deleted - return - } - this.store.put([{ ...this.getInstanceState(), currentPageId: fromId }]) - - this.updateRenderingBounds() - }, - squash: ({ fromId }, { toId }) => { - return { toId, fromId } - }, + if (!this.store.has(pageId)) { + console.error("Tried to set the current page id to a page that doesn't exist.") + return this } - ) + + this.stopFollowingUser() + + return this.batch( + () => this.store.put([{ ...this.getInstanceState(), currentPageId: pageId }]), + { history: 'record-preserveRedoStack' } + ) + } /** * Update a page. @@ -3420,45 +3332,20 @@ export class Editor extends EventEmitter { * @example * ```ts * editor.updatePage({ id: 'page2', name: 'Page 2' }) - * editor.updatePage({ id: 'page2', name: 'Page 2' }, { squashing: true }) * ``` * * @param partial - The partial of the shape to update. - * @param historyOptions - The history options for the change. * * @public */ - updatePage(partial: RequiredKeys, historyOptions?: TLCommandHistoryOptions): this { - this._updatePage(partial, historyOptions) - return this + updatePage(partial: RequiredKeys): this { + if (this.getInstanceState().isReadonly) return this + + const prev = this.getPage(partial.id) + if (!prev) return this + + return this.batch(() => this.store.update(partial.id, (page) => ({ ...page, ...partial }))) } - /** @internal */ - private _updatePage = this.history.createCommand( - 'updatePage', - (partial: RequiredKeys, historyOptions?: TLCommandHistoryOptions) => { - if (this.getInstanceState().isReadonly) return null - - const prev = this.getPage(partial.id) - - if (!prev) return null - - return { data: { prev, partial }, ...historyOptions } - }, - { - do: ({ partial }) => { - this.store.update(partial.id, (page) => ({ ...page, ...partial })) - }, - undo: ({ prev, partial }) => { - this.store.update(partial.id, () => prev) - }, - squash(prevData, nextData) { - return { - prev: { ...prevData.prev, ...nextData.prev }, - partial: nextData.partial, - } - }, - } - ) /** * Create a page. @@ -3474,15 +3361,9 @@ export class Editor extends EventEmitter { * @public */ createPage(page: Partial): this { - this._createPage(page) - return this - } - /** @internal */ - private _createPage = this.history.createCommand( - 'createPage', - (page: Partial) => { - if (this.getInstanceState().isReadonly) return null - if (this.getPages().length >= MAX_PAGES) return null + this.history.batch(() => { + if (this.getInstanceState().isReadonly) return + if (this.getPages().length >= MAX_PAGES) return const pages = this.getPages() const name = getIncrementedName( @@ -3503,33 +3384,10 @@ export class Editor extends EventEmitter { index, }) - const newCamera = CameraRecordType.create({ - id: CameraRecordType.createId(newPage.id), - }) - - const newTabPageState = InstancePageStateRecordType.create({ - id: InstancePageStateRecordType.createId(newPage.id), - pageId: newPage.id, - }) - - return { - data: { - newPage, - newTabPageState, - newCamera, - }, - } - }, - { - do: ({ newPage, newTabPageState, newCamera }) => { - this.store.put([newPage, newCamera, newTabPageState]) - }, - undo: ({ newPage, newTabPageState, newCamera }) => { - if (this.getPages().length === 1) return - this.store.remove([newTabPageState.id, newPage.id, newCamera.id]) - }, - } - ) + this.store.put([newPage]) + }) + return this + } /** * Delete a page. @@ -3545,21 +3403,13 @@ export class Editor extends EventEmitter { */ deletePage(page: TLPageId | TLPage): this { const id = typeof page === 'string' ? page : page.id - this._deletePage(id) - return this - } - /** @internal */ - private _deletePage = this.history.createCommand( - 'delete_page', - (id: TLPageId) => { - if (this.getInstanceState().isReadonly) return null + this.batch(() => { + if (this.getInstanceState().isReadonly) return const pages = this.getPages() - if (pages.length === 1) return null + if (pages.length === 1) return const deletedPage = this.getPage(id) - const deletedPageStates = this.getPageStates().filter((s) => s.pageId === id) - - if (!deletedPage) return null + if (!deletedPage) return if (id === this.getCurrentPageId()) { const index = pages.findIndex((page) => page.id === id) @@ -3567,30 +3417,11 @@ export class Editor extends EventEmitter { this.setCurrentPage(next.id) } - return { data: { id, deletedPage, deletedPageStates } } - }, - { - do: ({ deletedPage, deletedPageStates }) => { - const pages = this.getPages() - if (pages.length === 1) return - - if (deletedPage.id === this.getCurrentPageId()) { - const index = pages.findIndex((page) => page.id === deletedPage.id) - const next = pages[index - 1] ?? pages[index + 1] - this.setCurrentPage(next.id) - } - - this.store.remove(deletedPageStates.map((s) => s.id)) // remove the page state - this.store.remove([deletedPage.id]) // remove the page - this.updateRenderingBounds() - }, - undo: ({ deletedPage, deletedPageStates }) => { - this.store.put([deletedPage]) - this.store.put(deletedPageStates) - this.updateRenderingBounds() - }, - } - ) + this.store.remove([deletedPage.id]) + this.updateRenderingBounds() + }) + return this + } /** * Duplicate a page. @@ -3642,10 +3473,10 @@ export class Editor extends EventEmitter { * * @public */ - renamePage(page: TLPageId | TLPage, name: string, historyOptions?: TLCommandHistoryOptions) { + renamePage(page: TLPageId | TLPage, name: string) { const id = typeof page === 'string' ? page : page.id if (this.getInstanceState().isReadonly) return this - this.updatePage({ id, name }, historyOptions) + this.updatePage({ id, name }) return this } @@ -3678,28 +3509,10 @@ export class Editor extends EventEmitter { * @public */ createAssets(assets: TLAsset[]): this { - this._createAssets(assets) - return this + if (this.getInstanceState().isReadonly) return this + if (assets.length <= 0) return this + return this.batch(() => this.store.put(assets)) } - /** @internal */ - private _createAssets = this.history.createCommand( - 'createAssets', - (assets: TLAsset[]) => { - if (this.getInstanceState().isReadonly) return null - if (assets.length <= 0) return null - - return { data: { assets } } - }, - { - do: ({ assets }) => { - this.store.put(assets) - }, - undo: ({ assets }) => { - // todo: should we actually remove assets here? or on cleanup elsewhere? - this.store.remove(assets.map((a) => a.id)) - }, - } - ) /** * Update one or more assets. @@ -3714,39 +3527,17 @@ export class Editor extends EventEmitter { * @public */ updateAssets(assets: TLAssetPartial[]): this { - this._updateAssets(assets) - return this + if (this.getInstanceState().isReadonly) return this + if (assets.length <= 0) return this + return this.batch(() => { + this.store.put( + assets.map((partial) => ({ + ...this.store.get(partial.id)!, + ...partial, + })) + ) + }) } - /** @internal */ - private _updateAssets = this.history.createCommand( - 'updateAssets', - (assets: TLAssetPartial[]) => { - if (this.getInstanceState().isReadonly) return - if (assets.length <= 0) return - - const snapshots: Record = {} - - return { data: { snapshots, assets } } - }, - { - do: ({ assets, snapshots }) => { - this.store.put( - assets.map((a) => { - const asset = this.store.get(a.id)! - snapshots[a.id] = asset - - return { - ...asset, - ...a, - } - }) - ) - }, - undo: ({ snapshots }) => { - this.store.put(Object.values(snapshots)) - }, - } - ) /** * Delete one or more assets. @@ -3761,33 +3552,16 @@ export class Editor extends EventEmitter { * @public */ deleteAssets(assets: TLAssetId[] | TLAsset[]): this { + if (this.getInstanceState().isReadonly) return this + const ids = typeof assets[0] === 'string' ? (assets as TLAssetId[]) : (assets as TLAsset[]).map((a) => a.id) - this._deleteAssets(ids) - return this + if (ids.length <= 0) return this + + return this.batch(() => this.store.remove(ids)) } - /** @internal */ - private _deleteAssets = this.history.createCommand( - 'deleteAssets', - (ids: TLAssetId[]) => { - if (this.getInstanceState().isReadonly) return - if (ids.length <= 0) return - - const prev = compact(ids.map((id) => this.store.get(id))) - - return { data: { ids, prev } } - }, - { - do: ({ ids }) => { - this.store.remove(ids) - }, - undo: ({ prev }) => { - this.store.put(prev) - }, - } - ) /** * Get an asset by its id. @@ -5127,18 +4901,13 @@ export class Editor extends EventEmitter { * @example * ```ts * editor.nudgeShapes(['box1', 'box2'], { x: 8, y: 8 }) - * editor.nudgeShapes(editor.getSelectedShapes(), { x: 8, y: 8 }, { squashing: true }) * ``` * * @param shapes - The shapes (or shape ids) to move. * @param direction - The direction in which to move the shapes. * @param historyOptions - The history options for the change. */ - nudgeShapes( - shapes: TLShapeId[] | TLShape[], - offset: VecLike, - historyOptions?: TLCommandHistoryOptions - ): this { + nudgeShapes(shapes: TLShapeId[] | TLShape[], offset: VecLike): this { const ids = typeof shapes[0] === 'string' ? (shapes as TLShapeId[]) @@ -5156,10 +4925,7 @@ export class Editor extends EventEmitter { changes.push(this.getChangesToTranslateShape(shape, localDelta.add(shape))) } - this.updateShapes(changes, { - squashing: true, - ...historyOptions, - }) + this.updateShapes(changes) return this } @@ -6141,7 +5907,7 @@ export class Editor extends EventEmitter { if (parentTransform) localOffset.rot(-parentTransform.rotation()) const { x, y } = Vec.Add(localOffset, shape) - this.updateShapes([{ id: shape.id, type: shape.type, x, y }], { squashing: true }) + this.updateShapes([{ id: shape.id, type: shape.type, x, y }]) const scale = new Vec(1, commonBounds.height / pageBounds.height) this.resizeShape(shape.id, scale, { initialBounds: bounds, @@ -6164,7 +5930,7 @@ export class Editor extends EventEmitter { if (parentTransform) localOffset.rot(-parentTransform.rotation()) const { x, y } = Vec.Add(localOffset, shape) - this.updateShapes([{ id: shape.id, type: shape.type, x, y }], { squashing: true }) + this.updateShapes([{ id: shape.id, type: shape.type, x, y }]) const scale = new Vec(commonBounds.width / pageBounds.width, 1) this.resizeShape(shape.id, scale, { initialBounds: bounds, @@ -6277,30 +6043,27 @@ export class Editor extends EventEmitter { // need to adjust the shape's x and y points in case the parent has moved since start of resizing const { x, y } = this.getPointInParentSpace(initialShape.id, initialPagePoint) - this.updateShapes( - [ - { - id, - type: initialShape.type as any, - x: newLocalPoint.x, - y: newLocalPoint.y, - ...util.onResize( - { ...initialShape, x, y }, - { - newPoint: newLocalPoint, - handle: options.dragHandle ?? 'bottom_right', - // don't set isSingle to true for children - mode: options.mode ?? 'scale_shape', - scaleX: myScale.x, - scaleY: myScale.y, - initialBounds, - initialShape, - } - ), - }, - ], - { squashing: true } - ) + this.updateShapes([ + { + id, + type: initialShape.type as any, + x: newLocalPoint.x, + y: newLocalPoint.y, + ...util.onResize( + { ...initialShape, x, y }, + { + newPoint: newLocalPoint, + handle: options.dragHandle ?? 'bottom_right', + // don't set isSingle to true for children + mode: options.mode ?? 'scale_shape', + scaleX: myScale.x, + scaleY: myScale.y, + initialBounds, + initialShape, + } + ), + }, + ]) } else { const initialPageCenter = Mat.applyToPoint(pageTransform, initialBounds.center) // get the model changes from the shape util @@ -6319,17 +6082,14 @@ export class Editor extends EventEmitter { const delta = Vec.Sub(newPageCenterInParentSpace, initialPageCenterInParentSpace) // apply the changes to the model - this.updateShapes( - [ - { - id, - type: initialShape.type as any, - x: initialShape.x + delta.x, - y: initialShape.y + delta.y, - }, - ], - { squashing: true } - ) + this.updateShapes([ + { + id, + type: initialShape.type as any, + x: initialShape.x + delta.x, + y: initialShape.y + delta.y, + }, + ]) } return this @@ -6395,7 +6155,7 @@ export class Editor extends EventEmitter { if (Math.sign(scale.x) * Math.sign(scale.y) < 0) { let { rotation } = Mat.Decompose(options.initialPageTransform) rotation -= 2 * rotation - this.updateShapes([{ id, type, rotation }], { squashing: true }) + this.updateShapes([{ id, type, rotation }]) } // Next we need to translate the shape so that it's center point ends up in the right place. @@ -6425,7 +6185,7 @@ export class Editor extends EventEmitter { const postScaleShapePagePoint = Vec.Add(shapePageTransformOrigin, pageDelta) const { x, y } = this.getPointInParentSpace(id, postScaleShapePagePoint) - this.updateShapes([{ id, type, x, y }], { squashing: true }) + this.updateShapes([{ id, type, x, y }]) return this } @@ -6464,7 +6224,7 @@ export class Editor extends EventEmitter { * @public */ createShape(shape: OptionalKeys, 'id'>): this { - this._createShapes([shape]) + this.createShapes([shape]) return this } @@ -6482,204 +6242,184 @@ export class Editor extends EventEmitter { * * @public */ - createShapes(shapes: OptionalKeys, 'id'>[]) { + createShapes(shapes: OptionalKeys, 'id'>[]): this { if (!Array.isArray(shapes)) { throw Error('Editor.createShapes: must provide an array of shapes or shape partials') } - this._createShapes(shapes) - return this - } + if (this.getInstanceState().isReadonly) return this + if (shapes.length <= 0) return this - /** @internal */ - private _createShapes = this.history.createCommand( - 'createShapes', - (partials: OptionalKeys[]) => { - if (this.getInstanceState().isReadonly) return null - if (partials.length <= 0) return null + const currentPageShapeIds = this.getCurrentPageShapeIds() - const currentPageShapeIds = this.getCurrentPageShapeIds() + const maxShapesReached = shapes.length + currentPageShapeIds.size > MAX_SHAPES_PER_PAGE - const maxShapesReached = partials.length + currentPageShapeIds.size > MAX_SHAPES_PER_PAGE + if (maxShapesReached) { + // can't create more shapes than fit on the page + alertMaxShapes(this) + return this + } - if (maxShapesReached) { - // can't create more shapes than fit on the page - alertMaxShapes(this) - return - } + const focusedGroupId = this.getFocusedGroupId() - if (partials.length === 0) return null + return this.batch(() => { + // 1. Parents - return { - data: { - currentPageId: this.getCurrentPageId(), - partials: partials.map((p) => - p.id ? p : { ...p, id: createShapeId() } - ) as TLShapePartial[], - }, - } - }, - { - do: ({ partials }) => { - const focusedGroupId = this.getFocusedGroupId() + // Make sure that each partial will become the child of either the + // page or another shape that exists (or that will exist) in this page. - // 1. Parents + // find last parent id + const currentPageShapesSorted = this.getCurrentPageShapesSorted() - // Make sure that each partial will become the child of either the - // page or another shape that exists (or that will exist) in this page. - - // find last parent id - const currentPageShapesSorted = this.getCurrentPageShapesSorted() - - partials = partials.map((partial) => { - // If the partial does not provide the parentId OR if the provided - // parentId is NOT in the store AND NOT among the other shapes being - // created, then we need to find a parent for the shape. This can be - // another shape that exists under that point and which can receive - // children of the creating shape's type, or else the page itself. - if ( - !partial.parentId || - !(this.store.has(partial.parentId) || partials.some((p) => p.id === partial.parentId)) - ) { - let parentId: TLParentId = this.getFocusedGroupId() - - for (let i = currentPageShapesSorted.length - 1; i >= 0; i--) { - const parent = currentPageShapesSorted[i] - if ( - // parent.type === 'frame' - this.getShapeUtil(parent).canReceiveNewChildrenOfType(parent, partial.type) && - this.isPointInShape( - parent, - // If no parent is provided, then we can treat the - // shape's provided x/y as being in the page's space. - { x: partial.x ?? 0, y: partial.y ?? 0 }, - { - margin: 0, - hitInside: true, - } - ) - ) { - parentId = parent.id - break - } - } - - const prevParentId = partial.parentId - - // a shape cannot be it's own parent. This was a rare issue with frames/groups in the syncFuzz tests. - if (parentId === partial.id) { - parentId = focusedGroupId - } - - // If the parentid has changed... - if (parentId !== prevParentId) { - partial = { ...partial } - - partial.parentId = parentId - - // If the parent is a shape (rather than a page) then insert the - // shapes into the shape's children. Adjust the point and page rotation to be - // preserved relative to the parent. - if (isShapeId(parentId)) { - const point = this.getPointInShapeSpace(this.getShape(parentId)!, { - x: partial.x ?? 0, - y: partial.y ?? 0, - }) - partial.x = point.x - partial.y = point.y - partial.rotation = - -this.getShapePageTransform(parentId)!.rotation() + (partial.rotation ?? 0) - } - } - } - - return partial - }) - - // 2. Indices - - // Get the highest index among the parents of each of the - // the shapes being created; we'll increment from there. - - const parentIndices = new Map() - - const shapeRecordsToCreate: TLShape[] = [] - - for (const partial of partials) { - const util = this.getShapeUtil(partial) - - // If an index is not explicitly provided, then add the - // shapes to the top of their parents' children; using the - // value in parentsMappedToIndex, get the index above, use it, - // and set it back to parentsMappedToIndex for next time. - let index = partial.index - - if (!index) { - // Hello bug-seeker: have you just created a frame and then a shape - // and found that the shape is automatically the child of the frame? - // this is the reason why! It would be harder to have each shape specify - // the frame as the parent when creating a shape inside of a frame, so - // we do it here. - const parentId = partial.parentId ?? focusedGroupId - - if (!parentIndices.has(parentId)) { - parentIndices.set(parentId, this.getHighestIndexForParent(parentId)) - } - index = parentIndices.get(parentId)! - parentIndices.set(parentId, getIndexAbove(index)) - } - - // The initial props starts as the shape utility's default props - const initialProps = util.getDefaultProps() - - // We then look up each key in the tab state's styles; and if it's there, - // we use the value from the tab state's styles instead of the default. - for (const [style, propKey] of this.styleProps[partial.type]) { - ;(initialProps as any)[propKey] = this.getStyleForNextShape(style) - } - - // When we create the shape, take in the partial (the props coming into the - // function) and merge it with the default props. - let shapeRecordToCreate = ( - this.store.schema.types.shape as RecordType< - TLShape, - 'type' | 'props' | 'index' | 'parentId' - > - ).create({ - ...partial, - index, - opacity: partial.opacity ?? this.getInstanceState().opacityForNextShape, - parentId: partial.parentId ?? focusedGroupId, - props: 'props' in partial ? { ...initialProps, ...partial.props } : initialProps, - }) - - if (shapeRecordToCreate.index === undefined) { - throw Error('no index!') - } - - const next = this.getShapeUtil(shapeRecordToCreate).onBeforeCreate?.(shapeRecordToCreate) - - if (next) { - shapeRecordToCreate = next - } - - shapeRecordsToCreate.push(shapeRecordToCreate) + const partials = shapes.map((partial) => { + if (!partial.id) { + partial = { id: createShapeId(), ...partial } } - // Add meta properties, if any, to the shapes - shapeRecordsToCreate.forEach((shape) => { - shape.meta = { - ...this.getInitialMetaForShape(shape), - ...shape.meta, + // If the partial does not provide the parentId OR if the provided + // parentId is NOT in the store AND NOT among the other shapes being + // created, then we need to find a parent for the shape. This can be + // another shape that exists under that point and which can receive + // children of the creating shape's type, or else the page itself. + if ( + !partial.parentId || + !(this.store.has(partial.parentId) || shapes.some((p) => p.id === partial.parentId)) + ) { + let parentId: TLParentId = this.getFocusedGroupId() + + for (let i = currentPageShapesSorted.length - 1; i >= 0; i--) { + const parent = currentPageShapesSorted[i] + if ( + // parent.type === 'frame' + this.getShapeUtil(parent).canReceiveNewChildrenOfType(parent, partial.type) && + this.isPointInShape( + parent, + // If no parent is provided, then we can treat the + // shape's provided x/y as being in the page's space. + { x: partial.x ?? 0, y: partial.y ?? 0 }, + { + margin: 0, + hitInside: true, + } + ) + ) { + parentId = parent.id + break + } } + + const prevParentId = partial.parentId + + // a shape cannot be it's own parent. This was a rare issue with frames/groups in the syncFuzz tests. + if (parentId === partial.id) { + parentId = focusedGroupId + } + + // If the parentid has changed... + if (parentId !== prevParentId) { + partial = { ...partial } + + partial.parentId = parentId + + // If the parent is a shape (rather than a page) then insert the + // shapes into the shape's children. Adjust the point and page rotation to be + // preserved relative to the parent. + if (isShapeId(parentId)) { + const point = this.getPointInShapeSpace(this.getShape(parentId)!, { + x: partial.x ?? 0, + y: partial.y ?? 0, + }) + partial.x = point.x + partial.y = point.y + partial.rotation = + -this.getShapePageTransform(parentId)!.rotation() + (partial.rotation ?? 0) + } + } + } + + return partial + }) + + // 2. Indices + + // Get the highest index among the parents of each of the + // the shapes being created; we'll increment from there. + + const parentIndices = new Map() + + const shapeRecordsToCreate: TLShape[] = [] + + for (const partial of partials) { + const util = this.getShapeUtil(partial as TLShapePartial) + + // If an index is not explicitly provided, then add the + // shapes to the top of their parents' children; using the + // value in parentsMappedToIndex, get the index above, use it, + // and set it back to parentsMappedToIndex for next time. + let index = partial.index + + if (!index) { + // Hello bug-seeker: have you just created a frame and then a shape + // and found that the shape is automatically the child of the frame? + // this is the reason why! It would be harder to have each shape specify + // the frame as the parent when creating a shape inside of a frame, so + // we do it here. + const parentId = partial.parentId ?? focusedGroupId + + if (!parentIndices.has(parentId)) { + parentIndices.set(parentId, this.getHighestIndexForParent(parentId)) + } + index = parentIndices.get(parentId)! + parentIndices.set(parentId, getIndexAbove(index)) + } + + // The initial props starts as the shape utility's default props + const initialProps = util.getDefaultProps() + + // We then look up each key in the tab state's styles; and if it's there, + // we use the value from the tab state's styles instead of the default. + for (const [style, propKey] of this.styleProps[partial.type]) { + ;(initialProps as any)[propKey] = this.getStyleForNextShape(style) + } + + // When we create the shape, take in the partial (the props coming into the + // function) and merge it with the default props. + let shapeRecordToCreate = ( + this.store.schema.types.shape as RecordType< + TLShape, + 'type' | 'props' | 'index' | 'parentId' + > + ).create({ + ...partial, + index, + opacity: partial.opacity ?? this.getInstanceState().opacityForNextShape, + parentId: partial.parentId ?? focusedGroupId, + props: 'props' in partial ? { ...initialProps, ...partial.props } : initialProps, }) - this.store.put(shapeRecordsToCreate) - }, - undo: ({ partials }) => { - this.store.remove(partials.map((p) => p.id)) - }, - } - ) + if (shapeRecordToCreate.index === undefined) { + throw Error('no index!') + } + + const next = this.getShapeUtil(shapeRecordToCreate).onBeforeCreate?.(shapeRecordToCreate) + + if (next) { + shapeRecordToCreate = next + } + + shapeRecordsToCreate.push(shapeRecordToCreate) + } + + // Add meta properties, if any, to the shapes + shapeRecordsToCreate.forEach((shape) => { + shape.meta = { + ...this.getInitialMetaForShape(shape), + ...shape.meta, + } + }) + + this.store.put(shapeRecordsToCreate) + }) + } private animatingShapes = new Map() @@ -6771,7 +6511,7 @@ export class Editor extends EventEmitter { (p) => p && animatingShapes.get(p.id) === animationId ) if (partialsToUpdate.length) { - this.updateShapes(partialsToUpdate, { squashing: false }) + this.updateShapes(partialsToUpdate) // update shapes also removes the shape from animating shapes } @@ -6803,7 +6543,7 @@ export class Editor extends EventEmitter { }) } - this._updateShapes(updates, { squashing: true }) + this._updateShapes(updates) } this.addListener('tick', handleTick) @@ -6948,15 +6688,11 @@ export class Editor extends EventEmitter { * ``` * * @param partial - The shape partial to update. - * @param historyOptions - The history options for the change. * * @public */ - updateShape( - partial: TLShapePartial | null | undefined, - historyOptions?: TLCommandHistoryOptions - ) { - this.updateShapes([partial], historyOptions) + updateShape(partial: TLShapePartial | null | undefined) { + this.updateShapes([partial]) return this } @@ -6969,14 +6705,10 @@ export class Editor extends EventEmitter { * ``` * * @param partials - The shape partials to update. - * @param historyOptions - The history options for the change. * * @public */ - updateShapes( - partials: (TLShapePartial | null | undefined)[], - historyOptions?: TLCommandHistoryOptions - ) { + updateShapes(partials: (TLShapePartial | null | undefined)[]) { const compactedPartials: TLShapePartial[] = Array(partials.length) for (let i = 0, n = partials.length; i < n; i++) { @@ -6995,21 +6727,16 @@ export class Editor extends EventEmitter { compactedPartials.push(partial) } - this._updateShapes(compactedPartials, historyOptions) + this._updateShapes(compactedPartials) return this } /** @internal */ - private _updateShapes = this.history.createCommand( - 'updateShapes', - ( - _partials: (TLShapePartial | null | undefined)[], - historyOptions?: TLCommandHistoryOptions - ) => { - if (this.getInstanceState().isReadonly) return null + private _updateShapes = (_partials: (TLShapePartial | null | undefined)[]) => { + if (this.getInstanceState().isReadonly) return - const snapshots: Record = {} - const updates: Record = {} + this.batch(() => { + const updates = [] let shape: TLShape | undefined let updated: TLShape @@ -7029,42 +6756,17 @@ export class Editor extends EventEmitter { updated = applyPartialToShape(shape, partial) if (updated === shape) continue - snapshots[shape.id] = shape - updates[shape.id] = updated + //if any shape has an onBeforeUpdate handler, call it and, if the handler returns a + // new shape, replace the old shape with the new one. This is used for example when + // repositioning a text shape based on its new text content. + updated = this.getShapeUtil(shape).onBeforeUpdate?.(shape, updated) ?? updated + + updates.push(updated) } - return { data: { snapshots, updates }, ...historyOptions } - }, - { - do: ({ updates }) => { - // Iterate through array; if any shape has an onBeforeUpdate handler, call it - // and, if the handler returns a new shape, replace the old shape with - // the new one. This is used for example when repositioning a text shape - // based on its new text content. - this.store.put( - objectMapValues(updates).map((shape) => { - const current = this.store.get(shape.id) - if (current) { - const next = this.getShapeUtil(shape).onBeforeUpdate?.(current, shape) - if (next) return next - } - return shape - }) - ) - }, - undo: ({ snapshots }) => { - this.store.put(Object.values(snapshots)) - }, - squash(prevData, nextData) { - return { - // keep the oldest snapshots - snapshots: { ...nextData.snapshots, ...prevData.snapshots }, - // keep the newest updates - updates: { ...prevData.updates, ...nextData.updates }, - } - }, - } - ) + this.store.put(updates) + }) + } /** @internal */ private _getUnlockedShapeIds(ids: TLShapeId[]): TLShapeId[] { @@ -7085,16 +6787,28 @@ export class Editor extends EventEmitter { */ deleteShapes(ids: TLShapeId[]): this deleteShapes(shapes: TLShape[]): this - deleteShapes(_ids: TLShapeId[] | TLShape[]) { + deleteShapes(_ids: TLShapeId[] | TLShape[]): this { if (!Array.isArray(_ids)) { throw Error('Editor.deleteShapes: must provide an array of shapes or shapeIds') } - this._deleteShapes( - this._getUnlockedShapeIds( - typeof _ids[0] === 'string' ? (_ids as TLShapeId[]) : (_ids as TLShape[]).map((s) => s.id) - ) + + const ids = this._getUnlockedShapeIds( + typeof _ids[0] === 'string' ? (_ids as TLShapeId[]) : (_ids as TLShape[]).map((s) => s.id) ) - return this + + if (this.getInstanceState().isReadonly) return this + if (ids.length === 0) return this + + const allIds = new Set(ids) + + for (const id of ids) { + this.visitDescendants(id, (childId) => { + allIds.add(childId) + }) + } + + const deletedIds = [...allIds] + return this.batch(() => this.store.remove(deletedIds)) } /** @@ -7116,59 +6830,6 @@ export class Editor extends EventEmitter { return this } - /** @internal */ - private _deleteShapes = this.history.createCommand( - 'delete_shapes', - (ids: TLShapeId[]) => { - if (this.getInstanceState().isReadonly) return null - if (ids.length === 0) return null - const prevSelectedShapeIds = [...this.getCurrentPageState().selectedShapeIds] - - const allIds = new Set(ids) - - for (const id of ids) { - this.visitDescendants(id, (childId) => { - allIds.add(childId) - }) - } - - const deletedIds = [...allIds] - const arrowBindings = this._getArrowBindingsIndex().get() - const snapshots = compact( - deletedIds.flatMap((id) => { - const shape = this.getShape(id) - - // Add any bound arrows to the snapshots, so that we can restore the bindings on undo - const bindings = arrowBindings[id] - if (bindings && bindings.length > 0) { - return bindings.map(({ arrowId }) => this.getShape(arrowId)).concat(shape) - } - return shape - }) - ) - - const postSelectedShapeIds = prevSelectedShapeIds.filter((id) => !allIds.has(id)) - - return { data: { deletedIds, snapshots, prevSelectedShapeIds, postSelectedShapeIds } } - }, - { - do: ({ deletedIds, postSelectedShapeIds }) => { - this.store.remove(deletedIds) - this.store.update(this.getCurrentPageState().id, (state) => ({ - ...state, - selectedShapeIds: postSelectedShapeIds, - })) - }, - undo: ({ snapshots, prevSelectedShapeIds }) => { - this.store.put(snapshots) - this.store.update(this.getCurrentPageState().id, (state) => ({ - ...state, - selectedShapeIds: prevSelectedShapeIds, - })) - }, - } - ) - /* --------------------- Styles --------------------- */ /** @@ -7319,13 +6980,12 @@ export class Editor extends EventEmitter { * @example * ```ts * editor.setOpacityForNextShapes(0.5) - * editor.setOpacityForNextShapes(0.5, { squashing: true }) * ``` * * @param opacity - The opacity to set. Must be a number between 0 and 1 inclusive. * @param historyOptions - The history options for the change. */ - setOpacityForNextShapes(opacity: number, historyOptions?: TLCommandHistoryOptions): this { + setOpacityForNextShapes(opacity: number, historyOptions?: TLHistoryBatchOptions): this { this.updateInstanceState({ opacityForNextShape: opacity }, historyOptions) return this } @@ -7336,13 +6996,11 @@ export class Editor extends EventEmitter { * @example * ```ts * editor.setOpacityForSelectedShapes(0.5) - * editor.setOpacityForSelectedShapes(0.5, { squashing: true }) * ``` * * @param opacity - The opacity to set. Must be a number between 0 and 1 inclusive. - * @param historyOptions - The history options for the change. */ - setOpacityForSelectedShapes(opacity: number, historyOptions?: TLCommandHistoryOptions): this { + setOpacityForSelectedShapes(opacity: number): this { const selectedShapes = this.getSelectedShapes() if (selectedShapes.length > 0) { @@ -7372,8 +7030,7 @@ export class Editor extends EventEmitter { type: shape.type, opacity, } - }), - historyOptions + }) ) } @@ -7398,7 +7055,7 @@ export class Editor extends EventEmitter { setStyleForNextShapes( style: StyleProp, value: T, - historyOptions?: TLCommandHistoryOptions + historyOptions?: TLHistoryBatchOptions ): this { const stylesForNextShape = this.getInstanceState().stylesForNextShape @@ -7416,7 +7073,6 @@ export class Editor extends EventEmitter { * @example * ```ts * editor.setStyleForSelectedShapes(DefaultColorStyle, 'red') - * editor.setStyleForSelectedShapes(DefaultColorStyle, 'red', { ephemeral: true }) * ``` * * @param style - The style to set. @@ -7425,11 +7081,7 @@ export class Editor extends EventEmitter { * * @public */ - setStyleForSelectedShapes>( - style: S, - value: StylePropValue, - historyOptions?: TLCommandHistoryOptions - ): this { + setStyleForSelectedShapes>(style: S, value: StylePropValue): this { const selectedShapes = this.getSelectedShapes() if (selectedShapes.length > 0) { @@ -7469,10 +7121,7 @@ export class Editor extends EventEmitter { addShapeById(shape) } - this.updateShapes( - updates.map(({ updatePartial }) => updatePartial), - historyOptions - ) + this.updateShapes(updates.map(({ updatePartial }) => updatePartial)) } return this @@ -8212,22 +7861,24 @@ export class Editor extends EventEmitter { } // todo: We only have to do this if there are multiple users in the document - this.store.put([ - { - id: TLPOINTER_ID, - typeName: 'pointer', - x: currentPagePoint.x, - y: currentPagePoint.y, - lastActivityTimestamp: - // If our pointer moved only because we're following some other user, then don't - // update our last activity timestamp; otherwise, update it to the current timestamp. - info.type === 'pointer' && info.pointerId === INTERNAL_POINTER_IDS.CAMERA_MOVE - ? this.store.unsafeGetWithoutCapture(TLPOINTER_ID)?.lastActivityTimestamp ?? - this._tickManager.now - : this._tickManager.now, - meta: {}, - }, - ]) + this.history.ignore(() => { + this.store.put([ + { + id: TLPOINTER_ID, + typeName: 'pointer', + x: currentPagePoint.x, + y: currentPagePoint.y, + lastActivityTimestamp: + // If our pointer moved only because we're following some other user, then don't + // update our last activity timestamp; otherwise, update it to the current timestamp. + info.type === 'pointer' && info.pointerId === INTERNAL_POINTER_IDS.CAMERA_MOVE + ? this.store.unsafeGetWithoutCapture(TLPOINTER_ID)?.lastActivityTimestamp ?? + this._tickManager.now + : this._tickManager.now, + meta: {}, + }, + ]) + }) } /** @@ -8426,12 +8077,7 @@ export class Editor extends EventEmitter { if (this.inputs.isPanning) { this.inputs.isPanning = false - this.updateInstanceState({ - cursor: { - type: this._prevCursor, - rotation: 0, - }, - }) + this.setCursor({ type: this._prevCursor, rotation: 0 }) } } @@ -8529,14 +8175,14 @@ export class Editor extends EventEmitter { inputs.isPinching = false const { _selectedShapeIdsAtPointerDown } = this - this.setSelectedShapes(this._selectedShapeIdsAtPointerDown, { squashing: true }) + this.setSelectedShapes(this._selectedShapeIdsAtPointerDown) this._selectedShapeIdsAtPointerDown = [] if (this._didPinch) { this._didPinch = false this.once('tick', () => { if (!this._didPinch) { - this.setSelectedShapes(_selectedShapeIdsAtPointerDown, { squashing: true }) + this.setSelectedShapes(_selectedShapeIdsAtPointerDown) } }) } diff --git a/packages/editor/src/lib/editor/managers/HistoryManager.test.ts b/packages/editor/src/lib/editor/managers/HistoryManager.test.ts index 5dd0124df..12429bcf5 100644 --- a/packages/editor/src/lib/editor/managers/HistoryManager.test.ts +++ b/packages/editor/src/lib/editor/managers/HistoryManager.test.ts @@ -1,92 +1,75 @@ -import { TLCommandHistoryOptions } from '../types/history-types' +import { BaseRecord, RecordId, Store, StoreSchema, createRecordType } from '@tldraw/store' +import { TLHistoryBatchOptions } from '../types/history-types' import { HistoryManager } from './HistoryManager' import { stack } from './Stack' +interface TestRecord extends BaseRecord<'test', TestRecordId> { + value: number | string +} +type TestRecordId = RecordId +const testSchema = StoreSchema.create({ + test: createRecordType('test', { scope: 'document' }), +}) + +const ids = { + count: testSchema.types.test.createId('count'), + name: testSchema.types.test.createId('name'), + age: testSchema.types.test.createId('age'), + a: testSchema.types.test.createId('a'), + b: testSchema.types.test.createId('b'), +} + function createCounterHistoryManager() { - const manager = new HistoryManager({ emit: () => void null }, () => { - return - }) - const state = { - count: 0, - name: 'David', - age: 35, + const store = new Store({ schema: testSchema, props: null }) + store.put([ + testSchema.types.test.create({ id: ids.count, value: 0 }), + testSchema.types.test.create({ id: ids.name, value: 'David' }), + testSchema.types.test.create({ id: ids.age, value: 35 }), + ]) + + const manager = new HistoryManager({ store }) + + function getCount() { + return store.get(ids.count)!.value as number + } + function getName() { + return store.get(ids.name)!.value as string + } + function getAge() { + return store.get(ids.age)!.value as number + } + function _setCount(n: number) { + store.update(ids.count, (c) => ({ ...c, value: n })) + } + function _setName(name: string) { + store.update(ids.name, (c) => ({ ...c, value: name })) + } + function _setAge(age: number) { + store.update(ids.age, (c) => ({ ...c, value: age })) } - const increment = manager.createCommand( - 'increment', - (n = 1, squashing = false) => ({ - data: { n }, - squashing, - }), - { - do: ({ n }) => { - state.count += n - }, - undo: ({ n }) => { - state.count -= n - }, - squash: ({ n: n1 }, { n: n2 }) => ({ n: n1 + n2 }), - } - ) - const decrement = manager.createCommand( - 'decrement', - (n = 1, squashing = false) => ({ - data: { n }, - squashing, - }), - { - do: ({ n }) => { - state.count -= n - }, - undo: ({ n }) => { - state.count += n - }, - squash: ({ n: n1 }, { n: n2 }) => ({ n: n1 + n2 }), - } - ) + const increment = (n = 1) => { + _setCount(getCount() + n) + } - const setName = manager.createCommand( - 'setName', - (name = 'David') => ({ - data: { name, prev: state.name }, - ephemeral: true, - }), - { - do: ({ name }) => { - state.name = name - }, - undo: ({ prev }) => { - state.name = prev - }, - } - ) + const decrement = (n = 1) => { + _setCount(getCount() - n) + } - const setAge = manager.createCommand( - 'setAge', - (age = 35) => ({ - data: { age, prev: state.age }, - preservesRedoStack: true, - }), - { - do: ({ age }) => { - state.age = age - }, - undo: ({ prev }) => { - state.age = prev - }, - } - ) + const setName = (name = 'David') => { + manager.ignore(() => _setName(name)) + } - const incrementTwice = manager.createCommand('incrementTwice', () => ({ data: {} }), { - do: () => { + const setAge = (age = 35) => { + manager.batch(() => _setAge(age), { history: 'record-preserveRedoStack' }) + } + + const incrementTwice = () => { + manager.batch(() => { increment() increment() - }, - undo: () => { - decrement() - decrement() - }, - }) + }) + } return { increment, @@ -95,9 +78,9 @@ function createCounterHistoryManager() { setName, setAge, history: manager, - getCount: () => state.count, - getName: () => state.name, - getAge: () => state.age, + getCount, + getName, + getAge, } } @@ -116,9 +99,9 @@ describe(HistoryManager, () => { editor.decrement() expect(editor.getCount()).toBe(3) - const undos = [...editor.history._undos.get()] + const undos = [...editor.history.stacks.get().undos] const parsedUndos = JSON.parse(JSON.stringify(undos)) - editor.history._undos.set(stack(parsedUndos)) + editor.history.stacks.update(({ redos }) => ({ undos: stack(parsedUndos), redos })) editor.history.undo() @@ -200,17 +183,16 @@ describe(HistoryManager, () => { editor.history.mark('stop at 1') expect(editor.getCount()).toBe(1) - editor.increment(1, true) - editor.increment(1, true) - editor.increment(1, true) - editor.increment(1, true) + editor.increment(1) + editor.increment(1) + editor.increment(1) + editor.increment(1) expect(editor.getCount()).toBe(5) expect(editor.history.getNumUndos()).toBe(3) }) - - it('allows ephemeral commands that do not affect the stack', () => { + it('allows ignore commands that do not affect the stack', () => { editor.increment() editor.history.mark('stop at 1') editor.increment() @@ -263,7 +245,7 @@ describe(HistoryManager, () => { editor.history.mark('2') editor.incrementTwice() editor.incrementTwice() - expect(editor.history.getNumUndos()).toBe(5) + expect(editor.history.getNumUndos()).toBe(4) expect(editor.getCount()).toBe(6) editor.history.bail() expect(editor.getCount()).toBe(2) @@ -289,58 +271,35 @@ describe(HistoryManager, () => { }) describe('history options', () => { - let manager: HistoryManager - let state: { a: number; b: number } + let manager: HistoryManager - let setA: (n: number, historyOptions?: TLCommandHistoryOptions) => any - let setB: (n: number, historyOptions?: TLCommandHistoryOptions) => any + let getState: () => { a: number; b: number } + let setA: (n: number, historyOptions?: TLHistoryBatchOptions) => any + let setB: (n: number, historyOptions?: TLHistoryBatchOptions) => any beforeEach(() => { - manager = new HistoryManager({ emit: () => void null }, () => { - return - }) + const store = new Store({ schema: testSchema, props: null }) + store.put([ + testSchema.types.test.create({ id: ids.a, value: 0 }), + testSchema.types.test.create({ id: ids.b, value: 0 }), + ]) - state = { - a: 0, - b: 0, + manager = new HistoryManager({ store }) + + getState = () => { + return { a: store.get(ids.a)!.value as number, b: store.get(ids.b)!.value as number } } - setA = manager.createCommand( - 'setA', - (n: number, historyOptions?: TLCommandHistoryOptions) => ({ - data: { next: n, prev: state.a }, - ...historyOptions, - }), - { - do: ({ next }) => { - state = { ...state, a: next } - }, - undo: ({ prev }) => { - state = { ...state, a: prev } - }, - squash: ({ prev }, { next }) => ({ prev, next }), - } - ) + setA = (n: number, historyOptions?: TLHistoryBatchOptions) => { + manager.batch(() => store.update(ids.a, (s) => ({ ...s, value: n })), historyOptions) + } - setB = manager.createCommand( - 'setB', - (n: number, historyOptions?: TLCommandHistoryOptions) => ({ - data: { next: n, prev: state.b }, - ...historyOptions, - }), - { - do: ({ next }) => { - state = { ...state, b: next } - }, - undo: ({ prev }) => { - state = { ...state, b: prev } - }, - squash: ({ prev }, { next }) => ({ prev, next }), - } - ) + setB = (n: number, historyOptions?: TLHistoryBatchOptions) => { + manager.batch(() => store.update(ids.b, (s) => ({ ...s, value: n })), historyOptions) + } }) - it('sets, undoes, redoes', () => { + it('undos, redoes, separate marks', () => { manager.mark() setA(1) manager.mark() @@ -348,18 +307,18 @@ describe('history options', () => { manager.mark() setB(2) - expect(state).toMatchObject({ a: 1, b: 2 }) + expect(getState()).toMatchObject({ a: 1, b: 2 }) manager.undo() - expect(state).toMatchObject({ a: 1, b: 1 }) + expect(getState()).toMatchObject({ a: 1, b: 1 }) manager.redo() - expect(state).toMatchObject({ a: 1, b: 2 }) + expect(getState()).toMatchObject({ a: 1, b: 2 }) }) - it('sets, undoes, redoes', () => { + it('undos, redos, squashing', () => { manager.mark() setA(1) manager.mark() @@ -369,71 +328,107 @@ describe('history options', () => { setB(3) setB(4) - expect(state).toMatchObject({ a: 1, b: 4 }) + expect(getState()).toMatchObject({ a: 1, b: 4 }) manager.undo() - expect(state).toMatchObject({ a: 1, b: 1 }) + expect(getState()).toMatchObject({ a: 1, b: 1 }) manager.redo() - expect(state).toMatchObject({ a: 1, b: 4 }) + expect(getState()).toMatchObject({ a: 1, b: 4 }) }) - it('sets ephemeral, undoes, redos', () => { + it('undos, redos, ignore', () => { manager.mark() setA(1) manager.mark() setB(1) // B 0->1 manager.mark() - setB(2, { ephemeral: true }) // B 0->2, but ephemeral + setB(2, { history: 'ignore' }) // B 0->2, but ignore - expect(state).toMatchObject({ a: 1, b: 2 }) + expect(getState()).toMatchObject({ a: 1, b: 2 }) manager.undo() // undoes B 2->0 - expect(state).toMatchObject({ a: 1, b: 0 }) + expect(getState()).toMatchObject({ a: 1, b: 0 }) manager.redo() // redoes B 0->1, but not B 1-> 2 - expect(state).toMatchObject({ a: 1, b: 1 }) // no change, b 1->2 was ephemeral + expect(getState()).toMatchObject({ a: 1, b: 1 }) // no change, b 1->2 was ignore }) - it('sets squashing, undoes, redos', () => { + it('squashing, undos, redos', () => { manager.mark() setA(1) manager.mark() setB(1) - setB(2, { squashing: true }) // squashes with the previous command - setB(3, { squashing: true }) // squashes with the previous command + setB(2) // squashes with the previous command + setB(3) // squashes with the previous command - expect(state).toMatchObject({ a: 1, b: 3 }) + expect(getState()).toMatchObject({ a: 1, b: 3 }) manager.undo() - expect(state).toMatchObject({ a: 1, b: 0 }) + expect(getState()).toMatchObject({ a: 1, b: 0 }) manager.redo() - expect(state).toMatchObject({ a: 1, b: 3 }) + expect(getState()).toMatchObject({ a: 1, b: 3 }) }) - it('sets squashing and ephemeral, undoes, redos', () => { + it('squashing, undos, redos, ignore', () => { manager.mark() setA(1) manager.mark() setB(1) - setB(2, { squashing: true }) // squashes with the previous command - setB(3, { squashing: true, ephemeral: true }) // squashes with the previous command + setB(2) // squashes with the previous command + setB(3, { history: 'ignore' }) // squashes with the previous command - expect(state).toMatchObject({ a: 1, b: 3 }) + expect(getState()).toMatchObject({ a: 1, b: 3 }) manager.undo() - expect(state).toMatchObject({ a: 1, b: 0 }) + expect(getState()).toMatchObject({ a: 1, b: 0 }) manager.redo() - expect(state).toMatchObject({ a: 1, b: 2 }) // B2->3 was ephemeral + expect(getState()).toMatchObject({ a: 1, b: 2 }) // B2->3 was ignore + }) + + it('nested ignore', () => { + manager.mark() + manager.batch( + () => { + setA(1) + // even though we set this to record, it will still be ignored + manager.batch(() => setB(1), { history: 'record' }) + setA(2) + }, + { history: 'ignore' } + ) + expect(getState()).toMatchObject({ a: 2, b: 1 }) + + // changes were ignored: + manager.undo() + expect(getState()).toMatchObject({ a: 2, b: 1 }) + + manager.mark() + manager.batch( + () => { + setA(3) + manager.batch(() => setB(2), { history: 'ignore' }) + }, + { history: 'record-preserveRedoStack' } + ) + expect(getState()).toMatchObject({ a: 3, b: 2 }) + + // changes to A were recorded, but changes to B were ignore: + manager.undo() + expect(getState()).toMatchObject({ a: 2, b: 2 }) + + // We can still redo because we preserved the redo stack: + manager.redo() + expect(getState()).toMatchObject({ a: 3, b: 2 }) }) }) diff --git a/packages/editor/src/lib/editor/managers/HistoryManager.ts b/packages/editor/src/lib/editor/managers/HistoryManager.ts index 2b0511598..5a6a7bb29 100644 --- a/packages/editor/src/lib/editor/managers/HistoryManager.ts +++ b/packages/editor/src/lib/editor/managers/HistoryManager.ts @@ -1,156 +1,124 @@ import { atom, transact } from '@tldraw/state' -import { devFreeze } from '@tldraw/store' +import { + RecordsDiff, + Store, + UnknownRecord, + createEmptyRecordsDiff, + isRecordsDiffEmpty, + reverseRecordsDiff, + squashRecordDiffsMutable, +} from '@tldraw/store' +import { exhaustiveSwitchError, noop } from '@tldraw/utils' import { uniqueId } from '../../utils/uniqueId' -import { TLCommandHandler, TLCommandHistoryOptions, TLHistoryEntry } from '../types/history-types' -import { Stack, stack } from './Stack' +import { TLHistoryBatchOptions, TLHistoryEntry } from '../types/history-types' +import { stack } from './Stack' -type CommandFn = (...args: any[]) => - | ({ - data: Data - } & TLCommandHistoryOptions) - | null - | undefined - | void +enum HistoryRecorderState { + Recording = 'recording', + RecordingPreserveRedoStack = 'recordingPreserveRedoStack', + Paused = 'paused', +} -type ExtractData = Fn extends CommandFn ? Data : never -type ExtractArgs = Parameters any>> +/** @public */ +export class HistoryManager { + private readonly store: Store -export class HistoryManager< - CTX extends { - emit: (name: 'change-history' | 'mark-history', ...args: any) => void - }, -> { - _undos = atom>('HistoryManager.undos', stack()) // Updated by each action that includes and undo - _redos = atom>('HistoryManager.redos', stack()) // Updated when a user undoes - _batchDepth = 0 // A flag for whether the user is in a batch operation + readonly dispose: () => void - constructor( - private readonly ctx: CTX, - private readonly annotateError: (error: unknown) => void - ) {} + private state: HistoryRecorderState = HistoryRecorderState.Recording + private readonly pendingDiff = new PendingDiff() + /** @internal */ + stacks = atom( + 'HistoryManager.stacks', + { + undos: stack>(), + redos: stack>(), + }, + { + isEqual: (a, b) => a.undos === b.undos && a.redos === b.redos, + } + ) + + private readonly annotateError: (error: unknown) => void + + constructor(opts: { store: Store; annotateError?: (error: unknown) => void }) { + this.store = opts.store + this.annotateError = opts.annotateError ?? noop + this.dispose = this.store.addHistoryInterceptor((entry, source) => { + if (source !== 'user') return + + switch (this.state) { + case HistoryRecorderState.Recording: + this.pendingDiff.apply(entry.changes) + this.stacks.update(({ undos }) => ({ undos, redos: stack() })) + break + case HistoryRecorderState.RecordingPreserveRedoStack: + this.pendingDiff.apply(entry.changes) + break + case HistoryRecorderState.Paused: + break + default: + exhaustiveSwitchError(this.state) + } + }) + } + + private flushPendingDiff() { + if (this.pendingDiff.isEmpty()) return + + const diff = this.pendingDiff.clear() + this.stacks.update(({ undos, redos }) => ({ + undos: undos.push({ type: 'diff', diff }), + redos, + })) + } onBatchComplete: () => void = () => void null - private _commands: Record> = {} - getNumUndos() { - return this._undos.get().length + return this.stacks.get().undos.length + (this.pendingDiff.isEmpty() ? 0 : 1) } getNumRedos() { - return this._redos.get().length - } - createCommand = >( - name: Name, - constructor: Constructor, - handle: TLCommandHandler> - ) => { - if (this._commands[name]) { - throw new Error(`Duplicate command: ${name}`) - } - this._commands[name] = handle - - const exec = (...args: ExtractArgs) => { - if (!this._batchDepth) { - // If we're not batching, run again in a batch - this.batch(() => exec(...args)) - return this.ctx - } - - const result = constructor(...args) - - if (!result) { - return this.ctx - } - - const { data, ephemeral, squashing, preservesRedoStack } = result - - this.ignoringUpdates((undos, redos) => { - handle.do(data) - return { undos, redos } - }) - - if (!ephemeral) { - const prev = this._undos.get().head - if ( - squashing && - prev && - prev.type === 'command' && - prev.name === name && - prev.preservesRedoStack === preservesRedoStack - ) { - // replace the last command with a squashed version - this._undos.update((undos) => - undos.tail.push({ - ...prev, - data: devFreeze(handle.squash!(prev.data, data)), - }) - ) - } else { - // add to the undo stack - this._undos.update((undos) => - undos.push({ - type: 'command', - name, - data: devFreeze(data), - preservesRedoStack: preservesRedoStack, - }) - ) - } - - if (!result.preservesRedoStack) { - this._redos.set(stack()) - } - - this.ctx.emit('change-history', { reason: 'push' }) - } - - return this.ctx - } - - return exec + return this.stacks.get().redos.length } - batch = (fn: () => void) => { + /** @internal */ + _isInBatch = false + batch = (fn: () => void, opts?: TLHistoryBatchOptions) => { + const previousState = this.state + + // we move to the new state only if we haven't explicitly paused + if (previousState !== HistoryRecorderState.Paused && opts?.history) { + this.state = modeToState[opts.history] + } + try { - this._batchDepth++ - if (this._batchDepth === 1) { - transact(() => { - const mostRecentAction = this._undos.get().head - fn() - if (mostRecentAction !== this._undos.get().head) { - this.onBatchComplete() - } - }) - } else { + if (this._isInBatch) { fn() + return this } - } catch (error) { - this.annotateError(error) - throw error - } finally { - this._batchDepth-- - } - return this + this._isInBatch = true + try { + transact(() => { + fn() + this.onBatchComplete() + }) + } catch (error) { + this.annotateError(error) + throw error + } finally { + this._isInBatch = false + } + + return this + } finally { + this.state = previousState + } } - private ignoringUpdates = ( - fn: ( - undos: Stack, - redos: Stack - ) => { undos: Stack; redos: Stack } - ) => { - let undos = this._undos.get() - let redos = this._redos.get() - - this._undos.set(stack()) - this._redos.set(stack()) - try { - ;({ undos, redos } = transact(() => fn(undos, redos))) - } finally { - this._undos.set(undos) - this._redos.set(redos) - } + ignore(fn: () => void) { + return this.batch(fn, { history: 'ignore' }) } // History @@ -161,62 +129,66 @@ export class HistoryManager< pushToRedoStack: boolean toMark?: string }) => { - this.ignoringUpdates((undos, redos) => { - if (undos.length === 0) { - return { undos, redos } + const previousState = this.state + this.state = HistoryRecorderState.Paused + try { + let { undos, redos } = this.stacks.get() + + // start by collecting the pending diff (everything since the last mark). + // we'll accumulate the diff to undo in this variable so we can apply it atomically. + const pendingDiff = this.pendingDiff.clear() + const isPendingDiffEmpty = isRecordsDiffEmpty(pendingDiff) + const diffToUndo = reverseRecordsDiff(pendingDiff) + + if (pushToRedoStack && !isPendingDiffEmpty) { + redos = redos.push({ type: 'diff', diff: pendingDiff }) } - while (undos.head?.type === 'STOP') { - const mark = undos.head - undos = undos.tail - if (pushToRedoStack) { - redos = redos.push(mark) - } - if (mark.id === toMark) { - this.ctx.emit( - 'change-history', - pushToRedoStack ? { reason: 'undo' } : { reason: 'bail', markId: toMark } - ) - return { undos, redos } - } - } - - if (undos.length === 0) { - this.ctx.emit( - 'change-history', - pushToRedoStack ? { reason: 'undo' } : { reason: 'bail', markId: toMark } - ) - return { undos, redos } - } - - while (undos.head) { - const command = undos.head - undos = undos.tail - - if (pushToRedoStack) { - redos = redos.push(command) - } - - if (command.type === 'STOP') { - if (command.onUndo && (!toMark || command.id === toMark)) { - this.ctx.emit( - 'change-history', - pushToRedoStack ? { reason: 'undo' } : { reason: 'bail', markId: toMark } - ) - return { undos, redos } + let didFindMark = false + if (isPendingDiffEmpty) { + // if nothing has happened since the last mark, pop any intermediate marks off the stack + while (undos.head?.type === 'stop') { + const mark = undos.head + undos = undos.tail + if (pushToRedoStack) { + redos = redos.push(mark) + } + if (mark.id === toMark) { + didFindMark = true + break } - } else { - const handler = this._commands[command.name] - handler.undo(command.data) } } - this.ctx.emit( - 'change-history', - pushToRedoStack ? { reason: 'undo' } : { reason: 'bail', markId: toMark } - ) - return { undos, redos } - }) + if (!didFindMark) { + loop: while (undos.head) { + const undo = undos.head + undos = undos.tail + + if (pushToRedoStack) { + redos = redos.push(undo) + } + + switch (undo.type) { + case 'diff': + squashRecordDiffsMutable(diffToUndo, [reverseRecordsDiff(undo.diff)]) + break + case 'stop': + if (!toMark) break loop + if (undo.id === toMark) break loop + break + default: + exhaustiveSwitchError(undo) + } + } + } + + this.store.applyDiff(diffToUndo, { ignoreEphemeralKeys: true }) + this.store.ensureStoreIsUsable() + this.stacks.set({ undos, redos }) + } finally { + this.state = previousState + } return this } @@ -228,43 +200,43 @@ export class HistoryManager< } redo = () => { - this.ignoringUpdates((undos, redos) => { + const previousState = this.state + this.state = HistoryRecorderState.Paused + try { + this.flushPendingDiff() + + let { undos, redos } = this.stacks.get() if (redos.length === 0) { - return { undos, redos } + return } - while (redos.head?.type === 'STOP') { + // ignore any intermediate marks - this should take us to the first `diff` entry + while (redos.head?.type === 'stop') { undos = undos.push(redos.head) redos = redos.tail } - if (redos.length === 0) { - this.ctx.emit('change-history', { reason: 'redo' }) - return { undos, redos } - } + // accumulate diffs to be redone so they can be applied atomically + const diffToRedo = createEmptyRecordsDiff() while (redos.head) { - const command = redos.head - undos = undos.push(redos.head) + const redo = redos.head + undos = undos.push(redo) redos = redos.tail - if (command.type === 'STOP') { - if (command.onRedo) { - break - } + if (redo.type === 'diff') { + squashRecordDiffsMutable(diffToRedo, [redo.diff]) } else { - const handler = this._commands[command.name] - if (handler.redo) { - handler.redo(command.data) - } else { - handler.do(command.data) - } + break } } - this.ctx.emit('change-history', { reason: 'redo' }) - return { undos, redos } - }) + this.store.applyDiff(diffToRedo, { ignoreEphemeralKeys: true }) + this.store.ensureStoreIsUsable() + this.stacks.set({ undos, redos }) + } finally { + this.state = previousState + } return this } @@ -281,24 +253,59 @@ export class HistoryManager< return this } - mark = (id = uniqueId(), onUndo = true, onRedo = true) => { - const mostRecent = this._undos.get().head - // dedupe marks, why not - if (mostRecent && mostRecent.type === 'STOP') { - if (mostRecent.id === id && mostRecent.onUndo === onUndo && mostRecent.onRedo === onRedo) { - return mostRecent.id - } - } - - this._undos.update((undos) => undos.push({ type: 'STOP', id, onUndo, onRedo })) - - this.ctx.emit('mark-history', { id }) + mark = (id = uniqueId()) => { + transact(() => { + this.flushPendingDiff() + this.stacks.update(({ undos, redos }) => ({ undos: undos.push({ type: 'stop', id }), redos })) + }) return id } clear() { - this._undos.set(stack()) - this._redos.set(stack()) + this.stacks.set({ undos: stack(), redos: stack() }) + this.pendingDiff.clear() + } + + /** @internal */ + debug() { + const { undos, redos } = this.stacks.get() + return { + undos: undos.toArray(), + redos: redos.toArray(), + pendingDiff: this.pendingDiff.debug(), + state: this.state, + } + } +} + +const modeToState = { + record: HistoryRecorderState.Recording, + 'record-preserveRedoStack': HistoryRecorderState.RecordingPreserveRedoStack, + ignore: HistoryRecorderState.Paused, +} as const + +class PendingDiff { + private diff = createEmptyRecordsDiff() + private isEmptyAtom = atom('PendingDiff.isEmpty', true) + + clear() { + const diff = this.diff + this.diff = createEmptyRecordsDiff() + this.isEmptyAtom.set(true) + return diff + } + + isEmpty() { + return this.isEmptyAtom.get() + } + + apply(diff: RecordsDiff) { + squashRecordDiffsMutable(this.diff, [diff]) + this.isEmptyAtom.set(isRecordsDiffEmpty(this.diff)) + } + + debug() { + return { diff: this.diff, isEmpty: this.isEmpty() } } } diff --git a/packages/editor/src/lib/editor/managers/SideEffectManager.ts b/packages/editor/src/lib/editor/managers/SideEffectManager.ts index c7f569eec..51d742bf9 100644 --- a/packages/editor/src/lib/editor/managers/SideEffectManager.ts +++ b/packages/editor/src/lib/editor/managers/SideEffectManager.ts @@ -88,25 +88,13 @@ export class SideEffectManager< return next } - let updateDepth = 0 - editor.store.onAfterChange = (prev, next, source) => { - updateDepth++ - - if (updateDepth > 1000) { - console.error('[CleanupManager.onAfterChange] Maximum update depth exceeded, bailing out.') - } else { - const handlers = this._afterChangeHandlers[ - next.typeName - ] as TLAfterChangeHandler[] - if (handlers) { - for (const handler of handlers) { - handler(prev, next, source) - } + const handlers = this._afterChangeHandlers[next.typeName] as TLAfterChangeHandler[] + if (handlers) { + for (const handler of handlers) { + handler(prev, next, source) } } - - updateDepth-- } editor.store.onBeforeDelete = (record, source) => { @@ -161,6 +149,46 @@ export class SideEffectManager< private _batchCompleteHandlers: TLBatchCompleteHandler[] = [] + /** + * Internal helper for registering a bunch of side effects at once and keeping them organized. + * @internal + */ + register(handlersByType: { + [R in TLRecord as R['typeName']]?: { + beforeCreate?: TLBeforeCreateHandler + afterCreate?: TLAfterCreateHandler + beforeChange?: TLBeforeChangeHandler + afterChange?: TLAfterChangeHandler + beforeDelete?: TLBeforeDeleteHandler + afterDelete?: TLAfterDeleteHandler + } + }) { + const disposes: (() => void)[] = [] + for (const [type, handlers] of Object.entries(handlersByType) as any) { + if (handlers?.beforeCreate) { + disposes.push(this.registerBeforeCreateHandler(type, handlers.beforeCreate)) + } + if (handlers?.afterCreate) { + disposes.push(this.registerAfterCreateHandler(type, handlers.afterCreate)) + } + if (handlers?.beforeChange) { + disposes.push(this.registerBeforeChangeHandler(type, handlers.beforeChange)) + } + if (handlers?.afterChange) { + disposes.push(this.registerAfterChangeHandler(type, handlers.afterChange)) + } + if (handlers?.beforeDelete) { + disposes.push(this.registerBeforeDeleteHandler(type, handlers.beforeDelete)) + } + if (handlers?.afterDelete) { + disposes.push(this.registerAfterDeleteHandler(type, handlers.afterDelete)) + } + } + return () => { + for (const dispose of disposes) dispose() + } + } + /** * Register a handler to be called before a record of a certain type is created. Return a * modified record from the handler to change the record that will be created. diff --git a/packages/editor/src/lib/editor/types/emit-types.ts b/packages/editor/src/lib/editor/types/emit-types.ts index 6a6185d0a..3104f51e9 100644 --- a/packages/editor/src/lib/editor/types/emit-types.ts +++ b/packages/editor/src/lib/editor/types/emit-types.ts @@ -15,8 +15,6 @@ export interface TLEventMap { event: [TLEventInfo] tick: [number] frame: [number] - 'change-history': [{ reason: 'undo' | 'redo' | 'push' } | { reason: 'bail'; markId?: string }] - 'mark-history': [{ id: string }] 'select-all-text': [{ shapeId: TLShapeId }] } diff --git a/packages/editor/src/lib/editor/types/history-types.ts b/packages/editor/src/lib/editor/types/history-types.ts index 6adfa2de7..1df29aaaa 100644 --- a/packages/editor/src/lib/editor/types/history-types.ts +++ b/packages/editor/src/lib/editor/types/history-types.ts @@ -1,50 +1,27 @@ -/** @public */ -export type TLCommandHistoryOptions = Partial<{ - /** - * When true, this command will be squashed with the previous command in the undo / redo stack. - */ - squashing: boolean - /** - * When true, this command will not add anything to the undo / redo stack. Its change will never be undone or redone. - */ - ephemeral: boolean - /** - * When true, adding this this command will not clear out the redo stack. - */ - preservesRedoStack: boolean -}> +import { RecordsDiff, UnknownRecord } from '@tldraw/store' /** @public */ -export type TLHistoryMark = { - type: 'STOP' +export interface TLHistoryMark { + type: 'stop' id: string - onUndo: boolean - onRedo: boolean } /** @public */ -export type TLCommand = { - type: 'command' - data: Data - name: Name +export interface TLHistoryDiff { + type: 'diff' + diff: RecordsDiff +} + +/** @public */ +export type TLHistoryEntry = TLHistoryMark | TLHistoryDiff + +/** @public */ +export interface TLHistoryBatchOptions { /** - * Allows for commands that change state and should be undoable, but are 'inconsequential' and - * should not clear the redo stack. e.g. modifying the set of selected ids. + * How should this change interact with the history stack? + * - record: Add to the undo stack and clear the redo stack + * - record-preserveRedoStack: Add to the undo stack but do not clear the redo stack + * - ignore: Do not add to the undo stack or the redo stack */ - preservesRedoStack?: boolean -} - -/** @public */ -export type TLHistoryEntry = TLHistoryMark | TLCommand - -/** @public */ -export type TLCommandHandler = { - do: (data: Data) => void - undo: (data: Data) => void - redo?: (data: Data) => void - /** - * Allow to combine the next command with the previous one if possible. Useful for, e.g. combining - * a series of shape translation commands into one command in the undo stack - */ - squash?: (prevData: Data, nextData: Data) => Data + history?: 'record' | 'record-preserveRedoStack' | 'ignore' } diff --git a/packages/store/api-report.md b/packages/store/api-report.md index 3d125a866..cd2ba6928 100644 --- a/packages/store/api-report.md +++ b/packages/store/api-report.md @@ -33,6 +33,9 @@ export type ComputedCache = { get(id: IdOf): Data | undefined; }; +// @internal (undocumented) +export function createEmptyRecordsDiff(): RecordsDiff; + // @public export function createMigrationIds>(sequenceId: ID, versions: Versions): { [K in keyof Versions]: `${ID}/${Versions[K]}`; @@ -58,6 +61,9 @@ export function createRecordMigrationSequence(opts: { // @public export function createRecordType(typeName: R['typeName'], config: { + ephemeralKeys?: { + readonly [K in Exclude]: boolean; + }; scope: RecordScope; validator?: StoreValidator; }): RecordType>; @@ -98,6 +104,9 @@ export class IncrementalSetConstructor { remove(item: T): void; } +// @internal +export function isRecordsDiffEmpty(diff: RecordsDiff): boolean; + // @public (undocumented) export type LegacyMigration = { down: (newState: After) => Before; @@ -187,6 +196,9 @@ export class RecordType Exclude, RequiredProperties>; + readonly ephemeralKeys?: { + readonly [K in Exclude]: boolean; + }; readonly scope?: RecordScope; readonly validator?: StoreValidator; }); @@ -197,6 +209,12 @@ export class RecordType Exclude, RequiredProperties>; createId(customUniquePart?: string): IdOf; + // (undocumented) + readonly ephemeralKeys?: { + readonly [K in Exclude]: boolean; + }; + // (undocumented) + readonly ephemeralKeySet: ReadonlySet; isId(id?: string): id is IdOf; isInstance: (record?: UnknownRecord) => record is R; parseId(id: IdOf): string; @@ -244,22 +262,32 @@ export type SerializedStore = Record, R>; // @public export function squashRecordDiffs(diffs: RecordsDiff[]): RecordsDiff; +// @internal +export function squashRecordDiffsMutable(target: RecordsDiff, diffs: RecordsDiff[]): void; + // @public export class Store { constructor(config: { schema: StoreSchema; initialData?: SerializedStore; + id?: string; props: Props; }); + // @internal (undocumented) + addHistoryInterceptor(fn: (entry: HistoryEntry, source: ChangeSource) => void): () => void; allRecords: () => R[]; // (undocumented) - applyDiff(diff: RecordsDiff, runCallbacks?: boolean): void; + applyDiff(diff: RecordsDiff, { runCallbacks, ignoreEphemeralKeys, }?: { + ignoreEphemeralKeys?: boolean; + runCallbacks?: boolean; + }): void; + // @internal (undocumented) + atomic(fn: () => T, runCallbacks?: boolean): T; clear: () => void; createComputedCache: (name: string, derive: (record: V) => T | undefined, isEqual?: ((a: V, b: V) => boolean) | undefined) => ComputedCache; createSelectedComputedCache: (name: string, selector: (record: V) => T | undefined, derive: (input: T) => J | undefined) => ComputedCache; // @internal (undocumented) ensureStoreIsUsable(): void; - // (undocumented) extractingChanges(fn: () => void): RecordsDiff; filterChangesByScope(change: RecordsDiff, scope: RecordScope): { added: { [K in IdOf]: R; }; @@ -269,8 +297,6 @@ export class Store { // (undocumented) _flushHistory(): void; get: >(id: K) => RecFromId | undefined; - // (undocumented) - getRecordType: (record: R) => T; getSnapshot(scope?: 'all' | RecordScope): StoreSnapshot; has: >(id: K) => boolean; readonly history: Atom>; @@ -331,6 +357,8 @@ export class StoreSchema { createIntegrityChecker(store: Store): (() => void) | undefined; // (undocumented) getMigrationsSince(persistedSchema: SerializedSchema): Result; + // @internal (undocumented) + getType(typeName: string): RecordType; // (undocumented) migratePersistedRecord(record: R, persistedSchema: SerializedSchema, direction?: 'down' | 'up'): MigrationResult; // (undocumented) diff --git a/packages/store/src/index.ts b/packages/store/src/index.ts index e773ecd73..2216ec4d7 100644 --- a/packages/store/src/index.ts +++ b/packages/store/src/index.ts @@ -1,12 +1,19 @@ export type { BaseRecord, IdOf, RecordId, UnknownRecord } from './lib/BaseRecord' export { IncrementalSetConstructor } from './lib/IncrementalSetConstructor' export { RecordType, assertIdType, createRecordType } from './lib/RecordType' -export { Store, reverseRecordsDiff, squashRecordDiffs } from './lib/Store' +export { + createEmptyRecordsDiff, + isRecordsDiffEmpty, + reverseRecordsDiff, + squashRecordDiffs, + squashRecordDiffsMutable, + type RecordsDiff, +} from './lib/RecordsDiff' +export { Store } from './lib/Store' export type { CollectionDiff, ComputedCache, HistoryEntry, - RecordsDiff, SerializedStore, StoreError, StoreListener, diff --git a/packages/store/src/lib/RecordType.ts b/packages/store/src/lib/RecordType.ts index 40d568c00..9fd0dea7e 100644 --- a/packages/store/src/lib/RecordType.ts +++ b/packages/store/src/lib/RecordType.ts @@ -1,4 +1,4 @@ -import { structuredClone } from '@tldraw/utils' +import { objectMapEntries, structuredClone } from '@tldraw/utils' import { nanoid } from 'nanoid' import { IdOf, OmitMeta, UnknownRecord } from './BaseRecord' import { StoreValidator } from './Store' @@ -28,7 +28,8 @@ export class RecordType< > { readonly createDefaultProperties: () => Exclude, RequiredProperties> readonly validator: StoreValidator - + readonly ephemeralKeys?: { readonly [K in Exclude]: boolean } + readonly ephemeralKeySet: ReadonlySet readonly scope: RecordScope constructor( @@ -43,11 +44,21 @@ export class RecordType< readonly createDefaultProperties: () => Exclude, RequiredProperties> readonly validator?: StoreValidator readonly scope?: RecordScope + readonly ephemeralKeys?: { readonly [K in Exclude]: boolean } } ) { this.createDefaultProperties = config.createDefaultProperties this.validator = config.validator ?? { validate: (r: unknown) => r as R } this.scope = config.scope ?? 'document' + this.ephemeralKeys = config.ephemeralKeys + + const ephemeralKeySet = new Set() + if (config.ephemeralKeys) { + for (const [key, isEphemeral] of objectMapEntries(config.ephemeralKeys)) { + if (isEphemeral) ephemeralKeySet.add(key) + } + } + this.ephemeralKeySet = ephemeralKeySet } /** @@ -186,6 +197,7 @@ export class RecordType< createDefaultProperties: createDefaultProperties as any, validator: this.validator, scope: this.scope, + ephemeralKeys: this.ephemeralKeys, }) } @@ -218,12 +230,14 @@ export function createRecordType( config: { validator?: StoreValidator scope: RecordScope + ephemeralKeys?: { readonly [K in Exclude]: boolean } } ): RecordType> { return new RecordType>(typeName, { createDefaultProperties: () => ({}) as any, validator: config.validator, scope: config.scope, + ephemeralKeys: config.ephemeralKeys, }) } diff --git a/packages/store/src/lib/RecordsDiff.ts b/packages/store/src/lib/RecordsDiff.ts new file mode 100644 index 000000000..bf4834ae3 --- /dev/null +++ b/packages/store/src/lib/RecordsDiff.ts @@ -0,0 +1,107 @@ +import { objectMapEntries } from '@tldraw/utils' +import { IdOf, UnknownRecord } from './BaseRecord' + +/** + * A diff describing the changes to a record. + * + * @public + */ +export type RecordsDiff = { + added: Record, R> + updated: Record, [from: R, to: R]> + removed: Record, R> +} + +/** @internal */ +export function createEmptyRecordsDiff(): RecordsDiff { + return { added: {}, updated: {}, removed: {} } as RecordsDiff +} + +/** @public */ +export function reverseRecordsDiff(diff: RecordsDiff) { + const result: RecordsDiff = { added: diff.removed, removed: diff.added, updated: {} } + for (const [from, to] of Object.values(diff.updated)) { + result.updated[from.id] = [to, from] + } + return result +} + +/** + * Is a records diff empty? + * @internal + */ +export function isRecordsDiffEmpty(diff: RecordsDiff) { + return ( + Object.keys(diff.added).length === 0 && + Object.keys(diff.updated).length === 0 && + Object.keys(diff.removed).length === 0 + ) +} + +/** + * Squash a collection of diffs into a single diff. + * + * @param diffs - An array of diffs to squash. + * @returns A single diff that represents the squashed diffs. + * @public + */ +export function squashRecordDiffs( + diffs: RecordsDiff[] +): RecordsDiff { + const result = { added: {}, removed: {}, updated: {} } as RecordsDiff + + squashRecordDiffsMutable(result, diffs) + return result +} + +/** + * Apply the array `diffs` to the `target` diff, mutating it in-place. + * @internal + */ +export function squashRecordDiffsMutable( + target: RecordsDiff, + diffs: RecordsDiff[] +): void { + for (const diff of diffs) { + for (const [id, value] of objectMapEntries(diff.added)) { + if (target.removed[id]) { + const original = target.removed[id] + delete target.removed[id] + if (original !== value) { + target.updated[id] = [original, value] + } + } else { + target.added[id] = value + } + } + + for (const [id, [_from, to]] of objectMapEntries(diff.updated)) { + if (target.added[id]) { + target.added[id] = to + delete target.updated[id] + delete target.removed[id] + continue + } + if (target.updated[id]) { + target.updated[id] = [target.updated[id][0], to] + delete target.removed[id] + continue + } + + target.updated[id] = diff.updated[id] + delete target.removed[id] + } + + for (const [id, value] of objectMapEntries(diff.removed)) { + // the same record was added in this diff sequence, just drop it + if (target.added[id]) { + delete target.added[id] + } else if (target.updated[id]) { + target.removed[id] = target.updated[id][0] + delete target.updated[id] + } else { + target.removed[id] = value + } + } + } +} diff --git a/packages/store/src/lib/Store.ts b/packages/store/src/lib/Store.ts index 5ce958f3f..e718ab002 100644 --- a/packages/store/src/lib/Store.ts +++ b/packages/store/src/lib/Store.ts @@ -1,6 +1,8 @@ import { Atom, Computed, Reactor, atom, computed, reactor, transact } from '@tldraw/state' import { + assert, filterEntries, + getOwnProperty, objectMapEntries, objectMapFromEntries, objectMapKeys, @@ -11,23 +13,13 @@ import { nanoid } from 'nanoid' import { IdOf, RecordId, UnknownRecord } from './BaseRecord' import { Cache } from './Cache' import { RecordScope } from './RecordType' +import { RecordsDiff, squashRecordDiffs } from './RecordsDiff' import { StoreQueries } from './StoreQueries' import { SerializedSchema, StoreSchema } from './StoreSchema' import { devFreeze } from './devFreeze' type RecFromId> = K extends RecordId ? R : never -/** - * A diff describing the changes to a record. - * - * @public - */ -export type RecordsDiff = { - added: Record, R> - updated: Record, [from: R, to: R]> - removed: Record, R> -} - /** * A diff describing the changes to a collection. * @@ -113,7 +105,7 @@ export class Store { /** * The random id of the store. */ - public readonly id = nanoid() + public readonly id: string /** * An atom containing the store's atoms. * @@ -169,6 +161,7 @@ export class Store { public readonly scopedTypes: { readonly [K in RecordScope]: ReadonlySet } constructor(config: { + id?: string /** The store's initial data. */ initialData?: SerializedStore /** @@ -178,8 +171,9 @@ export class Store { schema: StoreSchema props: Props }) { - const { initialData, schema } = config + const { initialData, schema, id } = config + this.id = id ?? nanoid() this.schema = schema this.props = config.props @@ -357,7 +351,7 @@ export class Store { * @public */ put = (records: R[], phaseOverride?: 'initialize'): void => { - transact(() => { + this.atomic(() => { const updates: Record, [from: R, to: R]> = {} const additions: Record, R> = {} @@ -402,7 +396,9 @@ export class Store { recordAtom.set(devFreeze(record)) didChange = true - updates[record.id] = [initialValue, recordAtom.__unsafe__getWithoutCapture()] + const updated = recordAtom.__unsafe__getWithoutCapture() + updates[record.id] = [initialValue, updated] + this.addDiffForAfterEvent(initialValue, updated, source) } else { if (beforeCreate) record = beforeCreate(record, source) @@ -420,6 +416,7 @@ export class Store { // Mark the change as a new addition. additions[record.id] = record + this.addDiffForAfterEvent(null, record, source) // Assign the atom to the map under the record's id. if (!map) { @@ -441,24 +438,6 @@ export class Store { updated: updates, removed: {} as Record, R>, }) - - if (this._runCallbacks) { - const { onAfterCreate, onAfterChange } = this - - if (onAfterCreate) { - // Run the onAfterChange callback for addition. - Object.values(additions).forEach((record) => { - onAfterCreate(record, source) - }) - } - - if (onAfterChange) { - // Run the onAfterChange callback for update. - Object.values(updates).forEach(([from, to]) => { - onAfterChange(from, to, source) - }) - } - } }) } @@ -469,7 +448,7 @@ export class Store { * @public */ remove = (ids: IdOf[]): void => { - transact(() => { + this.atomic(() => { const cancelled = [] as IdOf[] const source = this.isMergingRemoteChanges ? 'remote' : 'user' @@ -496,7 +475,9 @@ export class Store { if (!result) result = { ...atoms } if (!removed) removed = {} as Record, R> delete result[id] - removed[id] = atoms[id].get() + const record = atoms[id].get() + removed[id] = record + this.addDiffForAfterEvent(record, null, source) } return result ?? atoms @@ -505,17 +486,6 @@ export class Store { if (!removed) return // Update the history with the removed records. this.updateHistory({ added: {}, updated: {}, removed } as RecordsDiff) - - // If we have an onAfterChange, run it for each removed record. - if (this.onAfterDelete && this._runCallbacks) { - let record: R - for (let i = 0, n = ids.length; i < n; i++) { - record = removed[ids[i]] - if (record) { - this.onAfterDelete(record, source) - } - } - } }) } @@ -620,7 +590,7 @@ export class Store { const prevRunCallbacks = this._runCallbacks try { this._runCallbacks = false - transact(() => { + this.atomic(() => { this.clear() this.put(Object.values(migrationResult.value)) this.ensureStoreIsUsable() @@ -731,9 +701,12 @@ export class Store { } } + /** + * Run `fn` and return a {@link RecordsDiff} of the changes that occurred as a result. + */ extractingChanges(fn: () => void): RecordsDiff { const changes: Array> = [] - const dispose = this.historyAccumulator.intercepting((entry) => changes.push(entry.changes)) + const dispose = this.historyAccumulator.addInterceptor((entry) => changes.push(entry.changes)) try { transact(fn) return squashRecordDiffs(changes) @@ -742,25 +715,47 @@ export class Store { } } - applyDiff(diff: RecordsDiff, runCallbacks = true) { - const prevRunCallbacks = this._runCallbacks - try { - this._runCallbacks = runCallbacks - transact(() => { - const toPut = objectMapValues(diff.added).concat( - objectMapValues(diff.updated).map(([_from, to]) => to) - ) - const toRemove = objectMapKeys(diff.removed) - if (toPut.length) { - this.put(toPut) + applyDiff( + diff: RecordsDiff, + { + runCallbacks = true, + ignoreEphemeralKeys = false, + }: { runCallbacks?: boolean; ignoreEphemeralKeys?: boolean } = {} + ) { + this.atomic(() => { + const toPut = objectMapValues(diff.added) + + for (const [_from, to] of objectMapValues(diff.updated)) { + const type = this.schema.getType(to.typeName) + if (ignoreEphemeralKeys && type.ephemeralKeySet.size) { + const existing = this.get(to.id) + if (!existing) { + toPut.push(to) + continue + } + let changed: R | null = null + for (const [key, value] of Object.entries(to)) { + if (type.ephemeralKeySet.has(key) || Object.is(value, getOwnProperty(existing, key))) { + continue + } + + if (!changed) changed = { ...existing } as R + ;(changed as any)[key] = value + } + if (changed) toPut.push(changed) + } else { + toPut.push(to) } - if (toRemove.length) { - this.remove(toRemove) - } - }) - } finally { - this._runCallbacks = prevRunCallbacks - } + } + + const toRemove = objectMapKeys(diff.removed) + if (toPut.length) { + this.put(toPut) + } + if (toRemove.length) { + this.remove(toRemove) + } + }, runCallbacks) } /** @@ -827,20 +822,14 @@ export class Store { } } - getRecordType = (record: R): T => { - const type = this.schema.types[record.typeName as R['typeName']] - if (!type) { - throw new Error(`Record type ${record.typeName} not found`) - } - return type as unknown as T - } - private _integrityChecker?: () => void | undefined /** @internal */ ensureStoreIsUsable() { - this._integrityChecker ??= this.schema.createIntegrityChecker(this) - this._integrityChecker?.() + this.atomic(() => { + this._integrityChecker ??= this.schema.createIntegrityChecker(this) + this._integrityChecker?.() + }) } private _isPossiblyCorrupted = false @@ -852,64 +841,82 @@ export class Store { isPossiblyCorrupted() { return this._isPossiblyCorrupted } -} -/** - * Squash a collection of diffs into a single diff. - * - * @param diffs - An array of diffs to squash. - * @returns A single diff that represents the squashed diffs. - * @public - */ -export function squashRecordDiffs( - diffs: RecordsDiff[] -): RecordsDiff { - const result = { added: {}, removed: {}, updated: {} } as RecordsDiff + private pendingAfterEvents: Map< + IdOf, + { before: R | null; after: R | null; source: 'remote' | 'user' } + > | null = null + private addDiffForAfterEvent(before: R | null, after: R | null, source: 'remote' | 'user') { + assert(this.pendingAfterEvents, 'must be in event operation') + if (before === after) return + if (before && after) assert(before.id === after.id) + if (!before && !after) return + const id = (before || after)!.id + const existing = this.pendingAfterEvents.get(id) + if (existing) { + assert(existing.source === source, 'source cannot change within a single event operation') + existing.after = after + } else { + this.pendingAfterEvents.set(id, { before, after, source }) + } + } + private flushAtomicCallbacks() { + let updateDepth = 0 + while (this.pendingAfterEvents) { + const events = this.pendingAfterEvents + this.pendingAfterEvents = null - for (const diff of diffs) { - for (const [id, value] of objectMapEntries(diff.added)) { - if (result.removed[id]) { - const original = result.removed[id] - delete result.removed[id] - if (original !== value) { - result.updated[id] = [original, value] + if (!this._runCallbacks) continue + + updateDepth++ + if (updateDepth > 100) { + throw new Error('Maximum store update depth exceeded, bailing out') + } + + for (const { before, after, source } of events.values()) { + if (before && after) { + this.onAfterChange?.(before, after, source) + } else if (before && !after) { + this.onAfterDelete?.(before, source) + } else if (!before && after) { + this.onAfterCreate?.(after, source) } - } else { - result.added[id] = value - } - } - - for (const [id, [_from, to]] of objectMapEntries(diff.updated)) { - if (result.added[id]) { - result.added[id] = to - delete result.updated[id] - delete result.removed[id] - continue - } - if (result.updated[id]) { - result.updated[id] = [result.updated[id][0], to] - delete result.removed[id] - continue - } - - result.updated[id] = diff.updated[id] - delete result.removed[id] - } - - for (const [id, value] of objectMapEntries(diff.removed)) { - // the same record was added in this diff sequence, just drop it - if (result.added[id]) { - delete result.added[id] - } else if (result.updated[id]) { - result.removed[id] = result.updated[id][0] - delete result.updated[id] - } else { - result.removed[id] = value } } } + private _isInAtomicOp = false + /** @internal */ + atomic(fn: () => T, runCallbacks = true): T { + return transact(() => { + if (this._isInAtomicOp) { + if (!this.pendingAfterEvents) this.pendingAfterEvents = new Map() + return fn() + } - return result + this.pendingAfterEvents = new Map() + const prevRunCallbacks = this._runCallbacks + this._runCallbacks = runCallbacks ?? prevRunCallbacks + this._isInAtomicOp = true + try { + const result = fn() + + this.flushAtomicCallbacks() + + return result + } finally { + this.pendingAfterEvents = null + this._runCallbacks = prevRunCallbacks + this._isInAtomicOp = false + } + }) + } + + /** @internal */ + addHistoryInterceptor(fn: (entry: HistoryEntry, source: ChangeSource) => void) { + return this.historyAccumulator.addInterceptor((entry) => + fn(entry, this.isMergingRemoteChanges ? 'remote' : 'user') + ) + } } /** @@ -949,21 +956,12 @@ function squashHistoryEntries( ) } -/** @public */ -export function reverseRecordsDiff(diff: RecordsDiff) { - const result: RecordsDiff = { added: diff.removed, removed: diff.added, updated: {} } - for (const [from, to] of Object.values(diff.updated)) { - result.updated[from.id] = [to, from] - } - return result -} - class HistoryAccumulator { private _history: HistoryEntry[] = [] private _interceptors: Set<(entry: HistoryEntry) => void> = new Set() - intercepting(fn: (entry: HistoryEntry) => void) { + addInterceptor(fn: (entry: HistoryEntry) => void) { this._interceptors.add(fn) return () => { this._interceptors.delete(fn) diff --git a/packages/store/src/lib/StoreQueries.ts b/packages/store/src/lib/StoreQueries.ts index ebfebc218..4a021a752 100644 --- a/packages/store/src/lib/StoreQueries.ts +++ b/packages/store/src/lib/StoreQueries.ts @@ -12,8 +12,9 @@ import isEqual from 'lodash.isequal' import { IdOf, UnknownRecord } from './BaseRecord' import { executeQuery, objectMatchesQuery, QueryExpression } from './executeQuery' import { IncrementalSetConstructor } from './IncrementalSetConstructor' +import { RecordsDiff } from './RecordsDiff' import { diffSets } from './setUtils' -import { CollectionDiff, RecordsDiff } from './Store' +import { CollectionDiff } from './Store' export type RSIndexDiff< R extends UnknownRecord, diff --git a/packages/store/src/lib/StoreSchema.ts b/packages/store/src/lib/StoreSchema.ts index d0cbe70aa..1f6fcdccd 100644 --- a/packages/store/src/lib/StoreSchema.ts +++ b/packages/store/src/lib/StoreSchema.ts @@ -329,4 +329,11 @@ export class StoreSchema { ), } } + + /** @internal */ + getType(typeName: string) { + const type = getOwnProperty(this.types, typeName) + assert(type, 'record type does not exists') + return type + } } diff --git a/packages/store/src/lib/test/recordStore.test.ts b/packages/store/src/lib/test/recordStore.test.ts index aa12b4bcb..6366e840d 100644 --- a/packages/store/src/lib/test/recordStore.test.ts +++ b/packages/store/src/lib/test/recordStore.test.ts @@ -1,8 +1,9 @@ import { Computed, react, RESET_VALUE, transact } from '@tldraw/state' import { BaseRecord, RecordId } from '../BaseRecord' import { createMigrationSequence } from '../migrate' +import { RecordsDiff, reverseRecordsDiff } from '../RecordsDiff' import { createRecordType } from '../RecordType' -import { CollectionDiff, RecordsDiff, Store } from '../Store' +import { CollectionDiff, Store } from '../Store' import { StoreSchema } from '../StoreSchema' interface Book extends BaseRecord<'book', RecordId> { @@ -881,3 +882,270 @@ describe('snapshots', () => { expect(store2.get(Book.createId('lotr'))!.numPages).toBe(42) }) }) + +describe('diffs', () => { + let store: Store + const authorId = Author.createId('tolkein') + const bookId = Book.createId('hobbit') + + beforeEach(() => { + store = new Store({ + props: {}, + schema: StoreSchema.create({ + book: Book, + author: Author, + visit: Visit, + }), + }) + }) + + it('produces diffs from `extractingChanges`', () => { + expect( + store.extractingChanges(() => { + store.put([Author.create({ name: 'J.R.R Tolkein', id: authorId })]) + store.put([ + Book.create({ title: 'The Hobbit', id: bookId, author: authorId, numPages: 300 }), + ]) + }) + ).toMatchInlineSnapshot(` + { + "added": { + "author:tolkein": { + "id": "author:tolkein", + "isPseudonym": false, + "name": "J.R.R Tolkein", + "typeName": "author", + }, + "book:hobbit": { + "author": "author:tolkein", + "id": "book:hobbit", + "numPages": 300, + "title": "The Hobbit", + "typeName": "book", + }, + }, + "removed": {}, + "updated": {}, + } + `) + + expect( + store.extractingChanges(() => { + store.remove([authorId]) + store.update(bookId, (book) => ({ ...book, title: 'The Hobbit: There and Back Again' })) + }) + ).toMatchInlineSnapshot(` + { + "added": {}, + "removed": { + "author:tolkein": { + "id": "author:tolkein", + "isPseudonym": false, + "name": "J.R.R Tolkein", + "typeName": "author", + }, + }, + "updated": { + "book:hobbit": [ + { + "author": "author:tolkein", + "id": "book:hobbit", + "numPages": 300, + "title": "The Hobbit", + "typeName": "book", + }, + { + "author": "author:tolkein", + "id": "book:hobbit", + "numPages": 300, + "title": "The Hobbit: There and Back Again", + "typeName": "book", + }, + ], + }, + } + `) + }) + it('produces diffs from `addHistoryInterceptor`', () => { + const diffs: any[] = [] + const interceptor = jest.fn((diff) => diffs.push(diff)) + store.addHistoryInterceptor(interceptor) + + store.put([ + Author.create({ name: 'J.R.R Tolkein', id: Author.createId('tolkein') }), + Book.create({ title: 'The Hobbit', id: bookId, author: authorId, numPages: 300 }), + ]) + expect(interceptor).toHaveBeenCalledTimes(1) + + store.extractingChanges(() => { + store.remove([authorId]) + + store.update(bookId, (book) => ({ ...book, title: 'The Hobbit: There and Back Again' })) + }) + expect(interceptor).toHaveBeenCalledTimes(3) + + expect(diffs).toMatchInlineSnapshot(` + [ + { + "changes": { + "added": { + "author:tolkein": { + "id": "author:tolkein", + "isPseudonym": false, + "name": "J.R.R Tolkein", + "typeName": "author", + }, + "book:hobbit": { + "author": "author:tolkein", + "id": "book:hobbit", + "numPages": 300, + "title": "The Hobbit", + "typeName": "book", + }, + }, + "removed": {}, + "updated": {}, + }, + "source": "user", + }, + { + "changes": { + "added": {}, + "removed": { + "author:tolkein": { + "id": "author:tolkein", + "isPseudonym": false, + "name": "J.R.R Tolkein", + "typeName": "author", + }, + }, + "updated": {}, + }, + "source": "user", + }, + { + "changes": { + "added": {}, + "removed": {}, + "updated": { + "book:hobbit": [ + { + "author": "author:tolkein", + "id": "book:hobbit", + "numPages": 300, + "title": "The Hobbit", + "typeName": "book", + }, + { + "author": "author:tolkein", + "id": "book:hobbit", + "numPages": 300, + "title": "The Hobbit: There and Back Again", + "typeName": "book", + }, + ], + }, + }, + "source": "user", + }, + ] + `) + }) + + it('can apply and invert diffs', () => { + store.put([ + Author.create({ name: 'J.R.R Tolkein', id: Author.createId('tolkein') }), + Book.create({ title: 'The Hobbit', id: bookId, author: authorId, numPages: 300 }), + ]) + + const checkpoint1 = store.getSnapshot() + + const forwardsDiff = store.extractingChanges(() => { + store.remove([authorId]) + store.update(bookId, (book) => ({ ...book, title: 'The Hobbit: There and Back Again' })) + }) + + const checkpoint2 = store.getSnapshot() + + store.applyDiff(reverseRecordsDiff(forwardsDiff)) + expect(store.getSnapshot()).toEqual(checkpoint1) + + store.applyDiff(forwardsDiff) + expect(store.getSnapshot()).toEqual(checkpoint2) + }) +}) + +describe('after callbacks', () => { + let store: Store + let callbacks: any[] = [] + + const authorId = Author.createId('tolkein') + const bookId = Book.createId('hobbit') + + beforeEach(() => { + store = new Store({ + props: {}, + schema: StoreSchema.create({ + book: Book, + author: Author, + visit: Visit, + }), + }) + + store.onAfterCreate = jest.fn((record) => callbacks.push({ type: 'create', record })) + store.onAfterChange = jest.fn((from, to) => callbacks.push({ type: 'change', from, to })) + store.onAfterDelete = jest.fn((record) => callbacks.push({ type: 'delete', record })) + callbacks = [] + }) + + it('fires callbacks at the end of an `atomic` op', () => { + store.atomic(() => { + expect(callbacks).toHaveLength(0) + + store.put([ + Author.create({ name: 'J.R.R Tolkein', id: authorId }), + Book.create({ title: 'The Hobbit', id: bookId, author: authorId, numPages: 300 }), + ]) + + expect(callbacks).toHaveLength(0) + }) + + expect(callbacks).toMatchObject([ + { type: 'create', record: { id: authorId } }, + { type: 'create', record: { id: bookId } }, + ]) + }) + + it('doesnt fire callback for a record created then deleted', () => { + store.atomic(() => { + store.put([Author.create({ name: 'J.R.R Tolkein', id: authorId })]) + store.remove([authorId]) + }) + expect(callbacks).toHaveLength(0) + }) + + it('bails out if too many callbacks are fired', () => { + let limit = 10 + store.onAfterCreate = (record) => { + if (record.typeName === 'book' && record.numPages < limit) { + store.put([{ ...record, numPages: record.numPages + 1 }]) + } + } + store.onAfterChange = (from, to) => { + if (to.typeName === 'book' && to.numPages < limit) { + store.put([{ ...to, numPages: to.numPages + 1 }]) + } + } + + // this should be fine: + store.put([Book.create({ title: 'The Hobbit', id: bookId, author: authorId, numPages: 0 })]) + expect(store.get(bookId)!.numPages).toBe(limit) + + // if we increase the limit thought, it should crash: + limit = 10000 + store.clear() + expect(() => { + store.put([Book.create({ title: 'The Hobbit', id: bookId, author: authorId, numPages: 0 })]) + }).toThrowErrorMatchingInlineSnapshot(`"Maximum store update depth exceeded, bailing out"`) + }) +}) diff --git a/packages/tldraw/api-report.md b/packages/tldraw/api-report.md index a5a6b46d1..b545a50d5 100644 --- a/packages/tldraw/api-report.md +++ b/packages/tldraw/api-report.md @@ -1814,7 +1814,7 @@ export interface TLUiButtonPickerProps { // (undocumented) items: StyleValuesForUi; // (undocumented) - onValueChange: (style: StyleProp, value: T, squashing: boolean) => void; + onValueChange: (style: StyleProp, value: T) => void; // (undocumented) style: StyleProp; // (undocumented) @@ -2206,6 +2206,8 @@ export interface TLUiInputProps { // (undocumented) onComplete?: (value: string) => void; // (undocumented) + onFocus?: () => void; + // (undocumented) onValueChange?: (value: string) => void; // (undocumented) placeholder?: string; @@ -2332,7 +2334,7 @@ export interface TLUiSliderProps { // (undocumented) label: string; // (undocumented) - onValueChange: (value: number, squashing: boolean) => void; + onValueChange: (value: number) => void; // (undocumented) steps: number; // (undocumented) diff --git a/packages/tldraw/src/lib/shapes/arrow/toolStates/Pointing.ts b/packages/tldraw/src/lib/shapes/arrow/toolStates/Pointing.ts index acd616f76..9e0b9414d 100644 --- a/packages/tldraw/src/lib/shapes/arrow/toolStates/Pointing.ts +++ b/packages/tldraw/src/lib/shapes/arrow/toolStates/Pointing.ts @@ -115,7 +115,7 @@ export class Pointing extends StateNode { if (startTerminal?.type === 'binding') { this.editor.setHintingShapes([startTerminal.boundShapeId]) } - this.editor.updateShapes([change], { squashing: true }) + this.editor.updateShapes([change]) } // Cache the current shape after those changes @@ -152,7 +152,7 @@ export class Pointing extends StateNode { if (endTerminal?.type === 'binding') { this.editor.setHintingShapes([endTerminal.boundShapeId]) } - this.editor.updateShapes([change], { squashing: true }) + this.editor.updateShapes([change]) } } @@ -168,7 +168,7 @@ export class Pointing extends StateNode { }) if (change) { - this.editor.updateShapes([change], { squashing: true }) + this.editor.updateShapes([change]) } } diff --git a/packages/tldraw/src/lib/shapes/draw/toolStates/Drawing.ts b/packages/tldraw/src/lib/shapes/draw/toolStates/Drawing.ts index 8c23577e1..2b049c675 100644 --- a/packages/tldraw/src/lib/shapes/draw/toolStates/Drawing.ts +++ b/packages/tldraw/src/lib/shapes/draw/toolStates/Drawing.ts @@ -370,9 +370,7 @@ export class Drawing extends StateNode { ) } - this.editor.updateShapes([shapePartial], { - squashing: true, - }) + this.editor.updateShapes([shapePartial]) } break } @@ -433,7 +431,7 @@ export class Drawing extends StateNode { ) } - this.editor.updateShapes([shapePartial], { squashing: true }) + this.editor.updateShapes([shapePartial]) } break @@ -574,7 +572,7 @@ export class Drawing extends StateNode { ) } - this.editor.updateShapes([shapePartial], { squashing: true }) + this.editor.updateShapes([shapePartial]) break } @@ -621,7 +619,7 @@ export class Drawing extends StateNode { ) } - this.editor.updateShapes([shapePartial], { squashing: true }) + this.editor.updateShapes([shapePartial]) // Set a maximum length for the lines array; after 200 points, complete the line. if (newPoints.length > 500) { diff --git a/packages/tldraw/src/lib/shapes/frame/components/FrameLabelInput.tsx b/packages/tldraw/src/lib/shapes/frame/components/FrameLabelInput.tsx index 0da28c544..70b5013a6 100644 --- a/packages/tldraw/src/lib/shapes/frame/components/FrameLabelInput.tsx +++ b/packages/tldraw/src/lib/shapes/frame/components/FrameLabelInput.tsx @@ -30,16 +30,13 @@ export const FrameLabelInput = forwardRef< const value = e.currentTarget.value.trim() if (name === value) return - editor.updateShapes( - [ - { - id, - type: 'frame', - props: { name: value }, - }, - ], - { squashing: true } - ) + editor.updateShapes([ + { + id, + type: 'frame', + props: { name: value }, + }, + ]) }, [id, editor] ) @@ -53,16 +50,13 @@ export const FrameLabelInput = forwardRef< const value = e.currentTarget.value if (name === value) return - editor.updateShapes( - [ - { - id, - type: 'frame', - props: { name: value }, - }, - ], - { squashing: true } - ) + editor.updateShapes([ + { + id, + type: 'frame', + props: { name: value }, + }, + ]) }, [id, editor] ) diff --git a/packages/tldraw/src/lib/shapes/note/NoteShapeTool.test.ts b/packages/tldraw/src/lib/shapes/note/NoteShapeTool.test.ts index 03013d2e9..1395e4c99 100644 --- a/packages/tldraw/src/lib/shapes/note/NoteShapeTool.test.ts +++ b/packages/tldraw/src/lib/shapes/note/NoteShapeTool.test.ts @@ -25,7 +25,6 @@ describe(NoteShapeTool, () => { editor.cancel() // leave edit mode - editor.undo() // undoes the selection change editor.undo() expect(editor.getCurrentPageShapes().length).toBe(0) diff --git a/packages/tldraw/src/lib/tools/HandTool/childStates/Pointing.ts b/packages/tldraw/src/lib/tools/HandTool/childStates/Pointing.ts index 0d8390d21..e5746c578 100644 --- a/packages/tldraw/src/lib/tools/HandTool/childStates/Pointing.ts +++ b/packages/tldraw/src/lib/tools/HandTool/childStates/Pointing.ts @@ -5,10 +5,7 @@ export class Pointing extends StateNode { override onEnter = () => { this.editor.stopCameraAnimation() - this.editor.updateInstanceState( - { cursor: { type: 'grabbing', rotation: 0 } }, - { ephemeral: true } - ) + this.editor.setCursor({ type: 'grabbing', rotation: 0 }) } override onLongPress: TLEventHandlers['onLongPress'] = () => { diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/Brushing.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/Brushing.ts index 593e3e36c..d44e2186e 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/Brushing.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/Brushing.ts @@ -79,7 +79,7 @@ export class Brushing extends StateNode { } override onCancel?: TLCancelEvent | undefined = (info) => { - this.editor.setSelectedShapes(this.initialSelectedShapeIds, { squashing: true }) + this.editor.setSelectedShapes(this.initialSelectedShapeIds) this.parent.transition('idle', info) } @@ -176,7 +176,7 @@ export class Brushing extends StateNode { const current = editor.getSelectedShapeIds() if (current.length !== results.size || current.some((id) => !results.has(id))) { - editor.setSelectedShapes(Array.from(results), { squashing: true }) + editor.setSelectedShapes(Array.from(results)) } } diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/Crop/children/Idle.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/Crop/children/Idle.ts index 1a3f22fa6..0c1a2493f 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/Crop/children/Idle.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/Crop/children/Idle.ts @@ -6,17 +6,14 @@ export class Idle extends StateNode { static override id = 'idle' override onEnter = () => { - this.editor.updateInstanceState( - { cursor: { type: 'default', rotation: 0 } }, - { ephemeral: true } - ) + this.editor.setCursor({ type: 'default', rotation: 0 }) const onlySelectedShape = this.editor.getOnlySelectedShape() // well this fucking sucks. what the fuck. // it's possible for a user to enter cropping, then undo // (which clears the cropping id) but still remain in this state. - this.editor.on('change-history', this.cleanupCroppingState) + this.editor.on('tick', this.cleanupCroppingState) if (onlySelectedShape) { this.editor.mark('crop') @@ -25,12 +22,9 @@ export class Idle extends StateNode { } override onExit: TLExitEventHandler = () => { - this.editor.updateInstanceState( - { cursor: { type: 'default', rotation: 0 } }, - { ephemeral: true } - ) + this.editor.setCursor({ type: 'default', rotation: 0 }) - this.editor.off('change-history', this.cleanupCroppingState) + this.editor.off('tick', this.cleanupCroppingState) } override onCancel: TLEventHandlers['onCancel'] = () => { diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/Crop/children/TranslatingCrop.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/Crop/children/TranslatingCrop.ts index 6d4ad7d85..d51c4a0b7 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/Crop/children/TranslatingCrop.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/Crop/children/TranslatingCrop.ts @@ -32,10 +32,7 @@ export class TranslatingCrop extends StateNode { } override onExit = () => { - this.editor.updateInstanceState( - { cursor: { type: 'default', rotation: 0 } }, - { ephemeral: true } - ) + this.editor.setCursor({ type: 'default', rotation: 0 }) } override onPointerMove = () => { @@ -102,7 +99,7 @@ export class TranslatingCrop extends StateNode { const partial = getTranslateCroppedImageChange(this.editor, shape, delta) if (partial) { - this.editor.updateShapes([partial], { squashing: true }) + this.editor.updateShapes([partial]) } } } diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/Cropping.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/Cropping.ts index a0da7a5ea..c2e327bca 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/Cropping.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/Cropping.ts @@ -65,12 +65,7 @@ export class Cropping extends StateNode { if (!selectedShape) return const cursorType = CursorTypeMap[this.info.handle!] - this.editor.updateInstanceState({ - cursor: { - type: cursorType, - rotation: this.editor.getSelectionRotation(), - }, - }) + this.editor.setCursor({ type: cursorType, rotation: this.editor.getSelectionRotation() }) } private getDefaultCrop = (): TLImageShapeCrop => ({ @@ -201,7 +196,7 @@ export class Cropping extends StateNode { }, } - this.editor.updateShapes([partial], { squashing: true }) + this.editor.updateShapes([partial]) this.updateCursor() } diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/DraggingHandle.tsx b/packages/tldraw/src/lib/tools/SelectTool/childStates/DraggingHandle.tsx index e6b81d14a..aab15b79d 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/DraggingHandle.tsx +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/DraggingHandle.tsx @@ -82,10 +82,7 @@ export class DraggingHandle extends StateNode { this.initialPageRotation = this.initialPageTransform.rotation() this.initialPagePoint = this.editor.inputs.originPagePoint.clone() - this.editor.updateInstanceState( - { cursor: { type: isCreating ? 'cross' : 'grabbing', rotation: 0 } }, - { ephemeral: true } - ) + this.editor.setCursor({ type: isCreating ? 'cross' : 'grabbing', rotation: 0 }) const handles = this.editor.getShapeHandles(shape)!.sort(sortByIndex) const index = handles.findIndex((h) => h.id === info.handle.id) @@ -196,10 +193,7 @@ export class DraggingHandle extends StateNode { this.editor.setHintingShapes([]) this.editor.snaps.clearIndicators() - this.editor.updateInstanceState( - { cursor: { type: 'default', rotation: 0 } }, - { ephemeral: true } - ) + this.editor.setCursor({ type: 'default', rotation: 0 }) } private complete() { @@ -312,7 +306,7 @@ export class DraggingHandle extends StateNode { } if (changes) { - editor.updateShapes([next], { squashing: true }) + editor.updateShapes([next]) } } } diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/Idle.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/Idle.ts index e9d885565..3c9952e03 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/Idle.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/Idle.ts @@ -39,10 +39,7 @@ export class Idle extends StateNode { override onEnter = () => { this.parent.setCurrentToolIdMask(undefined) updateHoveredId(this.editor) - this.editor.updateInstanceState( - { cursor: { type: 'default', rotation: 0 } }, - { ephemeral: true } - ) + this.editor.setCursor({ type: 'default', rotation: 0 }) } override onPointerMove: TLEventHandlers['onPointerMove'] = () => { diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingArrowLabel.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingArrowLabel.ts index fd84f09c0..35584682b 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingArrowLabel.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingArrowLabel.ts @@ -62,10 +62,7 @@ export class PointingArrowLabel extends StateNode { override onExit = () => { this.parent.setCurrentToolIdMask(undefined) - this.editor.updateInstanceState( - { cursor: { type: 'default', rotation: 0 } }, - { ephemeral: true } - ) + this.editor.setCursor({ type: 'default', rotation: 0 }) } private _labelDragOffset = new Vec(0, 0) @@ -105,10 +102,11 @@ export class PointingArrowLabel extends StateNode { } this.didDrag = true - this.editor.updateShape( - { id: shape.id, type: shape.type, props: { labelPosition: nextLabelPosition } }, - { squashing: true } - ) + this.editor.updateShape({ + id: shape.id, + type: shape.type, + props: { labelPosition: nextLabelPosition }, + }) } override onPointerUp = () => { diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingCropHandle.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingCropHandle.ts index aefbbdc7d..603aaa471 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingCropHandle.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingCropHandle.ts @@ -19,20 +19,12 @@ export class PointingCropHandle extends StateNode { if (!selectedShape) return const cursorType = CursorTypeMap[this.info.handle!] - this.editor.updateInstanceState({ - cursor: { - type: cursorType, - rotation: this.editor.getSelectionRotation(), - }, - }) + this.editor.setCursor({ type: cursorType, rotation: this.editor.getSelectionRotation() }) this.editor.setCroppingShape(selectedShape.id) } override onExit = () => { - this.editor.updateInstanceState( - { cursor: { type: 'default', rotation: 0 } }, - { ephemeral: true } - ) + this.editor.setCursor({ type: 'default', rotation: 0 }) this.parent.setCurrentToolIdMask(undefined) } diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingHandle.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingHandle.ts index 239b1a45c..95d5499ee 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingHandle.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingHandle.ts @@ -32,18 +32,12 @@ export class PointingHandle extends StateNode { } } - this.editor.updateInstanceState( - { cursor: { type: 'grabbing', rotation: 0 } }, - { ephemeral: true } - ) + this.editor.setCursor({ type: 'grabbing', rotation: 0 }) } override onExit = () => { this.editor.setHintingShapes([]) - this.editor.updateInstanceState( - { cursor: { type: 'default', rotation: 0 } }, - { ephemeral: true } - ) + this.editor.setCursor({ type: 'default', rotation: 0 }) } override onPointerUp: TLEventHandlers['onPointerUp'] = () => { diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingResizeHandle.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingResizeHandle.ts index e353d50ea..28e5ee041 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingResizeHandle.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingResizeHandle.ts @@ -34,11 +34,9 @@ export class PointingResizeHandle extends StateNode { private updateCursor() { const selected = this.editor.getSelectedShapes() const cursorType = CursorTypeMap[this.info.handle!] - this.editor.updateInstanceState({ - cursor: { - type: cursorType, - rotation: selected.length === 1 ? this.editor.getSelectionRotation() : 0, - }, + this.editor.setCursor({ + type: cursorType, + rotation: selected.length === 1 ? this.editor.getSelectionRotation() : 0, }) } diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingRotateHandle.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingRotateHandle.ts index 4053a7764..d5d9388c1 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingRotateHandle.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/PointingRotateHandle.ts @@ -11,11 +11,9 @@ export class PointingRotateHandle extends StateNode { private info = {} as PointingRotateHandleInfo private updateCursor() { - this.editor.updateInstanceState({ - cursor: { - type: CursorTypeMap[this.info.handle as RotateCorner], - rotation: this.editor.getSelectionRotation(), - }, + this.editor.setCursor({ + type: CursorTypeMap[this.info.handle as RotateCorner], + rotation: this.editor.getSelectionRotation(), }) } @@ -27,10 +25,7 @@ export class PointingRotateHandle extends StateNode { override onExit = () => { this.parent.setCurrentToolIdMask(undefined) - this.editor.updateInstanceState( - { cursor: { type: 'default', rotation: 0 } }, - { ephemeral: true } - ) + this.editor.setCursor({ type: 'default', rotation: 0 }) } override onPointerMove: TLEventHandlers['onPointerMove'] = () => { diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/Resizing.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/Resizing.ts index e07e70741..2c154bb9d 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/Resizing.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/Resizing.ts @@ -61,10 +61,7 @@ export class Resizing extends StateNode { if (isCreating) { this.markId = `creating:${this.editor.getOnlySelectedShape()!.id}` - this.editor.updateInstanceState( - { cursor: { type: 'cross', rotation: 0 } }, - { ephemeral: true } - ) + this.editor.setCursor({ type: 'cross', rotation: 0 }) } else { this.markId = 'starting resizing' this.editor.mark(this.markId) @@ -407,10 +404,7 @@ export class Resizing extends StateNode { override onExit = () => { this.parent.setCurrentToolIdMask(undefined) - this.editor.updateInstanceState( - { cursor: { type: 'default', rotation: 0 } }, - { ephemeral: true } - ) + this.editor.setCursor({ type: 'default', rotation: 0 }) this.editor.snaps.clearIndicators() } diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/Rotating.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/Rotating.ts index 2c001882a..9a441b850 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/Rotating.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/Rotating.ts @@ -51,11 +51,9 @@ export class Rotating extends StateNode { }) // Update cursor - this.editor.updateInstanceState({ - cursor: { - type: CursorTypeMap[this.info.handle as RotateCorner], - rotation: newSelectionRotation + this.snapshot.initialSelectionRotation, - }, + this.editor.setCursor({ + type: CursorTypeMap[this.info.handle as RotateCorner], + rotation: newSelectionRotation + this.snapshot.initialSelectionRotation, }) } @@ -105,11 +103,9 @@ export class Rotating extends StateNode { }) // Update cursor - this.editor.updateInstanceState({ - cursor: { - type: CursorTypeMap[this.info.handle as RotateCorner], - rotation: newSelectionRotation + this.snapshot.initialSelectionRotation, - }, + this.editor.setCursor({ + type: CursorTypeMap[this.info.handle as RotateCorner], + rotation: newSelectionRotation + this.snapshot.initialSelectionRotation, }) } diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/ScribbleBrushing.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/ScribbleBrushing.ts index 7f9504b42..b55fbb7aa 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/ScribbleBrushing.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/ScribbleBrushing.ts @@ -164,7 +164,7 @@ export class ScribbleBrushing extends StateNode { shiftKey ? [...newlySelectedShapeIds, ...initialSelectedShapeIds] : [...newlySelectedShapeIds] ) if (current.length !== next.size || current.some((id) => !next.has(id))) { - this.editor.setSelectedShapes(Array.from(next), { squashing: true }) + this.editor.setSelectedShapes(Array.from(next)) } } @@ -174,7 +174,7 @@ export class ScribbleBrushing extends StateNode { } private cancel() { - this.editor.setSelectedShapes([...this.initialSelectedShapeIds], { squashing: true }) + this.editor.setSelectedShapes([...this.initialSelectedShapeIds]) this.parent.transition('idle') } } diff --git a/packages/tldraw/src/lib/tools/SelectTool/childStates/Translating.ts b/packages/tldraw/src/lib/tools/SelectTool/childStates/Translating.ts index b6181d55f..04520f284 100644 --- a/packages/tldraw/src/lib/tools/SelectTool/childStates/Translating.ts +++ b/packages/tldraw/src/lib/tools/SelectTool/childStates/Translating.ts @@ -505,7 +505,6 @@ export function moveShapesToPoint({ y: newLocalPoint.y, } }) - ), - { squashing: true } + ) ) } diff --git a/packages/tldraw/src/lib/tools/ZoomTool/ZoomTool.ts b/packages/tldraw/src/lib/tools/ZoomTool/ZoomTool.ts index 009ff75d8..a9253644f 100644 --- a/packages/tldraw/src/lib/tools/ZoomTool/ZoomTool.ts +++ b/packages/tldraw/src/lib/tools/ZoomTool/ZoomTool.ts @@ -19,10 +19,7 @@ export class ZoomTool extends StateNode { override onExit = () => { this.parent.setCurrentToolIdMask(undefined) - this.editor.updateInstanceState( - { zoomBrush: null, cursor: { type: 'default', rotation: 0 } }, - { ephemeral: true } - ) + this.editor.updateInstanceState({ zoomBrush: null, cursor: { type: 'default', rotation: 0 } }) this.parent.setCurrentToolIdMask(undefined) } @@ -53,15 +50,9 @@ export class ZoomTool extends StateNode { private updateCursor() { if (this.editor.inputs.altKey) { - this.editor.updateInstanceState( - { cursor: { type: 'zoom-out', rotation: 0 } }, - { ephemeral: true } - ) + this.editor.setCursor({ type: 'zoom-out', rotation: 0 }) } else { - this.editor.updateInstanceState( - { cursor: { type: 'zoom-in', rotation: 0 } }, - { ephemeral: true } - ) + this.editor.setCursor({ type: 'zoom-in', rotation: 0 }) } } } diff --git a/packages/tldraw/src/lib/ui/components/MobileStylePanel.tsx b/packages/tldraw/src/lib/ui/components/MobileStylePanel.tsx index 30f34cbde..bd53018d9 100644 --- a/packages/tldraw/src/lib/ui/components/MobileStylePanel.tsx +++ b/packages/tldraw/src/lib/ui/components/MobileStylePanel.tsx @@ -37,7 +37,7 @@ export function MobileStylePanel() { const handleStylesOpenChange = useCallback( (isOpen: boolean) => { if (!isOpen) { - editor.updateInstanceState({ isChangingStyle: false }, { ephemeral: true }) + editor.updateInstanceState({ isChangingStyle: false }) } }, [editor] diff --git a/packages/tldraw/src/lib/ui/components/PageMenu/PageItemInput.tsx b/packages/tldraw/src/lib/ui/components/PageMenu/PageItemInput.tsx index 0f7cc48c7..b80aa34e2 100644 --- a/packages/tldraw/src/lib/ui/components/PageMenu/PageItemInput.tsx +++ b/packages/tldraw/src/lib/ui/components/PageMenu/PageItemInput.tsx @@ -15,17 +15,13 @@ export const PageItemInput = function PageItemInput({ const rInput = useRef(null) + const handleFocus = useCallback(() => { + editor.mark('rename page') + }, [editor]) + const handleChange = useCallback( (value: string) => { - editor.renamePage(id, value ? value : 'New Page', { ephemeral: true }) - }, - [editor, id] - ) - - const handleComplete = useCallback( - (value: string) => { - editor.mark('rename page') - editor.renamePage(id, value || 'New Page', { ephemeral: false }) + editor.renamePage(id, value || 'New Page') }, [editor, id] ) @@ -36,8 +32,7 @@ export const PageItemInput = function PageItemInput({ ref={(el) => (rInput.current = el)} defaultValue={name} onValueChange={handleChange} - onComplete={handleComplete} - onCancel={handleComplete} + onFocus={handleFocus} shouldManuallyMaintainScrollPositionWhenFocused autofocus={isCurrentPage} autoselect diff --git a/packages/tldraw/src/lib/ui/components/StylePanel/DefaultStylePanel.tsx b/packages/tldraw/src/lib/ui/components/StylePanel/DefaultStylePanel.tsx index b6c77140f..10ae96d3b 100644 --- a/packages/tldraw/src/lib/ui/components/StylePanel/DefaultStylePanel.tsx +++ b/packages/tldraw/src/lib/ui/components/StylePanel/DefaultStylePanel.tsx @@ -21,7 +21,7 @@ export const DefaultStylePanel = memo(function DefaultStylePanel({ const handlePointerOut = useCallback(() => { if (!isMobile) { - editor.updateInstanceState({ isChangingStyle: false }, { ephemeral: true }) + editor.updateInstanceState({ isChangingStyle: false }) } }, [editor, isMobile]) diff --git a/packages/tldraw/src/lib/ui/components/StylePanel/DefaultStylePanelContent.tsx b/packages/tldraw/src/lib/ui/components/StylePanel/DefaultStylePanelContent.tsx index 5d0e4d9ea..ca2d48367 100644 --- a/packages/tldraw/src/lib/ui/components/StylePanel/DefaultStylePanelContent.tsx +++ b/packages/tldraw/src/lib/ui/components/StylePanel/DefaultStylePanelContent.tsx @@ -78,13 +78,13 @@ function useStyleChangeCallback() { return React.useMemo( () => - function handleStyleChange(style: StyleProp, value: T, squashing: boolean) { + function handleStyleChange(style: StyleProp, value: T) { editor.batch(() => { if (editor.isIn('select')) { - editor.setStyleForSelectedShapes(style, value, { squashing }) + editor.setStyleForSelectedShapes(style, value) } - editor.setStyleForNextShapes(style, value, { squashing }) - editor.updateInstanceState({ isChangingStyle: true }, { ephemeral: true }) + editor.setStyleForNextShapes(style, value) + editor.updateInstanceState({ isChangingStyle: true }) }) trackEvent('set-style', { source: 'style-panel', id: style.id, value: value as string }) @@ -165,8 +165,8 @@ export function CommonStylePickerSet({ style={DefaultSizeStyle} items={STYLES.size} value={size} - onValueChange={(style, value, squashing) => { - handleValueChange(style, value, squashing) + onValueChange={(style, value) => { + handleValueChange(style, value) const selectedShapeIds = editor.getSelectedShapeIds() if (selectedShapeIds.length > 0) { kickoutOccludedShapes(editor, selectedShapeIds) @@ -333,14 +333,14 @@ export function OpacitySlider() { const msg = useTranslation() const handleOpacityValueChange = React.useCallback( - (value: number, squashing: boolean) => { + (value: number) => { const item = tldrawSupportedOpacities[value] editor.batch(() => { if (editor.isIn('select')) { - editor.setOpacityForSelectedShapes(item, { squashing }) + editor.setOpacityForSelectedShapes(item) } - editor.setOpacityForNextShapes(item, { squashing }) - editor.updateInstanceState({ isChangingStyle: true }, { ephemeral: true }) + editor.setOpacityForNextShapes(item) + editor.updateInstanceState({ isChangingStyle: true }) }) trackEvent('set-style', { source: 'style-panel', id: 'opacity', value }) diff --git a/packages/tldraw/src/lib/ui/components/StylePanel/DoubleDropdownPicker.tsx b/packages/tldraw/src/lib/ui/components/StylePanel/DoubleDropdownPicker.tsx index 18f4a53c2..8a5dff9d6 100644 --- a/packages/tldraw/src/lib/ui/components/StylePanel/DoubleDropdownPicker.tsx +++ b/packages/tldraw/src/lib/ui/components/StylePanel/DoubleDropdownPicker.tsx @@ -24,7 +24,7 @@ interface DoubleDropdownPickerProps { styleB: StyleProp valueA: SharedStyle valueB: SharedStyle - onValueChange: (style: StyleProp, value: T, squashing: boolean) => void + onValueChange: (style: StyleProp, value: T) => void } function _DoubleDropdownPicker({ @@ -88,7 +88,7 @@ function _DoubleDropdownPicker({ onValueChange(styleA, item.value, false)} + onClick={() => onValueChange(styleA, item.value)} title={`${msg(labelA)} — ${msg(`${uiTypeA}-style.${item.value}`)}`} > @@ -124,7 +124,7 @@ function _DoubleDropdownPicker({ type="icon" title={`${msg(labelB)} — ${msg(`${uiTypeB}-style.${item.value}` as TLUiTranslationKey)}`} data-testid={`style.${uiTypeB}.${item.value}`} - onClick={() => onValueChange(styleB, item.value, false)} + onClick={() => onValueChange(styleB, item.value)} > diff --git a/packages/tldraw/src/lib/ui/components/StylePanel/DropdownPicker.tsx b/packages/tldraw/src/lib/ui/components/StylePanel/DropdownPicker.tsx index 675692d55..6f50ab854 100644 --- a/packages/tldraw/src/lib/ui/components/StylePanel/DropdownPicker.tsx +++ b/packages/tldraw/src/lib/ui/components/StylePanel/DropdownPicker.tsx @@ -22,7 +22,7 @@ interface DropdownPickerProps { value: SharedStyle items: StyleValuesForUi type: TLUiButtonProps['type'] - onValueChange: (style: StyleProp, value: T, squashing: boolean) => void + onValueChange: (style: StyleProp, value: T) => void } function _DropdownPicker({ @@ -68,7 +68,7 @@ function _DropdownPicker({ title={msg(`${uiType}-style.${item.value}` as TLUiTranslationKey)} onClick={() => { editor.mark('select style dropdown item') - onValueChange(style, item.value, false) + onValueChange(style, item.value) }} > diff --git a/packages/tldraw/src/lib/ui/components/primitives/TldrawUiButtonPicker.tsx b/packages/tldraw/src/lib/ui/components/primitives/TldrawUiButtonPicker.tsx index bc90bf0b0..a1792463a 100644 --- a/packages/tldraw/src/lib/ui/components/primitives/TldrawUiButtonPicker.tsx +++ b/packages/tldraw/src/lib/ui/components/primitives/TldrawUiButtonPicker.tsx @@ -22,7 +22,7 @@ export interface TLUiButtonPickerProps { value: SharedStyle items: StyleValuesForUi theme: TLDefaultColorTheme - onValueChange: (style: StyleProp, value: T, squashing: boolean) => void + onValueChange: (style: StyleProp, value: T) => void } function _TldrawUiButtonPicker(props: TLUiButtonPickerProps) { @@ -57,14 +57,14 @@ function _TldrawUiButtonPicker(props: TLUiButtonPickerProps if (value.type === 'shared' && value.value === id) return editor.mark('point picker item') - onValueChange(style, id as T, false) + onValueChange(style, id as T) } const handleButtonPointerDown = (e: React.PointerEvent) => { const { id } = e.currentTarget.dataset editor.mark('point picker item') - onValueChange(style, id as T, true) + onValueChange(style, id as T) rPointing.current = true window.addEventListener('pointerup', handlePointerUp) // see TLD-658 @@ -74,14 +74,14 @@ function _TldrawUiButtonPicker(props: TLUiButtonPickerProps if (!rPointing.current) return const { id } = e.currentTarget.dataset - onValueChange(style, id as T, true) + onValueChange(style, id as T) } const handleButtonPointerUp = (e: React.PointerEvent) => { const { id } = e.currentTarget.dataset if (value.type === 'shared' && value.value === id) return - onValueChange(style, id as T, false) + onValueChange(style, id as T) } return { diff --git a/packages/tldraw/src/lib/ui/components/primitives/TldrawUiInput.tsx b/packages/tldraw/src/lib/ui/components/primitives/TldrawUiInput.tsx index 5ba5fe319..20742e06e 100644 --- a/packages/tldraw/src/lib/ui/components/primitives/TldrawUiInput.tsx +++ b/packages/tldraw/src/lib/ui/components/primitives/TldrawUiInput.tsx @@ -21,6 +21,7 @@ export interface TLUiInputProps { onValueChange?: (value: string) => void onCancel?: (value: string) => void onBlur?: (value: string) => void + onFocus?: () => void className?: string /** * Usually on iOS when you focus an input, the browser will adjust the viewport to bring the input @@ -49,6 +50,7 @@ export const TldrawUiInput = React.forwardRef( onComplete, onValueChange, onCancel, + onFocus, onBlur, shouldManuallyMaintainScrollPositionWhenFocused = false, children, @@ -77,8 +79,9 @@ export const TldrawUiInput = React.forwardRef( elm.select() } }) + onFocus?.() }, - [autoselect] + [autoselect, onFocus] ) const handleChange = React.useCallback( diff --git a/packages/tldraw/src/lib/ui/components/primitives/TldrawUiSlider.tsx b/packages/tldraw/src/lib/ui/components/primitives/TldrawUiSlider.tsx index 94b4079db..f9223a2e1 100644 --- a/packages/tldraw/src/lib/ui/components/primitives/TldrawUiSlider.tsx +++ b/packages/tldraw/src/lib/ui/components/primitives/TldrawUiSlider.tsx @@ -10,7 +10,7 @@ export interface TLUiSliderProps { value: number | null label: string title: string - onValueChange: (value: number, squashing: boolean) => void + onValueChange: (value: number) => void 'data-testid'?: string } @@ -22,7 +22,7 @@ export const TldrawUiSlider = memo(function Slider(props: TLUiSliderProps) { const handleValueChange = useCallback( (value: number[]) => { - onValueChange(value[0], true) + onValueChange(value[0]) }, [onValueChange] ) @@ -33,7 +33,7 @@ export const TldrawUiSlider = memo(function Slider(props: TLUiSliderProps) { const handlePointerUp = useCallback(() => { if (!value) return - onValueChange(value, false) + onValueChange(value) }, [value, onValueChange]) return ( diff --git a/packages/tldraw/src/lib/ui/context/actions.tsx b/packages/tldraw/src/lib/ui/context/actions.tsx index 850b44bca..b7dea063a 100644 --- a/packages/tldraw/src/lib/ui/context/actions.tsx +++ b/packages/tldraw/src/lib/ui/context/actions.tsx @@ -1164,12 +1164,9 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { readonlyOk: true, onSelect(source) { trackEvent('toggle-transparent', { source }) - editor.updateInstanceState( - { - exportBackground: !editor.getInstanceState().exportBackground, - }, - { ephemeral: true } - ) + editor.updateInstanceState({ + exportBackground: !editor.getInstanceState().exportBackground, + }) }, checkbox: true, }, @@ -1326,10 +1323,10 @@ export function ActionsProvider({ overrides, children }: ActionsProviderProps) { editor.batch(() => { editor.mark('change-color') if (editor.isIn('select')) { - editor.setStyleForSelectedShapes(style, 'white', { squashing: false }) + editor.setStyleForSelectedShapes(style, 'white') } - editor.setStyleForNextShapes(style, 'white', { squashing: false }) - editor.updateInstanceState({ isChangingStyle: true }, { ephemeral: true }) + editor.setStyleForNextShapes(style, 'white') + editor.updateInstanceState({ isChangingStyle: true }) }) trackEvent('set-style', { source, id: style.id, value: 'white' }) }, diff --git a/packages/tldraw/src/lib/ui/hooks/clipboard/pasteTldrawContent.ts b/packages/tldraw/src/lib/ui/hooks/clipboard/pasteTldrawContent.ts index 80106dd05..c7f6bb238 100644 --- a/packages/tldraw/src/lib/ui/hooks/clipboard/pasteTldrawContent.ts +++ b/packages/tldraw/src/lib/ui/hooks/clipboard/pasteTldrawContent.ts @@ -24,9 +24,9 @@ export function pasteTldrawContent(editor: Editor, clipboard: TLContent, point?: seletionBoundsBefore?.collides(selectedBoundsAfter) ) { // Creates a 'puff' to show a paste has happened. - editor.updateInstanceState({ isChangingStyle: true }, { ephemeral: true }) + editor.updateInstanceState({ isChangingStyle: true }) setTimeout(() => { - editor.updateInstanceState({ isChangingStyle: false }, { ephemeral: true }) + editor.updateInstanceState({ isChangingStyle: false }) }, 150) } } diff --git a/packages/tldraw/src/lib/ui/hooks/usePrint.ts b/packages/tldraw/src/lib/ui/hooks/usePrint.ts index 9728f4e22..a5c8e6cdb 100644 --- a/packages/tldraw/src/lib/ui/hooks/usePrint.ts +++ b/packages/tldraw/src/lib/ui/hooks/usePrint.ts @@ -134,7 +134,7 @@ export function usePrint() { } const afterPrintHandler = () => { - editor.once('change-history', () => { + editor.once('tick', () => { clearElements(el, style) }) } diff --git a/packages/tldraw/src/lib/ui/hooks/useTools.tsx b/packages/tldraw/src/lib/ui/hooks/useTools.tsx index 66fe31b33..fd05a340b 100644 --- a/packages/tldraw/src/lib/ui/hooks/useTools.tsx +++ b/packages/tldraw/src/lib/ui/hooks/useTools.tsx @@ -102,15 +102,7 @@ export function ToolsProvider({ overrides, children }: TLUiToolsProviderProps) { icon: ('geo-' + id) as TLUiIconType, onSelect(source: TLUiEventSource) { editor.batch(() => { - editor.updateInstanceState( - { - stylesForNextShape: { - ...editor.getInstanceState().stylesForNextShape, - [GeoShapeGeoStyle.id]: id, - }, - }, - { ephemeral: true } - ) + editor.setStyleForNextShapes(GeoShapeGeoStyle, id) editor.setCurrentTool('geo') trackEvent('select-tool', { source, id: `geo-${id}` }) }) diff --git a/packages/tldraw/src/test/TldrawEditor.test.tsx b/packages/tldraw/src/test/TldrawEditor.test.tsx index c25ac55e8..1f75a24d8 100644 --- a/packages/tldraw/src/test/TldrawEditor.test.tsx +++ b/packages/tldraw/src/test/TldrawEditor.test.tsx @@ -179,10 +179,7 @@ describe('', () => { expect(editor).toBeTruthy() await act(async () => { - editor.updateInstanceState( - { screenBounds: { x: 0, y: 0, w: 1080, h: 720 } }, - { ephemeral: true, squashing: true } - ) + editor.updateInstanceState({ screenBounds: { x: 0, y: 0, w: 1080, h: 720 } }) }) const id = createShapeId() @@ -299,10 +296,7 @@ describe('Custom shapes', () => { expect(editor).toBeTruthy() await act(async () => { - editor.updateInstanceState( - { screenBounds: { x: 0, y: 0, w: 1080, h: 720 } }, - { ephemeral: true, squashing: true } - ) + editor.updateInstanceState({ screenBounds: { x: 0, y: 0, w: 1080, h: 720 } }) }) expect(editor.shapeUtils.card).toBeTruthy() diff --git a/packages/tldraw/src/test/commands/alignShapes.test.tsx b/packages/tldraw/src/test/commands/alignShapes.test.tsx index 0ec1ec9cd..dbf7a520a 100644 --- a/packages/tldraw/src/test/commands/alignShapes.test.tsx +++ b/packages/tldraw/src/test/commands/alignShapes.test.tsx @@ -23,7 +23,7 @@ describe('when less than two shapes are selected', () => { editor.setSelectedShapes([ids.boxB]) const fn = jest.fn() - editor.on('update', fn) + editor.store.listen(fn) editor.alignShapes(editor.getSelectedShapeIds(), 'top') jest.advanceTimersByTime(1000) expect(fn).not.toHaveBeenCalled() diff --git a/packages/tldraw/src/test/commands/distributeShapes.test.ts b/packages/tldraw/src/test/commands/distributeShapes.test.ts index a6ef19a2f..241678154 100644 --- a/packages/tldraw/src/test/commands/distributeShapes.test.ts +++ b/packages/tldraw/src/test/commands/distributeShapes.test.ts @@ -46,7 +46,7 @@ describe('distributeShapes command', () => { it('does nothing', () => { editor.setSelectedShapes([ids.boxA, ids.boxB]) const fn = jest.fn() - editor.on('change-history', fn) + editor.store.listen(fn) editor.distributeShapes(editor.getSelectedShapeIds(), 'horizontal') jest.advanceTimersByTime(1000) expect(fn).not.toHaveBeenCalled() diff --git a/packages/tldraw/src/test/commands/moveShapesToPage.test.ts b/packages/tldraw/src/test/commands/moveShapesToPage.test.ts index 73d96646d..ed2e4c79e 100644 --- a/packages/tldraw/src/test/commands/moveShapesToPage.test.ts +++ b/packages/tldraw/src/test/commands/moveShapesToPage.test.ts @@ -60,8 +60,9 @@ describe('Editor.moveShapesToPage', () => { it('Adds undo items', () => { editor.history.clear() + expect(editor.history.getNumUndos()).toBe(0) editor.moveShapesToPage([ids.box1], ids.page2) - expect(editor.history.getNumUndos()).toBeGreaterThan(1) + expect(editor.history.getNumUndos()).toBe(1) }) it('Does nothing on an empty ids array', () => { diff --git a/packages/tldraw/src/test/commands/squash.test.ts b/packages/tldraw/src/test/commands/squash.test.ts deleted file mode 100644 index cb4e917de..000000000 --- a/packages/tldraw/src/test/commands/squash.test.ts +++ /dev/null @@ -1,13 +0,0 @@ -import { TestEditor } from '../TestEditor' - -let editor: TestEditor - -beforeEach(() => { - editor = new TestEditor() -}) - -describe('squashing', () => { - editor - - it.todo('squashes') -}) diff --git a/packages/tldraw/src/test/commands/stackShapes.test.ts b/packages/tldraw/src/test/commands/stackShapes.test.ts index 8c2f3e9f6..511258e4e 100644 --- a/packages/tldraw/src/test/commands/stackShapes.test.ts +++ b/packages/tldraw/src/test/commands/stackShapes.test.ts @@ -52,7 +52,7 @@ describe('distributeShapes command', () => { it('does nothing', () => { editor.setSelectedShapes([ids.boxA, ids.boxB]) const fn = jest.fn() - editor.on('change-history', fn) + editor.store.listen(fn) editor.stackShapes(editor.getSelectedShapeIds(), 'horizontal', 0) jest.advanceTimersByTime(1000) expect(fn).not.toHaveBeenCalled() diff --git a/packages/tldraw/src/test/commands/stretch.test.tsx b/packages/tldraw/src/test/commands/stretch.test.tsx index 309dfc174..8397c9a4b 100644 --- a/packages/tldraw/src/test/commands/stretch.test.tsx +++ b/packages/tldraw/src/test/commands/stretch.test.tsx @@ -27,7 +27,7 @@ describe('when less than two shapes are selected', () => { it('does nothing', () => { editor.setSelectedShapes([ids.boxB]) const fn = jest.fn() - editor.on('change-history', fn) + editor.store.listen(fn) editor.stretchShapes(editor.getSelectedShapeIds(), 'horizontal') jest.advanceTimersByTime(1000) diff --git a/packages/tldraw/src/test/testutils/pretty.ts b/packages/tldraw/src/test/testutils/pretty.ts new file mode 100644 index 000000000..f8d9a0cf4 --- /dev/null +++ b/packages/tldraw/src/test/testutils/pretty.ts @@ -0,0 +1,129 @@ +/* eslint-disable no-console */ +import { HistoryManager, RecordsDiff } from '@tldraw/editor' +// eslint-disable-next-line import/no-extraneous-dependencies +import { DiffOptions, diff as jestDiff } from 'jest-diff' +import { inspect } from 'util' + +class Printer { + private _output = '' + private _indent = 0 + + appendLines(str: string) { + const indent = ' '.repeat(this._indent) + this._output += + str + .split('\n') + .map((line) => indent + line) + .join('\n') + '\n' + } + + indent() { + this._indent++ + } + dedent() { + this._indent-- + } + + log(...args: any[]) { + this.appendLines(args.map((arg) => (typeof arg === 'string' ? arg : inspect(arg))).join(' ')) + } + + print() { + console.log(this._output) + } + + get() { + return this._output + } +} + +export function prettyPrintDiff(diff: RecordsDiff, opts?: DiffOptions) { + const before = {} as Record + const after = {} as Record + + for (const added of Object.values(diff.added)) { + after[added.id] = added + } + for (const [from, to] of Object.values(diff.updated)) { + before[from.id] = from + after[to.id] = to + } + for (const removed of Object.values(diff.removed)) { + before[removed.id] = removed + } + + const prettyDiff = jestDiff(after, before, { + aAnnotation: 'After', + bAnnotation: 'Before', + aIndicator: '+', + bIndicator: '-', + ...opts, + }) + + if (prettyDiff?.includes('Compared values have no visual difference.')) { + const p = new Printer() + p.log('Before & after have no visual difference.') + p.log('Diff:') + p.indent() + p.log(diff) + return p.get() + } + + return prettyDiff +} + +export function logHistory(history: HistoryManager) { + const { undos, redos, pendingDiff } = history.debug() + const p = new Printer() + p.log('=== History ===') + p.indent() + + p.log('Pending diff:') + p.indent() + if (pendingDiff.isEmpty) { + p.log('(empty)') + } else { + p.log(prettyPrintDiff(pendingDiff.diff)) + } + p.log('') + p.dedent() + + p.log('Undos:') + p.indent() + if (undos.length === 0) { + p.log('(empty)\n') + } + for (const undo of undos) { + if (!undo) continue + if (undo.type === 'stop') { + p.log('Stop', undo.id) + } else { + p.log('- Diff') + p.indent() + p.log(prettyPrintDiff(undo.diff)) + p.dedent() + } + p.log('') + } + p.dedent() + + p.log('Redos:') + p.indent() + if (redos.length === 0) { + p.log('(empty)\n') + } + for (const redo of redos) { + if (!redo) continue + if (redo.type === 'stop') { + p.log('> Stop', redo.id) + } else { + p.log('- Diff') + p.indent() + p.log(prettyPrintDiff(redo.diff)) + p.dedent() + } + p.log('') + } + + p.print() +} diff --git a/packages/tlschema/src/records/TLInstance.ts b/packages/tlschema/src/records/TLInstance.ts index d01bd16ad..ddf205698 100644 --- a/packages/tlschema/src/records/TLInstance.ts +++ b/packages/tlschema/src/records/TLInstance.ts @@ -27,7 +27,6 @@ export interface TLInstance extends BaseRecord<'instance', TLInstanceId> { currentPageId: TLPageId opacityForNextShape: TLOpacityType stylesForNextShape: Record - // ephemeral followingUserId: string | null highlightedUserIds: string[] brush: BoxModel | null @@ -129,6 +128,38 @@ export function createInstanceRecordType(stylesById: Map('instance', { validator: instanceTypeValidator, scope: 'session', + ephemeralKeys: { + currentPageId: false, + meta: false, + + followingUserId: true, + opacityForNextShape: true, + stylesForNextShape: true, + brush: true, + cursor: true, + scribbles: true, + isFocusMode: true, + isDebugMode: true, + isToolLocked: true, + exportBackground: true, + screenBounds: true, + insets: true, + zoomBrush: true, + isPenMode: true, + isGridMode: true, + chatMessage: true, + isChatting: true, + highlightedUserIds: true, + canMoveCamera: true, + isFocused: true, + devicePixelRatio: true, + isCoarsePointer: true, + isHoveringCanvas: true, + openMenus: true, + isChangingStyle: true, + isReadonly: true, + duplicateProps: true, + }, }).withDefaultProperties( (): Omit => ({ followingUserId: null, diff --git a/packages/tlschema/src/records/TLPageState.ts b/packages/tlschema/src/records/TLPageState.ts index 9599e300f..75e4b204c 100644 --- a/packages/tlschema/src/records/TLPageState.ts +++ b/packages/tlschema/src/records/TLPageState.ts @@ -138,6 +138,18 @@ export const InstancePageStateRecordType = createRecordType { validator: instancePageStateValidator, scope: 'session', + ephemeralKeys: { + pageId: false, + selectedShapeIds: false, + editingShapeId: false, + croppingShapeId: false, + meta: false, + + hintingShapeIds: true, + erasingShapeIds: true, + hoveredShapeId: true, + focusedGroupId: true, + }, } ).withDefaultProperties( (): Omit => ({ diff --git a/packages/tlsync/src/lib/TLSyncClient.ts b/packages/tlsync/src/lib/TLSyncClient.ts index 46b0b99e6..67c4ecde9 100644 --- a/packages/tlsync/src/lib/TLSyncClient.ts +++ b/packages/tlsync/src/lib/TLSyncClient.ts @@ -272,7 +272,9 @@ export class TLSyncClient = Store this.lastServerClock = 0 } // kill all presence state - this.store.remove(Object.keys(this.store.serialize('presence')) as any) + this.store.mergeRemoteChanges(() => { + this.store.remove(Object.keys(this.store.serialize('presence')) as any) + }) this.lastPushedPresenceState = null this.isConnectedToRoom = false this.pendingPushRequests = [] @@ -321,7 +323,7 @@ export class TLSyncClient = Store const wipeAll = event.hydrationType === 'wipe_all' if (!wipeAll) { // if we're only wiping presence data, undo the speculative changes first - this.store.applyDiff(reverseRecordsDiff(stashedChanges), false) + this.store.applyDiff(reverseRecordsDiff(stashedChanges), { runCallbacks: false }) } // now wipe all presence data and, if needed, all document data @@ -336,12 +338,22 @@ export class TLSyncClient = Store // then apply the upstream changes this.applyNetworkDiff({ ...wipeDiff, ...event.diff }, true) + + this.isConnectedToRoom = true + + // now re-apply the speculative changes creating a new push request with the + // appropriate diff + const speculativeChanges = this.store.filterChangesByScope( + this.store.extractingChanges(() => { + this.store.applyDiff(stashedChanges) + }), + 'document' + ) + if (speculativeChanges) this.push(speculativeChanges) }) - // now re-apply the speculative changes as a 'user' to trigger - // creating a new push request with the appropriate diff - this.isConnectedToRoom = true - this.store.applyDiff(stashedChanges) + // this.isConnectedToRoom = true + // this.store.applyDiff(stashedChanges, false) this.store.ensureStoreIsUsable() // TODO: reinstate isNew @@ -525,7 +537,7 @@ export class TLSyncClient = Store } } if (hasChanges) { - this.store.applyDiff(changes, runCallbacks) + this.store.applyDiff(changes, { runCallbacks }) } } @@ -541,7 +553,7 @@ export class TLSyncClient = Store try { this.store.mergeRemoteChanges(() => { // first undo speculative changes - this.store.applyDiff(reverseRecordsDiff(this.speculativeChanges), false) + this.store.applyDiff(reverseRecordsDiff(this.speculativeChanges), { runCallbacks: false }) // then apply network diffs on top of known-to-be-synced data for (const diff of diffs) { diff --git a/packages/tlsync/src/test/syncFuzz.test.ts b/packages/tlsync/src/test/syncFuzz.test.ts index 4974bc558..0a51ffcac 100644 --- a/packages/tlsync/src/test/syncFuzz.test.ts +++ b/packages/tlsync/src/test/syncFuzz.test.ts @@ -1,4 +1,3 @@ -import isEqual from 'lodash.isequal' import { nanoid } from 'nanoid' import { Editor, @@ -8,7 +7,9 @@ import { computed, createPresenceStateDerivation, createTLStore, + isRecordsDiffEmpty, } from 'tldraw' +import { prettyPrintDiff } from '../../../tldraw/src/test/testutils/pretty' import { TLSyncClient } from '../lib/TLSyncClient' import { schema } from '../lib/schema' import { FuzzEditor, Op } from './FuzzEditor' @@ -74,8 +75,8 @@ class FuzzTestInstance extends RandomSource { ) { super(seed) - this.store = createTLStore({ schema }) this.id = nanoid() + this.store = createTLStore({ schema, id: this.id }) this.socketPair = new TestSocketPair(this.id, server) this.client = new TLSyncClient({ store: this.store, @@ -105,6 +106,13 @@ class FuzzTestInstance extends RandomSource { } } +function assertPeerStoreIsUsable(peer: FuzzTestInstance) { + const diffToEnsureUsable = peer.store.extractingChanges(() => peer.store.ensureStoreIsUsable()) + if (!isRecordsDiffEmpty(diffToEnsureUsable)) { + throw new Error(`store of ${peer.id} was not usable\n${prettyPrintDiff(diffToEnsureUsable)}`) + } +} + let totalNumShapes = 0 let totalNumPages = 0 @@ -173,6 +181,7 @@ function runTest(seed: number) { allOk('before applyOp') peer.editor.applyOp(op) + assertPeerStoreIsUsable(peer) allOk('after applyOp') server.flushDebouncingMessages() @@ -210,6 +219,7 @@ function runTest(seed: number) { if (!peer.socketPair.isConnected && peer.randomInt(2) === 0) { peer.socketPair.connect() allOk('final connect') + assertPeerStoreIsUsable(peer) } } } @@ -223,33 +233,29 @@ function runTest(seed: number) { allOk('final flushServer') peer.socketPair.flushClientSentEvents() allOk('final flushClient') + assertPeerStoreIsUsable(peer) } } } - const equalityResults = [] - for (let i = 0; i < peers.length; i++) { - const row = [] - for (let j = 0; j < peers.length; j++) { - row.push( - isEqual( - peers[i].editor?.store.serialize('document'), - peers[j].editor?.store.serialize('document') - ) - ) - } - equalityResults.push(row) + // peers should all be usable without changes: + for (const peer of peers) { + assertPeerStoreIsUsable(peer) } - const [first, ...rest] = peers.map((peer) => peer.editor?.store.serialize('document')) + // all stores should be the same + for (let i = 1; i < peers.length; i++) { + const expected = peers[i - 1] + const actual = peers[i] + try { + expect(actual.store.serialize('document')).toEqual(expected.store.serialize('document')) + } catch (e: any) { + throw new Error(`received = ${actual.id}, expected = ${expected.id}\n${e.message}`) + } + } - // writeFileSync(`./test-results.${seed}.json`, JSON.stringify(ops, null, '\t')) - - expect(first).toEqual(rest[0]) - // all snapshots should be the same - expect(rest.every((other) => isEqual(other, first))).toBe(true) - totalNumPages += Object.values(first!).filter((v) => v.typeName === 'page').length - totalNumShapes += Object.values(first!).filter((v) => v.typeName === 'shape').length + totalNumPages += peers[0].store.query.ids('page').get().size + totalNumShapes += peers[0].store.query.ids('shape').get().size } catch (e) { console.error('seed', seed) console.error( @@ -269,21 +275,25 @@ const NUM_TESTS = 50 const NUM_OPS_PER_TEST = 100 const MAX_PEERS = 4 -// test.only('seed 8343632005032947', () => { -// runTest(8343632005032947) -// }) - -test('fuzzzzz', () => { - for (let i = 0; i < NUM_TESTS; i++) { - const seed = Math.floor(Math.random() * Number.MAX_SAFE_INTEGER) - try { - runTest(seed) - } catch (e) { - console.error('seed', seed) - throw e - } - } +test('seed 8360926944486245 - undo/redo page integrity regression', () => { + runTest(8360926944486245) }) +test('seed 3467175630814895 - undo/redo page integrity regression', () => { + runTest(3467175630814895) +}) +test('seed 6820615056006575 - undo/redo page integrity regression', () => { + runTest(6820615056006575) +}) +test('seed 5279266392988747 - undo/redo page integrity regression', () => { + runTest(5279266392988747) +}) + +for (let i = 0; i < NUM_TESTS; i++) { + const seed = Math.floor(Math.random() * Number.MAX_SAFE_INTEGER) + test(`seed ${seed}`, () => { + runTest(seed) + }) +} test('totalNumPages', () => { expect(totalNumPages).not.toBe(0)