Browse Source

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>
Renée Kooi 5 years ago
parent
commit
1463ee79ce

+ 163 - 52
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({

+ 25 - 18
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} />
           )}

+ 68 - 62
packages/@uppy/dashboard/src/components/FileItem/index.js

@@ -1,76 +1,82 @@
-const { h } = require('preact')
+const { h, Component } = require('preact')
 const classNames = require('classnames')
-const pure = require('../../utils/pure')
+const shallowEqual = require('is-shallow-equal')
 const FilePreviewAndLink = require('./FilePreviewAndLink')
 const FileProgress = require('./FileProgress')
 const FileInfo = require('./FileInfo')
 const Buttons = require('./Buttons')
 
-module.exports = pure(function FileItem (props) {
-  const file = props.file
+module.exports = class FileItem extends Component {
+  shouldComponentUpdate (nextProps) {
+    return !shallowEqual(this.props, nextProps)
+  }
 
-  const isProcessing = file.progress.preprocess || file.progress.postprocess
-  const isUploaded = file.progress.uploadComplete && !isProcessing && !file.error
-  const uploadInProgressOrComplete = file.progress.uploadStarted || isProcessing
-  const uploadInProgress = (file.progress.uploadStarted && !file.progress.uploadComplete) || isProcessing
-  const isPaused = file.isPaused || false
-  const error = file.error || false
+  render () {
+    const file = this.props.file
 
-  const showRemoveButton = props.individualCancellation
-    ? !isUploaded
-    : !uploadInProgress && !isUploaded
+    const isProcessing = file.progress.preprocess || file.progress.postprocess
+    const isUploaded = file.progress.uploadComplete && !isProcessing && !file.error
+    const uploadInProgressOrComplete = file.progress.uploadStarted || isProcessing
+    const uploadInProgress = (file.progress.uploadStarted && !file.progress.uploadComplete) || isProcessing
+    const isPaused = file.isPaused || false
+    const error = file.error || false
 
-  const dashboardItemClass = classNames(
-    'uppy-u-reset',
-    'uppy-DashboardItem',
-    { 'is-inprogress': uploadInProgress },
-    { 'is-processing': isProcessing },
-    { 'is-complete': isUploaded },
-    { 'is-paused': isPaused },
-    { 'is-error': !!error },
-    { 'is-resumable': props.resumableUploads },
-    { 'is-noIndividualCancellation': !props.individualCancellation }
-  )
+    const showRemoveButton = this.props.individualCancellation
+      ? !isUploaded
+      : !uploadInProgress && !isUploaded
 
-  return (
-    <li class={dashboardItemClass} id={`uppy_${file.id}`}>
-      <div class="uppy-DashboardItem-preview">
-        <FilePreviewAndLink
-          file={file}
-          showLinkToFileUploadResult={props.showLinkToFileUploadResult}
-        />
-        <FileProgress
-          {...props}
-          file={file}
-          error={error}
-          isUploaded={isUploaded}
-        />
-      </div>
+    const dashboardItemClass = classNames({
+      'uppy-u-reset': true,
+      'uppy-DashboardItem': true,
+      'is-inprogress': uploadInProgress,
+      'is-processing': isProcessing,
+      'is-complete': isUploaded,
+      'is-paused': isPaused,
+      'is-error': !!error,
+      'is-resumable': this.props.resumableUploads,
+      'is-noIndividualCancellation': !this.props.individualCancellation
+    })
 
-      <div class="uppy-DashboardItem-fileInfoAndButtons">
-        <FileInfo
-          file={file}
-          id={props.id}
-          acquirers={props.acquirers}
-          containerWidth={props.containerWidth}
-          i18n={props.i18n}
-        />
-        <Buttons
-          file={file}
-          metaFields={props.metaFields}
+    return (
+      <li class={dashboardItemClass} id={`uppy_${file.id}`}>
+        <div class="uppy-DashboardItem-preview">
+          <FilePreviewAndLink
+            file={file}
+            showLinkToFileUploadResult={this.props.showLinkToFileUploadResult}
+          />
+          <FileProgress
+            {...this.props}
+            file={file}
+            error={error}
+            isUploaded={isUploaded}
+          />
+        </div>
 
-          showLinkToFileUploadResult={props.showLinkToFileUploadResult}
-          showRemoveButton={showRemoveButton}
+        <div class="uppy-DashboardItem-fileInfoAndButtons">
+          <FileInfo
+            file={file}
+            id={this.props.id}
+            acquirers={this.props.acquirers}
+            containerWidth={this.props.containerWidth}
+            i18n={this.props.i18n}
+          />
+          <Buttons
+            file={file}
+            metaFields={this.props.metaFields}
 
-          uploadInProgressOrComplete={uploadInProgressOrComplete}
-          removeFile={props.removeFile}
-          toggleFileCard={props.toggleFileCard}
+            showLinkToFileUploadResult={this.props.showLinkToFileUploadResult}
+            showRemoveButton={showRemoveButton}
 
-          i18n={props.i18n}
-          log={props.log}
-          info={props.info}
-        />
-      </div>
-    </li>
-  )
-})
+            uploadInProgressOrComplete={uploadInProgressOrComplete}
+            removeFile={this.props.removeFile}
+            toggleFileCard={this.props.toggleFileCard}
+
+            i18n={this.props.i18n}
+            log={this.props.log}
+            info={this.props.info}
+          />
+        </div>
+      </li>
+    )
+  }
+}

