Improve performance of adding and removing files (#1949)

* core: add an addFiles() method that only updates state once

Previously, adding 1300-ish files took around 260ms, and looked like
this in Firefox's performance tab:

![Flamegraph](https://i.imgur.com/08veuoU.png)

All of the downward peaks are `setState()` calls and GC.

Now it takes around 60ms, and looks like this:

![Flamegraph](https://i.imgur.com/xFdwVBV.png)

Here, most of the time is spent generating file IDs and guessing file
types. Those would be areas to look at next.

* dashboard: prevent frequent state update if nothing changed

After the last commit, `addFiles()` still spends a lot of time in
`emit()` and `hideAllPanels()`. The reason is that `addFiles()` still
emits all the hundreds of file-added events, and the Dashboard responds
to each with `hideAllPanels()`, which does a state update. But this all
happens synchronously, and the state almost certainly did not change
since the last `file-added` event that fired a millisecond ago.

This adds a check to avoid the state update if it is not necessary.

![Flamegraph](https://i.imgur.com/KhuD035.png)

Adding 1300 files takes about 40ms now.

With this change, the `addFiles()` call is no longer the slowest
part—now preact rendering all the items is!

* utils: optimize generateFileID and getFileNameAndExtension

Replaces some clever things with more mundane and faster things!

Now, generateFileID is a bunch of string concatenations.
Now, getFileNameAndExtension uses `lastIndexOf()` instead of a regex.

![Flamegraph](https://i.imgur.com/ZQ1IhxI.png)

Adding 1300 files takes about 25ms.

* dashboard: use preact-virtual-list

* thumbnail-generator: add `lazy` option

* dashboard: request thumbnails once file item renders the first time

* dashboard: fork preact-virtual-list

* core: add removeFiles() to remove files in bulk

* Implement removeFile() in terms of removeFiles()

* thumbnail-generator: only queue files that can be previewed

* rename size constants to accommodate WIDTH/HEIGHT

* Use new uppy.addFiles in DragDrop and FileInput

* utils: fix getFileNameAndExtension() type

* Rip out the lazy thumbnail generation

* Rip out virtualization.

* Remove virtualization leftovers

* tell future people that this is intentionally verbose

* Update package-lock.json

* henlo i am spell

* Make `addFiles()` respect maxNumberOfFiles

* core: show an informer error if some files fail in bulk add

* locales: fix quotes to make build:locale-pack happy

Co-authored-by: Artur Paikin <artur@arturpaikin.com>

packages/@uppy/core/src/index.js

@@ -35,6 +35,10 @@ class Uppy {
   constructor (opts) {
     this.defaultLocale = {
       strings: {
+        addBulkFilesFailed: {
+          0: 'Failed to add %{smart_count} file due to an internal error',
+          1: 'Failed to add %{smart_count} files due to internal errors'
+        },
         youCanOnlyUploadX: {
           0: 'You can only upload %{smart_count} file',
           1: 'You can only upload %{smart_count} files',
@@ -416,14 +420,15 @@ class Uppy {
    * Check if file passes a set of restrictions set in options: maxFileSize,
    * maxNumberOfFiles and allowedFileTypes.
    *
+   * @param {object} files Object of IDs → files already added
    * @param {object} file object to check
    * @private
    */
-  _checkRestrictions (file) {
+  _checkRestrictions (files, file) {
     const { maxFileSize, maxNumberOfFiles, allowedFileTypes } = this.opts.restrictions
 
     if (maxNumberOfFiles) {
-      if (Object.keys(this.getState().files).length + 1 > maxNumberOfFiles) {
+      if (Object.keys(files).length + 1 > maxNumberOfFiles) {
         throw new RestrictionError(`${this.i18n('youCanOnlyUploadX', { smart_count: maxNumberOfFiles })}`)
       }
     }
@@ -479,21 +484,22 @@ class Uppy {
     throw (typeof err === 'object' ? err : new Error(err))
   }
 
-  /**
-   * Add a new file to `state.files`. This will run `onBeforeFileAdded`,
-   * try to guess file type in a clever way, check file against restrictions,
-   * and start an upload if `autoProceed === true`.
-   *
-   * @param {object} file object to add
-   * @returns {string} id for the added file
-   */
-  addFile (file) {
-    const { files, allowNewUpload } = this.getState()
+  _assertNewUploadAllowed (file) {
+    const { allowNewUpload } = this.getState()
 
     if (allowNewUpload === false) {
       this._showOrLogErrorAndThrow(new RestrictionError('Cannot add new files: already uploading.'), { file })
     }
+  }
 
+  /**
+   * Create a file state object based on user-provided `addFile()` options.
+   *
+   * Note this is extremely side-effectful and should only be done when a file state object will be added to state immediately afterward!
+   *
+   * The `files` value is passed in because it may be updated by the caller without updating the store.
+   */
+  _checkAndCreateFileStateObject (files, file) {
     const fileType = getFileType(file)
     file.type = fileType
 
@@ -536,7 +542,10 @@ class Uppy {
       id: fileID,
       name: fileName,
       extension: fileExtension || '',
-      meta: Object.assign({}, this.getState().meta, meta),
+      meta: {
+        ...this.getState().meta,
+        ...meta
+      },
       type: fileType,
       data: file.data,
       progress: {
@@ -553,20 +562,16 @@ class Uppy {
     }
 
     try {
-      this._checkRestrictions(newFile)
+      this._checkRestrictions(files, newFile)
     } catch (err) {
       this._showOrLogErrorAndThrow(err, { file: newFile })
     }
 
-    this.setState({
-      files: Object.assign({}, files, {
-        [fileID]: newFile
-      })
-    })
-
-    this.emit('file-added', newFile)
-    this.log(`Added file: ${fileName}, ${fileID}, mime type: ${fileType}`)
+    return newFile
+  }
 
+  // Schedule an upload if `autoProceed` is enabled.
+  _startIfAutoProceed () {
     if (this.opts.autoProceed && !this.scheduledAutoProceed) {
       this.scheduledAutoProceed = setTimeout(() => {
         this.scheduledAutoProceed = null
@@ -577,49 +582,153 @@ class Uppy {
         })
       }, 4)
     }
+  }
 
-    return fileID
+  /**
+   * Add a new file to `state.files`. This will run `onBeforeFileAdded`,
+   * try to guess file type in a clever way, check file against restrictions,
+   * and start an upload if `autoProceed === true`.
+   *
+   * @param {object} file object to add
+   * @returns {string} id for the added file
+   */
+  addFile (file) {
+    this._assertNewUploadAllowed(file)
+
+    const { files } = this.getState()
+    const newFile = this._checkAndCreateFileStateObject(files, file)
+
+    this.setState({
+      files: {
+        ...files,
+        [newFile.id]: newFile
+      }
+    })
+
+    this.emit('file-added', newFile)
+    this.log(`Added file: ${newFile.name}, ${newFile.id}, mime type: ${newFile.type}`)
+
+    this._startIfAutoProceed()
+
+    return newFile.id
   }
 
-  removeFile (fileID) {
+  /**
+   * Add multiple files to `state.files`. See the `addFile()` documentation.
+   *
+   * This cuts some corners for performance, so should typically only be used in cases where there may be a lot of files.
+   *
+   * If an error occurs while adding a file, it is logged and the user is notified. This is good for UI plugins, but not for programmatic use. Programmatic users should usually still use `addFile()` on individual files.
+   */
+  addFiles (fileDescriptors) {
+    this._assertNewUploadAllowed()
+
+    // create a copy of the files object only once
+    const files = { ...this.getState().files }
+    const newFiles = []
+    const errors = []
+    for (let i = 0; i < fileDescriptors.length; i++) {
+      try {
+        const newFile = this._checkAndCreateFileStateObject(files, fileDescriptors[i])
+        newFiles.push(newFile)
+        files[newFile.id] = newFile
+      } catch (err) {
+        if (!err.isRestriction) {
+          errors.push(err)
+        }
+      }
+    }
+
+    this.setState({ files })
+
+    newFiles.forEach((newFile) => {
+      this.emit('file-added', newFile)
+    })
+    this.log(`Added batch of ${newFiles.length} files`)
+
+    this._startIfAutoProceed()
+
+    if (errors.length > 0) {
+      let message = 'Multiple errors occurred while adding files:\n'
+      errors.forEach((subError) => {
+        message += `\n * ${subError.message}`
+      })
+
+      this.info({
+        message: this.i18n('addBulkFilesFailed', { smart_count: errors.length }),
+        details: message
+      }, 'error', 5000)
+
+      const err = new Error(message)
+      err.errors = errors
+      throw err
+    }
+  }
+
+  removeFiles (fileIDs) {
     const { files, currentUploads } = this.getState()
-    const updatedFiles = Object.assign({}, files)
-    const removedFile = updatedFiles[fileID]
-    delete updatedFiles[fileID]
+    const updatedFiles = { ...files }
+    const updatedUploads = { ...currentUploads }
+
+    const removedFiles = Object.create(null)
+    fileIDs.forEach((fileID) => {
+      if (files[fileID]) {
+        removedFiles[fileID] = files[fileID]
+        delete updatedFiles[fileID]
+      }
+    })
 
-    // Remove this file from its `currentUpload`.
-    const updatedUploads = Object.assign({}, currentUploads)
-    const removeUploads = []
+    // Remove files from the `fileIDs` list in each upload.
+    function fileIsNotRemoved (uploadFileID) {
+      return removedFiles[uploadFileID] === undefined
+    }
+    const uploadsToRemove = []
     Object.keys(updatedUploads).forEach((uploadID) => {
-      const newFileIDs = currentUploads[uploadID].fileIDs.filter((uploadFileID) => uploadFileID !== fileID)
+      const newFileIDs = currentUploads[uploadID].fileIDs.filter(fileIsNotRemoved)
+
       // Remove the upload if no files are associated with it anymore.
       if (newFileIDs.length === 0) {
-        removeUploads.push(uploadID)
+        uploadsToRemove.push(uploadID)
         return
       }
 
-      updatedUploads[uploadID] = Object.assign({}, currentUploads[uploadID], {
+      updatedUploads[uploadID] = {
+        ...currentUploads[uploadID],
         fileIDs: newFileIDs
-      })
+      }
     })
 
-    this.setState({
-      currentUploads: updatedUploads,
-      files: updatedFiles,
-      ...(
-        // If this is the last file we just removed - allow new uploads!
-        Object.keys(updatedFiles).length === 0 &&
-        { allowNewUpload: true }
-      )
+    uploadsToRemove.forEach((uploadID) => {
+      delete updatedUploads[uploadID]
     })
 
-    removeUploads.forEach((uploadID) => {
-      this._removeUpload(uploadID)
-    })
+    const stateUpdate = {
+      currentUploads: updatedUploads,
+      files: updatedFiles
+    }
+
+    // If all files were removed - allow new uploads!
+    if (Object.keys(updatedFiles).length === 0) {
+      stateUpdate.allowNewUpload = true
+    }
+
+    this.setState(stateUpdate)
 
     this._calculateTotalProgress()
-    this.emit('file-removed', removedFile)
-    this.log(`File removed: ${removedFile.id}`)
+
+    const removedFileIDs = Object.keys(removedFiles)
+    removedFileIDs.forEach((fileID) => {
+      this.emit('file-removed', removedFiles[fileID])
+    })
+    if (removedFileIDs.length > 5) {
+      this.log(`Removed ${removedFileIDs.length} files`)
+    } else {
+      this.log(`Removed files: ${removedFileIDs.join(', ')}`)
+    }
+  }
+
+  removeFile (fileID) {
+    this.removeFiles([fileID])
   }
 
   pauseResume (fileID) {
@@ -704,10 +813,12 @@ class Uppy {
   cancelAll () {
     this.emit('cancel-all')
 
-    const files = Object.keys(this.getState().files)
-    files.forEach((fileID) => {
-      this.removeFile(fileID)
-    })
+    const { files } = this.getState()
+
+    const fileIDs = Object.keys(files)
+    if (fileIDs.length) {
+      this.removeFiles(fileIDs)
+    }
 
     this.setState({
       totalProgress: 0,
@@ -1231,7 +1342,7 @@ class Uppy {
    * @param {string} uploadID The ID of the upload.
    */
   _removeUpload (uploadID) {
-    const currentUploads = Object.assign({}, this.getState().currentUploads)
+    const currentUploads = { ...this.getState().currentUploads }
     delete currentUploads[uploadID]
 
     this.setState({

packages/@uppy/dashboard/src/components/Dashboard.js

@@ -24,24 +24,31 @@ function TransitionWrapper (props) {
   )
 }
 
+const WIDTH_XL = 900
+const WIDTH_LG = 700
+const WIDTH_MD = 576
+const HEIGHT_MD = 576
+
 module.exports = function Dashboard (props) {
   const noFiles = props.totalFileCount === 0
 
-  const dashboardClassName = classNames(
-    { 'uppy-Root': props.isTargetDOMEl },
-    'uppy-Dashboard',
-    { 'Uppy--isTouchDevice': isTouchDevice() },
-    { 'uppy-Dashboard--animateOpenClose': props.animateOpenClose },
-    { 'uppy-Dashboard--isClosing': props.isClosing },
-    { 'uppy-Dashboard--isDraggingOver': props.isDraggingOver },
-    { 'uppy-Dashboard--modal': !props.inline },
-    { 'uppy-size--md': props.containerWidth > 576 },
-    { 'uppy-size--lg': props.containerWidth > 700 },
-    { 'uppy-size--xl': props.containerWidth > 900 },
-    { 'uppy-size--height-md': props.containerHeight > 400 },
-    { 'uppy-Dashboard--isAddFilesPanelVisible': props.showAddFilesPanel },
-    { 'uppy-Dashboard--isInnerWrapVisible': props.areInsidesReadyToBeVisible }
-  )
+  const dashboardClassName = classNames({
+    'uppy-Root': props.isTargetDOMEl,
+    'uppy-Dashboard': true,
+    'Uppy--isTouchDevice': isTouchDevice(),
+    'uppy-Dashboard--animateOpenClose': props.animateOpenClose,
+    'uppy-Dashboard--isClosing': props.isClosing,
+    'uppy-Dashboard--isDraggingOver': props.isDraggingOver,
+    'uppy-Dashboard--modal': !props.inline,
+    'uppy-size--md': props.containerWidth > WIDTH_MD,
+    'uppy-size--lg': props.containerWidth > WIDTH_LG,
+    'uppy-size--xl': props.containerWidth > WIDTH_XL,
+    'uppy-size--height-md': props.containerHeight > HEIGHT_MD,
+    'uppy-Dashboard--isAddFilesPanelVisible': props.showAddFilesPanel,
+    'uppy-Dashboard--isInnerWrapVisible': props.areInsidesReadyToBeVisible
+  })
+
+  const showFileList = props.showSelectedFiles && !noFiles
 
   return (
     <div
@@ -83,10 +90,10 @@ module.exports = function Dashboard (props) {
             {props.i18n('dropHint')}
           </div>
 
-          {(!noFiles && props.showSelectedFiles) && <PanelTopBar {...props} />}
+          {showFileList && <PanelTopBar {...props} />}
 
-          {props.showSelectedFiles ? (
-            noFiles ? <AddFiles {...props} /> : <FileList {...props} />
+          {showFileList ? (
+            <FileList {...props} />
           ) : (
             <AddFiles {...props} />
           )}