Recalculate the history entry

packages/element/src/delta.ts

@@ -1098,73 +1098,6 @@ export class ElementsDelta implements DeltaContainer<SceneElementsMap> {
     );
   }
 
-  /**
-   * Update delta/s based on the existing elements.
-   *
-   * @param elements current elements
-   * @param modifierOptions defines which of the delta (`deleted` or `inserted`) will be updated
-   * @returns new instance with modified delta/s
-   */
-  public applyLatestChanges(
-    elements: SceneElementsMap,
-    modifierOptions: "deleted" | "inserted",
-  ): ElementsDelta {
-    const modifier =
-      (element: OrderedExcalidrawElement) => (partial: ElementPartial) => {
-        const latestPartial: { [key: string]: unknown } = {};
-
-        for (const key of Object.keys(partial) as Array<keyof typeof partial>) {
-          // do not update following props:
-          // - `version` and `versionNonce`, as they should keep it's original value
-          // - `boundElements`, as it is a reference value which is postprocessed to contain only deleted/inserted keys
-          switch (key) {
-            case "version":
-            case "versionNonce":
-            case "boundElements":
-              latestPartial[key] = partial[key];
-              break;
-            default:
-              latestPartial[key] = element[key];
-          }
-        }
-
-        return latestPartial;
-      };
-
-    const applyLatestChangesInternal = (
-      deltas: Record<string, Delta<ElementPartial>>,
-    ) => {
-      const modifiedDeltas: Record<string, Delta<ElementPartial>> = {};
-
-      for (const [id, delta] of Object.entries(deltas)) {
-        const existingElement = elements.get(id);
-
-        if (existingElement) {
-          const modifiedDelta = Delta.create(
-            delta.deleted,
-            delta.inserted,
-            modifier(existingElement),
-            modifierOptions,
-          );
-
-          modifiedDeltas[id] = modifiedDelta;
-        } else {
-          modifiedDeltas[id] = delta;
-        }
-      }
-
-      return modifiedDeltas;
-    };
-
-    const added = applyLatestChangesInternal(this.added);
-    const removed = applyLatestChangesInternal(this.removed);
-    const updated = applyLatestChangesInternal(this.updated);
-
-    return ElementsDelta.create(added, removed, updated, {
-      shouldRedistribute: true, // redistribute the deltas as `isDeleted` could have been updated
-    });
-  }
-
   public applyTo(
     elements: SceneElementsMap,
     snapshot: StoreSnapshot["elements"] = StoreSnapshot.empty().elements,

packages/element/src/store.ts

@@ -213,16 +213,7 @@ export class Store {
       // using the same instance, since in history we have a check against `HistoryEntry`, so that we don't re-record the same delta again
       storeDelta = delta;
     } else {
-      // calculate the deltas based on the previous and next snapshot
+      storeDelta = StoreDelta.calculate(prevSnapshot, snapshot);
-      const elementsDelta = snapshot.metadata.didElementsChange
-        ? ElementsDelta.calculate(prevSnapshot.elements, snapshot.elements)
-        : ElementsDelta.empty();
-
-      const appStateDelta = snapshot.metadata.didAppStateChange
-        ? AppStateDelta.calculate(prevSnapshot.appState, snapshot.appState)
-        : AppStateDelta.empty();
-
-      storeDelta = StoreDelta.create(elementsDelta, appStateDelta);
     }
 
     if (!storeDelta.isEmpty()) {
@@ -505,6 +496,24 @@ export class StoreDelta {
     return new this(opts.id, elements, appState);
   }
 
+  /**
+   * Calculate the delta between the previous and next snapshot.
+   */
+  public static calculate(
+    prevSnapshot: StoreSnapshot,
+    nextSnapshot: StoreSnapshot,
+  ) {
+    const elementsDelta = nextSnapshot.metadata.didElementsChange
+      ? ElementsDelta.calculate(prevSnapshot.elements, nextSnapshot.elements)
+      : ElementsDelta.empty();
+
+    const appStateDelta = nextSnapshot.metadata.didAppStateChange
+      ? AppStateDelta.calculate(prevSnapshot.appState, nextSnapshot.appState)
+      : AppStateDelta.empty();
+
+    return this.create(elementsDelta, appStateDelta);
+  }
+
   /**
    * Restore a store delta instance from a DTO.
    */
@@ -538,23 +547,6 @@ export class StoreDelta {
     return this.create(delta.elements.inverse(), delta.appState.inverse());
   }
 
-  /**
-   * Apply latest (remote) changes to the delta, creates new instance of `StoreDelta`.
-   */
-  public static applyLatestChanges(
-    delta: StoreDelta,
-    elements: SceneElementsMap,
-    modifierOptions: "deleted" | "inserted",
-  ) {
-    return this.create(
-      delta.elements.applyLatestChanges(elements, modifierOptions),
-      delta.appState,
-      {
-        id: delta.id,
-      },
-    );
-  }
-
   /**
    * Apply the delta to the passed elements and appState, does not modify the snapshot.
    */

packages/excalidraw/history.ts

@@ -13,7 +13,7 @@ import type { SceneElementsMap } from "@excalidraw/element/types";
 
 import type { AppState } from "./types";
 
-class HistoryEntry extends StoreDelta {
+class HistoryDelta extends StoreDelta {
   /**
    * Apply the delta to the passed elements and appState, does not modify the snapshot.
    */
@@ -47,23 +47,18 @@ class HistoryEntry extends StoreDelta {
   /**
    * Overriding once to avoid type casting everywhere.
    */
-  public static override inverse(delta: StoreDelta): HistoryEntry {
+  public static override calculate(
-    return super.inverse(delta) as HistoryEntry;
+    prevSnapshot: StoreSnapshot,
+    nextSnapshot: StoreSnapshot,
+  ) {
+    return super.calculate(prevSnapshot, nextSnapshot) as HistoryDelta;
   }
 
   /**
    * Overriding once to avoid type casting everywhere.
    */
-  public static override applyLatestChanges(
+  public static override inverse(delta: StoreDelta): HistoryDelta {
-    delta: StoreDelta,
+    return super.inverse(delta) as HistoryDelta;
-    elements: SceneElementsMap,
-    modifierOptions: "deleted" | "inserted",
-  ): HistoryEntry {
-    return super.applyLatestChanges(
-      delta,
-      elements,
-      modifierOptions,
-    ) as HistoryEntry;
   }
 }
 
@@ -79,8 +74,8 @@ export class History {
     [HistoryChangedEvent]
   >();
 
-  public readonly undoStack: HistoryEntry[] = [];
+  public readonly undoStack: HistoryDelta[] = [];
-  public readonly redoStack: HistoryEntry[] = [];
+  public readonly redoStack: HistoryDelta[] = [];
 
   public get isUndoStackEmpty() {
     return this.undoStack.length === 0;
@@ -102,16 +97,16 @@ export class History {
    * Do not re-record history entries, which were already pushed to undo / redo stack, as part of history action.
    */
   public record(delta: StoreDelta) {
-    if (delta.isEmpty() || delta instanceof HistoryEntry) {
+    if (delta.isEmpty() || delta instanceof HistoryDelta) {
       return;
     }
 
     // construct history entry, so once it's emitted, it's not recorded again
-    const entry = HistoryEntry.inverse(delta);
+    const historyDelta = HistoryDelta.inverse(delta);
 
-    this.undoStack.push(entry);
+    this.undoStack.push(historyDelta);
 
-    if (!entry.elements.isEmpty()) {
+    if (!historyDelta.elements.isEmpty()) {
       // don't reset redo stack on local appState changes,
       // as a simple click (unselect) could lead to losing all the redo entries
       // only reset on non empty elements changes!
@@ -128,7 +123,7 @@ export class History {
       elements,
       appState,
       () => History.pop(this.undoStack),
-      (entry: HistoryEntry) => History.push(this.redoStack, entry, elements),
+      (entry: HistoryDelta) => History.push(this.redoStack, entry),
     );
   }
 
@@ -137,20 +132,20 @@ export class History {
       elements,
       appState,
       () => History.pop(this.redoStack),
-      (entry: HistoryEntry) => History.push(this.undoStack, entry, elements),
+      (entry: HistoryDelta) => History.push(this.undoStack, entry),
     );
   }
 
   private perform(
     elements: SceneElementsMap,
     appState: AppState,
-    pop: () => HistoryEntry | null,
+    pop: () => HistoryDelta | null,
-    push: (entry: HistoryEntry) => void,
+    push: (entry: HistoryDelta) => void,
   ): [SceneElementsMap, AppState] | void {
     try {
-      let historyEntry = pop();
+      let historyDelta = pop();
 
-      if (historyEntry === null) {
+      if (historyDelta === null) {
         return;
       }
 
@@ -163,10 +158,10 @@ export class History {
       let containsVisibleChange = false;
 
       // iterate through the history entries in case they result in no visible changes
-      while (historyEntry) {
+      while (historyDelta) {
         try {
           [nextElements, nextAppState, containsVisibleChange] =
-            historyEntry.applyTo(nextElements, nextAppState, prevSnapshot);
+            historyDelta.applyTo(nextElements, nextAppState, prevSnapshot);
 
           const nextSnapshot = prevSnapshot.maybeClone(
             action,
@@ -174,24 +169,31 @@ export class History {
             nextAppState,
           );
 
+          const change = StoreChange.create(prevSnapshot, nextSnapshot);
+
+          // update the history entry, so that it's the a new history entry instance
+          // and so that it contains the latest changes in both inserted and deleted partials
+          // including version and versionNonce or properties which were in the meantime updated by a remote client
+          historyDelta = HistoryDelta.calculate(prevSnapshot, nextSnapshot);
+
           // schedule immediate capture, so that it's emitted for the sync purposes
           this.store.scheduleMicroAction({
             action,
-            change: StoreChange.create(prevSnapshot, nextSnapshot),
+            change,
-            delta: historyEntry,
+            delta: historyDelta,
           });
 
           prevSnapshot = nextSnapshot;
         } finally {
           // make sure to always push, even if the delta is corrupted
-          push(historyEntry);
+          push(historyDelta);
         }
 
         if (containsVisibleChange) {
           break;
         }
 
-        historyEntry = pop();
+        historyDelta = pop();
       }
 
       return [nextElements, nextAppState];
@@ -204,7 +206,7 @@ export class History {
     }
   }
 
-  private static pop(stack: HistoryEntry[]): HistoryEntry | null {
+  private static pop(stack: HistoryDelta[]): HistoryDelta | null {
     if (!stack.length) {
       return null;
     }
@@ -218,18 +220,8 @@ export class History {
     return null;
   }
 
-  private static push(
+  private static push(stack: HistoryDelta[], entry: HistoryDelta) {
-    stack: HistoryEntry[],
+    const inversedEntry = HistoryDelta.inverse(entry);
-    entry: HistoryEntry,
+    return stack.push(inversedEntry);
-    prevElements: SceneElementsMap,
-  ) {
-    const inversedEntry = HistoryEntry.inverse(entry);
-    const updatedEntry = HistoryEntry.applyLatestChanges(
-      inversedEntry,
-      prevElements,
-      "inserted",
-    );
-
-    return stack.push(updatedEntry);
   }
 }