diff --git a/dev-docs/docs/codebase/transactions.mdx b/dev-docs/docs/codebase/transactions.mdx index 5d6f4727fb..627418358e 100644 --- a/dev-docs/docs/codebase/transactions.mdx +++ b/dev-docs/docs/codebase/transactions.mdx @@ -15,13 +15,13 @@ This system lets many canvas mutations over a time window (for example AI stream - `TransactionLedger`: keeps net mutation state across steps. - `recordStep()`: folds multiple updates into per-element baseline/target/touchedProps. - - `buildSyntheticSnapshots()`: reconciles ledger state with live scene under merge policy and returns `elementsBefore` / `elementsAfter` for history commit. -- `TransactionManager`: thin factory holding the app reference. `create(options?)` returns a `Transaction`. + - `buildSyntheticSnapshots()`: reconciles ledger state with live scene using fixed `live-wins-per-prop` behavior and returns `elementsBefore` / `elementsAfter` for history commit. +- `TransactionManager`: thin factory holding the app reference. `create()` returns a `Transaction`. - `Transaction` (class): - `tx.updateScene(data)`: wraps `app.updateScene` with `captureUpdate: NEVER`. Snapshots elements before/after and records the diff into the ledger. Accumulates appState intent. + - `tx.updateElements({ elements })`: convenience API for partial updates (`[{ id, strokeColor }, { id, x, y }]`). It materializes a next elements array and delegates to `tx.updateScene(...)`. - `tx.commit()`: builds synthetic before/after snapshots from the ledger, calls `store.commitSyntheticIncrement()`, returns a summary. - `tx.cancel()`: finalizes without committing history. -- Merge-policy types (`ConflictWinner`, `ConflictScope`, `TransactionMergePolicy`) and defaults. ### `packages/element/src/store.ts` @@ -58,6 +58,9 @@ const tx = app.transactionManager.create(); tx.updateScene({ elements: updatedElementsA }); tx.updateScene({ elements: updatedElementsB }); +tx.updateElements({ + elements: [{ id: "rect-1", strokeColor: "#f00" }, { id: "rect-2", x: 120 }], +}); tx.commit(); ``` @@ -85,6 +88,6 @@ try { ## Notes - `commit()` and `cancel()` are idempotent (repeated calls return the same summary). -- `updateScene()` throws after `commit()` or `cancel()`. -- Merge policy uses `conflictWinner` + `conflictScope` to resolve live/transaction divergence. +- `updateScene()` / `updateElements()` throw after `commit()` or `cancel()`. +- Element conflict handling is fixed to `live-wins-per-prop`. - Multi-transaction concurrency works naturally — each tx has its own ledger. diff --git a/packages/excalidraw/components/App.tsx b/packages/excalidraw/components/App.tsx index cbde098e68..c6d70f1891 100644 --- a/packages/excalidraw/components/App.tsx +++ b/packages/excalidraw/components/App.tsx @@ -4606,8 +4606,8 @@ class App extends React.Component { }; /** Creates a new transaction for batching mutations into a single undo entry. */ - public createTransaction: TransactionManager["create"] = (options) => { - return this.transactionManager.create(options); + public createTransaction: TransactionManager["create"] = () => { + return this.transactionManager.create(); }; public mutateElement = >( diff --git a/packages/excalidraw/tests/transaction.test.tsx b/packages/excalidraw/tests/transaction.test.tsx index 361d306c88..5729540fdf 100644 --- a/packages/excalidraw/tests/transaction.test.tsx +++ b/packages/excalidraw/tests/transaction.test.tsx @@ -3,15 +3,15 @@ import React from "react"; import { arrayToMap } from "@excalidraw/common"; import { CaptureUpdateAction, newElementWith } from "@excalidraw/element"; -import type { ExcalidrawElement } from "@excalidraw/element/types"; +import type { ElementUpdate } from "@excalidraw/element"; + +import type { + ExcalidrawElement, + OrderedExcalidrawElement, +} from "@excalidraw/element/types"; import { Excalidraw } from "../index"; -import { - TransactionLedger, - DEFAULT_TRANSACTION_MERGE_POLICY, - TRANSACTION_MERGE_MODES, - collectChangedElementIds, -} from "../transaction"; +import { TransactionLedger, collectChangedElementIds } from "../transaction"; import { API } from "./helpers/api"; import { Keyboard } from "./helpers/ui"; @@ -26,15 +26,13 @@ const getElement = (id: string) => const applyElementUpdate = ( id: string, - updates: Partial, + updates: ElementUpdate, captureUpdate: keyof typeof CaptureUpdateAction, ) => { const nextElements = h.app.scene .getElementsIncludingDeleted() .map((element) => - element.id === id - ? (newElementWith(element as any, updates as any) as ExcalidrawElement) - : element, + element.id === id ? newElementWith(element, updates) : element, ); API.updateScene({ @@ -58,6 +56,12 @@ const commitTransaction = (tx: Transaction) => { return summary; }; +const setupCreateTransactionSuite = async () => { + unmountComponent(); + vi.restoreAllMocks(); + await render(); +}; + // --------------------------------------------------------------------------- // TransactionLedger (unit tests — no React render needed) // --------------------------------------------------------------------------- @@ -108,14 +112,13 @@ describe("TransactionLedger", () => { const { elementsBefore, elementsAfter } = ledger.buildSyntheticSnapshots( arrayToMap([created]), - DEFAULT_TRANSACTION_MERGE_POLICY, ); expect(elementsBefore.has(created.id)).toBe(false); expect(elementsAfter.get(created.id)?.strokeColor).toBe("#ff006e"); }); - it("skips conflicting update when policy is live-wins", () => { + it("skips conflicting touched-prop updates and keeps live values", () => { const ledger = new TransactionLedger(); const baseline = API.createElement({ type: "rectangle", @@ -137,171 +140,18 @@ describe("TransactionLedger", () => { const { elementsBefore, elementsAfter } = ledger.buildSyntheticSnapshots( arrayToMap([live]), - DEFAULT_TRANSACTION_MERGE_POLICY, ); - expect(elementsBefore.get(live.id)?.strokeColor).toBe("#3a86ff"); expect(elementsAfter.get(live.id)?.strokeColor).toBe("#3a86ff"); }); - - it("applies conflicting update when policy is transaction-wins", () => { - const ledger = new TransactionLedger(); - const baseline = API.createElement({ - type: "rectangle", - id: "rect-1", - strokeColor: "#000000", - }); - const target = { - ...baseline, - strokeColor: "#ff006e", - version: baseline.version + 1, - }; - const live = { - ...target, - strokeColor: "#3a86ff", - version: target.version + 1, - }; - - ledger.recordStep(arrayToMap([baseline]), arrayToMap([target])); - - const { elementsBefore, elementsAfter } = ledger.buildSyntheticSnapshots( - arrayToMap([live]), - { - ...DEFAULT_TRANSACTION_MERGE_POLICY, - conflictWinner: "transaction", - }, - ); - - expect(elementsBefore.get(live.id)?.strokeColor).toBe("#000000"); - expect(elementsAfter.get(live.id)?.strokeColor).toBe("#ff006e"); - }); - - it("supports all winner/scope combinations for conflicting updates", () => { - const ledger = new TransactionLedger(); - const baseline = API.createElement({ - type: "rectangle", - id: "rect-1", - strokeColor: "#000000", - backgroundColor: "#ffffff", - x: 0, - }); - const target = { - ...baseline, - strokeColor: "#ff006e", - x: 200, - version: baseline.version + 1, - }; - const live = { - ...target, - strokeColor: "#3a86ff", - backgroundColor: "#00f5d4", - version: target.version + 1, - }; - - ledger.recordStep(arrayToMap([baseline]), arrayToMap([target])); - - const liveProp = ledger.buildSyntheticSnapshots( - arrayToMap([live]), - DEFAULT_TRANSACTION_MERGE_POLICY, - ); - expect(liveProp.elementsBefore.get(live.id)?.strokeColor).toBe("#3a86ff"); - expect(liveProp.elementsAfter.get(live.id)?.strokeColor).toBe("#3a86ff"); - expect(liveProp.elementsBefore.get(live.id)?.x).toBe(0); - expect(liveProp.elementsAfter.get(live.id)?.x).toBe(200); - expect(liveProp.elementsBefore.get(live.id)?.backgroundColor).toBe( - "#00f5d4", - ); - expect(liveProp.elementsAfter.get(live.id)?.backgroundColor).toBe( - "#00f5d4", - ); - - const liveElement = ledger.buildSyntheticSnapshots(arrayToMap([live]), { - conflictWinner: "live", - conflictScope: "element", - }); - expect(liveElement.elementsBefore.get(live.id)?.strokeColor).toBe("#3a86ff"); - expect(liveElement.elementsAfter.get(live.id)?.strokeColor).toBe("#3a86ff"); - expect(liveElement.elementsBefore.get(live.id)?.x).toBe(200); - expect(liveElement.elementsAfter.get(live.id)?.x).toBe(200); - expect(liveElement.elementsBefore.get(live.id)?.backgroundColor).toBe( - "#00f5d4", - ); - expect(liveElement.elementsAfter.get(live.id)?.backgroundColor).toBe( - "#00f5d4", - ); - - const transactionProp = ledger.buildSyntheticSnapshots(arrayToMap([live]), { - conflictWinner: "transaction", - conflictScope: "prop", - }); - expect(transactionProp.elementsBefore.get(live.id)?.strokeColor).toBe( - "#000000", - ); - expect(transactionProp.elementsAfter.get(live.id)?.strokeColor).toBe( - "#ff006e", - ); - expect(transactionProp.elementsBefore.get(live.id)?.x).toBe(0); - expect(transactionProp.elementsAfter.get(live.id)?.x).toBe(200); - expect(transactionProp.elementsBefore.get(live.id)?.backgroundColor).toBe( - "#00f5d4", - ); - expect(transactionProp.elementsAfter.get(live.id)?.backgroundColor).toBe( - "#00f5d4", - ); - - const transactionElement = ledger.buildSyntheticSnapshots( - arrayToMap([live]), - { - conflictWinner: "transaction", - conflictScope: "element", - }, - ); - expect(transactionElement.elementsBefore.get(live.id)?.strokeColor).toBe( - "#000000", - ); - expect(transactionElement.elementsAfter.get(live.id)?.strokeColor).toBe( - "#ff006e", - ); - expect(transactionElement.elementsBefore.get(live.id)?.x).toBe(0); - expect(transactionElement.elementsAfter.get(live.id)?.x).toBe(200); - expect(transactionElement.elementsBefore.get(live.id)?.backgroundColor).toBe( - "#ffffff", - ); - expect(transactionElement.elementsAfter.get(live.id)?.backgroundColor).toBe( - "#ffffff", - ); - }); - - it("maps merge modes to the same winner/scope policies", () => { - expect(TRANSACTION_MERGE_MODES["live-wins-per-prop"]).toEqual({ - conflictWinner: "live", - conflictScope: "prop", - }); - expect(TRANSACTION_MERGE_MODES["live-wins-per-element"]).toEqual({ - conflictWinner: "live", - conflictScope: "element", - }); - expect(TRANSACTION_MERGE_MODES["transaction-wins-per-prop"]).toEqual({ - conflictWinner: "transaction", - conflictScope: "prop", - }); - expect(TRANSACTION_MERGE_MODES["transaction-wins-per-element"]).toEqual({ - conflictWinner: "transaction", - conflictScope: "element", - }); - }); }); // --------------------------------------------------------------------------- // createTransaction (integration tests — requires full Excalidraw render) // --------------------------------------------------------------------------- -describe("createTransaction", () => { - beforeEach(async () => { - unmountComponent(); - vi.restoreAllMocks(); - await render(); - }); +describe("createTransaction lifecycle", () => { + beforeEach(setupCreateTransactionSuite); it("commits a single undo entry after tx.updateScene() calls", async () => { const element = API.createElement({ @@ -316,14 +166,16 @@ describe("createTransaction", () => { const tx = h.app.createTransaction(); - tx.updateScene({ - elements: h.app.scene - .getElementsIncludingDeleted() - .map((el) => - el.id === element.id - ? newElementWith(el as any, { strokeColor: "#ff006e" } as any) - : el, - ), + act(() => { + tx.updateScene({ + elements: h.app.scene + .getElementsIncludingDeleted() + .map((el) => + el.id === element.id + ? newElementWith(el, { strokeColor: "#ff006e" }) + : el, + ), + }); }); const summary = commitTransaction(tx); @@ -359,7 +211,7 @@ describe("createTransaction", () => { it("throws on updateScene after commit", () => { const tx = h.app.createTransaction(); - tx.commit(); + commitTransaction(tx); expect(() => tx.updateScene({ elements: [] })).toThrow(/already committed/); }); @@ -371,6 +223,116 @@ describe("createTransaction", () => { expect(() => tx.updateScene({ elements: [] })).toThrow(/already canceled/); }); + it("throws on updateElements after commit", () => { + const tx = h.app.createTransaction(); + commitTransaction(tx); + + expect(() => + tx.updateElements({ elements: [{ id: "missing", x: 10 }] }), + ).toThrow(/already committed/); + }); + + it("throws on updateElements after cancel", () => { + const tx = h.app.createTransaction(); + tx.cancel(); + + expect(() => + tx.updateElements({ elements: [{ id: "missing", x: 10 }] }), + ).toThrow(/already canceled/); + }); +}); + +describe("createTransaction updateElements", () => { + beforeEach(setupCreateTransactionSuite); + + it("supports partial element patches via tx.updateElements()", async () => { + const elementA = API.createElement({ + type: "rectangle", + id: "patch-a", + x: 0, + y: 0, + strokeColor: "#000", + backgroundColor: "#fff", + }); + const elementB = API.createElement({ + type: "rectangle", + id: "patch-b", + x: 300, + y: 100, + strokeColor: "#222", + backgroundColor: "#eee", + }); + setSceneBaseline([elementA, elementB]); + + const tx = h.app.createTransaction(); + + act(() => { + tx.updateElements({ + elements: [ + { id: elementA.id, strokeColor: "#f00" }, + { id: elementB.id, x: 420, y: 180 }, + ], + }); + }); + + const summary = commitTransaction(tx); + expect(summary.historyCommitted).toBe(true); + expect(API.getUndoStack().length).toBe(1); + + let liveA = getElement(elementA.id)!; + let liveB = getElement(elementB.id)!; + expect(liveA.strokeColor).toBe("#f00"); + expect(liveA.backgroundColor).toBe(elementA.backgroundColor); + expect(liveA.x).toBe(elementA.x); + expect(liveB.x).toBe(420); + expect(liveB.y).toBe(180); + expect(liveB.strokeColor).toBe(elementB.strokeColor); + + Keyboard.undo(); + await waitFor(() => { + liveA = getElement(elementA.id)!; + liveB = getElement(elementB.id)!; + expect(liveA.strokeColor).toBe(elementA.strokeColor); + expect(liveA.backgroundColor).toBe(elementA.backgroundColor); + expect(liveA.x).toBe(elementA.x); + expect(liveB.x).toBe(elementB.x); + expect(liveB.y).toBe(elementB.y); + expect(liveB.strokeColor).toBe(elementB.strokeColor); + }); + }); + + it("treats updateElements() with unknown ids as a no-op", () => { + const element = API.createElement({ + type: "rectangle", + id: "known", + x: 0, + y: 0, + strokeColor: "#000", + }); + setSceneBaseline([element]); + expect(API.getUndoStack().length).toBe(0); + + const tx = h.app.createTransaction(); + act(() => { + tx.updateElements({ + elements: [{ id: "missing-element-id", x: 999, strokeColor: "#f00" }], + }); + }); + + const summary = commitTransaction(tx); + expect(summary.historyCommitted).toBe(false); + expect(API.getUndoStack().length).toBe(0); + + const live = getElement(element.id)!; + expect(live.x).toBe(element.x); + expect(live.y).toBe(element.y); + expect(live.strokeColor).toBe(element.strokeColor); + }); +}); + +describe("createTransaction appState", () => { + beforeEach(setupCreateTransactionSuite); + it("forwards appState intent to commitSyntheticIncrement", async () => { const element = API.createElement({ type: "rectangle", @@ -384,29 +346,132 @@ describe("createTransaction", () => { const tx = h.app.createTransaction(); - tx.updateScene({ - elements: h.app.scene - .getElementsIncludingDeleted() - .map((el) => - el.id === element.id - ? newElementWith(el as any, { backgroundColor: "#ffbe0b" } as any) - : el, - ), - appState: { selectedElementIds: { [element.id]: true } }, + act(() => { + tx.updateScene({ + elements: h.app.scene + .getElementsIncludingDeleted() + .map((el) => + el.id === element.id + ? newElementWith(el, { backgroundColor: "#ffbe0b" }) + : el, + ), + appState: { selectedElementIds: { [element.id]: true } }, + }); }); - act(() => { - tx.commit(); - }); + tx.commit(); expect(commitSpy).toHaveBeenCalledTimes(1); const call = commitSpy.mock.calls[0]![0]; expect(call.logicalAfter.appState).toBeDefined(); - expect((call.logicalAfter.appState as any).selectedElementIds).toEqual({ + expect(call.logicalAfter.appState?.selectedElementIds).toEqual({ [element.id]: true, }); }); + it("uses resolveAppState output instead of accumulated appState intent", () => { + const element = API.createElement({ + type: "rectangle", + id: "resolver-source", + }); + setSceneBaseline([element]); + + const commitSpy = vi + .spyOn(h.store, "commitSyntheticIncrement") + .mockReturnValue(true); + + const tx = h.app.createTransaction(); + act(() => { + tx.updateScene({ + appState: { selectedElementIds: { [element.id]: true } }, + }); + }); + + const resolverTargetId = "resolver-target"; + let summary!: TransactionSummary; + act(() => { + summary = tx.commit({ + resolveAppState: ({ initial, accumulated, live }) => { + expect(initial.selectedElementIds).toEqual({}); + expect(accumulated.selectedElementIds).toEqual({ + [element.id]: true, + }); + expect(live.selectedElementIds).toEqual({ + [element.id]: true, + }); + return { selectedElementIds: { [resolverTargetId]: true } }; + }, + }); + }); + + expect(summary.historyCommitted).toBe(true); + expect(commitSpy).toHaveBeenCalledTimes(1); + const call = commitSpy.mock.calls[0]![0]; + expect(call.logicalAfter.appState?.selectedElementIds).toEqual({ + [resolverTargetId]: true, + }); + }); + + it("allows resolveAppState to suppress appState-only synthetic history", () => { + const element = API.createElement({ + type: "rectangle", + id: "resolver-suppress", + }); + setSceneBaseline([element]); + + const commitSpy = vi.spyOn(h.store, "commitSyntheticIncrement"); + + const tx = h.app.createTransaction(); + act(() => { + tx.updateScene({ + appState: { selectedElementIds: { [element.id]: true } }, + }); + }); + + let summary!: TransactionSummary; + act(() => { + summary = tx.commit({ + resolveAppState: () => undefined, + }); + }); + + expect(commitSpy).toHaveBeenCalledTimes(1); + expect(summary.historyCommitted).toBe(false); + expect(API.getUndoStack().length).toBe(0); + }); + + it("supports appState-only commit via tx.updateElements()", () => { + const element = API.createElement({ + type: "rectangle", + id: "appstate-only", + }); + setSceneBaseline([element]); + + const commitSpy = vi + .spyOn(h.store, "commitSyntheticIncrement") + .mockReturnValue(true); + + const tx = h.app.createTransaction(); + act(() => { + tx.updateElements({ + elements: [], + appState: { selectedElementIds: { [element.id]: true } }, + }); + }); + + const summary = commitTransaction(tx); + expect(summary.historyCommitted).toBe(true); + expect(commitSpy).toHaveBeenCalledTimes(1); + const call = commitSpy.mock.calls[0]![0]; + expect(call.logicalAfter.appState?.selectedElementIds).toEqual({ + [element.id]: true, + }); + }); +}); + +describe("createTransaction interleaving and undo ordering", () => { + beforeEach(setupCreateTransactionSuite); + it("keeps interleaved user edits and transaction history entries separated", async () => { const transactionElement = API.createElement({ type: "rectangle", @@ -430,18 +495,17 @@ describe("createTransaction", () => { const tx = h.app.createTransaction(); // First tx mutation - tx.updateScene({ - elements: h.app.scene.getElementsIncludingDeleted().map((el) => - el.id === transactionElement.id - ? newElementWith( - el as any, - { + act(() => { + tx.updateScene({ + elements: h.app.scene.getElementsIncludingDeleted().map((el) => + el.id === transactionElement.id + ? newElementWith(el, { x: 180, strokeColor: "#ff006e", - } as any, - ) - : el, - ), + }) + : el, + ), + }); }); // User edit interleaved @@ -452,14 +516,16 @@ describe("createTransaction", () => { ); // Second tx mutation - tx.updateScene({ - elements: h.app.scene - .getElementsIncludingDeleted() - .map((el) => - el.id === transactionElement.id - ? newElementWith(el as any, { opacity: 60 } as any) - : el, - ), + act(() => { + tx.updateScene({ + elements: h.app.scene + .getElementsIncludingDeleted() + .map((el) => + el.id === transactionElement.id + ? newElementWith(el, { opacity: 60 }) + : el, + ), + }); }); // Another user edit @@ -482,9 +548,7 @@ describe("createTransaction", () => { expect(liveUserElement.y).toBe(220); // Undo transaction entry - act(() => { - Keyboard.undo(); - }); + Keyboard.undo(); await waitFor(() => { liveTxElement = getElement(transactionElement.id)!; expect(liveTxElement.x).toBe(transactionElement.x); @@ -496,9 +560,7 @@ describe("createTransaction", () => { expect(liveUserElement.y).toBe(220); // Undo user edit - act(() => { - Keyboard.undo(); - }); + Keyboard.undo(); await waitFor(() => { liveUserElement = getElement(userElement.id)!; expect(liveUserElement.y).toBe(userElement.y); @@ -506,9 +568,7 @@ describe("createTransaction", () => { }); // Undo another user edit - act(() => { - Keyboard.undo(); - }); + Keyboard.undo(); await waitFor(() => { liveUserElement = getElement(userElement.id)!; expect(liveUserElement.backgroundColor).toBe(userElement.backgroundColor); @@ -536,8 +596,10 @@ describe("createTransaction", () => { const tx = h.app.createTransaction(); - tx.updateScene({ - elements: [...h.app.scene.getElementsIncludingDeleted(), txCreated], + act(() => { + tx.updateScene({ + elements: [...h.app.scene.getElementsIncludingDeleted(), txCreated], + }); }); applyElementUpdate(base.id, { x: 120 }, "IMMEDIATELY"); @@ -551,17 +613,13 @@ describe("createTransaction", () => { }); expect(getElement(txCreated.id)).not.toBeNull(); - act(() => { - Keyboard.undo(); - }); + Keyboard.undo(); await waitFor(() => { expect(getElement(txCreated.id)).toBeNull(); expect(getElement(base.id)?.x).toBe(120); }); - act(() => { - Keyboard.undo(); - }); + Keyboard.undo(); await waitFor(() => { expect(getElement(base.id)?.x).toBe(base.x); }); @@ -582,18 +640,17 @@ describe("createTransaction", () => { const tx = h.app.createTransaction(); - tx.updateScene({ - elements: h.app.scene.getElementsIncludingDeleted().map((el) => - el.id === element.id - ? newElementWith( - el as any, - { + act(() => { + tx.updateScene({ + elements: h.app.scene.getElementsIncludingDeleted().map((el) => + el.id === element.id + ? newElementWith(el, { strokeColor: "#ff006e", x: 200, - } as any, - ) - : el, - ), + }) + : el, + ), + }); }); applyElementUpdate( @@ -615,9 +672,7 @@ describe("createTransaction", () => { expect(live.backgroundColor).toBe("#00f5d4"); // Undo transaction - act(() => { - Keyboard.undo(); - }); + Keyboard.undo(); await waitFor(() => { live = getElement(element.id)!; expect(live.strokeColor).toBe(element.strokeColor); @@ -626,9 +681,7 @@ describe("createTransaction", () => { }); // Undo user edit - act(() => { - Keyboard.undo(); - }); + Keyboard.undo(); await waitFor(() => { live = getElement(element.id)!; expect(live.backgroundColor).toBe(element.backgroundColor); @@ -673,12 +726,8 @@ describe("createTransaction", () => { }); }); -describe("createTransaction merge policy behavior", () => { - beforeEach(async () => { - unmountComponent(); - vi.restoreAllMocks(); - await render(); - }); +describe("createTransaction live-wins-per-prop behavior", () => { + beforeEach(setupCreateTransactionSuite); const setupSamePropertyConflictScenario = () => { const element = API.createElement({ @@ -692,12 +741,7 @@ describe("createTransaction merge policy behavior", () => { setSceneBaseline([element]); expect(API.getUndoStack().length).toBe(0); - const tx = h.app.createTransaction({ - mergePolicy: { - conflictWinner: "live", - conflictScope: "prop", - }, - }); + const tx = h.app.createTransaction(); act(() => { tx.updateScene({ @@ -721,51 +765,6 @@ describe("createTransaction merge policy behavior", () => { return element; }; - const setupElementScopeConflictScenario = ( - options: - | Parameters[0] - | undefined = undefined, - ) => { - const element = API.createElement({ - type: "rectangle", - id: "shared-element-scope", - x: 0, - y: 0, - strokeColor: "#000", - backgroundColor: "#fff", - }); - setSceneBaseline([element]); - expect(API.getUndoStack().length).toBe(0); - - const tx = h.app.createTransaction(options); - - act(() => { - tx.updateScene({ - elements: h.app.scene.getElementsIncludingDeleted().map((el) => - el.id === element.id - ? newElementWith(el, { - strokeColor: "#f00", - x: 200, - }) - : el, - ), - }); - }); - - // non-conflicting live edit on untouched prop - applyElementUpdate( - element.id, - { backgroundColor: "#00f5d4" }, - "IMMEDIATELY", - ); - // conflicting live edit on touched prop - applyElementUpdate(element.id, { strokeColor: "#f0f" }, "IMMEDIATELY"); - - commitTransaction(tx); - - return element; - }; - it("tx merge strategy for conflicting edits should prefer live values", () => { const element = setupSamePropertyConflictScenario(); @@ -800,52 +799,4 @@ describe("createTransaction merge policy behavior", () => { expect(live.x).toBe(0); expect(live.backgroundColor).toBe("#fff"); }); - - it("mergeMode transaction-wins-per-element commits whole-element tx snapshots on conflict", () => { - const commitSpy = vi - .spyOn(h.store, "commitSyntheticIncrement") - .mockReturnValue(true); - const element = setupElementScopeConflictScenario({ - mergeMode: "transaction-wins-per-element", - }); - - expect(commitSpy).toHaveBeenCalledTimes(1); - const call = commitSpy.mock.calls[0]![0]; - expect(call.logicalBefore.elements.get(element.id)?.strokeColor).toBe( - "#000", - ); - expect(call.logicalBefore.elements.get(element.id)?.x).toBe(0); - expect(call.logicalBefore.elements.get(element.id)?.backgroundColor).toBe( - "#fff", - ); - expect(call.logicalAfter.elements.get(element.id)?.strokeColor).toBe("#f00"); - expect(call.logicalAfter.elements.get(element.id)?.x).toBe(200); - expect(call.logicalAfter.elements.get(element.id)?.backgroundColor).toBe( - "#fff", - ); - }); - - it("mergeMode transaction-wins-per-prop preserves untouched live props", () => { - const commitSpy = vi - .spyOn(h.store, "commitSyntheticIncrement") - .mockReturnValue(true); - const element = setupElementScopeConflictScenario({ - mergeMode: "transaction-wins-per-prop", - }); - - expect(commitSpy).toHaveBeenCalledTimes(1); - const call = commitSpy.mock.calls[0]![0]; - expect(call.logicalBefore.elements.get(element.id)?.strokeColor).toBe( - "#000", - ); - expect(call.logicalBefore.elements.get(element.id)?.x).toBe(0); - expect(call.logicalBefore.elements.get(element.id)?.backgroundColor).toBe( - "#00f5d4", - ); - expect(call.logicalAfter.elements.get(element.id)?.strokeColor).toBe("#f00"); - expect(call.logicalAfter.elements.get(element.id)?.x).toBe(200); - expect(call.logicalAfter.elements.get(element.id)?.backgroundColor).toBe( - "#00f5d4", - ); - }); }); diff --git a/packages/excalidraw/transaction.ts b/packages/excalidraw/transaction.ts index 286ba60b5f..b86a1e8463 100644 --- a/packages/excalidraw/transaction.ts +++ b/packages/excalidraw/transaction.ts @@ -1,5 +1,11 @@ import { randomId } from "@excalidraw/common"; -import { CaptureUpdateAction, deepCopyElement } from "@excalidraw/element"; +import { + CaptureUpdateAction, + deepCopyElement, + newElementWith, +} from "@excalidraw/element"; + +import type { ElementUpdate } from "@excalidraw/element"; import type { Mutable } from "@excalidraw/common/utility-types"; import type { @@ -15,79 +21,6 @@ import type { SceneData, } from "./types"; -// --------------------------------------------------------------------------- -// Ledger types -// --------------------------------------------------------------------------- - -/** - * Which side wins when transaction output and live scene diverge. - * - * - "live": keep what users currently see in the canvas for conflicting changes. - * - "transaction": force transaction changes for conflicting changes. - */ -export type ConflictWinner = "live" | "transaction"; - -/** - * Conflict granularity used by the merge policy. - * - * - "prop": resolve each touched property independently. - * - "element": when any touched property conflicts, resolve at whole-element level. - */ -export type ConflictScope = "prop" | "element"; - -/** - * Merge policy used when building synthetic before/after snapshots. - * - * Four meaningful combinations are supported: - * - live + prop: keep live only for conflicting props, still apply non-conflicting tx props. - * - live + element: if any touched prop conflicts, skip the whole element update. - * - transaction + prop: force tx values for conflicting props, keep untouched live props. - * - transaction + element: if any touched prop conflicts, force the whole tx element. - */ -export type TransactionMergePolicy = { - conflictWinner: ConflictWinner; - conflictScope: ConflictScope; -}; - -/** - * Named merge presets for clearer call-sites. - * - * These names are intentionally explicit to avoid ambiguity around - * winner/scope semantics. - */ -export type TransactionMergeMode = - | "live-wins-per-prop" - | "live-wins-per-element" - | "transaction-wins-per-prop" - | "transaction-wins-per-element"; - -export const DEFAULT_TRANSACTION_MERGE_POLICY: TransactionMergePolicy = { - conflictWinner: "live", - conflictScope: "prop", -}; - -export const TRANSACTION_MERGE_MODES: Record< - TransactionMergeMode, - TransactionMergePolicy -> = { - "live-wins-per-prop": { - conflictWinner: "live", - conflictScope: "prop", - }, - "live-wins-per-element": { - conflictWinner: "live", - conflictScope: "element", - }, - "transaction-wins-per-prop": { - conflictWinner: "transaction", - conflictScope: "prop", - }, - "transaction-wins-per-element": { - conflictWinner: "transaction", - conflictScope: "element", - }, -}; - /** Per-element ledger record captured during a transaction session. */ export type TransactionLedgerEntry = { baselineElement: ExcalidrawElement | null; @@ -214,60 +147,6 @@ export const collectChangedElementIds = ( return [...changedIds]; }; -/** Detects if an updated element has any live-vs-target conflict on touched props. */ -const hasTouchedPropConflict = ( - entry: TransactionLedgerEntry, - liveElement: ExcalidrawElement, - targetElement: ExcalidrawElement, -) => { - for (const prop of entry.touchedProps) { - const liveValue = getElementProp(liveElement, prop); - const targetValue = getElementProp(targetElement, prop); - if (!isLedgerValueEqual(liveValue, targetValue)) { - return true; - } - } - return false; -}; - -export type TransactionCreateOptions = - | { - mergeMode?: TransactionMergeMode; - mergePolicy?: never; - } - | { - mergeMode?: never; - mergePolicy?: Partial; - }; - -const resolveTransactionMergePolicy = ( - options?: TransactionCreateOptions, -): TransactionMergePolicy => { - if (!options) { - return DEFAULT_TRANSACTION_MERGE_POLICY; - } - - // Runtime guard for untyped callers. - if ("mergeMode" in options && "mergePolicy" in options) { - if (options.mergeMode && options.mergePolicy) { - throw new Error( - "Transaction options are ambiguous: pass either mergeMode or mergePolicy, not both.", - ); - } - } - - if ("mergeMode" in options && options.mergeMode) { - return TRANSACTION_MERGE_MODES[options.mergeMode]; - } - - const partialPolicy = - "mergePolicy" in options ? options.mergePolicy : undefined; - return { - ...DEFAULT_TRANSACTION_MERGE_POLICY, - ...partialPolicy, - }; -}; - // --------------------------------------------------------------------------- // TransactionLedger // --------------------------------------------------------------------------- @@ -338,13 +217,10 @@ export class TransactionLedger { } /** - * Builds synthetic element before/after snapshots by reconciling transaction - * targets with current live scene state under the selected merge policy. + * Builds synthetic element before/after snapshots with a fixed + * "live-wins-per-prop" strategy. */ - buildSyntheticSnapshots( - live: ReadonlyMap, - mergePolicy: TransactionMergePolicy, - ) { + buildSyntheticSnapshots(live: ReadonlyMap) { // Shallow copy — untouched elements stay as live references. // Only elements mutated in-place (prop-level updates) are deep-copied below. const elementsBefore = shallowCopySceneMap(live); @@ -358,10 +234,9 @@ export class TransactionLedger { continue; } if ( - mergePolicy.conflictWinner === "live" && - (!liveElement || - liveElement.isDeleted || - collectTouchedProps(targetElement, liveElement).size > 0) + !liveElement || + liveElement.isDeleted || + collectTouchedProps(targetElement, liveElement).size > 0 ) { continue; } @@ -375,11 +250,7 @@ export class TransactionLedger { if (!entry.targetElement) { const liveElement = live.get(elementId) ?? null; - if ( - mergePolicy.conflictWinner === "live" && - liveElement && - !liveElement.isDeleted - ) { + if (liveElement && !liveElement.isDeleted) { continue; } elementsBefore.set( @@ -409,7 +280,7 @@ export class TransactionLedger { if (entry.touchedProps.has("*")) { const hasLiveConflict = collectTouchedProps(targetElement, liveElement).size > 0; - if (mergePolicy.conflictWinner === "live" && hasLiveConflict) { + if (hasLiveConflict) { continue; } elementsBefore.set( @@ -423,32 +294,6 @@ export class TransactionLedger { continue; } - const hasElementConflict = hasTouchedPropConflict( - entry, - liveElement, - targetElement, - ); - - if (hasElementConflict && mergePolicy.conflictScope === "element") { - if (mergePolicy.conflictWinner === "live") { - // Example: user changed strokeColor while tx changed strokeColor+x. - // "live + element" keeps the whole live element, including x. - continue; - } - - // Example: same conflict as above but "transaction + element". - // This applies tx target for the whole element, including untouched props. - elementsBefore.set( - elementId, - deepCopyElement(baselineElement) as OrderedExcalidrawElement, - ); - elementsAfter.set( - elementId, - deepCopyElement(targetElement) as OrderedExcalidrawElement, - ); - continue; - } - // Deep-copy before mutating so we never touch live elements. const clonedBefore = deepCopyElement( beforeElement, @@ -467,7 +312,7 @@ export class TransactionLedger { const liveValue = getElementProp(liveElement, prop); const targetValue = getElementProp(targetElement, prop); const hasConflict = !isLedgerValueEqual(liveValue, targetValue); - if (mergePolicy.conflictWinner === "live" && hasConflict) { + if (hasConflict) { continue; } @@ -499,6 +344,11 @@ export class TransactionLedger { /** Lifecycle state of a transaction. */ export type TransactionStatus = "active" | "committed" | "canceled"; +/** Per-element partial patch used by tx.updateElements(). */ +export type TransactionElementUpdate = { + id: ExcalidrawElement["id"]; +} & ElementUpdate; + /** Final summary returned when a transaction is committed or canceled. */ export type TransactionSummary = { id: string; @@ -563,7 +413,6 @@ export class Transaction { public readonly id = `tx-${randomId()}`; private readonly app: AppClassProperties; - private readonly mergePolicy: TransactionMergePolicy; private readonly ledger = new TransactionLedger(); private readonly initialAppState: Partial; @@ -571,12 +420,8 @@ export class Transaction { private statusValue: TransactionStatus = "active"; private cachedSummary: TransactionSummary | null = null; - constructor( - app: AppClassProperties, - options?: TransactionCreateOptions, - ) { + constructor(app: AppClassProperties) { this.app = app; - this.mergePolicy = resolveTransactionMergePolicy(options); this.initialAppState = { ...app.store.snapshot.appState }; } @@ -625,6 +470,46 @@ export class Transaction { } } + /** + * Partial element updates convenience API. + * + * Example: + * tx.updateElements({ + * elements: [{ id: "a", strokeColor: "#f00" }, { id: "b", x: 10, y: 20 }], + * }) + */ + updateElements(data: { + elements: readonly TransactionElementUpdate[]; + appState?: Pick | null; + }): void { + const updatesById = new Map< + string, + ElementUpdate + >(); + + for (const update of data.elements) { + const { id, ...partialUpdate } = update; + updatesById.set(id, partialUpdate); + } + + if (updatesById.size === 0) { + this.updateScene({ appState: data.appState }); + return; + } + + const nextElements = this.app.scene + .getElementsIncludingDeleted() + .map((element) => { + const partialUpdate = updatesById.get(element.id); + return partialUpdate ? newElementWith(element, partialUpdate) : element; + }); + + this.updateScene({ + elements: nextElements, + appState: data.appState, + }); + } + commit(options?: { /** * Resolver that determines which appState changes are recorded in the @@ -659,7 +544,7 @@ export class Transaction { if (this.statusValue === "committed" && hasWork) { const liveMap = this.app.scene.getElementsMapIncludingDeleted(); const { elementsBefore, elementsAfter } = - this.ledger.buildSyntheticSnapshots(liveMap, this.mergePolicy); + this.ledger.buildSyntheticSnapshots(liveMap); // Resolve appState for the history entry. const hasAccumulatedAppState = @@ -733,7 +618,7 @@ export class TransactionManager { this.app = app; } - create(options?: TransactionCreateOptions): Transaction { - return new Transaction(this.app, options); + create(): Transaction { + return new Transaction(this.app); } }