factor reconcilation out of updateScene & remove replaceAll (#2266)

Co-authored-by: dwelle <luzar.david@gmail.com>

src/components/App.tsx

@@ -15,7 +15,6 @@ import {
   getCursorForResizingElement,
   getPerfectElementSize,
   getNormalizedDimensions,
-  getElementMap,
   getSceneVersion,
   getSyncableElements,
   newLinearElement,
@@ -1285,8 +1284,15 @@ class App extends React.Component<ExcalidrawProps, AppState> {
     if (init || initFromSnapshot) {
       this.setScrollToCenter(elements);
     }
+    const newElements = this.portal.reconcileElements(elements);
 
-    this.updateScene({ elements });
+    // Avoid broadcasting to the rest of the collaborators the scene
+    // we just received!
+    // Note: this needs to be set before updating the scene as it
+    // syncronously calls render.
+    this.setLastBroadcastedOrReceivedSceneVersion(getSceneVersion(newElements));
+
+    this.updateScene({ elements: newElements });
 
     if (!this.portal.socketInitialized && !initFromSnapshot) {
       this.initializeSocket();
@@ -1301,94 +1307,27 @@ class App extends React.Component<ExcalidrawProps, AppState> {
     this.portal.close();
   };
 
-  public updateScene = (
+  public updateScene = withBatchedUpdates(
-    sceneData: {
+    (sceneData: {
       elements: readonly ExcalidrawElement[];
       appState?: AppState;
-    },
+    }) => {
-    { replaceAll = false }: { replaceAll?: boolean } = {},
+      // currently we only support syncing background color
-  ) => {
+      if (sceneData.appState?.viewBackgroundColor) {
-    // currently we only support syncing background color
+        this.setState({
-    if (sceneData.appState?.viewBackgroundColor) {
+          viewBackgroundColor: sceneData.appState.viewBackgroundColor,
-      this.setState({
+        });
-        viewBackgroundColor: sceneData.appState.viewBackgroundColor,
+      }
-      });
+
-    }
-    // Perform reconciliation - in collaboration, if we encounter
-    // elements with more staler versions than ours, ignore them
-    // and keep ours.
-    const currentElements = this.scene.getElementsIncludingDeleted();
-    if (replaceAll || !currentElements.length) {
       this.scene.replaceAllElements(sceneData.elements);
-    } else {
-      // create a map of ids so we don't have to iterate
-      // over the array more than once.
-      const localElementMap = getElementMap(currentElements);
 
-      // Reconcile
+      // We haven't yet implemented multiplayer undo functionality, so we clear the undo stack
-      const newElements = sceneData.elements
+      // when we receive any messages from another peer. This UX can be pretty rough -- if you
-        .reduce((elements, element) => {
+      // undo, a user makes a change, and then try to redo, your element(s) will be lost. However,
-          // if the remote element references one that's currently
+      // right now we think this is the right tradeoff.
-          //  edited on local, skip it (it'll be added in the next
+      history.clear();
-          //  step)
+    },
-          if (
+  );
-            element.id === this.state.editingElement?.id ||
-            element.id === this.state.resizingElement?.id ||
-            element.id === this.state.draggingElement?.id
-          ) {
-            return elements;
-          }
-
-          if (
-            localElementMap.hasOwnProperty(element.id) &&
-            localElementMap[element.id].version > element.version
-          ) {
-            elements.push(localElementMap[element.id]);
-            delete localElementMap[element.id];
-          } else if (
-            localElementMap.hasOwnProperty(element.id) &&
-            localElementMap[element.id].version === element.version &&
-            localElementMap[element.id].versionNonce !== element.versionNonce
-          ) {
-            // resolve conflicting edits deterministically by taking the one with the lowest versionNonce
-            if (
-              localElementMap[element.id].versionNonce < element.versionNonce
-            ) {
-              elements.push(localElementMap[element.id]);
-            } else {
-              // it should be highly unlikely that the two versionNonces are the same. if we are
-              // really worried about this, we can replace the versionNonce with the socket id.
-              elements.push(element);
-            }
-            delete localElementMap[element.id];
-          } else {
-            elements.push(element);
-            delete localElementMap[element.id];
-          }
-
-          return elements;
-        }, [] as Mutable<typeof sceneData.elements>)
-        // add local elements that weren't deleted or on remote
-        .concat(...Object.values(localElementMap));
-
-      // Avoid broadcasting to the rest of the collaborators the scene
-      // we just received!
-      // Note: this needs to be set before replaceAllElements as it
-      // syncronously calls render.
-
-      this.setLastBroadcastedOrReceivedSceneVersion(
-        getSceneVersion(newElements),
-      );
-
-      this.scene.replaceAllElements(newElements);
-    }
-
-    // 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.
-    history.clear();
-  };
 
   private initializeSocket = () => {
     this.portal.socketInitialized = true;

src/components/Portal.tsx

@@ -3,7 +3,12 @@ import { encryptAESGEM, SocketUpdateDataSource } from "../data";
 import { SocketUpdateData } from "../types";
 import { BROADCAST, SCENE } from "../constants";
 import App from "./App";
-import { getSceneVersion, getSyncableElements } from "../element";
+import {
+  getElementMap,
+  getSceneVersion,
+  getSyncableElements,
+} from "../element";
+import { ExcalidrawElement } from "../element/types";
 
 class Portal {
   app: App;
@@ -151,6 +156,58 @@ class Portal {
       );
     }
   };
+
+  reconcileElements = (sceneElements: readonly ExcalidrawElement[]) => {
+    const currentElements = this.app.getSceneElementsIncludingDeleted();
+    // create a map of ids so we don't have to iterate
+    // over the array more than once.
+    const localElementMap = getElementMap(currentElements);
+
+    // Reconcile
+    const newElements = sceneElements
+      .reduce((elements, element) => {
+        // if the remote element references one that's currently
+        //  edited on local, skip it (it'll be added in the next
+        //  step)
+        if (
+          element.id === this.app.state.editingElement?.id ||
+          element.id === this.app.state.resizingElement?.id ||
+          element.id === this.app.state.draggingElement?.id
+        ) {
+          return elements;
+        }
+
+        if (
+          localElementMap.hasOwnProperty(element.id) &&
+          localElementMap[element.id].version > element.version
+        ) {
+          elements.push(localElementMap[element.id]);
+          delete localElementMap[element.id];
+        } else if (
+          localElementMap.hasOwnProperty(element.id) &&
+          localElementMap[element.id].version === element.version &&
+          localElementMap[element.id].versionNonce !== element.versionNonce
+        ) {
+          // resolve conflicting edits deterministically by taking the one with the lowest versionNonce
+          if (localElementMap[element.id].versionNonce < element.versionNonce) {
+            elements.push(localElementMap[element.id]);
+          } else {
+            // it should be highly unlikely that the two versionNonces are the same. if we are
+            // really worried about this, we can replace the versionNonce with the socket id.
+            elements.push(element);
+          }
+          delete localElementMap[element.id];
+        } else {
+          elements.push(element);
+          delete localElementMap[element.id];
+        }
+
+        return elements;
+      }, [] as Mutable<typeof sceneElements>)
+      // add local elements that weren't deleted or on remote
+      .concat(...Object.values(localElementMap));
+    return newElements;
+  };
 }
 
 export default Portal;