fix: restoreElementWithProperties drops "parent" property (#5742)

Co-authored-by: Yosyp Buchma <yo@yosyp.co>
Co-authored-by: dwelle <luzar.david@gmail.com>

src/constants.ts

@@ -212,3 +212,7 @@ export const ELEMENT_READY_TO_ERASE_OPACITY = 20;
 export const COOKIES = {
   AUTH_STATE_COOKIE: "excplus-auth",
 } as const;
+
+/** key containt id of precedeing elemnt id we use in reconciliation during
+ * collaboration */
+export const PRECEDING_ELEMENT_KEY = "__precedingElement__";

src/data/restore.ts

@@ -21,6 +21,7 @@ import {
   DEFAULT_FONT_FAMILY,
   DEFAULT_TEXT_ALIGN,
   DEFAULT_VERTICAL_ALIGN,
+  PRECEDING_ELEMENT_KEY,
   FONT_FAMILY,
 } from "../constants";
 import { getDefaultAppState } from "../appState";
@@ -71,6 +72,8 @@ const restoreElementWithProperties = <
     customData?: ExcalidrawElement["customData"];
     /** @deprecated */
     boundElementIds?: readonly ExcalidrawElement["id"][];
+    /** metadata that may be present in elements during collaboration */
+    [PRECEDING_ELEMENT_KEY]?: string;
   },
   K extends Pick<T, keyof Omit<Required<T>, keyof ExcalidrawElement>>,
 >(
@@ -83,7 +86,9 @@ const restoreElementWithProperties = <
   > &
     Partial<Pick<ExcalidrawElement, "type" | "x" | "y">>,
 ): T => {
-  const base: Pick<T, keyof ExcalidrawElement> = {
+  const base: Pick<T, keyof ExcalidrawElement> & {
+    [PRECEDING_ELEMENT_KEY]?: string;
+  } = {
     type: extra.type || element.type,
     // all elements must have version > 0 so getSceneVersion() will pick up
     // newly added elements
@@ -120,6 +125,10 @@ const restoreElementWithProperties = <
     base.customData = element.customData;
   }
 
+  if (PRECEDING_ELEMENT_KEY in element) {
+    base[PRECEDING_ELEMENT_KEY] = element[PRECEDING_ELEMENT_KEY];
+  }
+
   return {
     ...base,
     ...getNormalizedDimensions(base),

src/excalidraw-app/collab/Portal.tsx

@@ -18,6 +18,7 @@ import throttle from "lodash.throttle";
 import { newElementWith } from "../../element/mutateElement";
 import { BroadcastedExcalidrawElement } from "./reconciliation";
 import { encryptData } from "../../data/encryption";
+import { PRECEDING_ELEMENT_KEY } from "../../constants";
 
 class Portal {
   collab: TCollabClass;
@@ -152,7 +153,7 @@ class Portal {
           acc.push({
             ...element,
             // z-index info for the reconciler
-            parent: idx === 0 ? "^" : elements[idx - 1]?.id,
+            [PRECEDING_ELEMENT_KEY]: idx === 0 ? "^" : elements[idx - 1]?.id,
           });
         }
         return acc;

src/excalidraw-app/collab/reconciliation.ts

@@ -1,3 +1,4 @@
+import { PRECEDING_ELEMENT_KEY } from "../../constants";
 import { ExcalidrawElement } from "../../element/types";
 import { AppState } from "../../types";
 
@@ -6,7 +7,7 @@ export type ReconciledElements = readonly ExcalidrawElement[] & {
 };
 
 export type BroadcastedExcalidrawElement = ExcalidrawElement & {
-  parent?: string;
+  [PRECEDING_ELEMENT_KEY]?: string;
 };
 
 const shouldDiscardRemoteElement = (
@@ -71,8 +72,8 @@ export const reconcileElements = (
     const local = localElementsData[remoteElement.id];
 
     if (shouldDiscardRemoteElement(localAppState, local?.[0], remoteElement)) {
-      if (remoteElement.parent) {
-        delete remoteElement.parent;
+      if (remoteElement[PRECEDING_ELEMENT_KEY]) {
+        delete remoteElement[PRECEDING_ELEMENT_KEY];
       }
 
       continue;
@@ -92,10 +93,12 @@ export const reconcileElements = (
     // parent may not be defined in case the remote client is running an older
     // excalidraw version
     const parent =
-      remoteElement.parent || remoteElements[remoteElementIdx - 1]?.id || null;
+      remoteElement[PRECEDING_ELEMENT_KEY] ||
+      remoteElements[remoteElementIdx - 1]?.id ||
+      null;
 
     if (parent != null) {
-      delete remoteElement.parent;
+      delete remoteElement[PRECEDING_ELEMENT_KEY];
 
       // ^ indicates the element is the first in elements array
       if (parent === "^") {

src/tests/reconciliation.test.ts

@@ -1,4 +1,5 @@
 import { expect } from "chai";
+import { PRECEDING_ELEMENT_KEY } from "../constants";
 import { ExcalidrawElement } from "../element/types";
 import {
   BroadcastedExcalidrawElement,
@@ -13,7 +14,7 @@ type ElementLike = {
   id: string;
   version: number;
   versionNonce: number;
-  parent?: string | null;
+  [PRECEDING_ELEMENT_KEY]?: string | null;
 };
 
 type Cache = Record<string, ExcalidrawElement | undefined>;
@@ -42,7 +43,7 @@ const createElement = (opts: { uid: string } | ElementLike) => {
     id,
     version,
     versionNonce: versionNonce || randomInteger(),
-    parent: parent || null,
+    [PRECEDING_ELEMENT_KEY]: parent || null,
   };
 };
 
@@ -51,16 +52,20 @@ const idsToElements = (
   cache: Cache = {},
 ): readonly ExcalidrawElement[] => {
   return ids.reduce((acc, _uid, idx) => {
-    const { uid, id, version, parent, versionNonce } = createElement(
-      typeof _uid === "string" ? { uid: _uid } : _uid,
-    );
+    const {
+      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,
-      parent,
+      [PRECEDING_ELEMENT_KEY]: parent,
     } as BroadcastedExcalidrawElement;
     // @ts-ignore
     cache[uid] = elem;
@@ -71,7 +76,7 @@ const idsToElements = (
 
 const addParents = (elements: BroadcastedExcalidrawElement[]) => {
   return elements.map((el, idx, els) => {
-    el.parent = els[idx - 1]?.id || "^";
+    el[PRECEDING_ELEMENT_KEY] = els[idx - 1]?.id || "^";
     return el;
   });
 };
@@ -79,7 +84,7 @@ const addParents = (elements: BroadcastedExcalidrawElement[]) => {
 const cleanElements = (elements: ReconciledElements) => {
   return elements.map((el) => {
     // @ts-ignore
-    delete el.parent;
+    delete el[PRECEDING_ELEMENT_KEY];
     // @ts-ignore
     delete el.next;
     // @ts-ignore
@@ -385,13 +390,13 @@ describe("elements reconciliation", () => {
         id: "A",
         version: 1,
         versionNonce: 1,
-        parent: null,
+        [PRECEDING_ELEMENT_KEY]: null,
       },
       {
         id: "B",
         version: 1,
         versionNonce: 1,
-        parent: null,
+        [PRECEDING_ELEMENT_KEY]: null,
       },
     ];
 
@@ -404,13 +409,13 @@ describe("elements reconciliation", () => {
       id: "A",
       version: 1,
       versionNonce: 1,
-      parent: null,
+      [PRECEDING_ELEMENT_KEY]: null,
     };
     const el2 = {
       id: "B",
       version: 1,
       versionNonce: 1,
-      parent: null,
+      [PRECEDING_ELEMENT_KEY]: null,
     };
     testIdentical([el1, el2], [el2, el1], ["A", "B"]);
   });