Run _checkRestrictions and _checkMinNumberOfFiles before onBeforeFile…

…Added and onBeforeUpload callbacks Addresses transloadit#1545, but I’m not sure this is the right move. Is it better to check restrictions before or after the callbacks?
vymao · May 24, 2019 · 9614758 · 9614758
1 parent 738db80
commit 9614758
Showing 1 changed file with 23 additions and 20 deletions.
diff --git a/packages/@uppy/core/src/index.js b/packages/@uppy/core/src/index.js
@@ -443,6 +443,13 @@ class Uppy {
       preview: file.preview
     }
 
+    try {
+      this._checkRestrictions(newFile)
+    } catch (err) {
+      this.emit('restriction-failed', newFile, err)
+      onError(err)
+    }
+
     const onBeforeFileAddedResult = this.opts.onBeforeFileAdded(newFile, files)
 
     if (onBeforeFileAddedResult === false) {
@@ -454,13 +461,6 @@ class Uppy {
       newFile = onBeforeFileAddedResult
     }
 
-    try {
-      this._checkRestrictions(newFile)
-    } catch (err) {
-      this.emit('restriction-failed', newFile, err)
-      onError(err)
-    }
-
     this.setState({
       files: Object.assign({}, files, {
         [fileID]: newFile
@@ -1240,23 +1240,32 @@ class Uppy {
     }
 
     let files = this.getState().files
+
+    const handleError = (err) => {
+      const message = typeof err === 'object' ? err.message : err
+      const details = typeof err === 'object' ? err.details : null
+      this.log(`${message} ${details}`)
+      this.info({ message: message, details: details }, 'error', 4000)
+      return Promise.reject(typeof err === 'object' ? err : new Error(err))
+    }
+
+    try {
+      this._checkMinNumberOfFiles(files)
+    } catch (err) {
+      return handleError(err)
+    }
+
     const onBeforeUploadResult = this.opts.onBeforeUpload(files)
 
     if (onBeforeUploadResult === false) {
       return Promise.reject(new Error('Not starting the upload because onBeforeUpload returned false'))
     }
 
     if (onBeforeUploadResult && typeof onBeforeUploadResult === 'object') {
-      // warning after the change in 0.24
-      if (onBeforeUploadResult.then) {
-        throw new TypeError('onBeforeUpload() returned a Promise, but this is no longer supported. It must be synchronous.')
-      }
-
       files = onBeforeUploadResult
     }
 
     return Promise.resolve()
-      .then(() => this._checkMinNumberOfFiles(files))
       .then(() => {
         const { currentUploads } = this.getState()
         // get a list of files that are currently assigned to uploads
@@ -1274,13 +1283,7 @@ class Uppy {
         const uploadID = this._createUpload(waitingFileIDs)
         return this._runUpload(uploadID)
       })
-      .catch((err) => {
-        const message = typeof err === 'object' ? err.message : err
-        const details = typeof err === 'object' ? err.details : null
-        this.log(`${message} ${details}`)
-        this.info({ message: message, details: details }, 'error', 4000)
-        return Promise.reject(typeof err === 'object' ? err : new Error(err))
-      })
+      .catch(handleError)
   }
 }