Excalidraw loadFiles behaviour improvements (#7635)

* loadFiles behaviour improvements

* added comments and added one early exit to loadFiles

* more code improvements

Author: hero101

src/domain/common/whiteboard/excalidraw/collab/Collab.ts

@@ -319,17 +319,20 @@ class Collab {
       restoredRemoteElements as RemoteExcalidrawElement[],
       appState
     );
-
-    // Download the files that this instance is missing:
-    await this.filesManager.loadFiles({ files: remoteFiles });
-
     // Avoid broadcasting to the rest of the collaborators the scene
     // we just received!
     // Note: this needs to be set before updating the scene as it
     // synchronously calls render.
     this.lastBroadcastedOrReceivedSceneVersion = hashElementsVersion(reconciledElements);
 
-    return reconciledElements;
+    // Download the files that this instance is missing:
+    return this.filesManager.loadFiles({ files: remoteFiles }).then(() => {
+      // once the files are loaded, we need to update the scene again to render them
+      // instead of updating the scene twice, which is already expensive, we
+      // return the elements once the files have been loaded
+      // that way we render the elements and images at the same time
+      return reconciledElements;
+    });
   };
 
   private handleRemoteSceneUpdate = async (

src/domain/common/whiteboard/excalidraw/useWhiteboardFilesManager.ts

@@ -201,38 +201,42 @@ const useWhiteboardFilesManager = ({
     }
 
     const files = whiteboard.files;
+    // leave only the incoming files that don't have a dataURL and are not in the fileStore
+    const pendingFileIds = Object.keys(files).filter(fileId => !files[fileId]?.dataURL && !fileStore.current[fileId]);
+
+    if (!pendingFileIds.length) {
+      return;
+    }
 
-    const pendingFileIds = Object.keys(files).filter(fileId => !files[fileId]?.dataURL);
     log('I need to download these files', pendingFileIds);
-    const newFiles: BinaryFilesWithUrl = {};
 
     setDownloadingFiles(true);
 
-    try {
-      await Promise.all(
-        pendingFileIds.map(async fileId => {
-          const file = whiteboard!.files![fileId];
-          if (fileStore.current[fileId]?.dataURL) {
-            log(`No need to download ${fileId} already in the store`, fileStore.current[fileId]);
-            return;
-          }
-          if (file.url) {
-            log('DOWNLOADING ', file);
-            try {
-              const dataURL = await fetchFileToDataURL(file.url);
-              newFiles[fileId] = { ...file, dataURL } as BinaryFileDataWithUrl;
-              fileStoreAddFile(fileId, newFiles[fileId]);
-            } catch (e) {
-              error(`Error downloading file: ${file.url}`, { label: 'whiteboard-file-manager' });
-            }
-          } else {
-            error(`Cannot download: ${file.id}`, { label: 'whiteboard-file-manager' });
-          }
-        })
-      );
-    } finally {
-      setDownloadingFiles(false);
-    }
+    await Promise.allSettled(
+      pendingFileIds.map(async fileId => {
+        if (fileStore.current[fileId]?.dataURL) {
+          log(`No need to download ${fileId} already in the store`, fileStore.current[fileId]);
+          return;
+        }
+        const file = whiteboard!.files![fileId];
+        if (!file.url) {
+          error(`Cannot download: ${file.id}`, { label: 'whiteboard-file-manager' });
+          throw new Error(`Cannot download: ${file.id}`);
+        }
+
+        log('DOWNLOADING ', file);
+        try {
+          const dataURL = await fetchFileToDataURL(file.url);
+          // try-catch will avoid putting the file in the store if fetching fails
+          fileStoreAddFile(fileId, { ...file, dataURL } as BinaryFileDataWithUrl);
+        } catch (e) {
+          error(`Error downloading file: ${file.url}`, { label: 'whiteboard-file-manager' });
+          throw e;
+        }
+      })
+    );
+
+    setDownloadingFiles(false);
   };
 
   /**