reconciliate order based on fractional index

excalidraw-app/collab/Portal.tsx

@@ -18,7 +18,7 @@ import throttle from "lodash.throttle";
 import { newElementWith } from "../../src/element/mutateElement";
 import { BroadcastedExcalidrawElement } from "./reconciliation";
 import { encryptData } from "../../src/data/encryption";
-import { PRECEDING_ELEMENT_KEY } from "../../src/constants";
+import { normalizeFractionalIndexing } from "../../src/zindex";
 
 class Portal {
   collab: TCollabClass;
@@ -150,11 +150,7 @@ class Portal {
               this.broadcastedElementVersions.get(element.id)!) &&
           isSyncableElement(element)
         ) {
-          acc.push({
+          acc.push(element);
-            ...element,
-            // z-index info for the reconciler
-            [PRECEDING_ELEMENT_KEY]: idx === 0 ? "^" : elements[idx - 1]?.id,
-          });
         }
         return acc;
       },
@@ -164,7 +160,7 @@ class Portal {
     const data: SocketUpdateDataSource[typeof updateType] = {
       type: updateType,
       payload: {
-        elements: syncableElements,
+        elements: normalizeFractionalIndexing(syncableElements),
       },
     };
 

excalidraw-app/collab/reconciliation.ts

@@ -1,15 +1,13 @@
-import { PRECEDING_ELEMENT_KEY } from "../../src/constants";
 import { ExcalidrawElement } from "../../src/element/types";
 import { AppState } from "../../src/types";
-import { arrayToMapWithIndex } from "../../src/utils";
+import { arrayToMap, arrayToMapWithIndex } from "../../src/utils";
+import { orderByFractionalIndex } from "../../src/zindex";
 
 export type ReconciledElements = readonly ExcalidrawElement[] & {
   _brand: "reconciledElements";
 };
 
-export type BroadcastedExcalidrawElement = ExcalidrawElement & {
+export type BroadcastedExcalidrawElement = ExcalidrawElement;
-  [PRECEDING_ELEMENT_KEY]?: string;
-};
 
 const shouldDiscardRemoteElement = (
   localAppState: AppState,
@@ -39,116 +37,43 @@ export const reconcileElements = (
   remoteElements: readonly BroadcastedExcalidrawElement[],
   localAppState: AppState,
 ): ReconciledElements => {
-  const localElementsData =
+  const localElementsData = arrayToMap(localElements);
-    arrayToMapWithIndex<ExcalidrawElement>(localElements);
+  const reconciledElements: ExcalidrawElement[] = [];
+  const added = new Set<string>();
 
-  const reconciledElements: ExcalidrawElement[] = localElements.slice();
+  // process remote elements
-
-  const duplicates = new WeakMap<ExcalidrawElement, true>();
-
-  let cursor = 0;
-  let offset = 0;
-
-  let remoteElementIdx = -1;
   for (const remoteElement of remoteElements) {
-    remoteElementIdx++;
+    if (localElementsData.has(remoteElement.id)) {
+      const localElement = localElementsData.get(remoteElement.id);
 
-    const local = localElementsData.get(remoteElement.id);
+      if (
-
+        localElement &&
-    if (shouldDiscardRemoteElement(localAppState, local?.[0], remoteElement)) {
+        shouldDiscardRemoteElement(localAppState, localElement, remoteElement)
-      if (remoteElement[PRECEDING_ELEMENT_KEY]) {
+      ) {
-        delete remoteElement[PRECEDING_ELEMENT_KEY];
-      }
-
-      continue;
-    }
-
-    // Mark duplicate for removal as it'll be replaced with the remote element
-    if (local) {
-      // Unless the remote and local elements are the same element in which case
-      // we need to keep it as we'd otherwise discard it from the resulting
-      // array.
-      if (local[0] === remoteElement) {
         continue;
-      }
-      duplicates.set(local[0], true);
-    }
-
-    // parent may not be defined in case the remote client is running an older
-    // excalidraw version
-    const parent =
-      remoteElement[PRECEDING_ELEMENT_KEY] ||
-      remoteElements[remoteElementIdx - 1]?.id ||
-      null;
-
-    if (parent != null) {
-      delete remoteElement[PRECEDING_ELEMENT_KEY];
-
-      // ^ indicates the element is the first in elements array
-      if (parent === "^") {
-        offset++;
-        if (cursor === 0) {
-          reconciledElements.unshift(remoteElement);
-          localElementsData.set(remoteElement.id, [
-            remoteElement,
-            cursor - offset,
-          ]);
-        } else {
-          reconciledElements.splice(cursor + 1, 0, remoteElement);
-          localElementsData.set(remoteElement.id, [
-            remoteElement,
-            cursor + 1 - offset,
-          ]);
-          cursor++;
-        }
       } else {
-        let idx = localElementsData.has(parent)
+        if (!added.has(remoteElement.id)) {
-          ? localElementsData.get(parent)![1]
-          : null;
-        if (idx != null) {
-          idx += offset;
-        }
-        if (idx != null && idx >= cursor) {
-          reconciledElements.splice(idx + 1, 0, remoteElement);
-          offset++;
-          localElementsData.set(remoteElement.id, [
-            remoteElement,
-            idx + 1 - offset,
-          ]);
-          cursor = idx + 1;
-        } else if (idx != null) {
-          reconciledElements.splice(cursor + 1, 0, remoteElement);
-          offset++;
-          localElementsData.set(remoteElement.id, [
-            remoteElement,
-            cursor + 1 - offset,
-          ]);
-          cursor++;
-        } else {
           reconciledElements.push(remoteElement);
-          localElementsData.set(remoteElement.id, [
+          added.add(remoteElement.id);
-            remoteElement,
-            reconciledElements.length - 1 - offset,
-          ]);
         }
       }
-      // no parent z-index information, local element exists → replace in place
-    } else if (local) {
-      reconciledElements[local[1]] = remoteElement;
-      localElementsData.set(remoteElement.id, [remoteElement, local[1]]);
-      // otherwise push to the end
     } else {
-      reconciledElements.push(remoteElement);
+      if (!added.has(remoteElement.id)) {
-      localElementsData.set(remoteElement.id, [
+        reconciledElements.push(remoteElement);
-        remoteElement,
+        added.add(remoteElement.id);
-        reconciledElements.length - 1 - offset,
+      }
-      ]);
     }
   }
 
-  const ret: readonly ExcalidrawElement[] = reconciledElements.filter(
+  // process local elements
-    (element) => !duplicates.has(element),
+  for (const localElement of localElements) {
-  );
+    if (!added.has(localElement.id)) {
+      reconciledElements.push(localElement);
+      added.add(localElement.id);
+    }
+  }
 
-  return ret as ReconciledElements;
+  return orderByFractionalIndex(
+    reconciledElements,
+  ) as readonly ExcalidrawElement[] as ReconciledElements;
 };

excalidraw-app/tests/reconciliation.test.ts

@@ -1,5 +1,4 @@
 import { expect } from "chai";
-import { PRECEDING_ELEMENT_KEY } from "../../src/constants";
 import { ExcalidrawElement } from "../../src/element/types";
 import {
   BroadcastedExcalidrawElement,
@@ -15,7 +14,6 @@ type ElementLike = {
   id: string;
   version: number;
   versionNonce: number;
-  [PRECEDING_ELEMENT_KEY]?: string | null;
 };
 
 type Cache = Record<string, ExcalidrawElement | undefined>;
@@ -44,7 +42,6 @@ const createElement = (opts: { uid: string } | ElementLike) => {
     id,
     version,
     versionNonce: versionNonce || randomInteger(),
-    [PRECEDING_ELEMENT_KEY]: parent || null,
   };
 };
 
@@ -53,20 +50,15 @@ const idsToElements = (
   cache: Cache = {},
 ): readonly ExcalidrawElement[] => {
   return ids.reduce((acc, _uid, idx) => {
-    const {
+    const { uid, id, version, versionNonce } = createElement(
-      uid,
+      typeof _uid === "string" ? { uid: _uid } : _uid,
-      id,
+    );
-      version,
-      [PRECEDING_ELEMENT_KEY]: parent,
-      versionNonce,
-    } = createElement(typeof _uid === "string" ? { uid: _uid } : _uid);
     const cached = cache[uid];
     const elem = {
       id,
       version: version ?? 0,
       versionNonce,
       ...cached,
-      [PRECEDING_ELEMENT_KEY]: parent,
     } as BroadcastedExcalidrawElement;
     // @ts-ignore
     cache[uid] = elem;
@@ -77,7 +69,6 @@ const idsToElements = (
 
 const addParents = (elements: BroadcastedExcalidrawElement[]) => {
   return elements.map((el, idx, els) => {
-    el[PRECEDING_ELEMENT_KEY] = els[idx - 1]?.id || "^";
     return el;
   });
 };
@@ -389,13 +380,11 @@ describe("elements reconciliation", () => {
         id: "A",
         version: 1,
         versionNonce: 1,
-        [PRECEDING_ELEMENT_KEY]: null,
       },
       {
         id: "B",
         version: 1,
         versionNonce: 1,
-        [PRECEDING_ELEMENT_KEY]: null,
       },
     ];
 
@@ -408,13 +397,11 @@ describe("elements reconciliation", () => {
       id: "A",
       version: 1,
       versionNonce: 1,
-      [PRECEDING_ELEMENT_KEY]: null,
     };
     const el2 = {
       id: "B",
       version: 1,
       versionNonce: 1,
-      [PRECEDING_ELEMENT_KEY]: null,
     };
     testIdentical([el1, el2], [el2, el1], ["A", "B"]);
   });

src/constants.ts

@@ -302,10 +302,6 @@ export const ROUNDNESS = {
   ADAPTIVE_RADIUS: 3,
 } as const;
 
-/** key containt id of precedeing elemnt id we use in reconciliation during
- * collaboration */
-export const PRECEDING_ELEMENT_KEY = "__precedingElement__";
-
 export const ROUGHNESS = {
   architect: 0,
   artist: 1,