Rewrite restore to guard against missing migrations (#1664)

src/components/App.tsx

@@ -16,7 +16,7 @@ import {
   getCommonBounds,
   getCursorForResizingElement,
   getPerfectElementSize,
-  normalizeDimensions,
+  getNormalizedDimensions,
   getElementMap,
   getDrawingVersion,
   getSyncableElements,
@@ -2527,7 +2527,12 @@ class App extends React.Component<any, AppState> {
         return;
       }
 
-      normalizeDimensions(draggingElement);
+      if (draggingElement) {
+        mutateElement(
+          draggingElement,
+          getNormalizedDimensions(draggingElement),
+        );
+      }
 
       if (resizingElement) {
         history.resumeRecording();

src/data/restore.ts

@@ -1,17 +1,11 @@
-import { Point } from "../types";
-
 import {
   ExcalidrawElement,
-  ExcalidrawTextElement,
   FontFamily,
+  ExcalidrawSelectionElement,
 } from "../element/types";
 import { AppState } from "../types";
 import { DataState } from "./types";
-import {
-  isInvisiblySmallElement,
-  normalizeDimensions,
-  isTextElement,
-} from "../element";
+import { isInvisiblySmallElement, getNormalizedDimensions } from "../element";
 import { calculateScrollCenter } from "../scene";
 import { randomId } from "../random";
 import { DEFAULT_TEXT_ALIGN, DEFAULT_FONT_FAMILY } from "../appState";
@@ -26,96 +20,105 @@ const getFontFamilyByName = (fontFamilyName: string): FontFamily => {
   return DEFAULT_FONT_FAMILY;
 };
 
+function migrateElementWithProperties<T extends ExcalidrawElement>(
+  element: T,
+  extra: Omit<T, keyof ExcalidrawElement>,
+): T {
+  const base: Pick<T, keyof ExcalidrawElement> = {
+    type: element.type,
+    // all elements must have version > 0 so getDrawingVersion() will pick up
+    //  newly added elements
+    version: element.version || 1,
+    versionNonce: element.versionNonce ?? 0,
+    isDeleted: false,
+    id: element.id || randomId(),
+    fillStyle: element.fillStyle || "hachure",
+    strokeWidth: element.strokeWidth || 1,
+    strokeStyle: element.strokeStyle ?? "solid",
+    roughness: element.roughness ?? 1,
+    opacity: element.opacity == null ? 100 : element.opacity,
+    angle: element.angle || 0,
+    x: element.x || 0,
+    y: element.y || 0,
+    strokeColor: element.strokeColor,
+    backgroundColor: element.backgroundColor,
+    width: element.width || 0,
+    height: element.height || 0,
+    seed: element.seed ?? 1,
+    groupIds: element.groupIds || [],
+  };
+
+  return {
+    ...base,
+    ...getNormalizedDimensions(base),
+    ...extra,
+  } as T;
+}
+
+const migrateElement = (
+  element: Exclude<ExcalidrawElement, ExcalidrawSelectionElement>,
+): typeof element => {
+  switch (element.type) {
+    case "text":
+      let fontSize = element.fontSize;
+      let fontFamily = element.fontFamily;
+      if ("font" in element) {
+        const [fontPx, _fontFamily]: [
+          string,
+          string,
+        ] = (element as any).font.split(" ");
+        fontSize = parseInt(fontPx, 10);
+        fontFamily = getFontFamilyByName(_fontFamily);
+      }
+      return migrateElementWithProperties(element, {
+        fontSize,
+        fontFamily,
+        text: element.text ?? "",
+        baseline: element.baseline,
+        textAlign: element.textAlign ?? DEFAULT_TEXT_ALIGN,
+      });
+    case "draw":
+    case "line":
+    case "arrow": {
+      return migrateElementWithProperties(element, {
+        points:
+          // migrate old arrow model to new one
+          !Array.isArray(element.points) || element.points.length < 2
+            ? [
+                [0, 0],
+                [element.width, element.height],
+              ]
+            : element.points,
+      });
+    }
+    // generic elements
+    case "ellipse":
+    case "rectangle":
+    case "diamond":
+      return migrateElementWithProperties(element, {});
+
+    // don't use default case so as to catch a missing an element type case
+    //  (we also don't want to throw, but instead return void so we
+    //   filter out these unsupported elements from the restored array)
+  }
+};
+
 export const restore = (
-  // we're making the elements mutable for this API because we want to
-  //  efficiently remove/tweak properties on them (to migrate old scenes)
-  savedElements: readonly Mutable<ExcalidrawElement>[],
+  savedElements: readonly ExcalidrawElement[],
   savedState: AppState | null,
   opts?: { scrollToContent: boolean },
 ): DataState => {
-  const elements = savedElements
-    .filter((el) => {
-      // filtering out selection, which is legacy, no longer kept in elements,
-      //  and causing issues if retained
-      return el.type !== "selection" && !isInvisiblySmallElement(el);
-    })
-    .map((element) => {
-      let points: Point[] = [];
-      if (element.type === "arrow") {
-        if (Array.isArray(element.points)) {
-          // if point array is empty, add one point to the arrow
-          // this is used as fail safe to convert incoming data to a valid
-          // arrow. In the new arrow, width and height are not being usde
-          points = element.points.length > 0 ? element.points : [[0, 0]];
-        } else {
-          // convert old arrow type to a new one
-          // old arrow spec used width and height
-          // to determine the endpoints
-          points = [
-            [0, 0],
-            [element.width, element.height],
-          ];
-        }
-        element.points = points;
-      } else if (element.type === "line" || element.type === "draw") {
-        // old spec, pre-arrows
-        // old spec, post-arrows
-        if (!Array.isArray(element.points) || element.points.length === 0) {
-          points = [
-            [0, 0],
-            [element.width, element.height],
-          ];
-        } else {
-          points = element.points;
-        }
-        element.points = points;
-      } else {
-        if (isTextElement(element)) {
-          if ("font" in element) {
-            const [fontPx, fontFamily]: [
-              string,
-              string,
-            ] = (element as any).font.split(" ");
-            (element as Mutable<ExcalidrawTextElement>).fontSize = parseInt(
-              fontPx,
-              10,
-            );
-            (element as Mutable<
-              ExcalidrawTextElement
-            >).fontFamily = getFontFamilyByName(fontFamily);
-            delete (element as any).font;
-          }
-          if (!element.textAlign) {
-            element.textAlign = DEFAULT_TEXT_ALIGN;
-          }
-        }
-
-        normalizeDimensions(element);
-        // old spec, where non-linear elements used to have empty points arrays
-        if ("points" in element) {
-          delete element.points;
-        }
+  const elements = savedElements.reduce((elements, element) => {
+    // filtering out selection, which is legacy, no longer kept in elements,
+    //  and causing issues if retained
+    if (element.type !== "selection" && !isInvisiblySmallElement(element)) {
+      const migratedElement = migrateElement(element);
+      if (migratedElement) {
+        elements.push(migratedElement);
       }
-
-      return {
-        ...element,
-        // all elements must have version > 0 so getDrawingVersion() will pick
-        //  up newly added elements
-        version: element.version || 1,
-        id: element.id || randomId(),
-        isDeleted: false,
-        fillStyle: element.fillStyle || "hachure",
-        strokeWidth: element.strokeWidth || 1,
-        strokeStyle: element.strokeStyle ?? "solid",
-        roughness: element.roughness ?? 1,
-        opacity:
-          element.opacity === null || element.opacity === undefined
-            ? 100
-            : element.opacity,
-        angle: element.angle ?? 0,
-        groupIds: element.groupIds || [],
-      };
-    });
+    }
+    return elements;
+  }, [] as ExcalidrawElement[]);
 
   if (opts?.scrollToContent && savedState) {
     savedState = { ...savedState, ...calculateScrollCenter(elements) };

src/element/index.ts

@@ -45,7 +45,7 @@ export {
   getPerfectElementSize,
   isInvisiblySmallElement,
   resizePerfectLineForNWHandler,
-  normalizeDimensions,
+  getNormalizedDimensions,
 } from "./sizeHelpers";
 export { showSelectedShapeActions } from "./showSelectedShapeActions";
 

src/element/sizeHelpers.ts

@@ -81,31 +81,32 @@ export const resizePerfectLineForNWHandler = (
   }
 };
 
-/**
- * @returns {boolean} whether element was normalized
- */
-export const normalizeDimensions = (
-  element: ExcalidrawElement | null,
-): element is ExcalidrawElement => {
-  if (!element || (element.width >= 0 && element.height >= 0)) {
-    return false;
-  }
+export const getNormalizedDimensions = (
+  element: Pick<ExcalidrawElement, "width" | "height" | "x" | "y">,
+): {
+  width: ExcalidrawElement["width"];
+  height: ExcalidrawElement["height"];
+  x: ExcalidrawElement["x"];
+  y: ExcalidrawElement["y"];
+} => {
+  const ret = {
+    width: element.width,
+    height: element.height,
+    x: element.x,
+    y: element.y,
+  };
 
   if (element.width < 0) {
     const nextWidth = Math.abs(element.width);
-    mutateElement(element, {
-      width: nextWidth,
-      x: element.x - nextWidth,
-    });
+    ret.width = nextWidth;
+    ret.x = element.x - nextWidth;
   }
 
   if (element.height < 0) {
     const nextHeight = Math.abs(element.height);
-    mutateElement(element, {
-      height: nextHeight,
-      y: element.y - nextHeight,
-    });
+    ret.height = nextHeight;
+    ret.y = element.y - nextHeight;
   }
 
-  return true;
+  return ret;
 };

src/element/types.ts

@@ -24,12 +24,17 @@ type _ExcalidrawElementBase = Readonly<{
   groupIds: GroupId[];
 }>;
 
+export type ExcalidrawSelectionElement = _ExcalidrawElementBase & {
+  type: "selection";
+};
 /**
  * These are elements that don't have any additional properties.
  */
-export type ExcalidrawGenericElement = _ExcalidrawElementBase & {
-  type: "selection" | "rectangle" | "diamond" | "ellipse";
-};
+export type ExcalidrawGenericElement =
+  | ExcalidrawSelectionElement
+  | (_ExcalidrawElementBase & {
+      type: "rectangle" | "diamond" | "ellipse";
+    });
 
 /**
  * ExcalidrawElement should be JSON serializable and (eventually) contain