From c61f95a327384cd2429dd76fdc79c5a4d6df4932 Mon Sep 17 00:00:00 2001 From: David Luzar Date: Sat, 30 Oct 2021 23:40:35 +0200 Subject: [PATCH] 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 --- src/components/App.tsx | 12 +++++----- src/components/LayerUI.tsx | 1 + src/excalidraw-app/collab/CollabWrapper.tsx | 13 +++++------ src/excalidraw-app/collab/Portal.tsx | 23 +++++++++---------- .../components/ExportToExcalidrawPlus.tsx | 4 +++- src/excalidraw-app/data/FileManager.ts | 12 ++++------ src/excalidraw-app/index.tsx | 8 ++----- 7 files changed, 33 insertions(+), 40 deletions(-) diff --git a/src/components/App.tsx b/src/components/App.tsx index 0ff5c18d5..468f067fa 100644 --- a/src/components/App.tsx +++ b/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 { } if (erroredFiles.has(element.fileId)) { - mutateElement( - element, - { status: "error" }, - /* informMutation */ false, - ); + newElementWith(element, { status: "error" }); } } } diff --git a/src/components/LayerUI.tsx b/src/components/LayerUI.tsx index c958abf77..98fbb8d2e 100644 --- a/src/components/LayerUI.tsx +++ b/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]) ); }; diff --git a/src/excalidraw-app/collab/CollabWrapper.tsx b/src/excalidraw-app/collab/CollabWrapper.tsx index e334be2ba..a2f320e14 100644 --- a/src/excalidraw-app/collab/CollabWrapper.tsx +++ b/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 { }; 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 { .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 { } 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; }); diff --git a/src/excalidraw-app/collab/Portal.tsx b/src/excalidraw-app/collab/Portal.tsx index 4f1d6c6af..9d4479dbe 100644 --- a/src/excalidraw-app/collab/Portal.tsx +++ b/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; }), diff --git a/src/excalidraw-app/components/ExportToExcalidrawPlus.tsx b/src/excalidraw-app/components/ExportToExcalidrawPlus.tsx index d0fbe1c54..da73d879b 100644 --- a/src/excalidraw-app/components/ExportToExcalidrawPlus.tsx +++ b/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"))); + } } }} /> diff --git a/src/excalidraw-app/data/FileManager.ts b/src/excalidraw-app/data/FileManager.ts index 94a29f302..b70ff0340 100644 --- a/src/excalidraw-app/data/FileManager.ts +++ b/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; }), diff --git a/src/excalidraw-app/index.tsx b/src/excalidraw-app/index.tsx index a5f823881..d4bbff0ae 100644 --- a/src/excalidraw-app/index.tsx +++ b/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; });