+ 1 - 3
packages/@uppy/dashboard/src/components/FileItem/index.scss

@@ -17,9 +17,7 @@
   .uppy-size--md & {
     // For the Remove button
     position: relative;
-
-    display: block;
-    float: left;
+    display: inline-block;
     margin: 5px $rl-margin;
     width: calc(33.333% - #{$rl-margin} - #{$rl-margin});
     height: 215px;

+ 18 - 18
packages/@uppy/dashboard/src/components/FileList.js

@@ -3,11 +3,10 @@ const classNames = require('classnames')
 const { h } = require('preact')
 
 module.exports = (props) => {
-  const noFiles = props.totalFileCount === 0
-  const dashboardFilesClass = classNames(
-    'uppy-Dashboard-files',
-    { 'uppy-Dashboard-files--noFiles': noFiles }
-  )
+  const dashboardFilesClass = classNames({
+    'uppy-Dashboard-files': true,
+    'uppy-Dashboard-files--noFiles': props.totalFileCount === 0
+  })
 
   const fileProps = {
     // FIXME This is confusing, it's actually the Dashboard's plugin ID
@@ -32,22 +31,23 @@ module.exports = (props) => {
     pauseUpload: props.pauseUpload,
     cancelUpload: props.cancelUpload,
     toggleFileCard: props.toggleFileCard,
-    removeFile: props.removeFile
+    removeFile: props.removeFile,
+    handleRequestThumbnail: props.handleRequestThumbnail
+  }
+
+  function renderItem (fileID) {
+    return (
+      <FileItem
+        key={fileID}
+        {...fileProps}
+        file={props.files[fileID]}
+      />
+    )
   }
 
   return (
-    <ul
-      class={dashboardFilesClass}
-      // making <ul> not focusable for firefox
-      tabindex="-1"
-    >
-      {Object.keys(props.files).map((fileID) => (
-        <FileItem
-          key={fileID}
-          {...fileProps}
-          file={props.files[fileID]}
-        />
-      ))}
+    <ul class={dashboardFilesClass}>
+      {Object.keys(props.files).map(renderItem)}
     </ul>
   )
 }

+ 30 - 28
packages/@uppy/dashboard/src/index.js

@@ -220,11 +220,21 @@ module.exports = class Dashboard extends Plugin {
   }
 
   hideAllPanels () {
-    this.setPluginState({
+    const update = {
       activePickerPanel: false,
       showAddFilesPanel: false,
       activeOverlayType: null
-    })
+    }
+
+    const current = this.getPluginState()
+    if (current.activePickerPanel === update.activePickerPanel &&
+        current.showAddFilesPanel === update.showAddFilesPanel &&
+        current.activeOverlayType === update.activeOverlayType) {
+      // avoid doing a state update if nothing changed
+      return
+    }
+
+    this.setPluginState(update)
   }
 
   showPanel (id) {
@@ -373,23 +383,23 @@ module.exports = class Dashboard extends Plugin {
     })
   }
 
-  addFile (file) {
+  addFiles (files) {
+    const descriptors = files.map((file) => ({
+      source: this.id,
+      name: file.name,
+      type: file.type,
+      data: file,
+      meta: {
+        // path of the file relative to the ancestor directory the user selected.
+        // e.g. 'docs/Old Prague/airbnb.pdf'
+        relativePath: file.relativePath || null
+      }
+    }))
+
     try {
-      this.uppy.addFile({
-        source: this.id,
-        name: file.name,
-        type: file.type,
-        data: file,
-        meta: {
-          // path of the file relative to the ancestor directory the user selected.
-          // e.g. 'docs/Old Prague/airbnb.pdf'
-          relativePath: file.relativePath || null
-        }
-      })
+      this.uppy.addFiles(descriptors)
     } catch (err) {
-      if (!err.isRestriction) {
-        this.uppy.log(err)
-      }
+      this.uppy.log(err)
     }
   }
 
@@ -504,18 +514,13 @@ module.exports = class Dashboard extends Plugin {
 
     // 2. Add all dropped files
     const files = toArray(event.clipboardData.files)
-    files.forEach((file) => {
-      this.uppy.log('[Dashboard] File pasted')
-      this.addFile(file)
-    })
+    this.addFiles(files)
   }
 
   handleInputChange (event) {
     event.preventDefault()
     const files = toArray(event.target.files)
-    files.forEach((file) =>
-      this.addFile(file)
-    )
+    this.addFiles(files)
   }
 
   handleDragOver (event) {
@@ -572,9 +577,7 @@ module.exports = class Dashboard extends Plugin {
       .then((files) => {
         if (files.length > 0) {
           this.uppy.log('[Dashboard] Files were dropped')
-          files.forEach((file) =>
-            this.addFile(file)
-          )
+          this.addFiles(files)
         }
       })
   }
@@ -817,7 +820,6 @@ module.exports = class Dashboard extends Plugin {
       log: this.uppy.log,
       i18n: this.i18n,
       i18nArray: this.i18nArray,
-      addFile: this.uppy.addFile,
       removeFile: this.uppy.removeFile,
       info: this.uppy.info,
       note: this.opts.note,

+ 0 - 21
packages/@uppy/dashboard/src/utils/pure.js

@@ -1,21 +0,0 @@
-const shallowEqual = require('is-shallow-equal')
-const { h, Component } = require('preact')
-
-/**
- * Higher order component that doesn't rerender an element if its props didn't change.
- */
-module.exports = function pure (Inner) {
-  return class Pure extends Component {
-    shouldComponentUpdate (nextProps) {
-      return !shallowEqual(this.props, nextProps)
-    }
-
-    render () {
-      // we have to clone this or Preact mutates it:
-      // https://github.com/preactjs/preact/issues/836
-      // TODO can be removed if we upgrade to Preact X
-      const props = { ...this.props }
-      return <Inner {...props} />
-    }
-  }
-}

+ 21 - 21
packages/@uppy/drag-drop/src/index.js

@@ -44,11 +44,11 @@ module.exports = class DragDrop extends Plugin {
     this.i18nInit()
 
     // Bind `this` to class methods
-    this.handleInputChange = this.handleInputChange.bind(this)
+    this.onInputChange = this.onInputChange.bind(this)
     this.handleDragOver = this.handleDragOver.bind(this)
     this.handleDragLeave = this.handleDragLeave.bind(this)
     this.handleDrop = this.handleDrop.bind(this)
-    this.addFile = this.addFile.bind(this)
+    this.addFiles = this.addFiles.bind(this)
     this.render = this.render.bind(this)
   }
 
@@ -64,28 +64,30 @@ module.exports = class DragDrop extends Plugin {
     this.setPluginState() // so that UI re-renders and we see the updated locale
   }
 
-  addFile (file) {
+  addFiles (files) {
+    const descriptors = files.map((file) => ({
+      source: this.id,
+      name: file.name,
+      type: file.type,
+      data: file,
+      meta: {
+        // path of the file relative to the ancestor directory the user selected.
+        // e.g. 'docs/Old Prague/airbnb.pdf'
+        relativePath: file.relativePath || null
+      }
+    }))
+
     try {
-      this.uppy.addFile({
-        source: this.id,
-        name: file.name,
-        type: file.type,
-        data: file,
-        meta: {
-          relativePath: file.relativePath || null
-        }
-      })
+      this.uppy.addFiles(descriptors)
     } catch (err) {
-      if (!err.isRestriction) {
-        this.uppy.log(err)
-      }
+      this.uppy.log(err)
     }
   }
 
-  handleInputChange (event) {
+  onInputChange (event) {
     this.uppy.log('[DragDrop] Files selected through input')
     const files = toArray(event.target.files)
-    files.forEach(this.addFile)
+    this.addFiles(files)
 
     // We clear the input after a file is selected, because otherwise
     // change event is not fired in Chrome and Safari when a file
@@ -110,9 +112,7 @@ module.exports = class DragDrop extends Plugin {
       this.uppy.log(error, 'error')
     }
     getDroppedFiles(event.dataTransfer, { logDropError })
-      .then((files) => {
-        files.forEach(this.addFile)
-      })
+      .then((files) => this.addFiles(files))
   }
 
   handleDragOver (event) {
@@ -150,7 +150,7 @@ module.exports = class DragDrop extends Plugin {
         name={this.opts.inputName}
         multiple={restrictions.maxNumberOfFiles !== 1}
         accept={restrictions.allowedFileTypes}
-        onchange={this.handleInputChange}
+        onchange={this.onInputChange}
       />
     )
   }

+ 16 - 16
packages/@uppy/file-input/src/index.js

@@ -50,25 +50,25 @@ module.exports = class FileInput extends Plugin {
     this.setPluginState() // so that UI re-renders and we see the updated locale
   }
 
+  addFiles (files) {
+    const descriptors = files.map((file) => ({
+      source: this.id,
+      name: file.name,
+      type: file.type,
+      data: file
+    }))
+
+    try {
+      this.uppy.addFiles(descriptors)
+    } catch (err) {
+      this.uppy.log(err)
+    }
+  }
+
   handleInputChange (event) {
     this.uppy.log('[FileInput] Something selected through input...')
-
     const files = toArray(event.target.files)
-
-    files.forEach((file) => {
-      try {
-        this.uppy.addFile({
-          source: this.id,
-          name: file.name,
-          type: file.type,
-          data: file
-        })
-      } catch (err) {
-        if (!err.isRestriction) {
-          this.uppy.log(err)
-        }
-      }
-    })
+    this.addFiles(files)
 
     // We clear the input after a file is selected, because otherwise
     // change event is not fired in Chrome and Safari when a file

+ 4 - 0
packages/@uppy/locales/src/en_US.js

@@ -1,6 +1,10 @@
 const en_US = {}
 
 en_US.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'
+  },
   addMore: 'Add more',
   addMoreFiles: 'Add more files',
   addingMoreFiles: 'Adding more files',

+ 3 - 3
packages/@uppy/thumbnail-generator/src/index.js

@@ -302,7 +302,7 @@ module.exports = class ThumbnailGenerator extends Plugin {
   }
 
   onFileAdded = (file) => {
-    if (!file.preview) {
+    if (!file.preview && isPreviewSupported(file.type) && !file.isRemote) {
       this.addToQueue(file)
     }
   }
@@ -362,8 +362,8 @@ module.exports = class ThumbnailGenerator extends Plugin {
   }
 
   install () {
-    this.uppy.on('file-added', this.onFileAdded)
     this.uppy.on('file-removed', this.onFileRemoved)
+    this.uppy.on('file-added', this.onFileAdded)
     this.uppy.on('restored', this.onRestored)
 
     if (this.opts.waitForThumbnailsBeforeUpload) {
@@ -372,8 +372,8 @@ module.exports = class ThumbnailGenerator extends Plugin {
   }
 
   uninstall () {
-    this.uppy.off('file-added', this.onFileAdded)
     this.uppy.off('file-removed', this.onFileRemoved)
+    this.uppy.off('file-added', this.onFileAdded)
     this.uppy.off('restored', this.onRestored)
 
     if (this.opts.waitForThumbnailsBeforeUpload) {

+ 24 - 10
packages/@uppy/utils/src/generateFileID.js

@@ -4,18 +4,32 @@
  *
  * @param {object} file
  * @returns {string} the fileID
- *
  */
 module.exports = function generateFileID (file) {
-  // filter is needed to not join empty values with `-`
-  return [
-    'uppy',
-    file.name ? encodeFilename(file.name.toLowerCase()) : '',
-    file.type,
-    file.meta && file.meta.relativePath ? encodeFilename(file.meta.relativePath.toLowerCase()) : '',
-    file.data.size,
-    file.data.lastModified
-  ].filter(val => val).join('-')
+  // It's tempting to do `[items].filter(Boolean).join('-')` here, but that
+  // is slower! simple string concatenation is fast
+
+  let id = 'uppy'
+  if (typeof file.name === 'string') {
+    id += '-' + encodeFilename(file.name.toLowerCase())
+  }
+
+  if (file.type !== undefined) {
+    id += '-' + file.type
+  }
+
+  if (file.meta && typeof file.meta.relativePath === 'string') {
+    id += '-' + encodeFilename(file.meta.relativePath.toLowerCase())
+  }
+
+  if (file.data.size !== undefined) {
+    id += '-' + file.data.size
+  }
+  if (file.data.lastModified !== undefined) {
+    id += '-' + file.data.lastModified
+  }
+
+  return id
 }
 
 function encodeFilename (name) {

+ 12 - 6
packages/@uppy/utils/src/getFileNameAndExtension.js

@@ -5,11 +5,17 @@
  * @returns {object} {name, extension}
  */
 module.exports = function getFileNameAndExtension (fullFileName) {
-  var re = /(?:\.([^.]+))?$/
-  var fileExt = re.exec(fullFileName)[1]
-  var fileName = fullFileName.replace('.' + fileExt, '')
-  return {
-    name: fileName,
-    extension: fileExt
+  const lastDot = fullFileName.lastIndexOf('.')
+  // these count as no extension: "no-dot", "trailing-dot."
+  if (lastDot === -1 || lastDot === fullFileName.length - 1) {
+    return {
+      name: fullFileName,
+      extension: undefined
+    }
+  } else {
+    return {
+      name: fullFileName.slice(0, lastDot),
+      extension: fullFileName.slice(lastDot + 1)
+    }
   }
 }

+ 1 - 1
packages/@uppy/utils/types/index.d.ts

@@ -119,7 +119,7 @@ declare module '@uppy/utils/lib/getETA' {
 }
 
 declare module '@uppy/utils/lib/getFileNameAndExtension' {
-  function getFileNameAndExtension(filename: string): { name: string, extension: string };
+  function getFileNameAndExtension(filename: string): { name: string, extension: string | undefined };
   export = getFileNameAndExtension
 }