Browse Source

fix: image-related fixes (#4147)

* flush queues on portal close

* fix mouse broadcast race condition

* stop mutating image elements when updating status

to fix race condition when closing/opening collab room

* check `files` when resolving `LayerUI`

* fix displaying AbortError
David Luzar 3 years ago
parent
commit
c61f95a327

+ 6 - 6
src/components/App.tsx

@@ -113,7 +113,11 @@ import {
   updateBoundElements,
 } from "../element/binding";
 import { LinearElementEditor } from "../element/linearElementEditor";
-import { bumpVersion, mutateElement } from "../element/mutateElement";
+import {
+  bumpVersion,
+  mutateElement,
+  newElementWith,
+} from "../element/mutateElement";
 import { deepCopyElement, newFreeDrawElement } from "../element/newElement";
 import {
   isBindingElement,
@@ -4268,11 +4272,7 @@ class App extends React.Component<AppProps, AppState> {
         }
 
         if (erroredFiles.has(element.fileId)) {
-          mutateElement(
-            element,
-            { status: "error" },
-            /* informMutation */ false,
-          );
+          newElementWith(element, { status: "error" });
         }
       }
     }

+ 1 - 0
src/components/LayerUI.tsx

@@ -845,6 +845,7 @@ const areEqual = (prev: LayerUIProps, next: LayerUIProps) => {
     prev.renderCustomFooter === next.renderCustomFooter &&
     prev.langCode === next.langCode &&
     prev.elements === next.elements &&
+    prev.files === next.files &&
     keys.every((key) => prevAppState[key] === nextAppState[key])
   );
 };

+ 6 - 7
src/excalidraw-app/collab/CollabWrapper.tsx

@@ -60,7 +60,7 @@ import {
   isImageElement,
   isInitializedImageElement,
 } from "../../element/typeChecks";
-import { mutateElement } from "../../element/mutateElement";
+import { newElementWith } from "../../element/mutateElement";
 import {
   ReconciledElements,
   reconcileElements as _reconcileElements,
@@ -241,6 +241,9 @@ class CollabWrapper extends PureComponent<Props, CollabState> {
   };
 
   closePortal = () => {
+    this.queueBroadcastAllElements.cancel();
+    this.loadImageFiles.cancel();
+
     this.saveCollabRoomToFirebase();
     if (window.confirm(t("alerts.collabStopOverridePrompt"))) {
       window.history.pushState({}, APP_NAME, window.location.origin);
@@ -253,7 +256,7 @@ class CollabWrapper extends PureComponent<Props, CollabState> {
         .getSceneElementsIncludingDeleted()
         .map((element) => {
           if (isImageElement(element) && element.status === "saved") {
-            return mutateElement(element, { status: "pending" }, false);
+            return newElementWith(element, { status: "pending" });
           }
           return element;
         });
@@ -351,11 +354,7 @@ class CollabWrapper extends PureComponent<Props, CollabState> {
     } else {
       const elements = this.excalidrawAPI.getSceneElements().map((element) => {
         if (isImageElement(element) && element.status === "saved") {
-          return mutateElement(
-            element,
-            { status: "pending" },
-            /* informMutation */ false,
-          );
+          return newElementWith(element, { status: "pending" });
         }
         return element;
       });

+ 11 - 12
src/excalidraw-app/collab/Portal.tsx

@@ -11,7 +11,7 @@ import { BROADCAST, FILE_UPLOAD_TIMEOUT, SCENE } from "../app_constants";
 import { UserIdleState } from "../../types";
 import { trackEvent } from "../../analytics";
 import { throttle } from "lodash";
-import { mutateElement } from "../../element/mutateElement";
+import { newElementWith } from "../../element/mutateElement";
 import { BroadcastedExcalidrawElement } from "./reconciliation";
 
 class Portal {
@@ -54,6 +54,7 @@ class Portal {
     if (!this.socket) {
       return;
     }
+    this.queueFileUpload.flush();
     this.socket.close();
     this.socket = null;
     this.roomId = null;
@@ -79,7 +80,7 @@ class Portal {
       const json = JSON.stringify(data);
       const encoded = new TextEncoder().encode(json);
       const encrypted = await encryptAESGEM(encoded, this.roomKey!);
-      this.socket!.emit(
+      this.socket?.emit(
         volatile ? BROADCAST.SERVER_VOLATILE : BROADCAST.SERVER,
         this.roomId,
         encrypted.data,
@@ -95,11 +96,13 @@ class Portal {
         files: this.collab.excalidrawAPI.getFiles(),
       });
     } catch (error) {
-      this.collab.excalidrawAPI.updateScene({
-        appState: {
-          errorMessage: error.message,
-        },
-      });
+      if (error.name !== "AbortError") {
+        this.collab.excalidrawAPI.updateScene({
+          appState: {
+            errorMessage: error.message,
+          },
+        });
+      }
     }
 
     this.collab.excalidrawAPI.updateScene({
@@ -110,11 +113,7 @@ class Portal {
             // this will signal collaborators to pull image data from server
             // (using mutation instead of newElementWith otherwise it'd break
             // in-progress dragging)
-            return mutateElement(
-              element,
-              { status: "saved" },
-              /* informMutation */ false,
-            );
+            return newElementWith(element, { status: "saved" });
           }
           return element;
         }),

+ 3 - 1
src/excalidraw-app/components/ExportToExcalidrawPlus.tsx

@@ -95,7 +95,9 @@ export const ExportToExcalidrawPlus: React.FC<{
             await exportToExcalidrawPlus(elements, appState, files);
           } catch (error) {
             console.error(error);
-            onError(new Error(t("exportDialog.excalidrawplus_exportError")));
+            if (error.name !== "AbortError") {
+              onError(new Error(t("exportDialog.excalidrawplus_exportError")));
+            }
           }
         }}
       />

+ 4 - 8
src/excalidraw-app/data/FileManager.ts

@@ -1,5 +1,5 @@
 import { compressData } from "../../data/encode";
-import { mutateElement } from "../../element/mutateElement";
+import { newElementWith } from "../../element/mutateElement";
 import { isInitializedImageElement } from "../../element/typeChecks";
 import {
   ExcalidrawElement,
@@ -235,13 +235,9 @@ export const updateStaleImageStatuses = (params: {
           isInitializedImageElement(element) &&
           params.erroredFiles.has(element.fileId)
         ) {
-          return mutateElement(
-            element,
-            {
-              status: "error",
-            },
-            false,
-          );
+          return newElementWith(element, {
+            status: "error",
+          });
         }
         return element;
       }),

+ 2 - 6
src/excalidraw-app/index.tsx

@@ -64,7 +64,7 @@ import { ExportToExcalidrawPlus } from "./components/ExportToExcalidrawPlus";
 
 import { getMany, set, del, keys, createStore } from "idb-keyval";
 import { FileManager, updateStaleImageStatuses } from "./data/FileManager";
-import { mutateElement } from "../element/mutateElement";
+import { newElementWith } from "../element/mutateElement";
 import { isInitializedImageElement } from "../element/typeChecks";
 import { loadFilesFromFirebase } from "./data/firebase";
 
@@ -465,11 +465,7 @@ const ExcalidrawWrapper = () => {
             .map((element) => {
               if (localFileStorage.shouldUpdateImageElementStatus(element)) {
                 didChange = true;
-                return mutateElement(
-                  element,
-                  { status: "saved" },
-                  /* informMutation */ false,
-                );
+                return newElementWith(element, { status: "saved" });
               }
               return element;
             });