fix: restoring deleted bindings (#6029)

* fix: restoring deleted bindings

* add tests

* add one more test

* merge restore tests files

src/data/restore.ts

@@ -273,6 +273,14 @@ const repairContainerElement = (
       ) => {
         const boundElement = elementsMap.get(binding.id);
         if (boundElement && !boundIds.has(binding.id)) {
+          boundIds.add(binding.id);
+
+          if (boundElement.isDeleted) {
+            return acc;
+          }
+
+          acc.push(binding);
+
           if (
             isTextElement(boundElement) &&
             // being slightly conservative here, preserving existing containerId
@@ -282,9 +290,6 @@ const repairContainerElement = (
             (boundElement as Mutable<ExcalidrawTextElement>).containerId =
               container.id;
           }
-
-          acc.push(binding);
-          boundIds.add(binding.id);
         }
         return acc;
       },
@@ -312,6 +317,10 @@ const repairBoundElement = (
     return;
   }
 
+  if (boundElement.isDeleted) {
+    return;
+  }
+
   if (
     container.boundElements &&
     !container.boundElements.find((binding) => binding.id === boundElement.id)

src/tests/data/restore.test.ts

@@ -13,13 +13,17 @@ import { NormalizedZoomValue } from "../../types";
 import { FONT_FAMILY, ROUNDNESS } from "../../constants";
 import { newElementWith } from "../../element/mutateElement";
 
-const mockSizeHelper = jest.spyOn(sizeHelpers, "isInvisiblySmallElement");
+describe("restoreElements", () => {
+  const mockSizeHelper = jest.spyOn(sizeHelpers, "isInvisiblySmallElement");
 
-beforeEach(() => {
-  mockSizeHelper.mockReset();
-});
+  beforeEach(() => {
+    mockSizeHelper.mockReset();
+  });
+
+  afterAll(() => {
+    mockSizeHelper.mockRestore();
+  });
 
-describe("restoreElements", () => {
   it("should return empty array when element is null", () => {
     expect(restore.restoreElements(null, null)).toStrictEqual([]);
   });
@@ -528,3 +532,172 @@ describe("restore", () => {
     ]);
   });
 });
+
+describe("repairing bindings", () => {
+  it("should repair container boundElements", () => {
+    const container = API.createElement({
+      type: "rectangle",
+      boundElements: [],
+    });
+    const boundElement = API.createElement({
+      type: "text",
+      containerId: container.id,
+    });
+
+    expect(container.boundElements).toEqual([]);
+
+    const restoredElements = restore.restoreElements(
+      [container, boundElement],
+      null,
+    );
+
+    expect(restoredElements).toEqual([
+      expect.objectContaining({
+        id: container.id,
+        boundElements: [{ type: boundElement.type, id: boundElement.id }],
+      }),
+      expect.objectContaining({
+        id: boundElement.id,
+        containerId: container.id,
+      }),
+    ]);
+  });
+
+  it("should repair containerId of boundElements", () => {
+    const boundElement = API.createElement({
+      type: "text",
+      containerId: null,
+    });
+    const container = API.createElement({
+      type: "rectangle",
+      boundElements: [{ type: boundElement.type, id: boundElement.id }],
+    });
+
+    const restoredElements = restore.restoreElements(
+      [container, boundElement],
+      null,
+    );
+
+    expect(restoredElements).toEqual([
+      expect.objectContaining({
+        id: container.id,
+        boundElements: [{ type: boundElement.type, id: boundElement.id }],
+      }),
+      expect.objectContaining({
+        id: boundElement.id,
+        containerId: container.id,
+      }),
+    ]);
+  });
+
+  it("should ignore bound element if deleted", () => {
+    const container = API.createElement({
+      type: "rectangle",
+      boundElements: [],
+    });
+    const boundElement = API.createElement({
+      type: "text",
+      containerId: container.id,
+      isDeleted: true,
+    });
+
+    expect(container.boundElements).toEqual([]);
+
+    const restoredElements = restore.restoreElements(
+      [container, boundElement],
+      null,
+    );
+
+    expect(restoredElements).toEqual([
+      expect.objectContaining({
+        id: container.id,
+        boundElements: [],
+      }),
+      expect.objectContaining({
+        id: boundElement.id,
+        containerId: container.id,
+      }),
+    ]);
+  });
+
+  it("should remove bindings of deleted elements from boundElements", () => {
+    const container = API.createElement({
+      type: "rectangle",
+      boundElements: [],
+    });
+    const boundElement = API.createElement({
+      type: "text",
+      containerId: container.id,
+      isDeleted: true,
+    });
+    const invisibleBoundElement = API.createElement({
+      type: "text",
+      containerId: container.id,
+      width: 0,
+      height: 0,
+    });
+
+    const obsoleteBinding = { type: boundElement.type, id: boundElement.id };
+    const invisibleBinding = {
+      type: invisibleBoundElement.type,
+      id: invisibleBoundElement.id,
+    };
+    const nonExistentBinding = { type: "text", id: "non-existent" };
+    // @ts-ignore
+    container.boundElements = [
+      obsoleteBinding,
+      invisibleBinding,
+      nonExistentBinding,
+    ];
+
+    expect(container.boundElements).toEqual([
+      obsoleteBinding,
+      invisibleBinding,
+      nonExistentBinding,
+    ]);
+
+    const restoredElements = restore.restoreElements(
+      [container, invisibleBoundElement, boundElement],
+      null,
+    );
+
+    expect(restoredElements).toEqual([
+      expect.objectContaining({
+        id: container.id,
+        boundElements: [],
+      }),
+      expect.objectContaining({
+        id: boundElement.id,
+        containerId: container.id,
+      }),
+    ]);
+  });
+
+  it("should remove containerId if container not exists", () => {
+    const boundElement = API.createElement({
+      type: "text",
+      containerId: "non-existent",
+    });
+    const boundElementDeleted = API.createElement({
+      type: "text",
+      containerId: "non-existent",
+      isDeleted: true,
+    });
+
+    const restoredElements = restore.restoreElements(
+      [boundElement, boundElementDeleted],
+      null,
+    );
+
+    expect(restoredElements).toEqual([
+      expect.objectContaining({
+        id: boundElement.id,
+        containerId: null,
+      }),
+      expect.objectContaining({
+        id: boundElementDeleted.id,
+        containerId: null,
+      }),
+    ]);
+  });
+});