feat: fix delta apply to issues (#9830)

packages/element/src/delta.ts

@@ -151,6 +151,16 @@ export class Delta<T> {
     );
   }
 
+  /**
+   * Merges two deltas into a new one.
+   */
+  public static merge<T>(delta1: Delta<T>, delta2: Delta<T>) {
+    return Delta.create(
+      { ...delta1.deleted, ...delta2.deleted },
+      { ...delta1.inserted, ...delta2.inserted },
+    );
+  }
+
   /**
    * Merges deleted and inserted object partials.
    */
@@ -497,6 +507,11 @@ export interface DeltaContainer<T> {
    */
   applyTo(previous: T, ...options: unknown[]): [T, boolean];
 
+  /**
+   * Squashes the current delta with the given one.
+   */
+  squash(delta: DeltaContainer<T>): this;
+
   /**
    * Checks whether all `Delta`s are empty.
    */
@@ -504,7 +519,7 @@ export interface DeltaContainer<T> {
 }
 
 export class AppStateDelta implements DeltaContainer<AppState> {
-  private constructor(public readonly delta: Delta<ObservedAppState>) {}
+  private constructor(public delta: Delta<ObservedAppState>) {}
 
   public static calculate<T extends ObservedAppState>(
     prevAppState: T,
@@ -535,6 +550,11 @@ export class AppStateDelta implements DeltaContainer<AppState> {
     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<SceneElementsMap> {
     const inverseInternal = (deltas: Record<string, Delta<ElementPartial>>) => {
       const inversedDeltas: Record<string, Delta<ElementPartial>> = {};
 
-      for (const [id, delta] of Object.entries(deltas)) {
+      for (const [id, { inserted, deleted }] of Object.entries(deltas)) {
-        inversedDeltas[id] = Delta.create(delta.inserted, delta.deleted);
+        inversedDeltas[id] = Delta.create({ ...inserted }, { ...deleted });
       }
 
       return inversedDeltas;
@@ -1395,6 +1415,42 @@ export class ElementsDelta implements DeltaContainer<SceneElementsMap> {
     }
   }
 
+  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<SceneElementsMap> {
       Array.from(prevElements).filter(([id]) => nextAffectedElements.has(id)),
     );
 
-    // calculate complete deltas for affected elements, and assign them back to all the deltas
+    // calculate complete deltas for affected elements, and squash them back to the current deltas
-    // technically we could do better here if perf. would become an issue
+    this.squash(
-    const { added, removed, updated } = ElementsDelta.calculate(
+      // technically we could do better here if perf. would become an issue
-      prevAffectedElements,
+      ElementsDelta.calculate(prevAffectedElements, nextAffectedElements),
-      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;
   }
 

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

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

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();