Quellcode durchsuchen

fix: utils leaking Scene state (#6461

* fix: utils leaking Scene state

* remove debug

* doc

* add tests for group duplicating

* tweaks
David Luzar vor 2 Jahren
Ursprung
Commit
399c92d882

+ 337 - 62
src/element/newElement.test.ts

@@ -1,8 +1,9 @@
-import { duplicateElement } from "./newElement";
+import { duplicateElement, duplicateElements } from "./newElement";
 import { mutateElement } from "./mutateElement";
 import { API } from "../tests/helpers/api";
 import { FONT_FAMILY, ROUNDNESS } from "../constants";
 import { isPrimitive } from "../utils";
+import { ExcalidrawLinearElement } from "./types";
 
 const assertCloneObjects = (source: any, clone: any) => {
   for (const key in clone) {
@@ -15,79 +16,353 @@ const assertCloneObjects = (source: any, clone: any) => {
   }
 };
 
-it("clones arrow element", () => {
-  const element = API.createElement({
-    type: "arrow",
-    x: 0,
-    y: 0,
-    strokeColor: "#000000",
-    backgroundColor: "transparent",
-    fillStyle: "hachure",
-    strokeWidth: 1,
-    strokeStyle: "solid",
-    roundness: { type: ROUNDNESS.PROPORTIONAL_RADIUS },
-    roughness: 1,
-    opacity: 100,
-  });
+describe("duplicating single elements", () => {
+  it("clones arrow element", () => {
+    const element = API.createElement({
+      type: "arrow",
+      x: 0,
+      y: 0,
+      strokeColor: "#000000",
+      backgroundColor: "transparent",
+      fillStyle: "hachure",
+      strokeWidth: 1,
+      strokeStyle: "solid",
+      roundness: { type: ROUNDNESS.PROPORTIONAL_RADIUS },
+      roughness: 1,
+      opacity: 100,
+    });
+
+    // @ts-ignore
+    element.__proto__ = { hello: "world" };
+
+    mutateElement(element, {
+      points: [
+        [1, 2],
+        [3, 4],
+      ],
+    });
+
+    const copy = duplicateElement(null, new Map(), element);
+
+    assertCloneObjects(element, copy);
 
-  // @ts-ignore
-  element.__proto__ = { hello: "world" };
+    // assert we clone the object's prototype
+    // @ts-ignore
+    expect(copy.__proto__).toEqual({ hello: "world" });
+    expect(copy.hasOwnProperty("hello")).toBe(false);
 
-  mutateElement(element, {
-    points: [
-      [1, 2],
-      [3, 4],
-    ],
+    expect(copy.points).not.toBe(element.points);
+    expect(copy).not.toHaveProperty("shape");
+    expect(copy.id).not.toBe(element.id);
+    expect(typeof copy.id).toBe("string");
+    expect(copy.seed).not.toBe(element.seed);
+    expect(typeof copy.seed).toBe("number");
+    expect(copy).toEqual({
+      ...element,
+      id: copy.id,
+      seed: copy.seed,
+    });
   });
 
-  const copy = duplicateElement(null, new Map(), element);
+  it("clones text element", () => {
+    const element = API.createElement({
+      type: "text",
+      x: 0,
+      y: 0,
+      strokeColor: "#000000",
+      backgroundColor: "transparent",
+      fillStyle: "hachure",
+      strokeWidth: 1,
+      strokeStyle: "solid",
+      roundness: null,
+      roughness: 1,
+      opacity: 100,
+      text: "hello",
+      fontSize: 20,
+      fontFamily: FONT_FAMILY.Virgil,
+      textAlign: "left",
+      verticalAlign: "top",
+    });
 
-  assertCloneObjects(element, copy);
+    const copy = duplicateElement(null, new Map(), element);
 
-  // @ts-ignore
-  expect(copy.__proto__).toEqual({ hello: "world" });
-  expect(copy.hasOwnProperty("hello")).toBe(false);
+    assertCloneObjects(element, copy);
 
-  expect(copy.points).not.toBe(element.points);
-  expect(copy).not.toHaveProperty("shape");
-  expect(copy.id).not.toBe(element.id);
-  expect(typeof copy.id).toBe("string");
-  expect(copy.seed).not.toBe(element.seed);
-  expect(typeof copy.seed).toBe("number");
-  expect(copy).toEqual({
-    ...element,
-    id: copy.id,
-    seed: copy.seed,
+    expect(copy).not.toHaveProperty("points");
+    expect(copy).not.toHaveProperty("shape");
+    expect(copy.id).not.toBe(element.id);
+    expect(typeof copy.id).toBe("string");
+    expect(typeof copy.seed).toBe("number");
   });
 });
 
-it("clones text element", () => {
-  const element = API.createElement({
-    type: "text",
-    x: 0,
-    y: 0,
-    strokeColor: "#000000",
-    backgroundColor: "transparent",
-    fillStyle: "hachure",
-    strokeWidth: 1,
-    strokeStyle: "solid",
-    roundness: null,
-    roughness: 1,
-    opacity: 100,
-    text: "hello",
-    fontSize: 20,
-    fontFamily: FONT_FAMILY.Virgil,
-    textAlign: "left",
-    verticalAlign: "top",
+describe("duplicating multiple elements", () => {
+  it("duplicateElements should clone bindings", () => {
+    const rectangle1 = API.createElement({
+      type: "rectangle",
+      id: "rectangle1",
+      boundElements: [
+        { id: "arrow1", type: "arrow" },
+        { id: "arrow2", type: "arrow" },
+        { id: "text1", type: "text" },
+      ],
+    });
+
+    const text1 = API.createElement({
+      type: "text",
+      id: "text1",
+      containerId: "rectangle1",
+    });
+
+    const arrow1 = API.createElement({
+      type: "arrow",
+      id: "arrow1",
+      startBinding: {
+        elementId: "rectangle1",
+        focus: 0.2,
+        gap: 7,
+      },
+    });
+
+    const arrow2 = API.createElement({
+      type: "arrow",
+      id: "arrow2",
+      endBinding: {
+        elementId: "rectangle1",
+        focus: 0.2,
+        gap: 7,
+      },
+      boundElements: [{ id: "text2", type: "text" }],
+    });
+
+    const text2 = API.createElement({
+      type: "text",
+      id: "text2",
+      containerId: "arrow2",
+    });
+
+    // -------------------------------------------------------------------------
+
+    const origElements = [rectangle1, text1, arrow1, arrow2, text2] as const;
+    const clonedElements = duplicateElements(origElements);
+
+    // generic id in-equality checks
+    // --------------------------------------------------------------------------
+    expect(origElements.map((e) => e.type)).toEqual(
+      clonedElements.map((e) => e.type),
+    );
+    origElements.forEach((origElement, idx) => {
+      const clonedElement = clonedElements[idx];
+      expect(origElement).toEqual(
+        expect.objectContaining({
+          id: expect.not.stringMatching(clonedElement.id),
+          type: clonedElement.type,
+        }),
+      );
+      if ("containerId" in origElement) {
+        expect(origElement.containerId).not.toBe(
+          (clonedElement as any).containerId,
+        );
+      }
+      if ("endBinding" in origElement) {
+        if (origElement.endBinding) {
+          expect(origElement.endBinding.elementId).not.toBe(
+            (clonedElement as any).endBinding?.elementId,
+          );
+        } else {
+          expect((clonedElement as any).endBinding).toBeNull();
+        }
+      }
+      if ("startBinding" in origElement) {
+        if (origElement.startBinding) {
+          expect(origElement.startBinding.elementId).not.toBe(
+            (clonedElement as any).startBinding?.elementId,
+          );
+        } else {
+          expect((clonedElement as any).startBinding).toBeNull();
+        }
+      }
+    });
+    // --------------------------------------------------------------------------
+
+    const clonedArrows = clonedElements.filter(
+      (e) => e.type === "arrow",
+    ) as ExcalidrawLinearElement[];
+
+    const [clonedRectangle, clonedText1, , clonedArrow2, clonedArrowLabel] =
+      clonedElements as any as typeof origElements;
+
+    expect(clonedText1.containerId).toBe(clonedRectangle.id);
+    expect(
+      clonedRectangle.boundElements!.find((e) => e.id === clonedText1.id),
+    ).toEqual(
+      expect.objectContaining({
+        id: clonedText1.id,
+        type: clonedText1.type,
+      }),
+    );
+
+    clonedArrows.forEach((arrow) => {
+      // console.log(arrow);
+      expect(
+        clonedRectangle.boundElements!.find((e) => e.id === arrow.id),
+      ).toEqual(
+        expect.objectContaining({
+          id: arrow.id,
+          type: arrow.type,
+        }),
+      );
+
+      if (arrow.endBinding) {
+        expect(arrow.endBinding.elementId).toBe(clonedRectangle.id);
+      }
+      if (arrow.startBinding) {
+        expect(arrow.startBinding.elementId).toBe(clonedRectangle.id);
+      }
+    });
+
+    expect(clonedArrow2.boundElements).toEqual([
+      { type: "text", id: clonedArrowLabel.id },
+    ]);
+    expect(clonedArrowLabel.containerId).toBe(clonedArrow2.id);
+  });
+
+  it("should remove id references of elements that aren't found", () => {
+    const rectangle1 = API.createElement({
+      type: "rectangle",
+      id: "rectangle1",
+      boundElements: [
+        // should keep
+        { id: "arrow1", type: "arrow" },
+        // should drop
+        { id: "arrow-not-exists", type: "arrow" },
+        // should drop
+        { id: "text-not-exists", type: "text" },
+      ],
+    });
+
+    const arrow1 = API.createElement({
+      type: "arrow",
+      id: "arrow1",
+      startBinding: {
+        elementId: "rectangle1",
+        focus: 0.2,
+        gap: 7,
+      },
+    });
+
+    const text1 = API.createElement({
+      type: "text",
+      id: "text1",
+      containerId: "rectangle-not-exists",
+    });
+
+    const arrow2 = API.createElement({
+      type: "arrow",
+      id: "arrow2",
+      startBinding: {
+        elementId: "rectangle1",
+        focus: 0.2,
+        gap: 7,
+      },
+      endBinding: {
+        elementId: "rectangle-not-exists",
+        focus: 0.2,
+        gap: 7,
+      },
+    });
+
+    const arrow3 = API.createElement({
+      type: "arrow",
+      id: "arrow2",
+      startBinding: {
+        elementId: "rectangle-not-exists",
+        focus: 0.2,
+        gap: 7,
+      },
+      endBinding: {
+        elementId: "rectangle1",
+        focus: 0.2,
+        gap: 7,
+      },
+    });
+
+    // -------------------------------------------------------------------------
+
+    const origElements = [rectangle1, text1, arrow1, arrow2, arrow3] as const;
+    const clonedElements = duplicateElements(
+      origElements,
+    ) as any as typeof origElements;
+    const [
+      clonedRectangle,
+      clonedText1,
+      clonedArrow1,
+      clonedArrow2,
+      clonedArrow3,
+    ] = clonedElements;
+
+    expect(clonedRectangle.boundElements).toEqual([
+      { id: clonedArrow1.id, type: "arrow" },
+    ]);
+
+    expect(clonedText1.containerId).toBe(null);
+
+    expect(clonedArrow2.startBinding).toEqual({
+      ...arrow2.startBinding,
+      elementId: clonedRectangle.id,
+    });
+    expect(clonedArrow2.endBinding).toBe(null);
+
+    expect(clonedArrow3.startBinding).toBe(null);
+    expect(clonedArrow3.endBinding).toEqual({
+      ...arrow3.endBinding,
+      elementId: clonedRectangle.id,
+    });
   });
 
-  const copy = duplicateElement(null, new Map(), element);
+  describe("should duplicate all group ids", () => {
+    it("should regenerate all group ids and keep them consistent across elements", () => {
+      const rectangle1 = API.createElement({
+        type: "rectangle",
+        groupIds: ["g1"],
+      });
+      const rectangle2 = API.createElement({
+        type: "rectangle",
+        groupIds: ["g2", "g1"],
+      });
+      const rectangle3 = API.createElement({
+        type: "rectangle",
+        groupIds: ["g2", "g1"],
+      });
 
-  assertCloneObjects(element, copy);
+      const origElements = [rectangle1, rectangle2, rectangle3] as const;
+      const clonedElements = duplicateElements(
+        origElements,
+      ) as any as typeof origElements;
+      const [clonedRectangle1, clonedRectangle2, clonedRectangle3] =
+        clonedElements;
 
-  expect(copy).not.toHaveProperty("points");
-  expect(copy).not.toHaveProperty("shape");
-  expect(copy.id).not.toBe(element.id);
-  expect(typeof copy.id).toBe("string");
-  expect(typeof copy.seed).toBe("number");
+      expect(rectangle1.groupIds[0]).not.toBe(clonedRectangle1.groupIds[0]);
+      expect(rectangle2.groupIds[0]).not.toBe(clonedRectangle2.groupIds[0]);
+      expect(rectangle2.groupIds[1]).not.toBe(clonedRectangle2.groupIds[1]);
+
+      expect(clonedRectangle1.groupIds[0]).toBe(clonedRectangle2.groupIds[1]);
+      expect(clonedRectangle2.groupIds[0]).toBe(clonedRectangle3.groupIds[0]);
+      expect(clonedRectangle2.groupIds[1]).toBe(clonedRectangle3.groupIds[1]);
+    });
+
+    it("should keep and regenerate ids of groups even if invalid", () => {
+      // lone element shouldn't be able to be grouped with itself,
+      // but hard to check against in a performant way so we ignore it
+      const rectangle1 = API.createElement({
+        type: "rectangle",
+        groupIds: ["g1"],
+      });
+
+      const [clonedRectangle1] = duplicateElements([rectangle1]);
+
+      expect(typeof clonedRectangle1.groupIds[0]).toBe("string");
+      expect(rectangle1.groupIds[0]).not.toBe(clonedRectangle1.groupIds[0]);
+    });
+  });
 });

+ 152 - 10
src/element/newElement.ts

@@ -13,7 +13,12 @@ import {
   FontFamilyValues,
   ExcalidrawTextContainer,
 } from "../element/types";
-import { getFontString, getUpdatedTimestamp, isTestEnv } from "../utils";
+import {
+  arrayToMap,
+  getFontString,
+  getUpdatedTimestamp,
+  isTestEnv,
+} from "../utils";
 import { randomInteger, randomId } from "../random";
 import { mutateElement, newElementWith } from "./mutateElement";
 import { getNewGroupIdsForDuplication } from "../groups";
@@ -357,16 +362,24 @@ export const newImageElement = (
   };
 };
 
-// Simplified deep clone for the purpose of cloning ExcalidrawElement only
-// (doesn't clone Date, RegExp, Map, Set, Typed arrays etc.)
+// Simplified deep clone for the purpose of cloning ExcalidrawElement.
+//
+// Only clones plain objects and arrays. Doesn't clone Date, RegExp, Map, Set,
+// Typed arrays and other non-null objects.
 //
 // Adapted from https://github.com/lukeed/klona
-export const deepCopyElement = (val: any, depth: number = 0) => {
+//
+// The reason for `deepCopyElement()` wrapper is type safety (only allow
+// passing ExcalidrawElement as the top-level argument).
+const _deepCopyElement = (val: any, depth: number = 0) => {
+  // only clone non-primitives
   if (val == null || typeof val !== "object") {
     return val;
   }
 
-  if (Object.prototype.toString.call(val) === "[object Object]") {
+  const objectType = Object.prototype.toString.call(val);
+
+  if (objectType === "[object Object]") {
     const tmp =
       typeof val.constructor === "function"
         ? Object.create(Object.getPrototypeOf(val))
@@ -378,7 +391,7 @@ export const deepCopyElement = (val: any, depth: number = 0) => {
         if (depth === 0 && (key === "shape" || key === "canvas")) {
           continue;
         }
-        tmp[key] = deepCopyElement(val[key], depth + 1);
+        tmp[key] = _deepCopyElement(val[key], depth + 1);
       }
     }
     return tmp;
@@ -388,14 +401,44 @@ export const deepCopyElement = (val: any, depth: number = 0) => {
     let k = val.length;
     const arr = new Array(k);
     while (k--) {
-      arr[k] = deepCopyElement(val[k], depth + 1);
+      arr[k] = _deepCopyElement(val[k], depth + 1);
     }
     return arr;
   }
 
+  // we're not cloning non-array & non-plain-object objects because we
+  // don't support them on excalidraw elements yet. If we do, we need to make
+  // sure we start cloning them, so let's warn about it.
+  if (process.env.NODE_ENV === "development") {
+    if (
+      objectType !== "[object Object]" &&
+      objectType !== "[object Array]" &&
+      objectType.startsWith("[object ")
+    ) {
+      console.warn(
+        `_deepCloneElement: unexpected object type ${objectType}. This value will not be cloned!`,
+      );
+    }
+  }
+
   return val;
 };
 
+/**
+ * Clones ExcalidrawElement data structure. Does not regenerate id, nonce, or
+ * any value. The purpose is to to break object references for immutability
+ * reasons, whenever we want to keep the original element, but ensure it's not
+ * mutated.
+ *
+ * Only clones plain objects and arrays. Doesn't clone Date, RegExp, Map, Set,
+ * Typed arrays and other non-null objects.
+ */
+export const deepCopyElement = <T extends ExcalidrawElement>(
+  val: T,
+): Mutable<T> => {
+  return _deepCopyElement(val);
+};
+
 /**
  * Duplicate an element, often used in the alt-drag operation.
  * Note that this method has gotten a bit complicated since the
@@ -410,13 +453,13 @@ export const deepCopyElement = (val: any, depth: number = 0) => {
  * @param element Element to duplicate
  * @param overrides Any element properties to override
  */
-export const duplicateElement = <TElement extends Mutable<ExcalidrawElement>>(
+export const duplicateElement = <TElement extends ExcalidrawElement>(
   editingGroupId: AppState["editingGroupId"],
   groupIdMapForOperation: Map<GroupId, GroupId>,
   element: TElement,
   overrides?: Partial<TElement>,
-): TElement => {
-  let copy: TElement = deepCopyElement(element);
+): Readonly<TElement> => {
+  let copy = deepCopyElement(element);
 
   if (isTestEnv()) {
     copy.id = `${copy.id}_copy`;
@@ -449,3 +492,102 @@ export const duplicateElement = <TElement extends Mutable<ExcalidrawElement>>(
   }
   return copy;
 };
+
+/**
+ * Clones elements, regenerating their ids (including bindings) and group ids.
+ *
+ * If bindings don't exist in the elements array, they are removed. Therefore,
+ * it's advised to supply the whole elements array, or sets of elements that
+ * are encapsulated (such as library items), if the purpose is to retain
+ * bindings to the cloned elements intact.
+ */
+export const duplicateElements = (elements: readonly ExcalidrawElement[]) => {
+  const clonedElements: ExcalidrawElement[] = [];
+
+  const origElementsMap = arrayToMap(elements);
+
+  // used for for migrating old ids to new ids
+  const elementNewIdsMap = new Map<
+    /* orig */ ExcalidrawElement["id"],
+    /* new */ ExcalidrawElement["id"]
+  >();
+
+  const maybeGetNewId = (id: ExcalidrawElement["id"]) => {
+    // if we've already migrated the element id, return the new one directly
+    if (elementNewIdsMap.has(id)) {
+      return elementNewIdsMap.get(id)!;
+    }
+    // if we haven't migrated the element id, but an old element with the same
+    // id exists, generate a new id for it and return it
+    if (origElementsMap.has(id)) {
+      const newId = randomId();
+      elementNewIdsMap.set(id, newId);
+      return newId;
+    }
+    // if old element doesn't exist, return null to mark it for removal
+    return null;
+  };
+
+  const groupNewIdsMap = new Map</* orig */ GroupId, /* new */ GroupId>();
+
+  for (const element of elements) {
+    const clonedElement: Mutable<ExcalidrawElement> = _deepCopyElement(element);
+
+    clonedElement.id = maybeGetNewId(element.id)!;
+
+    if (clonedElement.groupIds) {
+      clonedElement.groupIds = clonedElement.groupIds.map((groupId) => {
+        if (!groupNewIdsMap.has(groupId)) {
+          groupNewIdsMap.set(groupId, randomId());
+        }
+        return groupNewIdsMap.get(groupId)!;
+      });
+    }
+
+    if ("containerId" in clonedElement && clonedElement.containerId) {
+      const newContainerId = maybeGetNewId(clonedElement.containerId);
+      clonedElement.containerId = newContainerId;
+    }
+
+    if ("boundElements" in clonedElement && clonedElement.boundElements) {
+      clonedElement.boundElements = clonedElement.boundElements.reduce(
+        (
+          acc: Mutable<NonNullable<ExcalidrawElement["boundElements"]>>,
+          binding,
+        ) => {
+          const newBindingId = maybeGetNewId(binding.id);
+          if (newBindingId) {
+            acc.push({ ...binding, id: newBindingId });
+          }
+          return acc;
+        },
+        [],
+      );
+    }
+
+    if ("endBinding" in clonedElement && clonedElement.endBinding) {
+      const newEndBindingId = maybeGetNewId(clonedElement.endBinding.elementId);
+      clonedElement.endBinding = newEndBindingId
+        ? {
+            ...clonedElement.endBinding,
+            elementId: newEndBindingId,
+          }
+        : null;
+    }
+    if ("startBinding" in clonedElement && clonedElement.startBinding) {
+      const newEndBindingId = maybeGetNewId(
+        clonedElement.startBinding.elementId,
+      );
+      clonedElement.startBinding = newEndBindingId
+        ? {
+            ...clonedElement.startBinding,
+            elementId: newEndBindingId,
+          }
+        : null;
+    }
+
+    clonedElements.push(clonedElement);
+  }
+
+  return clonedElements;
+};

+ 39 - 40
src/packages/utils.ts

@@ -15,6 +15,23 @@ import {
   copyToClipboard,
 } from "../clipboard";
 import Scene from "../scene/Scene";
+import { duplicateElements } from "../element/newElement";
+
+// getContainerElement and getBoundTextElement and potentially other helpers
+// depend on `Scene` which will not be available when these pure utils are
+// called outside initialized Excalidraw editor instance or even if called
+// from inside Excalidraw if the elements were never cached by Scene (e.g.
+// for library elements).
+//
+// As such, before passing the elements down, we need to initialize a custom
+// Scene instance and assign them to it.
+//
+// FIXME This is a super hacky workaround and we'll need to rewrite this soon.
+const passElementsSafely = (elements: readonly ExcalidrawElement[]) => {
+  const scene = new Scene();
+  scene.replaceAllElements(duplicateElements(elements));
+  return scene.getNonDeletedElements();
+};
 
 export { MIME_TYPES };
 
@@ -44,17 +61,9 @@ export const exportToCanvas = ({
     null,
     null,
   );
-  // The helper methods getContainerElement and getBoundTextElement are
-  // dependent on Scene which will not be available
-  // when these pure utils are called outside Excalidraw or even if called
-  // from inside Excalidraw when Scene isn't available eg when using library items from store, as a result the element cannot be extracted
-  // hence initailizing a new scene with the elements
-  // so its always available to helper methods
-  const scene = new Scene();
-  scene.replaceAllElements(restoredElements);
   const { exportBackground, viewBackgroundColor } = restoredAppState;
   return _exportToCanvas(
-    scene.getNonDeletedElements(),
+    passElementsSafely(restoredElements),
     { ...restoredAppState, offsetTop: 0, offsetLeft: 0, width: 0, height: 0 },
     files || {},
     { exportBackground, exportPadding, viewBackgroundColor },
@@ -122,17 +131,9 @@ export const exportToBlob = async (
     };
   }
 
-  // The helper methods getContainerElement and getBoundTextElement are
-  // dependent on Scene which will not be available
-  // when these pure utils are called outside Excalidraw or even if called
-  // from inside Excalidraw when Scene isn't available eg when using library items from store, as a result the element cannot be extracted
-  // hence initailizing a new scene with the elements
-  // so its always available to helper methods
-  const scene = new Scene();
-  scene.replaceAllElements(opts.elements);
   const canvas = await exportToCanvas({
     ...opts,
-    elements: scene.getNonDeletedElements(),
+    elements: passElementsSafely(opts.elements),
   });
   quality = quality ? quality : /image\/jpe?g/.test(mimeType) ? 0.92 : 0.8;
 
@@ -150,7 +151,10 @@ export const exportToBlob = async (
           blob = await encodePngMetadata({
             blob,
             metadata: serializeAsJSON(
-              scene.getNonDeletedElements(),
+              // NOTE as long as we're using the Scene hack, we need to ensure
+              // we pass the original, uncloned elements when serializing
+              // so that we keep ids stable
+              opts.elements,
               opts.appState,
               opts.files || {},
               "local",
@@ -178,21 +182,24 @@ export const exportToSvg = async ({
     null,
     null,
   );
-  // The helper methods getContainerElement and getBoundTextElement are
-  // dependent on Scene which will not be available
-  // when these pure utils are called outside Excalidraw or even if called
-  // from inside Excalidraw when Scene isn't available eg when using library items from store, as a result the element cannot be extracted
-  // hence initailizing a new scene with the elements
-  // so its always available to helper methods
-  const scene = new Scene();
-  scene.replaceAllElements(restoredElements);
+
+  const exportAppState = {
+    ...restoredAppState,
+    exportPadding,
+  };
+
   return _exportToSvg(
-    scene.getNonDeletedElements(),
+    passElementsSafely(restoredElements),
+    exportAppState,
+    files,
     {
-      ...restoredAppState,
-      exportPadding,
+      // NOTE as long as we're using the Scene hack, we need to ensure
+      // we pass the original, uncloned elements when serializing
+      // so that we keep ids stable. Hence adding the serializeAsJSON helper
+      // support into the downstream exportToSvg function.
+      serializeAsJSON: () =>
+        serializeAsJSON(restoredElements, exportAppState, files || {}, "local"),
     },
-    files,
   );
 };
 
@@ -203,14 +210,6 @@ export const exportToClipboard = async (
     type: "png" | "svg" | "json";
   },
 ) => {
-  // The helper methods getContainerElement and getBoundTextElement are
-  // dependent on Scene which will not be available
-  // when these pure utils are called outside Excalidraw or even if called
-  // from inside Excalidraw when Scene isn't available eg when using library items from store, as a result the element cannot be extracted
-  // hence initailizing a new scene with the elements
-  // so its always available to helper methods
-  const scene = new Scene();
-  scene.replaceAllElements(opts.elements);
   if (opts.type === "svg") {
     const svg = await exportToSvg(opts);
     await copyTextToSystemClipboard(svg.outerHTML);
@@ -225,7 +224,7 @@ export const exportToClipboard = async (
       ...getDefaultAppState(),
       ...opts.appState,
     };
-    await copyToClipboard(scene.getNonDeletedElements(), appState, opts.files);
+    await copyToClipboard(opts.elements, appState, opts.files);
   } else {
     throw new Error("Invalid export type");
   }

+ 6 - 1
src/scene/export.ts

@@ -90,6 +90,9 @@ export const exportToSvg = async (
     exportEmbedScene?: boolean;
   },
   files: BinaryFiles | null,
+  opts?: {
+    serializeAsJSON?: () => string;
+  },
 ): Promise<SVGSVGElement> => {
   const {
     exportPadding = DEFAULT_EXPORT_PADDING,
@@ -103,7 +106,9 @@ export const exportToSvg = async (
       metadata = await (
         await import(/* webpackChunkName: "image" */ "../../src/data/image")
       ).encodeSvgMetadata({
-        text: serializeAsJSON(elements, appState, files || {}, "local"),
+        text: opts?.serializeAsJSON
+          ? opts?.serializeAsJSON?.()
+          : serializeAsJSON(elements, appState, files || {}, "local"),
       });
     } catch (error: any) {
       console.error(error);

+ 67 - 0
src/tests/packages/utils.unmocked.test.ts

@@ -0,0 +1,67 @@
+import { decodePngMetadata, decodeSvgMetadata } from "../../data/image";
+import { ImportedDataState } from "../../data/types";
+import * as utils from "../../packages/utils";
+import { API } from "../helpers/api";
+
+// NOTE this test file is using the actual API, unmocked. Hence splitting it
+// from the other test file, because I couldn't figure out how to test
+// mocked and unmocked API in the same file.
+
+describe("embedding scene data", () => {
+  describe("exportToSvg", () => {
+    it("embedding scene data shouldn't modify them", async () => {
+      const rectangle = API.createElement({ type: "rectangle" });
+      const ellipse = API.createElement({ type: "ellipse" });
+
+      const sourceElements = [rectangle, ellipse];
+
+      const svgNode = await utils.exportToSvg({
+        elements: sourceElements,
+        appState: {
+          viewBackgroundColor: "#ffffff",
+          gridSize: null,
+          exportEmbedScene: true,
+        },
+        files: null,
+      });
+
+      const svg = svgNode.outerHTML;
+
+      const parsedString = await decodeSvgMetadata({ svg });
+      const importedData: ImportedDataState = JSON.parse(parsedString);
+
+      expect(sourceElements.map((x) => x.id)).toEqual(
+        importedData.elements?.map((el) => el.id),
+      );
+    });
+  });
+
+  // skipped because we can't test png encoding right now
+  // (canvas.toBlob not supported in jsdom)
+  describe.skip("exportToBlob", () => {
+    it("embedding scene data shouldn't modify them", async () => {
+      const rectangle = API.createElement({ type: "rectangle" });
+      const ellipse = API.createElement({ type: "ellipse" });
+
+      const sourceElements = [rectangle, ellipse];
+
+      const blob = await utils.exportToBlob({
+        mimeType: "image/png",
+        elements: sourceElements,
+        appState: {
+          viewBackgroundColor: "#ffffff",
+          gridSize: null,
+          exportEmbedScene: true,
+        },
+        files: null,
+      });
+
+      const parsedString = await decodePngMetadata(blob);
+      const importedData: ImportedDataState = JSON.parse(parsedString);
+
+      expect(sourceElements.map((x) => x.id)).toEqual(
+        importedData.elements?.map((el) => el.id),
+      );
+    });
+  });
+});