fix: reconciliation of server updates & refactor restore (#10597)

excalidraw-app/App.tsx

@@ -48,7 +48,11 @@ import {
   youtubeIcon,
 } from "@excalidraw/excalidraw/components/icons";
 import { isElementLink } from "@excalidraw/element";
-import { restore, restoreAppState } from "@excalidraw/excalidraw/data/restore";
+import {
+  bumpElementVersions,
+  restoreAppState,
+  restoreElements,
+} from "@excalidraw/excalidraw/data/restore";
 import { newElementWith } from "@excalidraw/element";
 import { isInitializedImageElement } from "@excalidraw/element";
 import clsx from "clsx";
@@ -105,8 +109,8 @@ import { TopErrorBoundary } from "./components/TopErrorBoundary";
 import {
   exportToBackend,
   getCollaborationLinkData,
+  importFromBackend,
   isCollaborationLink,
-  loadScene,
 } from "./data";
 
 import { updateStaleImageStatuses } from "./data/FileManager";
@@ -224,9 +228,20 @@ const initializeScene = async (opts: {
 
   const localDataState = importFromLocalStorage();
 
-  let scene: RestoredDataState & {
+  let scene: Omit<
+    RestoredDataState,
+    // we're not storing files in the scene database/localStorage, and instead
+    // fetch them async from a different store
+    "files"
+  > & {
     scrollToContent?: boolean;
-  } = await loadScene(null, null, localDataState);
+  } = {
+    elements: restoreElements(localDataState?.elements, null, {
+      repairBindings: true,
+      deleteInvisibleElements: true,
+    }),
+    appState: restoreAppState(localDataState?.appState, null),
+  };
 
   let roomLinkData = getCollaborationLinkData(window.location.href);
   const isExternalScene = !!(id || jsonBackendMatch || roomLinkData);
@@ -240,11 +255,26 @@ const initializeScene = async (opts: {
       (await openConfirmModal(shareableLinkConfirmDialog))
     ) {
       if (jsonBackendMatch) {
-        scene = await loadScene(
+        const imported = await importFromBackend(
           jsonBackendMatch[1],
           jsonBackendMatch[2],
-          localDataState,
         );
+
+        scene = {
+          elements: bumpElementVersions(
+            restoreElements(imported.elements, null, {
+              repairBindings: true,
+              deleteInvisibleElements: true,
+            }),
+            localDataState?.elements,
+          ),
+          appState: restoreAppState(
+            imported.appState,
+            // local appState when importing from backend to ensure we restore
+            // localStorage user settings which we do not persist on server.
+            localDataState?.appState,
+          ),
+        };
       }
       scene.scrollToContent = true;
       if (!roomLinkData) {
@@ -496,8 +526,10 @@ const ExcalidrawWrapper = () => {
           loadImages(data);
           if (data.scene) {
             excalidrawAPI.updateScene({
-              ...data.scene,
+              elements: restoreElements(data.scene.elements, null, {
-              ...restore(data.scene, null, null, { repairBindings: true }),
+                repairBindings: true,
+              }),
+              appState: restoreAppState(data.scene.appState, null),
               captureUpdate: CaptureUpdateAction.IMMEDIATELY,
             });
           }

excalidraw-app/collab/Collab.tsx

@@ -6,7 +6,7 @@ import {
   reconcileElements,
 } from "@excalidraw/excalidraw";
 import { ErrorDialog } from "@excalidraw/excalidraw/components/ErrorDialog";
-import { APP_NAME, EVENT } from "@excalidraw/common";
+import { APP_NAME, cloneJSON, EVENT, toBrandedType } from "@excalidraw/common";
 import {
   IDLE_THRESHOLD,
   ACTIVE_THRESHOLD,
@@ -29,6 +29,8 @@ import { withBatchedUpdates } from "@excalidraw/excalidraw/reactUtils";
 import throttle from "lodash.throttle";
 import { PureComponent } from "react";
 
+import { bumpElementVersions } from "@excalidraw/excalidraw/data/restore";
+
 import type {
   ReconciledExcalidrawElement,
   RemoteExcalidrawElement,
@@ -311,6 +313,7 @@ class Collab extends PureComponent<CollabProps, CollabState> {
   saveCollabRoomToFirebase = async (
     syncableElements: readonly SyncableExcalidrawElement[],
   ) => {
+    syncableElements = cloneJSON(syncableElements);
     try {
       const storedElements = await saveToFirebase(
         this.portal,
@@ -579,7 +582,9 @@ class Collab extends PureComponent<CollabProps, CollabState> {
           case WS_SUBTYPES.INIT: {
             if (!this.portal.socketInitialized) {
               this.initializeRoom({ fetchScene: false });
-              const remoteElements = decryptedData.payload.elements;
+              const remoteElements = toBrandedType<
+                readonly RemoteExcalidrawElement[]
+              >(decryptedData.payload.elements);
               const reconciledElements =
                 this._reconcileElements(remoteElements);
               this.handleRemoteSceneUpdate(reconciledElements);
@@ -593,7 +598,11 @@ class Collab extends PureComponent<CollabProps, CollabState> {
           }
           case WS_SUBTYPES.UPDATE:
             this.handleRemoteSceneUpdate(
-              this._reconcileElements(decryptedData.payload.elements),
+              this._reconcileElements(
+                toBrandedType<readonly RemoteExcalidrawElement[]>(
+                  decryptedData.payload.elements,
+                ),
+              ),
             );
             break;
           case WS_SUBTYPES.MOUSE_LOCATION: {
@@ -742,20 +751,28 @@ class Collab extends PureComponent<CollabProps, CollabState> {
   };
 
   private _reconcileElements = (
-    remoteElements: readonly ExcalidrawElement[],
+    remoteElements: readonly RemoteExcalidrawElement[],
   ): ReconciledExcalidrawElement[] => {
-    const localElements = this.getSceneElementsIncludingDeleted();
     const appState = this.excalidrawAPI.getAppState();
-    const restoredRemoteElements = restoreElements(
+
+    const existingElements = this.getSceneElementsIncludingDeleted();
+
+    // NOTE ideally we restore _after_ reconciliation but we can't do that
+    // as we'd regenerate even elements such as appState.newElement which would
+    // break the state
+    remoteElements = restoreElements(remoteElements, existingElements);
+
+    let reconciledElements = reconcileElements(
+      existingElements,
       remoteElements,
-      this.excalidrawAPI.getSceneElementsMapIncludingDeleted(),
-    );
-    const reconciledElements = reconcileElements(
-      localElements,
-      restoredRemoteElements as RemoteExcalidrawElement[],
       appState,
     );
 
+    reconciledElements = bumpElementVersions(
+      reconciledElements,
+      existingElements,
+    );
+
     // Avoid broadcasting to the rest of the collaborators the scene
     // we just received!
     // Note: this needs to be set before updating the scene as it

excalidraw-app/data/firebase.ts

@@ -1,5 +1,5 @@
 import { reconcileElements } from "@excalidraw/excalidraw";
-import { MIME_TYPES } from "@excalidraw/common";
+import { MIME_TYPES, toBrandedType } from "@excalidraw/common";
 import { decompressData } from "@excalidraw/excalidraw/data/encode";
 import {
   encryptData,
@@ -243,7 +243,7 @@ export const saveToFirebase = async (
 
   FirebaseSceneVersionCache.set(socket, storedElements);
 
-  return storedElements;
+  return toBrandedType<RemoteExcalidrawElement[]>(storedElements);
 };
 
 export const loadFromFirebase = async (

excalidraw-app/data/index.ts

@@ -8,7 +8,6 @@ import {
   IV_LENGTH_BYTES,
 } from "@excalidraw/excalidraw/data/encryption";
 import { serializeAsJSON } from "@excalidraw/excalidraw/data/json";
-import { restore } from "@excalidraw/excalidraw/data/restore";
 import { isInvisiblySmallElement } from "@excalidraw/element";
 import { isInitializedImageElement } from "@excalidraw/element";
 import { t } from "@excalidraw/excalidraw/i18n";
@@ -84,13 +83,13 @@ export type SocketUpdateDataSource = {
   SCENE_INIT: {
     type: WS_SUBTYPES.INIT;
     payload: {
-      elements: readonly ExcalidrawElement[];
+      elements: readonly OrderedExcalidrawElement[];
     };
   };
   SCENE_UPDATE: {
     type: WS_SUBTYPES.UPDATE;
     payload: {
-      elements: readonly ExcalidrawElement[];
+      elements: readonly OrderedExcalidrawElement[];
     };
   };
   MOUSE_LOCATION: {
@@ -200,7 +199,7 @@ const legacy_decodeFromBackend = async ({
   };
 };
 
-const importFromBackend = async (
+export const importFromBackend = async (
   id: string,
   decryptionKey: string,
 ): Promise<ImportedDataState> => {
@@ -242,45 +241,6 @@ const importFromBackend = async (
   }
 };
 
-export const loadScene = async (
-  id: string | null,
-  privateKey: string | null,
-  // Supply local state even if importing from backend to ensure we restore
-  // localStorage user settings which we do not persist on server.
-  // Non-optional so we don't forget to pass it even if `undefined`.
-  localDataState: ImportedDataState | undefined | null,
-) => {
-  let data;
-  if (id != null && privateKey != null) {
-    // the private key is used to decrypt the content from the server, take
-    // extra care not to leak it
-    data = restore(
-      await importFromBackend(id, privateKey),
-      localDataState?.appState,
-      localDataState?.elements,
-      {
-        repairBindings: true,
-        refreshDimensions: false,
-        deleteInvisibleElements: true,
-      },
-    );
-  } else {
-    data = restore(localDataState || null, null, null, {
-      repairBindings: true,
-      deleteInvisibleElements: true,
-    });
-  }
-
-  return {
-    elements: data.elements,
-    appState: data.appState,
-    // note: this will always be empty because we're not storing files
-    // in the scene database/localStorage, and instead fetch them async
-    // from a different database
-    files: data.files,
-  };
-};
-
 type ExportToBackendResult =
   | { url: null; errorMessage: string }
   | { url: string; errorMessage: null };

packages/common/src/utils.ts

@@ -1157,39 +1157,69 @@ export const normalizeEOL = (str: string) => {
 };
 
 // -----------------------------------------------------------------------------
-type HasBrand<T> = {
+export type HasBrand<T> = {
   // eslint-disable-next-line @typescript-eslint/no-unused-vars
-  [K in keyof T]: K extends `~brand${infer _}` ? true : never;
+  [K in keyof T]: K extends `~brand${infer _}` | "_brand" ? true : never;
 }[keyof T];
 
 type RemoveAllBrands<T> = HasBrand<T> extends true
   ? {
       // eslint-disable-next-line @typescript-eslint/no-unused-vars
-      [K in keyof T as K extends `~brand~${infer _}` ? never : K]: T[K];
+      [K in keyof T as K extends `~brand~${infer _}` | "_brand"
+        ? never
+        : K]: T[K];
     }
-  : never;
+  : T;
 
-// adapted from https://github.com/colinhacks/zod/discussions/1994#discussioncomment-6068940
+// For accepting values - uses loose matching for branded types
-// currently does not cover all types (e.g. tuples, promises...)
+// Preserves readonly modifier: mutable array requires mutable input
-type Unbrand<T> = T extends Map<infer E, infer F>
+type UnbrandForValue<T> = T extends Map<infer E, infer F>
-  ? Map<E, F>
+  ? Map<UnbrandForValue<E>, UnbrandForValue<F>>
   : T extends Set<infer E>
-  ? Set<E>
+  ? Set<UnbrandForValue<E>>
-  : T extends Array<infer E>
+  : T extends readonly any[]
-  ? Array<E>
+  ? T extends any[]
+    ? unknown[] // mutable array - require mutable input
+    : readonly unknown[] // readonly array - accept readonly input
   : RemoveAllBrands<T>;
 
+// For return types - preserves array element unbranding
+export type Unbrand<T> = T extends Map<infer E, infer F>
+  ? Map<Unbrand<E>, Unbrand<F>>
+  : T extends Set<infer E>
+  ? Set<Unbrand<E>>
+  : T extends readonly (infer E)[]
+  ? Array<Unbrand<E>>
+  : RemoveAllBrands<T>;
+
+export type CombineBrands<BrandedType, CurrentType> =
+  BrandedType extends readonly (infer BE)[]
+    ? CurrentType extends readonly (infer CE)[]
+      ? Array<CE & BE>
+      : CurrentType & BrandedType
+    : CurrentType & BrandedType;
+
+export type CombineBrandsIfNeeded<T, Required> = [T] extends [Required]
+  ? T[]
+  : HasBrand<T> extends true
+  ? CombineBrands<T, Required>[]
+  : Required[];
+
 /**
  * Makes type into a branded type, ensuring that value is assignable to
- * the base ubranded type. Optionally you can explicitly supply current value
+ * the base unbranded type. Optionally you can explicitly supply current value
  * type to combine both (useful for composite branded types. Make sure you
  * compose branded types which are not composite themselves.)
  */
-export const toBrandedType = <BrandedType, CurrentType = BrandedType>(
+export function toBrandedType<BrandedType>(
-  value: Unbrand<BrandedType>,
+  value: UnbrandForValue<BrandedType>,
-) => {
+): BrandedType;
-  return value as CurrentType & BrandedType;
+export function toBrandedType<BrandedType, CurrentType>(
-};
+  value: CurrentType,
+): CombineBrands<BrandedType, CurrentType>;
+export function toBrandedType(value: unknown) {
+  return value;
+}
 
 // -----------------------------------------------------------------------------
 

packages/excalidraw/components/App.tsx

@@ -346,7 +346,7 @@ import {
 
 import { exportCanvas, loadFromBlob } from "../data";
 import Library, { distributeLibraryItemsOnSquareGrid } from "../data/library";
-import { restore, restoreElements } from "../data/restore";
+import { restoreAppState, restoreElements } from "../data/restore";
 import { getCenter, getDistance } from "../gesture";
 import { History } from "../history";
 import { defaultLang, getLanguage, languages, setLanguage, t } from "../i18n";
@@ -2701,46 +2701,47 @@ class App extends React.Component<AppProps, AppState> {
         },
       };
     }
-    const scene = restore(initialData, null, null, {
+    const restoredElements = restoreElements(initialData?.elements, null, {
       repairBindings: true,
       deleteInvisibleElements: true,
     });
-    const activeTool = scene.appState.activeTool;
+    let restoredAppState = restoreAppState(initialData?.appState, null);
+    const activeTool = restoredAppState.activeTool;
 
-    if (!scene.appState.preferredSelectionTool.initialized) {
+    if (!restoredAppState.preferredSelectionTool.initialized) {
-      scene.appState.preferredSelectionTool = {
+      restoredAppState.preferredSelectionTool = {
         type:
           this.editorInterface.formFactor === "phone" ? "lasso" : "selection",
         initialized: true,
       };
     }
 
-    scene.appState = {
+    restoredAppState = {
-      ...scene.appState,
+      ...restoredAppState,
-      theme: this.props.theme || scene.appState.theme,
+      theme: this.props.theme || restoredAppState.theme,
       // we're falling back to current (pre-init) state when deciding
       // whether to open the library, to handle a case where we
       // update the state outside of initialData (e.g. when loading the app
       // with a library install link, which should auto-open the library)
-      openSidebar: scene.appState?.openSidebar || this.state.openSidebar,
+      openSidebar: restoredAppState?.openSidebar || this.state.openSidebar,
       activeTool:
         activeTool.type === "image" ||
         activeTool.type === "lasso" ||
         activeTool.type === "selection"
           ? {
               ...activeTool,
-              type: scene.appState.preferredSelectionTool.type,
+              type: restoredAppState.preferredSelectionTool.type,
             }
-          : scene.appState.activeTool,
+          : restoredAppState.activeTool,
       isLoading: false,
       toast: this.state.toast,
     };
 
     if (initialData?.scrollToContent) {
-      scene.appState = {
+      restoredAppState = {
-        ...scene.appState,
+        ...restoredAppState,
-        ...calculateScrollCenter(scene.elements, {
+        ...calculateScrollCenter(restoredElements, {
-          ...scene.appState,
+          ...restoredAppState,
           width: this.state.width,
           height: this.state.height,
           offsetTop: this.state.offsetTop,
@@ -2752,7 +2753,9 @@ class App extends React.Component<AppProps, AppState> {
     this.resetStore();
     this.resetHistory();
     this.syncActionResult({
-      ...scene,
+      elements: restoredElements,
+      appState: restoredAppState,
+      files: initialData?.files,
       captureUpdate: CaptureUpdateAction.NEVER,
     });
 

packages/excalidraw/data/blob.ts

@@ -19,7 +19,11 @@ import { decodeSvgBase64Payload } from "../scene/export";
 import { base64ToString, stringToBase64, toByteString } from "./encode";
 import { nativeFileSystemSupported } from "./filesystem";
 import { isValidExcalidrawData, isValidLibrary } from "./json";
-import { restore, restoreLibraryItems } from "./restore";
+import {
+  restoreAppState,
+  restoreElements,
+  restoreLibraryItems,
+} from "./restore";
 
 import type { AppState, DataURL, LibraryItem } from "../types";
 
@@ -155,10 +159,13 @@ export const loadSceneOrLibraryFromBlob = async (
     if (isValidExcalidrawData(data)) {
       return {
         type: MIME_TYPES.excalidraw,
-        data: restore(
+        data: {
-          {
+          elements: restoreElements(data.elements, localElements, {
-            elements: data.elements || [],
+            repairBindings: true,
-            appState: {
+            deleteInvisibleElements: true,
+          }),
+          appState: restoreAppState(
+            {
               theme: localAppState?.theme,
               fileHandle: fileHandle || blob.handle || null,
               ...cleanAppStateForExport(data.appState || {}),
@@ -166,16 +173,10 @@ export const loadSceneOrLibraryFromBlob = async (
                 ? calculateScrollCenter(data.elements || [], localAppState)
                 : {}),
             },
-            files: data.files,
+            localAppState,
-          },
+          ),
-          localAppState,
+          files: data.files || {},
-          localElements,
+        },
-          {
-            repairBindings: true,
-            refreshDimensions: false,
-            deleteInvisibleElements: true,
-          },
-        ),
       };
     } else if (isValidLibrary(data)) {
       return {

packages/excalidraw/data/reconcile.ts

@@ -36,7 +36,7 @@ export const shouldDiscardRemoteElement = (
       // resolve conflicting edits deterministically by taking the one with
       // the lowest versionNonce
       (local.version === remote.version &&
-        local.versionNonce < remote.versionNonce))
+        local.versionNonce <= remote.versionNonce))
   ) {
     return true;
   }

packages/excalidraw/data/restore.ts

@@ -1,6 +1,7 @@
 import { isFiniteNumber, pointFrom } from "@excalidraw/math";
 
 import {
+  type CombineBrandsIfNeeded,
   DEFAULT_FONT_FAMILY,
   DEFAULT_TEXT_ALIGN,
   DEFAULT_VERTICAL_ALIGN,
@@ -131,13 +132,18 @@ const repairBinding = <T extends ExcalidrawArrowElement>(
   element: T,
   binding: FixedPointBinding | null,
   targetElementsMap: Readonly<ElementsMap>,
-  localElementsMap: Readonly<ElementsMap> | null | undefined,
+  /** used for context (arrow bindings) */
+  existingElementsMap: Readonly<ElementsMap> | null | undefined,
   startOrEnd: "start" | "end",
 ): FixedPointBinding | null => {
   if (!binding) {
     return null;
   }
 
+  // ---------------------------------------------------------------------------
+  // elbow arrows
+  // ---------------------------------------------------------------------------
+
   if (isElbowArrow(element)) {
     const fixedPointBinding:
       | ExcalidrawElbowArrowElement["startBinding"]
@@ -150,24 +156,41 @@ const repairBinding = <T extends ExcalidrawArrowElement>(
     return fixedPointBinding;
   }
 
-  // Fallback if the bound element is missing but the binding is at least
+  // ---------------------------------------------------------------------------
-  // looking like a valid one shape-wise
+  // simple arrows
-  if (binding.mode && binding.fixedPoint && binding.elementId) {
+  // ---------------------------------------------------------------------------
-    return {
+
-      elementId: binding.elementId,
+  // binding schema v2
-      mode: binding.mode,
+  // ---------------------------------------------------------------------------
-      fixedPoint: normalizeFixedPoint(binding.fixedPoint || [0.5, 0.5]),
+
-    } as FixedPointBinding | null;
+  if (binding.mode) {
+    // if latest binding schema, don't check if binding.elementId exists
+    // (it's done in a separate pass)
+    if (binding.elementId) {
+      return {
+        elementId: binding.elementId,
+        mode: binding.mode,
+        fixedPoint: normalizeFixedPoint(binding.fixedPoint || [0.5, 0.5]),
+      } as FixedPointBinding | null;
+    }
+    return null;
   }
 
+  // binding schema v1 (legacy) -> attempt to migrate to v2
+  // ---------------------------------------------------------------------------
+
   const targetBoundElement =
     (targetElementsMap.get(binding.elementId) as ExcalidrawBindableElement) ||
     undefined;
   const boundElement =
     targetBoundElement ||
-    (localElementsMap?.get(binding.elementId) as ExcalidrawBindableElement) ||
+    (existingElementsMap?.get(
+      binding.elementId,
+    ) as ExcalidrawBindableElement) ||
     undefined;
-  const elementsMap = targetBoundElement ? targetElementsMap : localElementsMap;
+  const elementsMap = targetBoundElement
+    ? targetElementsMap
+    : existingElementsMap;
 
   // migrating legacy focus point bindings
   if (boundElement && elementsMap) {
@@ -296,9 +319,12 @@ const restoreElementWithProperties = <
 };
 
 export const restoreElement = (
+  /** element to be restored */
   element: Exclude<ExcalidrawElement, ExcalidrawSelectionElement>,
+  /** all elements to be restored */
   targetElementsMap: Readonly<ElementsMap>,
-  localElementsMap: Readonly<ElementsMap> | null | undefined,
+  /** used for additional context */
+  existingElementsMap: Readonly<ElementsMap> | null | undefined,
   opts?: {
     deleteInvisibleElements?: boolean;
   },
@@ -420,14 +446,14 @@ export const restoreElement = (
           element as ExcalidrawArrowElement,
           element.startBinding,
           targetElementsMap,
-          localElementsMap,
+          existingElementsMap,
           "start",
         ),
         endBinding: repairBinding(
           element as ExcalidrawArrowElement,
           element.endBinding,
           targetElementsMap,
-          localElementsMap,
+          existingElementsMap,
           "end",
         ),
         startArrowhead,
@@ -588,10 +614,10 @@ const repairFrameMembership = (
   }
 };
 
-export const restoreElements = (
+export const restoreElements = <T extends ExcalidrawElement>(
-  targetElements: ImportedDataState["elements"],
+  targetElements: readonly T[] | undefined | null,
-  /** NOTE doesn't serve for reconciliation */
+  /** used for additional context (e.g. repairing arrow bindings) */
-  localElements: Readonly<ElementsMapOrArray> | null | undefined,
+  existingElements: Readonly<ElementsMapOrArray> | null | undefined,
   opts?:
     | {
         refreshDimensions?: boolean;
@@ -599,11 +625,13 @@ export const restoreElements = (
         deleteInvisibleElements?: boolean;
       }
     | undefined,
-): OrderedExcalidrawElement[] => {
+): CombineBrandsIfNeeded<T, OrderedExcalidrawElement> => {
   // used to detect duplicate top-level element ids
   const existingIds = new Set<string>();
   const targetElementsMap = arrayToMap(targetElements || []);
-  const localElementsMap = localElements ? arrayToMap(localElements) : null;
+  const existingElementsMap = existingElements
+    ? arrayToMap(existingElements)
+    : null;
   const restoredElements = syncInvalidIndices(
     (targetElements || []).reduce((elements, element) => {
       // filtering out selection, which is legacy, no longer kept in elements,
@@ -615,21 +643,18 @@ export const restoreElements = (
       let migratedElement: ExcalidrawElement | null = restoreElement(
         element,
         targetElementsMap,
-        localElementsMap,
+        existingElementsMap,
         {
           deleteInvisibleElements: opts?.deleteInvisibleElements,
         },
       );
       if (migratedElement) {
-        const localElement = localElementsMap?.get(element.id);
+        const localElement = existingElementsMap?.get(element.id);
 
         const shouldMarkAsDeleted =
           opts?.deleteInvisibleElements && isInvisiblySmallElement(element);
 
-        if (
+        if (shouldMarkAsDeleted) {
-          shouldMarkAsDeleted ||
-          (localElement && localElement.version > migratedElement.version)
-        ) {
           migratedElement = bumpVersion(migratedElement, localElement?.version);
         }
 
@@ -650,7 +675,10 @@ export const restoreElements = (
   );
 
   if (!opts?.repairBindings) {
-    return restoredElements;
+    return restoredElements as CombineBrandsIfNeeded<
+      T,
+      OrderedExcalidrawElement
+    >;
   }
 
   // repair binding. Mutates elements.
@@ -759,6 +787,41 @@ export const restoreElements = (
       };
     }
 
+    return element;
+  }) as CombineBrandsIfNeeded<T, OrderedExcalidrawElement>;
+};
+
+/**
+ * When replacing elements that may exist locally, this bumps their versions
+ * to the local version + 1. Mainly for later reconciliation to work properly.
+ *
+ * See https://github.com/excalidraw/excalidraw/issues/3795
+ *
+ * Generally use this on editor boundaries (importing from file etc.), though
+ * it does not apply universally (e.g. we don't want to do this for collab
+ * updates).
+ */
+export const bumpElementVersions = <T extends ExcalidrawElement>(
+  targetElements: readonly T[],
+  localElements: Readonly<ElementsMapOrArray> | null | undefined,
+) => {
+  const localElementsMap = localElements ? arrayToMap(localElements) : null;
+
+  return targetElements.map((element) => {
+    const localElement = localElementsMap?.get(element.id);
+
+    if (
+      localElement &&
+      (localElement.version > element.version ||
+        // same versions but different versionNonce means different edits
+        // (this often means the element was bumped during restore e.g. due
+        // to re-indexing, and the original element was modified elsewhere
+        // and supplied as localElements)
+        (localElement.version === element.version &&
+          localElement.versionNonce !== element.versionNonce))
+    ) {
+      return bumpVersion(element, localElement.version);
+    }
     return element;
   });
 };
@@ -875,29 +938,6 @@ export const restoreAppState = (
   };
 };
 
-export const restore = (
-  data: Pick<ImportedDataState, "appState" | "elements" | "files"> | null,
-  /**
-   * Local AppState (`this.state` or initial state from localStorage) so that we
-   * don't overwrite local state with default values (when values not
-   * explicitly specified).
-   * Supply `null` if you can't get access to it.
-   */
-  localAppState: Partial<AppState> | null | undefined,
-  localElements: readonly ExcalidrawElement[] | null | undefined,
-  elementsConfig?: {
-    refreshDimensions?: boolean;
-    repairBindings?: boolean;
-    deleteInvisibleElements?: boolean;
-  },
-): RestoredDataState => {
-  return {
-    elements: restoreElements(data?.elements, localElements, elementsConfig),
-    appState: restoreAppState(data?.appState, localAppState || null),
-    files: data?.files || {},
-  };
-};
-
 const restoreLibraryItem = (libraryItem: LibraryItem) => {
   const elements = restoreElements(
     getNonDeletedElements(libraryItem.elements),

packages/excalidraw/index.tsx

@@ -229,7 +229,6 @@ export { isInvisiblySmallElement } from "@excalidraw/element";
 
 export { defaultLang, useI18n, languages } from "./i18n";
 export {
-  restore,
   restoreAppState,
   restoreElement,
   restoreElements,

packages/excalidraw/tests/data/restore.test.ts

@@ -34,6 +34,20 @@ describe("restoreElements", () => {
     mockSizeHelper.mockRestore();
   });
 
+  it("basic restoreElements", () => {
+    const textElement = API.createElement({ type: "text" });
+    const rectElement = API.createElement({ type: "rectangle" });
+    const elements = [textElement, rectElement];
+
+    const restoredElements = restore.restoreElements(elements, null);
+    expect(restoredElements.length).toBe(elements.length);
+  });
+
+  it("when imported data state is null it should return an empty array of elements", () => {
+    const restoredElements = restore.restoreElements(null, null);
+    expect(restoredElements.length).toBe(0);
+  });
+
   it("should return empty array when element is null", () => {
     expect(restore.restoreElements(null, null)).toStrictEqual([]);
   });
@@ -434,12 +448,12 @@ describe("restoreElements", () => {
   });
 
   it("bump versions of local duplicate elements when supplied", () => {
-    const rectangle = API.createElement({ type: "rectangle" });
+    const rectangle = API.createElement({ type: "rectangle" }); // version=1
     const ellipse = API.createElement({ type: "ellipse" });
-    const rectangle_modified = newElementWith(rectangle, { isDeleted: true });
+    const rectangle_modified = newElementWith(rectangle, { isDeleted: true }); // version=2
 
-    const restoredElements = restore.restoreElements(
+    const restoredElements = restore.bumpElementVersions(
-      [rectangle, ellipse],
+      restore.restoreElements([rectangle, ellipse], null),
       [rectangle_modified],
     );
 
@@ -448,7 +462,7 @@ describe("restoreElements", () => {
     expect(restoredElements).toEqual([
       expect.objectContaining({
         id: rectangle.id,
-        version: rectangle_modified.version + 2,
+        version: rectangle_modified.version + 1,
       }),
       expect.objectContaining({
         id: ellipse.id,
@@ -456,9 +470,73 @@ describe("restoreElements", () => {
       }),
     ]);
   });
+
+  it("bump versions of local duplicate elements when supplied even if both have same version", () => {
+    const rectangle = API.createElement({ type: "rectangle" });
+
+    const restored_rectangle_1 = restore.restoreElements([rectangle], null)[0];
+    const restored_rectangle_2 = restore.restoreElements(
+      [restored_rectangle_1],
+      null,
+    )[0];
+
+    // restored rectangle version should be +1 because of re-index
+    expect(rectangle.version).not.toBe(restored_rectangle_1.version);
+
+    // restoring it again shouldn't re-index again
+    expect(restored_rectangle_1.version).toBe(restored_rectangle_2.version);
+    expect(restored_rectangle_1.versionNonce).toBe(
+      restored_rectangle_2.versionNonce,
+    );
+
+    const modified_rectangle_1 = newElementWith(restored_rectangle_1, {
+      width: 500,
+    });
+    const modified_rectangle_2 = newElementWith(restored_rectangle_2, {
+      width: 600,
+    });
+
+    const restoredElements = restore.bumpElementVersions(
+      restore.restoreElements([modified_rectangle_1], null),
+      [modified_rectangle_2],
+    );
+
+    expect(restoredElements[0].id).toBe(rectangle.id);
+    expect(restoredElements[0].id).toBe(modified_rectangle_1.id);
+    expect(restoredElements[0].versionNonce).not.toBe(
+      modified_rectangle_1.versionNonce,
+    );
+    expect(restoredElements[0].version).toBe(modified_rectangle_2.version + 1);
+  });
 });
 
 describe("restoreAppState", () => {
+  it("when appState is null it should return the local app state property", () => {
+    const stubLocalAppState = getDefaultAppState();
+    stubLocalAppState.cursorButton = "down";
+    stubLocalAppState.name = "local app state";
+
+    const restoredAppState = restore.restoreAppState(null, stubLocalAppState);
+    expect(restoredAppState.cursorButton).toBe(stubLocalAppState.cursorButton);
+    expect(restoredAppState.name).toBe(stubLocalAppState.name);
+  });
+
+  it("when local appState is null but imported app state is supplied", () => {
+    const stubImportedAppState = getDefaultAppState();
+    stubImportedAppState.cursorButton = "down";
+    stubImportedAppState.name = "imported app state";
+
+    const importedDataState = {} as ImportedDataState;
+    importedDataState.appState = stubImportedAppState;
+
+    const restoredAppState = restore.restoreAppState(
+      importedDataState.appState,
+      null,
+    );
+    expect(restoredAppState.cursorButton).toBe("up");
+    expect(restoredAppState.name).toBe(stubImportedAppState.name);
+  });
+
   it("should restore with imported data", () => {
     const stubImportedAppState = getDefaultAppState();
     stubImportedAppState.activeTool.type = "selection";
@@ -638,83 +716,6 @@ describe("restoreAppState", () => {
   });
 });
 
-describe("restore", () => {
-  it("when imported data state is null it should return an empty array of elements", () => {
-    const stubLocalAppState = getDefaultAppState();
-
-    const restoredData = restore.restore(null, stubLocalAppState, null);
-    expect(restoredData.elements.length).toBe(0);
-  });
-
-  it("when imported data state is null it should return the local app state property", () => {
-    const stubLocalAppState = getDefaultAppState();
-    stubLocalAppState.cursorButton = "down";
-    stubLocalAppState.name = "local app state";
-
-    const restoredData = restore.restore(null, stubLocalAppState, null);
-    expect(restoredData.appState.cursorButton).toBe(
-      stubLocalAppState.cursorButton,
-    );
-    expect(restoredData.appState.name).toBe(stubLocalAppState.name);
-  });
-
-  it("when imported data state has elements", () => {
-    const stubLocalAppState = getDefaultAppState();
-
-    const textElement = API.createElement({ type: "text" });
-    const rectElement = API.createElement({ type: "rectangle" });
-    const elements = [textElement, rectElement];
-
-    const importedDataState = {} as ImportedDataState;
-    importedDataState.elements = elements;
-
-    const restoredData = restore.restore(
-      importedDataState,
-      stubLocalAppState,
-      null,
-    );
-    expect(restoredData.elements.length).toBe(elements.length);
-  });
-
-  it("when local app state is null but imported app state is supplied", () => {
-    const stubImportedAppState = getDefaultAppState();
-    stubImportedAppState.cursorButton = "down";
-    stubImportedAppState.name = "imported app state";
-
-    const importedDataState = {} as ImportedDataState;
-    importedDataState.appState = stubImportedAppState;
-
-    const restoredData = restore.restore(importedDataState, null, null);
-    expect(restoredData.appState.cursorButton).toBe("up");
-    expect(restoredData.appState.name).toBe(stubImportedAppState.name);
-  });
-
-  it("bump versions of local duplicate elements when supplied", () => {
-    const rectangle = API.createElement({ type: "rectangle" });
-    const ellipse = API.createElement({ type: "ellipse" });
-
-    const rectangle_modified = newElementWith(rectangle, { isDeleted: true });
-
-    const restoredData = restore.restore(
-      { elements: [rectangle, ellipse] },
-      null,
-      [rectangle_modified],
-    );
-
-    expect(restoredData.elements[0].id).toBe(rectangle.id);
-    expect(restoredData.elements[0].versionNonce).not.toBe(
-      rectangle.versionNonce,
-    );
-    expect(restoredData.elements).toEqual([
-      expect.objectContaining({ version: rectangle_modified.version + 2 }),
-      expect.objectContaining({
-        id: ellipse.id,
-        version: ellipse.version + 1,
-      }),
-    ]);
-  });
-});
-
 describe("repairing bindings", () => {
   it("should repair container boundElements when repair is true", () => {
     const container = API.createElement({

packages/utils/src/export.ts

@@ -7,7 +7,10 @@ import {
 } from "@excalidraw/excalidraw/clipboard";
 import { encodePngMetadata } from "@excalidraw/excalidraw/data/image";
 import { serializeAsJSON } from "@excalidraw/excalidraw/data/json";
-import { restore } from "@excalidraw/excalidraw/data/restore";
+import {
+  restoreAppState,
+  restoreElements,
+} from "@excalidraw/excalidraw/data/restore";
 import {
   exportToCanvas as _exportToCanvas,
   exportToSvg as _exportToSvg,
@@ -45,12 +48,11 @@ export const exportToCanvas = ({
 }: ExportOpts & {
   exportPadding?: number;
 }) => {
-  const { elements: restoredElements, appState: restoredAppState } = restore(
+  const restoredElements = restoreElements(elements, null, {
-    { elements, appState },
+    deleteInvisibleElements: true,
-    null,
+  });
-    null,
+  const restoredAppState = restoreAppState(appState, null);
-    { deleteInvisibleElements: true },
+
-  );
   const { exportBackground, viewBackgroundColor } = restoredAppState;
   return _exportToCanvas(
     restoredElements,
@@ -176,12 +178,10 @@ export const exportToSvg = async ({
   skipInliningFonts?: true;
   reuseImages?: boolean;
 }): Promise<SVGSVGElement> => {
-  const { elements: restoredElements, appState: restoredAppState } = restore(
+  const restoredElements = restoreElements(elements, null, {
-    { elements, appState },
+    deleteInvisibleElements: true,
-    null,
+  });
-    null,
+  const restoredAppState = restoreAppState(appState, null);
-    { deleteInvisibleElements: true },
-  );
 
   const exportAppState = {
     ...restoredAppState,