fix: Don't save deleted ExcalidrawElements to Firebase (#5108)

Co-authored-by: dwelle <luzar.david@gmail.com>

src/excalidraw-app/app_constants.ts

@@ -6,6 +6,7 @@ export const LOAD_IMAGES_TIMEOUT = 500;
 export const SYNC_FULL_SCENE_INTERVAL_MS = 20000;
 export const SYNC_BROWSER_TABS_TIMEOUT = 50;
 export const CURSOR_SYNC_TIMEOUT = 33; // ~30fps
+export const DELETED_ELEMENT_TIMEOUT = 24 * 60 * 60 * 1000; // 1 day
 
 export const FILE_UPLOAD_MAX_BYTES = 3 * 1024 * 1024; // 3 MiB
 // 1 year (https://stackoverflow.com/a/25201898/927631)

src/excalidraw-app/collab/CollabWrapper.tsx

@@ -30,7 +30,9 @@ import {
   generateCollaborationLinkData,
   getCollaborationLink,
   getCollabServer,
+  getSyncableElements,
   SocketUpdateDataSource,
+  SyncableExcalidrawElement,
 } from "../data";
 import {
   isSavedToFirebase,
@@ -50,7 +52,6 @@ import { t } from "../../i18n";
 import { UserIdleState } from "../../types";
 import { IDLE_THRESHOLD, ACTIVE_THRESHOLD } from "../../constants";
 import { trackEvent } from "../../analytics";
-import { isInvisiblySmallElement } from "../../element";
 import {
   encodeFilesForUpload,
   FileManager,
@@ -202,7 +203,7 @@ class CollabWrapper extends PureComponent<Props, CollabState> {
   };
 
   private beforeUnload = withBatchedUpdates((event: BeforeUnloadEvent) => {
-    const syncableElements = this.getSyncableElements(
+    const syncableElements = getSyncableElements(
       this.getSceneElementsIncludingDeleted(),
     );
 
@@ -232,7 +233,7 @@ class CollabWrapper extends PureComponent<Props, CollabState> {
   });
 
   saveCollabRoomToFirebase = async (
-    syncableElements: readonly ExcalidrawElement[],
+    syncableElements: readonly SyncableExcalidrawElement[],
   ) => {
     try {
       const savedData = await saveToFirebase(
@@ -262,7 +263,7 @@ class CollabWrapper extends PureComponent<Props, CollabState> {
     this.loadImageFiles.cancel();
 
     this.saveCollabRoomToFirebase(
-      this.getSyncableElements(
+      getSyncableElements(
         this.excalidrawAPI.getSceneElementsIncludingDeleted(),
       ),
     );
@@ -413,7 +414,7 @@ class CollabWrapper extends PureComponent<Props, CollabState> {
         commitToHistory: true,
       });
 
-      this.saveCollabRoomToFirebase(this.getSyncableElements(elements));
+      this.saveCollabRoomToFirebase(getSyncableElements(elements));
     }
 
     // fallback in case you're not alone in the room but still don't receive
@@ -749,7 +750,7 @@ class CollabWrapper extends PureComponent<Props, CollabState> {
   queueSaveToFirebase = throttle(() => {
     if (this.portal.socketInitialized) {
       this.saveCollabRoomToFirebase(
-        this.getSyncableElements(
+        getSyncableElements(
           this.excalidrawAPI.getSceneElementsIncludingDeleted(),
         ),
       );
@@ -775,13 +776,6 @@ class CollabWrapper extends PureComponent<Props, CollabState> {
     });
   };
 
-  isSyncableElement = (element: ExcalidrawElement) => {
-    return element.isDeleted || !isInvisiblySmallElement(element);
-  };
-
-  getSyncableElements = (elements: readonly ExcalidrawElement[]) =>
-    elements.filter((element) => this.isSyncableElement(element));
-
   /** PRIVATE. Use `this.getContextValue()` instead. */
   private contextValue: CollabAPI | null = null;
 

src/excalidraw-app/collab/Portal.tsx

@@ -1,4 +1,8 @@
-import { SocketUpdateData, SocketUpdateDataSource } from "../data";
+import {
+  isSyncableElement,
+  SocketUpdateData,
+  SocketUpdateDataSource,
+} from "../data";
 
 import CollabWrapper from "./CollabWrapper";
 
@@ -143,7 +147,7 @@ class Portal {
             !this.broadcastedElementVersions.has(element.id) ||
             element.version >
               this.broadcastedElementVersions.get(element.id)!) &&
-          this.collab.isSyncableElement(element)
+          isSyncableElement(element)
         ) {
           acc.push({
             ...element,

src/excalidraw-app/data/firebase.ts

@@ -13,6 +13,7 @@ import { decompressData } from "../../data/encode";
 import { encryptData, decryptData } from "../../data/encryption";
 import { MIME_TYPES } from "../../constants";
 import { reconcileElements } from "../collab/reconciliation";
+import { getSyncableElements, SyncableExcalidrawElement } from ".";
 
 // private
 // -----------------------------------------------------------------------------
@@ -127,7 +128,18 @@ const decryptElements = async (
   return JSON.parse(decodedData);
 };
 
-const firebaseSceneVersionCache = new WeakMap<SocketIOClient.Socket, number>();
+class FirebaseSceneVersionCache {
+  private static cache = new WeakMap<SocketIOClient.Socket, number>();
+  static get = (socket: SocketIOClient.Socket) => {
+    return FirebaseSceneVersionCache.cache.get(socket);
+  };
+  static set = (
+    socket: SocketIOClient.Socket,
+    elements: readonly SyncableExcalidrawElement[],
+  ) => {
+    FirebaseSceneVersionCache.cache.set(socket, getSceneVersion(elements));
+  };
+}
 
 export const isSavedToFirebase = (
   portal: Portal,
@@ -136,7 +148,7 @@ export const isSavedToFirebase = (
   if (portal.socket && portal.roomId && portal.roomKey) {
     const sceneVersion = getSceneVersion(elements);
 
-    return firebaseSceneVersionCache.get(portal.socket) === sceneVersion;
+    return FirebaseSceneVersionCache.get(portal.socket) === sceneVersion;
   }
   // if no room exists, consider the room saved so that we don't unnecessarily
   // prevent unload (there's nothing we could do at that point anyway)
@@ -181,7 +193,7 @@ export const saveFilesToFirebase = async ({
 
 const createFirebaseSceneDocument = async (
   firebase: ResolutionType<typeof loadFirestore>,
-  elements: readonly ExcalidrawElement[],
+  elements: readonly SyncableExcalidrawElement[],
   roomKey: string,
 ) => {
   const sceneVersion = getSceneVersion(elements);
@@ -197,7 +209,7 @@ const createFirebaseSceneDocument = async (
 
 export const saveToFirebase = async (
   portal: Portal,
-  elements: readonly ExcalidrawElement[],
+  elements: readonly SyncableExcalidrawElement[],
   appState: AppState,
 ) => {
   const { roomId, roomKey, socket } = portal;
@@ -229,18 +241,18 @@ export const saveToFirebase = async (
       transaction.set(docRef, sceneDocument);
 
       return {
-        sceneVersion: sceneDocument.sceneVersion,
+        elements,
         reconciledElements: null,
       };
     }
 
     const prevDocData = snapshot.data() as FirebaseStoredScene;
-    const prevElements = await decryptElements(prevDocData, roomKey);
+    const prevElements = getSyncableElements(
+      await decryptElements(prevDocData, roomKey),
+    );
 
-    const reconciledElements = reconcileElements(
-      elements,
-      prevElements,
-      appState,
+    const reconciledElements = getSyncableElements(
+      reconcileElements(elements, prevElements, appState),
     );
 
     const sceneDocument = await createFirebaseSceneDocument(
@@ -251,14 +263,14 @@ export const saveToFirebase = async (
 
     transaction.update(docRef, sceneDocument);
     return {
+      elements,
       reconciledElements,
-      sceneVersion: sceneDocument.sceneVersion,
     };
   });
 
-  firebaseSceneVersionCache.set(socket, savedData.sceneVersion);
+  FirebaseSceneVersionCache.set(socket, savedData.elements);
 
-  return savedData;
+  return { reconciledElements: savedData.reconciledElements };
 };
 
 export const loadFromFirebase = async (
@@ -275,10 +287,12 @@ export const loadFromFirebase = async (
     return null;
   }
   const storedScene = doc.data() as FirebaseStoredScene;
-  const elements = await decryptElements(storedScene, roomKey);
+  const elements = getSyncableElements(
+    await decryptElements(storedScene, roomKey),
+  );
 
   if (socket) {
-    firebaseSceneVersionCache.set(socket, getSceneVersion(elements));
+    FirebaseSceneVersionCache.set(socket, elements);
   }
 
   return restoreElements(elements, null);

src/excalidraw-app/data/index.ts

@@ -7,6 +7,7 @@ import {
 import { serializeAsJSON } from "../../data/json";
 import { restore } from "../../data/restore";
 import { ImportedDataState } from "../../data/types";
+import { isInvisiblySmallElement } from "../../element/sizeHelpers";
 import { isInitializedImageElement } from "../../element/typeChecks";
 import { ExcalidrawElement, FileId } from "../../element/types";
 import { t } from "../../i18n";
@@ -17,10 +18,35 @@ import {
   UserIdleState,
 } from "../../types";
 import { bytesToHexString } from "../../utils";
-import { FILE_UPLOAD_MAX_BYTES, ROOM_ID_BYTES } from "../app_constants";
+import {
+  DELETED_ELEMENT_TIMEOUT,
+  FILE_UPLOAD_MAX_BYTES,
+  ROOM_ID_BYTES,
+} from "../app_constants";
 import { encodeFilesForUpload } from "./FileManager";
 import { saveFilesToFirebase } from "./firebase";
 
+export type SyncableExcalidrawElement = ExcalidrawElement & {
+  _brand: "SyncableExcalidrawElement";
+};
+
+export const isSyncableElement = (
+  element: ExcalidrawElement,
+): element is SyncableExcalidrawElement => {
+  if (element.isDeleted) {
+    if (element.updated > Date.now() - DELETED_ELEMENT_TIMEOUT) {
+      return true;
+    }
+    return false;
+  }
+  return !isInvisiblySmallElement(element);
+};
+
+export const getSyncableElements = (elements: readonly ExcalidrawElement[]) =>
+  elements.filter((element) =>
+    isSyncableElement(element),
+  ) as SyncableExcalidrawElement[];
+
 const BACKEND_V2_GET = process.env.REACT_APP_BACKEND_V2_GET_URL;
 const BACKEND_V2_POST = process.env.REACT_APP_BACKEND_V2_POST_URL;