First step towards delta based history

Introducing independent change detection for appState and elements

Generalizing object change, cleanup, refactoring, comments, solving typing issues

Shaping increment, change, delta hierarchy

Structural clone of elements

Introducing store and incremental API

Disabling buttons for canvas actions, smaller store and changes improvements

Update history entry based on latest changes, iterate through the stack for visible changes to limit empty commands

Solving concurrency issues, solving (partly) linear element issues,  introducing commitToStore breaking change

Fixing existing tests, updating snapshots

Trying to be smarter on the appstate change detection

Extending collab test, refactoring action / updateScene params, bugfixes

Resetting snapshots

Resetting snapshots

UI / API tests for history - WIP

Changing actions related to the observed appstate to at least update the store snapshot - WIP

Adding skipping of snapshot update flag for most no-breaking changes compatible solution

Ignoring uncomitted elements from local async actions, updating store directly in updateScene

Bound element issues - WIP

excalidraw-app/collab/Collab.tsx

@@ -302,7 +302,6 @@ class Collab extends PureComponent<Props, CollabState> {
 
       this.excalidrawAPI.updateScene({
         elements,
-        commitToHistory: false,
       });
     }
   };
@@ -449,14 +448,12 @@ class Collab extends PureComponent<Props, CollabState> {
         }
         return element;
       });
-      // remove deleted elements from elements array & history to ensure we don't
+      // remove deleted elements from elements array to ensure we don't
       // expose potentially sensitive user data in case user manually deletes
       // existing elements (or clears scene), which would otherwise be persisted
       // to database even if deleted before creating the room.
-      this.excalidrawAPI.history.clear();
       this.excalidrawAPI.updateScene({
         elements,
-        commitToHistory: true,
       });
 
       this.saveCollabRoomToFirebase(getSyncableElements(elements));
@@ -491,9 +488,7 @@ class Collab extends PureComponent<Props, CollabState> {
               this.initializeRoom({ fetchScene: false });
               const remoteElements = decryptedData.payload.elements;
               const reconciledElements = this.reconcileElements(remoteElements);
-              this.handleRemoteSceneUpdate(reconciledElements, {
-                init: true,
-              });
+              this.handleRemoteSceneUpdate(reconciledElements);
               // noop if already resolved via init from firebase
               scenePromise.resolve({
                 elements: reconciledElements,
@@ -649,21 +644,11 @@ class Collab extends PureComponent<Props, CollabState> {
     });
   }, LOAD_IMAGES_TIMEOUT);
 
-  private handleRemoteSceneUpdate = (
-    elements: ReconciledElements,
-    { init = false }: { init?: boolean } = {},
-  ) => {
+  private handleRemoteSceneUpdate = (elements: ReconciledElements) => {
     this.excalidrawAPI.updateScene({
       elements,
-      commitToHistory: !!init,
     });
 
-    // We haven't yet implemented multiplayer undo functionality, so we clear the undo stack
-    // when we receive any messages from another peer. This UX can be pretty rough -- if you
-    // undo, a user makes a change, and then try to redo, your element(s) will be lost. However,
-    // right now we think this is the right tradeoff.
-    this.excalidrawAPI.history.clear();
-
     this.loadImageFiles();
   };
 

excalidraw-app/collab/reconciliation.ts

@@ -19,7 +19,7 @@ const shouldDiscardRemoteElement = (
     // local element is being edited
     (local.id === localAppState.editingElement?.id ||
       local.id === localAppState.resizingElement?.id ||
-      local.id === localAppState.draggingElement?.id ||
+      local.id === localAppState.draggingElement?.id || // Is this still valid? As draggingElement is selection element, which is never part of the elements array
       // local element is newer
       local.version > remote.version ||
       // resolve conflicting edits deterministically by taking the one with

excalidraw-app/data/index.ts

@@ -278,7 +278,7 @@ export const loadScene = async (
     // in the scene database/localStorage, and instead fetch them async
     // from a different database
     files: data.files,
-    commitToHistory: false,
+    commitToStore: false,
   };
 };
 

excalidraw-app/index.tsx

@@ -409,7 +409,7 @@ const ExcalidrawWrapper = () => {
             excalidrawAPI.updateScene({
               ...data.scene,
               ...restore(data.scene, null, null, { repairBindings: true }),
-              commitToHistory: true,
+              commitToStore: true,
             });
           }
         });
@@ -590,6 +590,7 @@ const ExcalidrawWrapper = () => {
           if (didChange) {
             excalidrawAPI.updateScene({
               elements,
+              skipSnapshotUpdate: true,
             });
           }
         }

excalidraw-app/tests/collab.test.tsx

@@ -64,7 +64,7 @@ vi.mock("socket.io-client", () => {
 });
 
 describe("collaboration", () => {
-  it("creating room should reset deleted elements", async () => {
+  it("creating room should reset deleted elements while keeping store snapshot in sync", async () => {
     await render(<ExcalidrawApp />);
     // To update the scene with deleted elements before starting collab
     updateSceneData({
@@ -76,26 +76,43 @@ describe("collaboration", () => {
           isDeleted: true,
         }),
       ],
+      commitToStore: true,
     });
     await waitFor(() => {
+      expect(API.getUndoStack().length).toBe(1);
       expect(h.elements).toEqual([
         expect.objectContaining({ id: "A" }),
         expect.objectContaining({ id: "B", isDeleted: true }),
       ]);
-      expect(API.getStateHistory().length).toBe(1);
+      expect(Array.from(h.store.snapshot.elements.values())).toEqual([
+        expect.objectContaining({ id: "A" }),
+        expect.objectContaining({ id: "B", isDeleted: true }),
+      ]);
     });
     window.collab.startCollaboration(null);
     await waitFor(() => {
+      expect(API.getUndoStack().length).toBe(1);
       expect(h.elements).toEqual([expect.objectContaining({ id: "A" })]);
-      expect(API.getStateHistory().length).toBe(1);
+      // We never delete from the local store as it is used for correct diff calculation
+      expect(Array.from(h.store.snapshot.elements.values())).toEqual([
+        expect.objectContaining({ id: "A" }),
+        expect.objectContaining({ id: "B", isDeleted: true }),
+      ]);
     });
 
     const undoAction = createUndoAction(h.history);
     // noop
     h.app.actionManager.executeAction(undoAction);
+
+    // As it was introduced #2270, undo is a noop here, but we might want to re-enable it,
+    // since inability to undo your own deletions could be a bigger upsetting factor here
     await waitFor(() => {
+      expect(h.history.isUndoStackEmpty).toBeTruthy();
       expect(h.elements).toEqual([expect.objectContaining({ id: "A" })]);
-      expect(API.getStateHistory().length).toBe(1);
+      expect(Array.from(h.store.snapshot.elements.values())).toEqual([
+        expect.objectContaining({ id: "A" }),
+        expect.objectContaining({ id: "B", isDeleted: true }),
+      ]);
     });
   });
 });