retain local appState props on restore (#2224)

Co-authored-by: Lipis <[email protected]>

src/components/App.tsx

@@ -599,9 +599,13 @@ class App extends React.Component<ExcalidrawProps, AppState> {
       ) {
         // Backwards compatibility with legacy url format
         if (id) {
-          scene = await loadScene(id);
+          scene = await loadScene(id, null, this.props.initialData);
         } else if (jsonMatch) {
-          scene = await loadScene(jsonMatch[1], jsonMatch[2]);
+          scene = await loadScene(
+            jsonMatch[1],
+            jsonMatch[2],
+            this.props.initialData,
+          );
         }
         if (!isCollaborationScene) {
           window.history.replaceState({}, "Excalidraw", window.location.origin);

src/data/blob.ts

@@ -23,11 +23,11 @@ const loadFileContents = async (blob: any) => {
   return contents;
 };
 
-/**
- * @param blob
- * @param appState if provided, used for centering scroll to restored scene
- */
-export const loadFromBlob = async (blob: any, appState?: AppState) => {
+export const loadFromBlob = async (
+  blob: any,
+  /** @see restore.localAppState */
+  localAppState: AppState | null,
+) => {
   if (blob.handle) {
     // TODO: Make this part of `AppState`.
     (window as any).handle = blob.handle;
@@ -39,16 +39,19 @@ export const loadFromBlob = async (blob: any, appState?: AppState) => {
     if (data.type !== "excalidraw") {
       throw new Error(t("alerts.couldNotLoadInvalidFile"));
     }
-    return restore({
-      elements: data.elements,
-      appState: {
-        appearance: appState?.appearance,
-        ...cleanAppStateForExport(data.appState || {}),
-        ...(appState
-          ? calculateScrollCenter(data.elements || [], appState, null)
-          : {}),
+    return restore(
+      {
+        elements: data.elements,
+        appState: {
+          appearance: localAppState?.appearance,
+          ...cleanAppStateForExport(data.appState || {}),
+          ...(localAppState
+            ? calculateScrollCenter(data.elements || [], localAppState, null)
+            : {}),
+        },
       },
-    });
+      localAppState,
+    );
   } catch {
     throw new Error(t("alerts.couldNotLoadInvalidFile"));
   }

src/data/index.ts

@@ -233,18 +233,15 @@ const importFromBackend = async (
   id: string | null,
   privateKey?: string | null,
 ): Promise<ImportedDataState> => {
-  let elements: readonly ExcalidrawElement[] = [];
-  let appState = getDefaultAppState();
-
   try {
     const response = await fetch(
       privateKey ? `${BACKEND_V2_GET}${id}` : `${BACKEND_GET}${id}.json`,
     );
     if (!response.ok) {
       window.alert(t("alerts.importBackendFailed"));
-      return { elements, appState };
+      return {};
     }
-    let data;
+    let data: ImportedDataState;
     if (privateKey) {
       const buffer = await response.arrayBuffer();
       const key = await getImportedKey(privateKey, "decrypt");
@@ -267,13 +264,14 @@ const importFromBackend = async (
       data = await response.json();
     }
 
-    elements = data.elements || elements;
-    appState = { ...appState, ...data.appState };
+    return {
+      elements: data.elements || null,
+      appState: data.appState || null,
+    };
   } catch (error) {
     window.alert(t("alerts.importBackendFailed"));
     console.error(error);
-  } finally {
-    return { elements, appState };
+    return {};
   }
 };
 
@@ -363,16 +361,22 @@ export const exportCanvas = async (
 
 export const loadScene = async (
   id: string | null,
-  privateKey?: string | null,
-  initialData?: ImportedDataState,
+  privateKey: string | null,
+  // Supply initialData 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`.
+  initialData: ImportedDataState | undefined | null,
 ) => {
   let data;
   if (id != 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));
+    data = restore(
+      await importFromBackend(id, privateKey),
+      initialData?.appState,
+    );
   } else {
-    data = restore(initialData || {});
+    data = restore(initialData || {}, null);
   }
 
   return {

src/data/json.ts

@@ -45,13 +45,13 @@ export const saveAsJSON = async (
   );
 };
 
-export const loadFromJSON = async (appState: AppState) => {
+export const loadFromJSON = async (localAppState: AppState) => {
   const blob = await fileOpen({
     description: "Excalidraw files",
     extensions: [".json", ".excalidraw"],
     mimeTypes: ["application/json"],
   });
-  return loadFromBlob(blob, appState);
+  return loadFromBlob(blob, localAppState);
 };
 
 export const isValidLibrary = (json: any) => {

src/data/localStorage.ts

@@ -1,7 +1,7 @@
 import { ExcalidrawElement } from "../element/types";
 import { AppState, LibraryItems } from "../types";
 import { clearAppStateForLocalStorage, getDefaultAppState } from "../appState";
-import { restore } from "./restore";
+import { restoreElements } from "./restore";
 
 const LOCAL_STORAGE_KEY = "excalidraw";
 const LOCAL_STORAGE_KEY_STATE = "excalidraw-state";
@@ -21,8 +21,8 @@ export const loadLibrary = (): Promise<LibraryItems> => {
         return resolve([]);
       }
 
-      const items = (JSON.parse(data) as LibraryItems).map(
-        (elements) => restore({ elements, appState: null }).elements,
+      const items = (JSON.parse(data) as LibraryItems).map((elements) =>
+        restoreElements(elements),
       ) as Mutable<LibraryItems>;
 
       // clone to ensure we don't mutate the cached library elements in the app

src/data/restore.ts

@@ -118,7 +118,7 @@ const restoreElement = (
   }
 };
 
-const restoreElements = (
+export const restoreElements = (
   elements: ImportedDataState["elements"],
 ): ExcalidrawElement[] => {
   return (elements || []).reduce((elements, element) => {
@@ -134,18 +134,27 @@ const restoreElements = (
   }, [] as ExcalidrawElement[]);
 };
 
-const restoreAppState = (appState: ImportedDataState["appState"]): AppState => {
+const restoreAppState = (
+  appState: ImportedDataState["appState"],
+  localAppState: Partial<AppState> | null,
+): AppState => {
   appState = appState || {};
 
   const defaultAppState = getDefaultAppState();
   const nextAppState = {} as typeof defaultAppState;
 
-  for (const [key, val] of Object.entries(defaultAppState)) {
-    if ((appState as any)[key] !== undefined) {
-      (nextAppState as any)[key] = (appState as any)[key];
-    } else {
-      (nextAppState as any)[key] = val;
-    }
+  for (const [key, val] of Object.entries(defaultAppState) as [
+    keyof typeof defaultAppState,
+    any,
+  ][]) {
+    const restoredValue = appState[key];
+    const localValue = localAppState ? localAppState[key] : undefined;
+    (nextAppState as any)[key] =
+      restoredValue !== undefined
+        ? restoredValue
+        : localValue !== undefined
+        ? localValue
+        : val;
   }
 
   return {
@@ -155,9 +164,18 @@ const restoreAppState = (appState: ImportedDataState["appState"]): AppState => {
   };
 };
 
-export const restore = (data: ImportedDataState): DataState => {
+export const restore = (
+  data: ImportedDataState,
+  /**
+   * 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,
+): DataState => {
   return {
     elements: restoreElements(data.elements),
-    appState: restoreAppState(data.appState),
+    appState: restoreAppState(data.appState, localAppState || null),
   };
 };

src/tests/appState.test.tsx

@@ -0,0 +1,46 @@
+import React from "react";
+import { render, waitFor } from "./test-utils";
+import App from "../components/App";
+import { API } from "./helpers/api";
+import { getDefaultAppState } from "../appState";
+
+const { h } = window;
+
+describe("appState", () => {
+  it("drag&drop file doesn't reset non-persisted appState", async () => {
+    const defaultAppState = getDefaultAppState();
+    const exportBackground = !defaultAppState.exportBackground;
+    render(
+      <App
+        initialData={{
+          appState: {
+            ...defaultAppState,
+            exportBackground,
+            viewBackgroundColor: "#F00",
+          },
+          elements: [],
+        }}
+      />,
+    );
+
+    await waitFor(() => {
+      expect(h.state.exportBackground).toBe(exportBackground);
+      expect(h.state.viewBackgroundColor).toBe("#F00");
+    });
+
+    API.dropFile({
+      appState: {
+        viewBackgroundColor: "#000",
+      },
+      elements: [API.createElement({ type: "rectangle", id: "A" })],
+    });
+
+    await waitFor(() => {
+      expect(h.elements).toEqual([expect.objectContaining({ id: "A" })]);
+      // non-imported prop → retain
+      expect(h.state.exportBackground).toBe(exportBackground);
+      // imported prop → overwrite
+      expect(h.state.viewBackgroundColor).toBe("#000");
+    });
+  });
+});

src/tests/helpers/api.ts

@@ -7,6 +7,8 @@ import {
 import { newElement, newTextElement, newLinearElement } from "../../element";
 import { DEFAULT_VERTICAL_ALIGN } from "../../constants";
 import { getDefaultAppState } from "../../appState";
+import { GlobalTestState, createEvent, fireEvent } from "../test-utils";
+import { ImportedDataState } from "../../data/types";
 
 const { h } = window;
 
@@ -135,4 +137,28 @@ export class API {
     }
     return element as any;
   };
+
+  static dropFile(sceneData: ImportedDataState) {
+    const fileDropEvent = createEvent.drop(GlobalTestState.canvas);
+    const file = new Blob(
+      [
+        JSON.stringify({
+          type: "excalidraw",
+          ...sceneData,
+        }),
+      ],
+      {
+        type: "application/json",
+      },
+    );
+    Object.defineProperty(fileDropEvent, "dataTransfer", {
+      value: {
+        files: [file],
+        getData: (_type: string) => {
+          return "";
+        },
+      },
+    });
+    fireEvent(GlobalTestState.canvas, fileDropEvent);
+  }
 }

src/tests/history.test.tsx

@@ -1,10 +1,10 @@
 import React from "react";
-import { render, GlobalTestState } from "./test-utils";
+import { render } from "./test-utils";
 import App from "../components/App";
 import { UI } from "./helpers/ui";
 import { API } from "./helpers/api";
 import { getDefaultAppState } from "../appState";
-import { waitFor, fireEvent, createEvent } from "@testing-library/react";
+import { waitFor } from "@testing-library/react";
 import { createUndoAction, createRedoAction } from "../actions/actionHistory";
 
 const { h } = window;
@@ -77,31 +77,14 @@ describe("history", () => {
     await waitFor(() =>
       expect(h.elements).toEqual([expect.objectContaining({ id: "A" })]),
     );
-    const fileDropEvent = createEvent.drop(GlobalTestState.canvas);
-    const file = new Blob(
-      [
-        JSON.stringify({
-          type: "excalidraw",
-          appState: {
-            ...getDefaultAppState(),
-            viewBackgroundColor: "#000",
-          },
-          elements: [API.createElement({ type: "rectangle", id: "B" })],
-        }),
-      ],
-      {
-        type: "application/json",
-      },
-    );
-    Object.defineProperty(fileDropEvent, "dataTransfer", {
-      value: {
-        files: [file],
-        getData: (_type: string) => {
-          return "";
-        },
+
+    API.dropFile({
+      appState: {
+        ...getDefaultAppState(),
+        viewBackgroundColor: "#000",
       },
+      elements: [API.createElement({ type: "rectangle", id: "B" })],
     });
-    fireEvent(GlobalTestState.canvas, fileDropEvent);
 
     await waitFor(() => expect(API.getStateHistory().length).toBe(2));
     expect(h.state.viewBackgroundColor).toBe("#000");