From df25de7e6816cccc72187aca7b1b865da7803fca Mon Sep 17 00:00:00 2001 From: Marcel Mraz Date: Thu, 7 Aug 2025 15:38:58 +0200 Subject: [PATCH] feat: fix delta apply to issues (#9830) --- packages/element/src/delta.ts | 83 +++++++++---- packages/element/src/store.ts | 4 +- .../tests/__snapshots__/history.test.tsx.snap | 110 +++++++++++++++++- scripts/release.js | 4 +- 4 files changed, 174 insertions(+), 27 deletions(-) diff --git a/packages/element/src/delta.ts b/packages/element/src/delta.ts index bd428d856..a86b5b523 100644 --- a/packages/element/src/delta.ts +++ b/packages/element/src/delta.ts @@ -151,6 +151,16 @@ export class Delta { ); } + /** + * Merges two deltas into a new one. + */ + public static merge(delta1: Delta, delta2: Delta) { + return Delta.create( + { ...delta1.deleted, ...delta2.deleted }, + { ...delta1.inserted, ...delta2.inserted }, + ); + } + /** * Merges deleted and inserted object partials. */ @@ -497,6 +507,11 @@ export interface DeltaContainer { */ applyTo(previous: T, ...options: unknown[]): [T, boolean]; + /** + * Squashes the current delta with the given one. + */ + squash(delta: DeltaContainer): this; + /** * Checks whether all `Delta`s are empty. */ @@ -504,7 +519,7 @@ export interface DeltaContainer { } export class AppStateDelta implements DeltaContainer { - private constructor(public readonly delta: Delta) {} + private constructor(public delta: Delta) {} public static calculate( prevAppState: T, @@ -535,6 +550,11 @@ export class AppStateDelta implements DeltaContainer { return new AppStateDelta(inversedDelta); } + public squash(delta: AppStateDelta): this { + this.delta = Delta.merge(this.delta, delta.delta); + return this; + } + public applyTo( appState: AppState, nextElements: SceneElementsMap, @@ -1196,8 +1216,8 @@ export class ElementsDelta implements DeltaContainer { const inverseInternal = (deltas: Record>) => { const inversedDeltas: Record> = {}; - for (const [id, delta] of Object.entries(deltas)) { - inversedDeltas[id] = Delta.create(delta.inserted, delta.deleted); + for (const [id, { inserted, deleted }] of Object.entries(deltas)) { + inversedDeltas[id] = Delta.create({ ...inserted }, { ...deleted }); } return inversedDeltas; @@ -1395,6 +1415,42 @@ export class ElementsDelta implements DeltaContainer { } } + public squash(delta: ElementsDelta): this { + const { added, removed, updated } = delta; + + for (const [id, nextDelta] of Object.entries(added)) { + const prevDelta = this.added[id]; + + if (!prevDelta) { + this.added[id] = nextDelta; + } else { + this.added[id] = Delta.merge(prevDelta, nextDelta); + } + } + + for (const [id, nextDelta] of Object.entries(removed)) { + const prevDelta = this.removed[id]; + + if (!prevDelta) { + this.removed[id] = nextDelta; + } else { + this.removed[id] = Delta.merge(prevDelta, nextDelta); + } + } + + for (const [id, nextDelta] of Object.entries(updated)) { + const prevDelta = this.updated[id]; + + if (!prevDelta) { + this.updated[id] = nextDelta; + } else { + this.updated[id] = Delta.merge(prevDelta, nextDelta); + } + } + + return this; + } + private static createApplier = ( nextElements: SceneElementsMap, @@ -1624,25 +1680,12 @@ export class ElementsDelta implements DeltaContainer { Array.from(prevElements).filter(([id]) => nextAffectedElements.has(id)), ); - // calculate complete deltas for affected elements, and assign them back to all the deltas - // technically we could do better here if perf. would become an issue - const { added, removed, updated } = ElementsDelta.calculate( - prevAffectedElements, - nextAffectedElements, + // calculate complete deltas for affected elements, and squash them back to the current deltas + this.squash( + // technically we could do better here if perf. would become an issue + ElementsDelta.calculate(prevAffectedElements, nextAffectedElements), ); - for (const [id, delta] of Object.entries(added)) { - this.added[id] = delta; - } - - for (const [id, delta] of Object.entries(removed)) { - this.removed[id] = delta; - } - - for (const [id, delta] of Object.entries(updated)) { - this.updated[id] = delta; - } - return nextAffectedElements; } diff --git a/packages/element/src/store.ts b/packages/element/src/store.ts index 2bf70f581..56316c5e3 100644 --- a/packages/element/src/store.ts +++ b/packages/element/src/store.ts @@ -76,8 +76,9 @@ type MicroActionsQueue = (() => void)[]; * Store which captures the observed changes and emits them as `StoreIncrement` events. */ export class Store { - // internally used by history + // for internal use by history public readonly onDurableIncrementEmitter = new Emitter<[DurableIncrement]>(); + // for public use as part of onIncrement API public readonly onStoreIncrementEmitter = new Emitter< [DurableIncrement | EphemeralIncrement] >(); @@ -239,7 +240,6 @@ export class Store { if (!storeDelta.isEmpty()) { const increment = new DurableIncrement(storeChange, storeDelta); - // Notify listeners with the increment this.onDurableIncrementEmitter.trigger(increment); this.onStoreIncrementEmitter.trigger(increment); } diff --git a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap b/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap index 70269263d..9d7be4e0d 100644 --- a/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap +++ b/packages/excalidraw/tests/__snapshots__/history.test.tsx.snap @@ -2924,7 +2924,7 @@ exports[`history > multiplayer undo/redo > conflicts in bound text elements and "strokeWidth": 2, "type": "rectangle", "updated": 1, - "version": 7, + "version": 11, "width": 100, "x": 10, "y": 10, @@ -3001,7 +3001,7 @@ exports[`history > multiplayer undo/redo > conflicts in bound text elements and "textAlign": "left", "type": "text", "updated": 1, - "version": 5, + "version": 11, "verticalAlign": "top", "width": 30, "x": 15, @@ -3031,14 +3031,67 @@ exports[`history > multiplayer undo/redo > conflicts in bound text elements and "version": 9, }, "inserted": { + "angle": 0, + "autoResize": true, + "backgroundColor": "transparent", + "boundElements": null, "containerId": null, + "customData": undefined, + "fillStyle": "solid", + "fontFamily": 5, + "fontSize": 20, + "frameId": null, + "groupIds": [], + "height": 100, + "index": "a0", "isDeleted": false, + "lineHeight": "1.25000", + "link": null, + "locked": false, + "opacity": 100, + "originalText": "que pasa", + "roughness": 1, + "roundness": null, + "strokeColor": "#1e1e1e", + "strokeStyle": "solid", + "strokeWidth": 2, + "text": "que pasa", + "textAlign": "left", + "type": "text", "version": 8, + "verticalAlign": "top", + "width": 100, + "x": 15, + "y": 15, }, }, }, "removed": {}, - "updated": {}, + "updated": { + "id0": { + "deleted": { + "boundElements": [], + "version": 11, + }, + "inserted": { + "boundElements": [ + { + "id": "id1", + "type": "text", + }, + ], + "version": 10, + }, + }, + "id5": { + "deleted": { + "version": 11, + }, + "inserted": { + "version": 9, + }, + }, + }, }, "id": "id9", }, @@ -5036,9 +5089,29 @@ exports[`history > multiplayer undo/redo > conflicts in bound text elements and "removed": { "id0": { "deleted": { + "angle": 0, + "backgroundColor": "transparent", "boundElements": [], + "customData": undefined, + "fillStyle": "solid", + "frameId": null, + "groupIds": [], + "height": 100, + "index": "a0", "isDeleted": false, + "link": null, + "locked": false, + "opacity": 100, + "roughness": 1, + "roundness": null, + "strokeColor": "#1e1e1e", + "strokeStyle": "solid", + "strokeWidth": 2, + "type": "rectangle", "version": 8, + "width": 100, + "x": 10, + "y": 10, }, "inserted": { "boundElements": [ @@ -5266,9 +5339,38 @@ exports[`history > multiplayer undo/redo > conflicts in bound text elements and "removed": { "id1": { "deleted": { + "angle": 0, + "autoResize": true, + "backgroundColor": "transparent", + "boundElements": null, "containerId": null, + "customData": undefined, + "fillStyle": "solid", + "fontFamily": 5, + "fontSize": 20, + "frameId": null, + "groupIds": [], + "height": 100, + "index": "a0", "isDeleted": false, + "lineHeight": "1.25000", + "link": null, + "locked": false, + "opacity": 100, + "originalText": "que pasa", + "roughness": 1, + "roundness": null, + "strokeColor": "#1e1e1e", + "strokeStyle": "solid", + "strokeWidth": 2, + "text": "que pasa", + "textAlign": "left", + "type": "text", "version": 8, + "verticalAlign": "top", + "width": 100, + "x": 15, + "y": 15, }, "inserted": { "containerId": "id0", @@ -5525,9 +5627,11 @@ exports[`history > multiplayer undo/redo > conflicts in frames and their childre "updated": { "id1": { "deleted": { + "frameId": null, "version": 10, }, "inserted": { + "frameId": null, "version": 8, }, }, diff --git a/scripts/release.js b/scripts/release.js index 3c7552341..f45afc66d 100644 --- a/scripts/release.js +++ b/scripts/release.js @@ -144,7 +144,7 @@ const askToCommit = (tag, nextVersion) => { }); rl.question( - "Do you want to commit these changes to git? (Y/n): ", + "Would you like to commit these changes to git? (Y/n): ", (answer) => { rl.close(); @@ -189,7 +189,7 @@ const askToPublish = (tag, version) => { }); rl.question( - "Do you want to publish these changes to npm? (Y/n): ", + "Would you like to publish these changes to npm? (Y/n): ", (answer) => { rl.close();