Browse Source

core: Remove `core:upload-complete` event.

XHRUpload and Tus were relying on `core:upload-complete` to determine
when their uploads had all finished, but `core:upload-complete` could
fire before all XHR `onload` handlers were called. Then, because
XHRUpload and Tus would resolve their `handleUpload` promises at that
point, the `core:success` event could fire *before* all
`core:upload-success` events had fired, and before the plugins properly
handled responses. In particular, some files may not have `.uploadURL`
properties when `core:success` is fired:

https://community.transloadit.com/t/unable-to-get-to-work-the-awss3-plugin/14477/16

I think, that the XHR `onprogress` event would fire first at 100%, and
then the `onload` handler fired at some point later (after the response
was parsed or whatever the browser does to it). This could cause the
`core:upload-complete` event to fire before responses were handled by
Uppy.

The XHRUpload and Tus plugins now use the Promises that they were
already creating to wait for uploads to complete. This way they will
always have handled responses before handing over control to
postprocessors and before the `core:success` event is fired.

Since `core:upload-complete` is now unused, I just removed it for now.
Renée Kooi 7 years ago
parent
commit
2ef0f04ac2
3 changed files with 10 additions and 25 deletions
  1. 0 7
      src/core/Core.js
  2. 5 9
      src/plugins/Tus10.js
  3. 5 9
      src/plugins/XHRUpload.js

+ 0 - 7
src/core/Core.js

@@ -525,13 +525,6 @@ class Uppy {
       })
 
       this.calculateTotalProgress()
-
-      if (this.getState().totalProgress === 100) {
-        const completeFiles = Object.keys(updatedFiles).filter((file) => {
-          return updatedFiles[file].progress.uploadComplete
-        })
-        this.emit('core:upload-complete', completeFiles.length)
-      }
     })
 
     this.on('core:update-meta', (data, fileID) => {

+ 5 - 9
src/plugins/Tus10.js

@@ -324,16 +324,16 @@ module.exports = class Tus10 extends Plugin {
   }
 
   uploadFiles (files) {
-    files.forEach((file, index) => {
+    return Promise.all(files.map((file, index) => {
       const current = parseInt(index, 10) + 1
       const total = files.length
 
       if (!file.isRemote) {
-        this.upload(file, current, total)
+        return this.upload(file, current, total)
       } else {
-        this.uploadRemote(file, current, total)
+        return this.uploadRemote(file, current, total)
       }
-    })
+    }))
   }
 
   handleUpload (fileIDs) {
@@ -345,11 +345,7 @@ module.exports = class Tus10 extends Plugin {
     this.core.log('Tus is uploading...')
     const filesToUpload = fileIDs.map((fileID) => this.core.getFile(fileID))
 
-    this.uploadFiles(filesToUpload)
-
-    return new Promise((resolve) => {
-      this.core.once('core:upload-complete', resolve)
-    })
+    return this.uploadFiles(filesToUpload)
   }
 
   actions () {

+ 5 - 9
src/plugins/XHRUpload.js

@@ -176,16 +176,16 @@ module.exports = class XHRUpload extends Plugin {
   }
 
   selectForUpload (files) {
-    files.forEach((file, i) => {
+    return Promise.all(files.map((file, i) => {
       const current = parseInt(i, 10) + 1
       const total = files.length
 
       if (file.isRemote) {
-        this.uploadRemote(file, current, total)
+        return this.uploadRemote(file, current, total)
       } else {
-        this.upload(file, current, total)
+        return this.upload(file, current, total)
       }
-    })
+    }))
 
     //   if (this.opts.bundle) {
     //     uploaders.push(this.upload(files, 0, files.length))
@@ -208,11 +208,7 @@ module.exports = class XHRUpload extends Plugin {
       return this.core.state.files[fileID]
     }
 
-    this.selectForUpload(files)
-
-    return new Promise((resolve) => {
-      this.core.once('core:upload-complete', resolve)
-    })
+    return this.selectForUpload(files).then(() => null)
   }
 
   install () {