alex/auto-undo-redo: cleanup

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])
 

packages/editor/api-report.md

@@ -794,7 +794,7 @@ export class Editor extends EventEmitter<TLEventMap> {
     getZoomLevel(): number;
     groupShapes(shapes: TLShape[] | TLShapeId[], groupId?: TLShapeId): this;
     hasAncestor(shape: TLShape | TLShapeId | undefined, ancestorId: TLShapeId): boolean;
-    readonly history: HistoryManager<TLRecord, this>;
+    readonly history: HistoryManager<TLRecord>;
     inputs: {
         originPagePoint: Vec;
         originScreenPoint: Vec;
@@ -2160,17 +2160,6 @@ export type TLEventInfo = TLCancelEventInfo | TLClickEventInfo | TLCompleteEvent
 
 // @public (undocumented)
 export interface TLEventMap {
-    // (undocumented)
-    'change-history': [{
-        reason: 'bail';
-        markId?: string;
-    } | {
-        reason: 'push' | 'redo' | 'undo';
-    }];
-    // (undocumented)
-    'mark-history': [{
-        id: string;
-    }];
     // (undocumented)
     'max-shapes': [{
         name: string;

packages/editor/api/api.json

@@ -14509,7 +14509,7 @@
                 },
                 {
                   "kind": "Content",
-                  "text": ", this>"
+                  "text": ">"
                 },
                 {
                   "kind": "Content",
@@ -38487,60 +38487,6 @@
           "name": "TLEventMap",
           "preserveMemberOrder": false,
           "members": [
-            {
-              "kind": "PropertySignature",
-              "canonicalReference": "@tldraw/editor!TLEventMap#\"change-history\":member",
-              "docComment": "",
-              "excerptTokens": [
-                {
-                  "kind": "Content",
-                  "text": "'change-history': "
-                },
-                {
-                  "kind": "Content",
-                  "text": "[{\n        reason: 'bail';\n        markId?: string;\n    } | {\n        reason: 'push' | 'redo' | 'undo';\n    }]"
-                },
-                {
-                  "kind": "Content",
-                  "text": ";"
-                }
-              ],
-              "isReadonly": false,
-              "isOptional": false,
-              "releaseTag": "Public",
-              "name": "\"change-history\"",
-              "propertyTypeTokenRange": {
-                "startIndex": 1,
-                "endIndex": 2
-              }
-            },
-            {
-              "kind": "PropertySignature",
-              "canonicalReference": "@tldraw/editor!TLEventMap#\"mark-history\":member",
-              "docComment": "",
-              "excerptTokens": [
-                {
-                  "kind": "Content",
-                  "text": "'mark-history': "
-                },
-                {
-                  "kind": "Content",
-                  "text": "[{\n        id: string;\n    }]"
-                },
-                {
-                  "kind": "Content",
-                  "text": ";"
-                }
-              ],
-              "isReadonly": false,
-              "isOptional": false,
-              "releaseTag": "Public",
-              "name": "\"mark-history\"",
-              "propertyTypeTokenRange": {
-                "startIndex": 1,
-                "endIndex": 2
-              }
-            },
             {
               "kind": "PropertySignature",
               "canonicalReference": "@tldraw/editor!TLEventMap#\"max-shapes\":member",

packages/editor/src/lib/editor/Editor.ts

@@ -198,9 +198,12 @@ export class Editor extends EventEmitter<TLEventMap> {
 		super()
 
 		this.store = store
-		this.history = new HistoryManager<TLRecord, this>(this, (error) => {
-			this.annotateError(error, { origin: 'history.batch', willCrashApp: true })
-			this.crash(error)
+		this.history = new HistoryManager<TLRecord>({
+			store,
+			annotateError: (error) => {
+				this.annotateError(error, { origin: 'history.batch', willCrashApp: true })
+				this.crash(error)
+			},
 		})
 
 		this.snaps = new SnapManager(this)
@@ -814,7 +817,7 @@ export class Editor extends EventEmitter<TLEventMap> {
 	 *
 	 * @readonly
 	 */
-	readonly history: HistoryManager<TLRecord, this>
+	readonly history: HistoryManager<TLRecord>
 
 	/**
 	 * Undo to the last mark.
@@ -1477,16 +1480,18 @@ export class Editor extends EventEmitter<TLEventMap> {
 	 * @public
 	 */
 	setSelectedShapes(shapes: TLShapeId[] | TLShape[]): this {
-		this.history.preserveRedoStack(() => {
-			const ids = shapes.map((shape) => (typeof shape === 'string' ? shape : shape.id))
-			const { selectedShapeIds: prevSelectedShapeIds } = this.getCurrentPageState()
-			const prevSet = new Set(prevSelectedShapeIds)
+		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
+				if (ids.length === prevSet.size && ids.every((id) => prevSet.has(id))) return
 
-			this.store.put([{ ...this.getCurrentPageState(), selectedShapeIds: ids }])
-		})
-		return this
+				this.store.put([{ ...this.getCurrentPageState(), selectedShapeIds: ids }])
+			},
+			{ history: 'preserveRedoStack' }
+		)
 	}
 
 	/**
@@ -1743,11 +1748,12 @@ export class Editor extends EventEmitter<TLEventMap> {
 
 		if (id === this.getFocusedGroupId()) return this
 
-		this.history.preserveRedoStack(() => {
-			this.store.update(this.getCurrentPageState().id, (s) => ({ ...s, focusedGroupId: id }))
-		})
-
-		return this
+		return this.batch(
+			() => {
+				this.store.update(this.getCurrentPageState().id, (s) => ({ ...s, focusedGroupId: id }))
+			},
+			{ history: 'preserveRedoStack' }
+		)
 	}
 
 	/**
@@ -3362,11 +3368,10 @@ export class Editor extends EventEmitter<TLEventMap> {
 
 		this.stopFollowingUser()
 
-		this.history.preserveRedoStack(() =>
-			this.store.put([{ ...this.getInstanceState(), currentPageId: pageId }])
+		return this.batch(
+			() => this.store.put([{ ...this.getInstanceState(), currentPageId: pageId }]),
+			{ history: 'preserveRedoStack' }
 		)
-
-		return this
 	}
 
 	/**

packages/editor/src/lib/editor/managers/HistoryManager.test.ts

@@ -1,5 +1,4 @@
 import { BaseRecord, RecordId, Store, StoreSchema, createRecordType } from '@tldraw/store'
-import { noop } from '@tldraw/utils'
 import { TLHistoryBatchOptions } from '../types/history-types'
 import { HistoryManager } from './HistoryManager'
 import { stack } from './Stack'
@@ -28,10 +27,7 @@ function createCounterHistoryManager() {
 		testSchema.types.test.create({ id: ids.age, value: 35 }),
 	])
 
-	const ctx = { store, emit: noop }
-	const manager = new HistoryManager<TestRecord, typeof ctx>(ctx, () => {
-		return
-	})
+	const manager = new HistoryManager<TestRecord>({ store })
 
 	function getCount() {
 		return store.get(ids.count)!.value as number
@@ -65,7 +61,7 @@ function createCounterHistoryManager() {
 	}
 
 	const setAge = (age = 35) => {
-		manager.preserveRedoStack(() => _setAge(age))
+		manager.batch(() => _setAge(age), { history: 'preserveRedoStack' })
 	}
 
 	const incrementTwice = () => {
@@ -275,7 +271,7 @@ describe(HistoryManager, () => {
 })
 
 describe('history options', () => {
-	let manager: HistoryManager<TestRecord, any>
+	let manager: HistoryManager<TestRecord>
 
 	let getState: () => { a: number; b: number }
 	let setA: (n: number, historyOptions?: TLHistoryBatchOptions) => any
@@ -288,8 +284,7 @@ describe('history options', () => {
 			testSchema.types.test.create({ id: ids.b, value: 0 }),
 		])
 
-		const ctx = { store, emit: noop }
-		manager = new HistoryManager<TestRecord, typeof ctx>(ctx, noop)
+		manager = new HistoryManager<TestRecord>({ store })
 
 		getState = () => {
 			return { a: store.get(ids.a)!.value as number, b: store.get(ids.b)!.value as number }

packages/editor/src/lib/editor/managers/HistoryManager.ts

@@ -8,7 +8,7 @@ import {
 	reverseRecordsDiff,
 	squashRecordDiffsMutable,
 } from '@tldraw/store'
-import { exhaustiveSwitchError } from '@tldraw/utils'
+import { exhaustiveSwitchError, noop } from '@tldraw/utils'
 import { uniqueId } from '../../utils/uniqueId'
 import { TLHistoryBatchOptions, TLHistoryEntry } from '../types/history-types'
 import { stack } from './Stack'
@@ -19,13 +19,13 @@ enum HistoryRecorderState {
 	Paused = 'paused',
 }
 
-export class HistoryManager<
-	R extends UnknownRecord,
-	CTX extends {
-		store: Store<R>
-		emit: (name: 'change-history' | 'mark-history', ...args: any) => void
-	},
-> {
+export class HistoryManager<R extends UnknownRecord> {
+	private readonly store: Store<R>
+
+	readonly dispose: () => void
+
+	private state: HistoryRecorderState = HistoryRecorderState.Recording
+	private readonly pendingDiff = new PendingDiff<R>()
 	/** @internal */
 	stacks = atom(
 		'HistoryManager.stacks',
@@ -38,15 +38,12 @@ export class HistoryManager<
 		}
 	)
 
-	private state: HistoryRecorderState = HistoryRecorderState.Recording
-	private readonly pendingDiff = new PendingDiff<R>()
-	readonly dispose: () => void
+	private readonly annotateError: (error: unknown) => void
 
-	constructor(
-		private readonly ctx: CTX,
-		private readonly annotateError: (error: unknown) => void
-	) {
-		this.dispose = this.ctx.store.addHistoryInterceptor((entry, source) => {
+	constructor(opts: { store: Store<R>; 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) {
@@ -65,19 +62,6 @@ export class HistoryManager<
 		})
 	}
 
-	// private hasPendingDiff = atom('HistoryManager.hasPendingDiff', false)
-	// private _pendingDiff = createEmptyDiff<R>()
-	// private getPendingDiff() {
-	// 	return this._pendingDiff
-	// }
-	// private setPendingDiff(diff: RecordsDiff<R>) {
-	// 	this._pendingDiff = diff
-	// 	this.didUpdatePendingDiff()
-	// }
-	// private didUpdatePendingDiff() {
-	// 	this.hasPendingDiff.set(isRecordsDiffEmpty(this._pendingDiff))
-	// }
-	// private appendToPending
 	flushPendingDiff() {
 		if (this.pendingDiff.isEmpty()) return
 
@@ -114,7 +98,6 @@ export class HistoryManager<
 				transact(() => {
 					fn()
 					this.onBatchComplete()
-					this.ctx.emit('change-history', { reason: 'push' })
 				})
 			} catch (error) {
 				this.annotateError(error)
@@ -132,9 +115,6 @@ export class HistoryManager<
 	ephemeral(fn: () => void) {
 		return this.batch(fn, { history: 'ephemeral' })
 	}
-	preserveRedoStack(fn: () => void) {
-		return this.batch(fn, { history: 'preserveRedoStack' })
-	}
 
 	// History
 	private _undo = ({
@@ -198,13 +178,8 @@ export class HistoryManager<
 				}
 			}
 
-			this.ctx.store.applyDiff(diffToUndo)
+			this.store.applyDiff(diffToUndo)
 			this.stacks.set({ undos, redos })
-
-			this.ctx.emit(
-				'change-history',
-				pushToRedoStack ? { reason: 'undo' } : { reason: 'bail', markId: toMark }
-			)
 		} finally {
 			this.state = previousState
 		}
@@ -250,10 +225,8 @@ export class HistoryManager<
 				}
 			}
 
-			this.ctx.store.applyDiff(diffToRedo)
+			this.store.applyDiff(diffToRedo)
 			this.stacks.set({ undos, redos })
-
-			this.ctx.emit('change-history', { reason: 'redo' })
 		} finally {
 			this.state = previousState
 		}
@@ -279,8 +252,6 @@ export class HistoryManager<
 			this.stacks.update(({ undos, redos }) => ({ undos: undos.push({ type: 'stop', id }), redos }))
 		})
 
-		this.ctx.emit('mark-history', { id })
-
 		return id
 	}
 

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 }]
 }
 
 /** @public */

packages/tldraw/src/lib/tools/SelectTool/childStates/Crop/children/Idle.ts

@@ -13,7 +13,7 @@ export class Idle extends StateNode {
 		// 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')
@@ -24,7 +24,7 @@ export class Idle extends StateNode {
 	override onExit: TLExitEventHandler = () => {
 		this.editor.setCursor({ type: 'default', rotation: 0 })
 
-		this.editor.off('change-history', this.cleanupCroppingState)
+		this.editor.off('tick', this.cleanupCroppingState)
 	}
 
 	override onCancel: TLEventHandlers['onCancel'] = () => {

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)
 				})
 			}

packages/tlsync/src/test/syncFuzz.test.ts

@@ -292,7 +292,7 @@ const MAX_PEERS = 4
 test('seed 8360926944486245 - undo/redo page integrity regression', () => {
 	runTest(8360926944486245)
 })
-test.only('seed 3467175630814895 - undo/redo page integrity regression', () => {
+test('seed 3467175630814895 - undo/redo page integrity regression', () => {
 	runTest(3467175630814895)
 })