feat: Make repair and refreshDimensions configurable in restoreElements (#6238)

* fix: don't repair during reconcilation

* Add opts to restoreElement and enable refreshDimensions and repair via config

* remove

* update changelog

* fix tests

* rename to repairBindings

src/components/App.tsx

@@ -836,7 +836,7 @@ class App extends React.Component<AppProps, AppState> {
         },
       };
     }
-    const scene = restore(initialData, null, null);
+    const scene = restore(initialData, null, null, { repairBindings: true });
     scene.appState = {
       ...scene.appState,
       theme: this.props.theme || scene.appState.theme,

src/data/blob.ts

@@ -156,6 +156,7 @@ export const loadSceneOrLibraryFromBlob = async (
           },
           localAppState,
           localElements,
+          { repairBindings: true },
         ),
       };
     } else if (isValidLibrary(data)) {

src/data/restore.ts

@@ -339,7 +339,7 @@ export const restoreElements = (
   elements: ImportedDataState["elements"],
   /** NOTE doesn't serve for reconciliation */
   localElements: readonly ExcalidrawElement[] | null | undefined,
-  refreshDimensions = false,
+  opts?: { refreshDimensions?: boolean; repairBindings?: boolean } | undefined,
 ): ExcalidrawElement[] => {
   const localElementsMap = localElements ? arrayToMap(localElements) : null;
   const restoredElements = (elements || []).reduce((elements, element) => {
@@ -348,7 +348,7 @@ export const restoreElements = (
     if (element.type !== "selection" && !isInvisiblySmallElement(element)) {
       let migratedElement: ExcalidrawElement | null = restoreElement(
         element,
-        refreshDimensions,
+        opts?.refreshDimensions,
       );
       if (migratedElement) {
         const localElement = localElementsMap?.get(element.id);
@@ -361,6 +361,10 @@ export const restoreElements = (
     return elements;
   }, [] as ExcalidrawElement[]);
 
+  if (!opts?.repairBindings) {
+    return restoredElements;
+  }
+
   // repair binding. Mutates elements.
   const restoredElementsMap = arrayToMap(restoredElements);
   for (const element of restoredElements) {
@@ -497,9 +501,10 @@ export const restore = (
    */
   localAppState: Partial<AppState> | null | undefined,
   localElements: readonly ExcalidrawElement[] | null | undefined,
+  elementsConfig?: { refreshDimensions?: boolean; repairBindings?: boolean },
 ): RestoredDataState => {
   return {
-    elements: restoreElements(data?.elements, localElements),
+    elements: restoreElements(data?.elements, localElements, elementsConfig),
     appState: restoreAppState(data?.appState, localAppState || null),
     files: data?.files || {},
   };

src/excalidraw-app/collab/Collab.tsx

@@ -610,7 +610,7 @@ class Collab extends PureComponent<Props, CollabState> {
     const localElements = this.getSceneElementsIncludingDeleted();
     const appState = this.excalidrawAPI.getAppState();
 
-    remoteElements = restoreElements(remoteElements, null, false);
+    remoteElements = restoreElements(remoteElements, null);
 
     const reconciledElements = _reconcileElements(
       localElements,

src/excalidraw-app/data/index.ts

@@ -263,9 +263,12 @@ export const loadScene = async (
       await importFromBackend(id, privateKey),
       localDataState?.appState,
       localDataState?.elements,
+      { repairBindings: true },
     );
   } else {
-    data = restore(localDataState || null, null, null);
+    data = restore(localDataState || null, null, null, {
+      repairBindings: true,
+    });
   }
 
   return {

src/excalidraw-app/index.tsx

@@ -362,7 +362,7 @@ const ExcalidrawWrapper = () => {
           if (data.scene) {
             excalidrawAPI.updateScene({
               ...data.scene,
-              ...restore(data.scene, null, null),
+              ...restore(data.scene, null, null, { repairBindings: true }),
               commitToHistory: true,
             });
           }

src/packages/excalidraw/CHANGELOG.md

@@ -11,6 +11,24 @@ The change should be grouped under one of the below section and must contain PR
 Please add the latest change on the top under the correct section.
 -->
 
+## Unreleased
+
+### Features
+
+- [`restoreElements`](https://docs.excalidraw.com/docs/@excalidraw/excalidraw/api/utils/restore#restoreelements) API now takes an optional parameter `opts` which currently supports the below attributes
+
+```js
+{ refreshDimensions?: boolean, repair?: boolean }
+```
+
+The same `opts` param has been added to [`restore`](https://docs.excalidraw.com/docs/@excalidraw/excalidraw/api/utils/restore#restore) API as well.
+
+For more details refer to the [docs](https://docs.excalidraw.com)
+
+#### BREAKING CHANGE
+
+- The optional parameter `refreshDimensions` in [`restoreElements`](https://docs.excalidraw.com/docs/@excalidraw/excalidraw/api/utils/restore#restoreelements) has been removed and can be enabled via `opts`
+
 ## 0.14.2 (2023-02-01)
 
 ### Features

src/tests/data/restore.test.ts

@@ -534,7 +534,7 @@ describe("restore", () => {
 });
 
 describe("repairing bindings", () => {
-  it("should repair container boundElements", () => {
+  it("should repair container boundElements when repair is true", () => {
     const container = API.createElement({
       type: "rectangle",
       boundElements: [],
@@ -546,9 +546,26 @@ describe("repairing bindings", () => {
 
     expect(container.boundElements).toEqual([]);
 
-    const restoredElements = restore.restoreElements(
+    let restoredElements = restore.restoreElements(
+      [container, boundElement],
+      null,
+    );
+
+    expect(restoredElements).toEqual([
+      expect.objectContaining({
+        id: container.id,
+        boundElements: [],
+      }),
+      expect.objectContaining({
+        id: boundElement.id,
+        containerId: container.id,
+      }),
+    ]);
+
+    restoredElements = restore.restoreElements(
       [container, boundElement],
       null,
+      { repairBindings: true },
     );
 
     expect(restoredElements).toEqual([
@@ -563,7 +580,7 @@ describe("repairing bindings", () => {
     ]);
   });
 
-  it("should repair containerId of boundElements", () => {
+  it("should repair containerId of boundElements when repair is true", () => {
     const boundElement = API.createElement({
       type: "text",
       containerId: null,
@@ -573,9 +590,26 @@ describe("repairing bindings", () => {
       boundElements: [{ type: boundElement.type, id: boundElement.id }],
     });
 
-    const restoredElements = restore.restoreElements(
+    let 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: null,
+      }),
+    ]);
+
+    restoredElements = restore.restoreElements(
       [container, boundElement],
       null,
+      { repairBindings: true },
     );
 
     expect(restoredElements).toEqual([
@@ -620,7 +654,7 @@ describe("repairing bindings", () => {
     ]);
   });
 
-  it("should remove bindings of deleted elements from boundElements", () => {
+  it("should remove bindings of deleted elements from boundElements when repair is true", () => {
     const container = API.createElement({
       type: "rectangle",
       boundElements: [],
@@ -642,6 +676,8 @@ describe("repairing bindings", () => {
       type: invisibleBoundElement.type,
       id: invisibleBoundElement.id,
     };
+    expect(container.boundElements).toEqual([]);
+
     const nonExistentBinding = { type: "text", id: "non-existent" };
     // @ts-ignore
     container.boundElements = [
@@ -650,15 +686,26 @@ describe("repairing bindings", () => {
       nonExistentBinding,
     ];
 
-    expect(container.boundElements).toEqual([
-      obsoleteBinding,
-      invisibleBinding,
-      nonExistentBinding,
+    let restoredElements = restore.restoreElements(
+      [container, invisibleBoundElement, boundElement],
+      null,
+    );
+
+    expect(restoredElements).toEqual([
+      expect.objectContaining({
+        id: container.id,
+        boundElements: [obsoleteBinding, invisibleBinding, nonExistentBinding],
+      }),
+      expect.objectContaining({
+        id: boundElement.id,
+        containerId: container.id,
+      }),
     ]);
 
-    const restoredElements = restore.restoreElements(
+    restoredElements = restore.restoreElements(
       [container, invisibleBoundElement, boundElement],
       null,
+      { repairBindings: true },
     );
 
     expect(restoredElements).toEqual([
@@ -673,7 +720,7 @@ describe("repairing bindings", () => {
     ]);
   });
 
-  it("should remove containerId if container not exists", () => {
+  it("should remove containerId if container not exists when repair is true", () => {
     const boundElement = API.createElement({
       type: "text",
       containerId: "non-existent",
@@ -684,9 +731,26 @@ describe("repairing bindings", () => {
       isDeleted: true,
     });
 
-    const restoredElements = restore.restoreElements(
+    let restoredElements = restore.restoreElements(
+      [boundElement, boundElementDeleted],
+      null,
+    );
+
+    expect(restoredElements).toEqual([
+      expect.objectContaining({
+        id: boundElement.id,
+        containerId: "non-existent",
+      }),
+      expect.objectContaining({
+        id: boundElementDeleted.id,
+        containerId: "non-existent",
+      }),
+    ]);
+
+    restoredElements = restore.restoreElements(
       [boundElement, boundElementDeleted],
       null,
+      { repairBindings: true },
     );
 
     expect(restoredElements).toEqual([