From 1acfcbe4a51cf4559c5cacae0c99ce093db834cd Mon Sep 17 00:00:00 2001 From: syntaxbullet Date: Sat, 4 Jul 2026 15:49:13 +0200 Subject: [PATCH] perf(history): batch transform drag undo --- commands/command.ts | 6 +++ commands/dispatcher.ts | 76 ++++++++++++++++++++++++++++++++++---- commands/history.test.ts | 80 +++++++++++++++++++++++++++++++++++++++- commands/history.ts | 2 + commands/transform.ts | 3 ++ 5 files changed, 159 insertions(+), 8 deletions(-) diff --git a/commands/command.ts b/commands/command.ts index ffc9bf3..3bdb814 100644 --- a/commands/command.ts +++ b/commands/command.ts @@ -4,8 +4,14 @@ export type CommandContext = { state: AppState; }; +export type CommandHistoryPolicy = + | { mode: "auto" } + | { mode: "ignore" } + | { mode: "deferred"; phase: "begin" | "update" | "commit" }; + export type Command = { id: string; name: string; + history?: CommandHistoryPolicy; execute(context: CommandContext, payload: TPayload): AppState; }; diff --git a/commands/dispatcher.ts b/commands/dispatcher.ts index ff939c0..107afd7 100644 --- a/commands/dispatcher.ts +++ b/commands/dispatcher.ts @@ -1,6 +1,5 @@ import type { AppState, HistorySnapshot } from "@editor/state"; -import type { CommandContext } from "./command"; -import { commandIds } from "./ids"; +import type { CommandContext, CommandHistoryPolicy } from "./command"; import type { CommandId, CommandPayloads } from "./payloads"; import type { CommandRegistry } from "./registry"; @@ -15,6 +14,8 @@ export function createCommandDispatcher(options: { getState: () => AppState; setState: (state: AppState) => void; }): CommandDispatcher { + let deferredHistory: { snapshot: HistorySnapshot; changed: boolean } | undefined; + return { dispatch(commandId, payload) { const command = options.registry.get(commandId); @@ -23,28 +24,89 @@ export function createCommandDispatcher(options: { } const currentState = options.getState(); + const historyPolicy = command.history ?? defaultHistoryPolicy; + const deferredSnapshot = historyPolicy.mode === "deferred" && historyPolicy.phase === "begin" ? snapshot(currentState) : undefined; const context: CommandContext = { state: currentState }; const executedState = command.execute(context, payload); if (executedState === currentState) { + if (historyPolicy.mode === "deferred" && historyPolicy.phase === "commit") deferredHistory = undefined; return currentState; } - const nextState = shouldRecordHistory(commandId, currentState, executedState) ? recordHistory(currentState, executedState) : executedState; + + const nextState = applyHistoryPolicy({ + currentState, + nextState: executedState, + historyPolicy, + deferredSnapshot, + getDeferredHistory: () => deferredHistory, + setDeferredHistory: (nextDeferredHistory) => { + deferredHistory = nextDeferredHistory; + }, + }); options.setState(nextState); return nextState; }, }; } -function shouldRecordHistory(commandId: CommandId, currentState: AppState, nextState: AppState) { - if (commandId === commandIds.historyUndo || commandId === commandIds.historyRedo) return false; +const defaultHistoryPolicy: CommandHistoryPolicy = { mode: "auto" }; + +function applyHistoryPolicy(options: { + currentState: AppState; + nextState: AppState; + historyPolicy: CommandHistoryPolicy; + deferredSnapshot?: HistorySnapshot; + getDeferredHistory: () => { snapshot: HistorySnapshot; changed: boolean } | undefined; + setDeferredHistory: (nextDeferredHistory: { snapshot: HistorySnapshot; changed: boolean } | undefined) => void; +}): AppState { + if (options.historyPolicy.mode === "ignore") { + options.setDeferredHistory(undefined); + return options.nextState; + } + + if (options.historyPolicy.mode === "deferred") { + return applyDeferredHistoryPolicy(options); + } + + return shouldRecordHistory(options.currentState, options.nextState) ? recordHistory(snapshot(options.currentState), options.nextState) : options.nextState; +} + +function applyDeferredHistoryPolicy(options: { + currentState: AppState; + nextState: AppState; + historyPolicy: Extract; + deferredSnapshot?: HistorySnapshot; + getDeferredHistory: () => { snapshot: HistorySnapshot; changed: boolean } | undefined; + setDeferredHistory: (nextDeferredHistory: { snapshot: HistorySnapshot; changed: boolean } | undefined) => void; +}): AppState { + switch (options.historyPolicy.phase) { + case "begin": + options.setDeferredHistory(options.deferredSnapshot ? { snapshot: options.deferredSnapshot, changed: false } : undefined); + return options.nextState; + case "update": { + const deferredHistory = options.getDeferredHistory(); + if (deferredHistory && options.currentState.document !== options.nextState.document) { + options.setDeferredHistory({ ...deferredHistory, changed: true }); + } + return options.nextState; + } + case "commit": { + const deferredHistory = options.getDeferredHistory(); + options.setDeferredHistory(undefined); + return deferredHistory?.changed ? recordHistory(deferredHistory.snapshot, options.nextState) : options.nextState; + } + } +} + +function shouldRecordHistory(currentState: AppState, nextState: AppState) { return currentState.document !== nextState.document; } -function recordHistory(currentState: AppState, nextState: AppState): AppState { +function recordHistory(historySnapshot: HistorySnapshot, nextState: AppState): AppState { return { ...nextState, history: { - past: [...currentState.history.past, snapshot(currentState)].slice(-100), + past: [...nextState.history.past, historySnapshot].slice(-100), future: [], }, }; diff --git a/commands/history.test.ts b/commands/history.test.ts index b86d44f..2a3a218 100644 --- a/commands/history.test.ts +++ b/commands/history.test.ts @@ -5,8 +5,9 @@ import { documentAddArtboardCommand } from "./document"; import { historyCommands } from "./history"; import { commandIds } from "./ids"; import { createCommandRegistry } from "./registry"; +import { transformCommands } from "./transform"; -const registry = createCommandRegistry([documentAddArtboardCommand, ...historyCommands]); +const registry = createCommandRegistry([documentAddArtboardCommand, ...historyCommands, ...transformCommands]); describe("history commands", () => { test("records document changes and undoes/redoes them", () => { @@ -26,4 +27,81 @@ describe("history commands", () => { expect(store.getState().document.artboards.map((artboard) => artboard.id)).toEqual(["a1"]); }); + + test("records one history entry for a transform drag", () => { + const store = createAppStore(artboardState(), registry); + + store.dispatch(commandIds.transformBegin, { + target: { type: "artboard", id: "a1" }, + handle: "body", + point: { x: 0, y: 0 }, + initialBounds: { x: 0, y: 0, w: 100, h: 80 }, + }); + store.dispatch(commandIds.transformUpdate, { point: { x: 5, y: 10 } }); + store.dispatch(commandIds.transformUpdate, { point: { x: 10, y: 20 } }); + store.dispatch(commandIds.transformUpdate, { point: { x: 15, y: 25 } }); + + expect(store.getState().document.artboards[0]?.bounds).toEqual({ x: 15, y: 25, w: 100, h: 80 }); + expect(store.getState().history.past).toHaveLength(0); + + store.dispatch(commandIds.transformEnd, undefined); + + expect(store.getState().editor.transformSession).toBeUndefined(); + expect(store.getState().history.past).toHaveLength(1); + expect(store.getState().history.past[0]?.document.artboards[0]?.bounds).toEqual({ x: 0, y: 0, w: 100, h: 80 }); + expect(store.getState().history.past[0]?.editor.transformSession).toBeUndefined(); + + store.dispatch(commandIds.historyUndo, undefined); + + expect(store.getState().document.artboards[0]?.bounds).toEqual({ x: 0, y: 0, w: 100, h: 80 }); + expect(store.getState().editor.transformSession).toBeUndefined(); + expect(store.getState().history.future).toHaveLength(1); + }); + + test("keeps direct transform bounds edits normally undoable", () => { + const store = createAppStore(artboardState(), registry); + + store.dispatch(commandIds.transformSetBounds, { target: { type: "artboard", id: "a1" }, bounds: { x: 12, y: 24, w: 120, h: 90 } }); + + expect(store.getState().document.artboards[0]?.bounds).toEqual({ x: 12, y: 24, w: 120, h: 90 }); + expect(store.getState().history.past).toHaveLength(1); + + store.dispatch(commandIds.historyUndo, undefined); + + expect(store.getState().document.artboards[0]?.bounds).toEqual({ x: 0, y: 0, w: 100, h: 80 }); + }); + + test("does not record history for transform update or end without a session", () => { + const store = createAppStore(artboardState(), registry); + + store.dispatch(commandIds.transformUpdate, { point: { x: 10, y: 20 } }); + store.dispatch(commandIds.transformEnd, undefined); + + expect(store.getState().document.artboards[0]?.bounds).toEqual({ x: 0, y: 0, w: 100, h: 80 }); + expect(store.getState().history.past).toHaveLength(0); + expect(store.getState().history.future).toHaveLength(0); + }); + + test("does not record history for a transform session with no document update", () => { + const store = createAppStore(artboardState(), registry); + + store.dispatch(commandIds.transformBegin, { + target: { type: "artboard", id: "a1" }, + handle: "body", + point: { x: 0, y: 0 }, + initialBounds: { x: 0, y: 0, w: 100, h: 80 }, + }); + store.dispatch(commandIds.transformEnd, undefined); + + expect(store.getState().editor.transformSession).toBeUndefined(); + expect(store.getState().document.artboards[0]?.bounds).toEqual({ x: 0, y: 0, w: 100, h: 80 }); + expect(store.getState().history.past).toHaveLength(0); + }); }); + +function artboardState() { + return documentAddArtboardCommand.execute( + { state: createInitialAppState("Test") }, + { id: "a1", name: "Artboard", bounds: { x: 0, y: 0, w: 100, h: 80 } }, + ); +} diff --git a/commands/history.ts b/commands/history.ts index 3e7e9c5..7c08375 100644 --- a/commands/history.ts +++ b/commands/history.ts @@ -4,6 +4,7 @@ import { commandIds } from "./ids"; export const historyUndoCommand: Command = { id: commandIds.historyUndo, name: "Undo", + history: { mode: "ignore" }, execute({ state }) { const previous = state.history.past.at(-1); if (!previous) return state; @@ -23,6 +24,7 @@ export const historyUndoCommand: Command = { export const historyRedoCommand: Command = { id: commandIds.historyRedo, name: "Redo", + history: { mode: "ignore" }, execute({ state }) { const next = state.history.future[0]; if (!next) return state; diff --git a/commands/transform.ts b/commands/transform.ts index 2bcb726..c76e3e2 100644 --- a/commands/transform.ts +++ b/commands/transform.ts @@ -24,6 +24,7 @@ export type TransformSetBoundsPayload = { export const transformBeginCommand: Command = { id: commandIds.transformBegin, name: "Begin transform", + history: { mode: "deferred", phase: "begin" }, execute({ state }, payload) { return { ...state, @@ -43,6 +44,7 @@ export const transformBeginCommand: Command = { export const transformUpdateCommand: Command = { id: commandIds.transformUpdate, name: "Update transform", + history: { mode: "deferred", phase: "update" }, execute({ state }, payload) { const session = state.editor.transformSession; if (!session) return state; @@ -78,6 +80,7 @@ export const transformSetBoundsCommand: Command = { export const transformEndCommand: Command = { id: commandIds.transformEnd, name: "End transform", + history: { mode: "deferred", phase: "commit" }, execute({ state }) { if (!state.editor.transformSession) return state;