fix progress throttling

only do it on total progress

Author: mifi

packages/@uppy/core/src/Uppy.test.ts

@@ -1187,7 +1187,7 @@ describe('src/Core', () => {
       core.addUploader((fileIDs) => {
         fileIDs.forEach((fileID) => {
           const file = core.getFile(fileID)
-          if (/bar/.test(file.name)) {
+          if (file.name != null && /bar/.test(file.name)) {
             // @ts-ignore
             core.emit(
               'upload-error',
@@ -1701,6 +1701,9 @@ describe('src/Core', () => {
 
       const fileId = Object.keys(core.getState().files)[0]
       const file = core.getFile(fileId)
+
+      core.emit('upload-start', [core.getFile(fileId)])
+
       // @ts-ignore
       core.emit('upload-progress', file, {
         bytesUploaded: 12345,
@@ -1711,7 +1714,7 @@ describe('src/Core', () => {
         bytesUploaded: 12345,
         bytesTotal: 17175,
         uploadComplete: false,
-        uploadStarted: null,
+        uploadStarted: expect.any(Number),
       })
 
       // @ts-ignore
@@ -1720,14 +1723,12 @@ describe('src/Core', () => {
         bytesTotal: 17175,
       })
 
-      core.calculateProgress.flush()
-
       expect(core.getFile(fileId).progress).toEqual({
         percentage: 100,
         bytesUploaded: 17175,
         bytesTotal: 17175,
         uploadComplete: false,
-        uploadStarted: null,
+        uploadStarted: expect.any(Number),
       })
     })
 
@@ -1897,7 +1898,6 @@ describe('src/Core', () => {
 
       // @ts-ignore
       core[Symbol.for('uppy test: updateTotalProgress')]()
-      core.calculateProgress.flush()
 
       expect(core.getState().totalProgress).toEqual(66)
     })
@@ -1942,7 +1942,6 @@ describe('src/Core', () => {
 
       // @ts-ignore
       core[Symbol.for('uppy test: updateTotalProgress')]()
-      core.calculateProgress.flush()
 
       expect(core.getState().totalProgress).toEqual(66)
       expect(core.getState().allowNewUpload).toEqual(true)

packages/@uppy/core/src/Uppy.ts

@@ -1270,7 +1270,7 @@ export class Uppy<
     }
 
     this.setState(stateUpdate)
-    this.#updateTotalProgress()
+    this.#updateTotalProgressThrottled()
 
     const removedFileIDs = Object.keys(removedFiles)
     removedFileIDs.forEach((fileID) => {
@@ -1416,121 +1416,133 @@ export class Uppy<
     })
   }
 
-  // ___Why throttle at 500ms?
-  //    - We must throttle at >250ms for superfocus in Dashboard to work well
-  //    (because animation takes 0.25s, and we want to wait for all animations to be over before refocusing).
-  //    [Practical Check]: if thottle is at 100ms, then if you are uploading a file,
-  //    and click 'ADD MORE FILES', - focus won't activate in Firefox.
-  //    - We must throttle at around >500ms to avoid performance lags.
-  //    [Practical Check] Firefox, try to upload a big file for a prolonged period of time. Laptop will start to heat up.
-  // todo when uploading multiple files, this will cause problems because they share the same throttle,
-  // meaning some files might never get their progress reported (eaten up by progress events from other files)
-  calculateProgress = throttle(
-    (file, data) => {
-      const fileInState = this.getFile(file?.id)
-      if (file == null || !fileInState) {
-        this.log(
-          `Not setting progress for a file that has been removed: ${file?.id}`,
-        )
-        return
-      }
+  #handleUploadProgress = (
+    file: UppyFile<M, B> | undefined,
+    progress: FileProgressStarted,
+  ) => {
+    const fileInState = file ? this.getFile(file.id) : undefined
+    if (file == null || !fileInState) {
+      this.log(
+        `Not setting progress for a file that has been removed: ${file?.id}`,
+      )
+      return
+    }
 
-      if (fileInState.progress.percentage === 100) {
-        this.log(
-          `Not setting progress for a file that has been already uploaded: ${file.id}`,
-        )
-        return
-      }
+    if (fileInState.progress.percentage === 100) {
+      this.log(
+        `Not setting progress for a file that has been already uploaded: ${file.id}`,
+      )
+      return
+    }
 
+    const newProgress = {
+      bytesTotal: progress.bytesTotal,
       // bytesTotal may be null or zero; in that case we can't divide by it
-      const canHavePercentage =
-        Number.isFinite(data.bytesTotal) && data.bytesTotal > 0
+      percentage:
+        (
+          progress.bytesTotal != null &&
+          Number.isFinite(progress.bytesTotal) &&
+          progress.bytesTotal > 0
+        ) ?
+          Math.round((progress.bytesUploaded / progress.bytesTotal) * 100)
+        : 0,
+    }
+
+    if (fileInState.progress.uploadStarted != null) {
       this.setFileState(file.id, {
         progress: {
           ...fileInState.progress,
-          bytesUploaded: data.bytesUploaded,
-          bytesTotal: data.bytesTotal,
-          percentage:
-            canHavePercentage ?
-              Math.round((data.bytesUploaded / data.bytesTotal) * 100)
-            : 0,
+          bytesUploaded: progress.bytesUploaded,
+          ...newProgress,
         },
       })
+    } else {
+      this.setFileState(file.id, {
+        progress: {
+          ...fileInState.progress,
+          ...newProgress,
+        },
+      })
+    }
 
-      this.#updateTotalProgress()
-    },
-    500,
-    { leading: true, trailing: true },
-  )
+    this.#updateTotalProgressThrottled()
+  }
 
   #updateTotalProgress() {
-    const totalProgress = this.#calculateTotalProgress()
+    let totalProgress = Math.round(this.#calculateTotalProgress() * 100)
+    if (totalProgress > 100) totalProgress = 100
+    else if (totalProgress < 0) totalProgress = 0
+
     this.emit('progress', totalProgress)
     this.setState({ totalProgress })
   }
 
+  // ___Why throttle at 500ms?
+  //    - We must throttle at >250ms for superfocus in Dashboard to work well
+  //    (because animation takes 0.25s, and we want to wait for all animations to be over before refocusing).
+  //    [Practical Check]: if thottle is at 100ms, then if you are uploading a file,
+  //    and click 'ADD MORE FILES', - focus won't activate in Firefox.
+  //    - We must throttle at around >500ms to avoid performance lags.
+  //    [Practical Check] Firefox, try to upload a big file for a prolonged period of time. Laptop will start to heat up.
+  #updateTotalProgressThrottled = throttle(
+    () => this.#updateTotalProgress(),
+    500,
+    { leading: true, trailing: true },
+  )
+
   // eslint-disable-next-line class-methods-use-this, @typescript-eslint/explicit-module-boundary-types
   private [Symbol.for('uppy test: updateTotalProgress')]() {
     return this.#updateTotalProgress()
   }
 
   #calculateTotalProgress() {
     // calculate total progress, using the number of files currently uploading,
-    // multiplied by 100 and the summ of individual progress of each file
+    // between 0 and 1 and sum of individual progress of each file
     const files = this.getFiles()
 
-    const inProgress = files.filter((file) => {
+    const filesInProgress = files.filter((file) => {
       return (
         file.progress.uploadStarted ||
         file.progress.preprocess ||
         file.progress.postprocess
       )
     })
 
-    if (inProgress.length === 0) {
+    if (filesInProgress.length === 0) {
       return 0
     }
 
-    const sizedFiles = inProgress.filter(
+    const sizedFiles = filesInProgress.filter(
       (file) => file.progress.bytesTotal != null,
     )
-    const unsizedFiles = inProgress.filter(
+    const unsizedFiles = filesInProgress.filter(
       (file) => file.progress.bytesTotal == null,
     )
 
     if (sizedFiles.length === 0) {
-      const progressMax = inProgress.length * 100
-      const currentProgress = unsizedFiles.reduce((acc, file) => {
-        return acc + (file.progress.percentage as number)
-      }, 0)
-      const totalProgress = Math.round((currentProgress / progressMax) * 100)
-      return totalProgress
+      const totalUnsizedProgress = unsizedFiles.reduce(
+        (acc, file) => acc + (file.progress.percentage ?? 0) / 100,
+        0,
+      )
+
+      return totalUnsizedProgress / unsizedFiles.length
     }
 
-    let totalSize = sizedFiles.reduce((acc, file) => {
+    let totalFilesSize = sizedFiles.reduce((acc, file) => {
       return (acc + (file.progress.bytesTotal ?? 0)) as number
     }, 0)
-    const averageSize = totalSize / sizedFiles.length
-    totalSize += averageSize * unsizedFiles.length
+    const averageSize = totalFilesSize / sizedFiles.length
+    totalFilesSize += averageSize * unsizedFiles.length
 
-    let uploadedSize = 0
+    let totalUploadedSize = 0
     sizedFiles.forEach((file) => {
-      uploadedSize += file.progress.bytesUploaded as number
+      totalUploadedSize += file.progress.bytesUploaded || 0
     })
     unsizedFiles.forEach((file) => {
-      uploadedSize += (averageSize * (file.progress.percentage || 0)) / 100
+      totalUploadedSize += averageSize * ((file.progress.percentage ?? 0) / 100)
     })
 
-    let totalProgress =
-      totalSize === 0 ? 0 : Math.round((uploadedSize / totalSize) * 100)
-
-    // hot fix, because:
-    // uploadedSize ended up larger than totalSize, resulting in 1325% total
-    if (totalProgress > 100) {
-      totalProgress = 100
-    }
-
-    return totalProgress
+    return totalFilesSize === 0 ? 0 : totalUploadedSize / totalFilesSize
   }
 
   /**
@@ -1629,7 +1641,7 @@ export class Uppy<
 
     this.on('upload-start', onUploadStarted)
 
-    this.on('upload-progress', this.calculateProgress)
+    this.on('upload-progress', this.#handleUploadProgress)
 
     this.on('upload-success', (file, uploadResp) => {
       if (file == null || !this.getFile(file.id)) {
@@ -1666,7 +1678,7 @@ export class Uppy<
         })
       }
 
-      this.#updateTotalProgress()
+      this.#updateTotalProgressThrottled()
     })
 
     this.on('preprocess-progress', (file, progress) => {
@@ -1736,7 +1748,7 @@ export class Uppy<
 
     this.on('restored', () => {
       // Files may have changed--ensure progress is still accurate.
-      this.#updateTotalProgress()
+      this.#updateTotalProgressThrottled()
     })
 
     // @ts-expect-error should fix itself when dashboard it typed (also this doesn't belong here)

packages/@uppy/utils/src/emitSocketProgress.ts

@@ -1,4 +1,3 @@
-import throttle from 'lodash/throttle.js'
 import type { Uppy } from '@uppy/core'
 import type { UppyFile } from './UppyFile.ts'
 
@@ -22,7 +21,4 @@ function emitSocketProgress(
   }
 }
 
-export default throttle(emitSocketProgress, 300, {
-  leading: true,
-  trailing: true,
-})
+export default emitSocketProgress