From 24fd4158a99aebdb50df8717c9a5d92707d16084 Mon Sep 17 00:00:00 2001 From: Mikael Finstad Date: Sat, 7 Dec 2024 10:30:00 +0800 Subject: [PATCH] Companion stream upload unknown size files (#5489) * stream upload unknown size files behind a new option streamingUploadSizeless COMPANION_STREAMING_UPLOAD_SIZELESS for tus * allow for all upload protocols seems to be working closes #5305 * refactor and fix bug where progress was not always emitted * fix type * fix progress throttling only do it on total progress * Improve progress in UI - only show progress percent and total bytes for files that we know the size of. (but all files will still be included in number of files) - use `null` as an unknown value for progress and ETA, allowing us to remove ETA from UI when unknown - `percentage` make use of `undefined` when progress is not yet known - don't show percentage in UI when unknown - add a new state field `progress` that's the same as `totalProgress` but can also be `null` * fix build error * format * fix progress when upload complete * use execa for companion load balancer if not, then it leaves zombie companion instances running in the background when e2e stops have to be manually killed before running e2e again * update docs and tests for new state.progress * revert progress/totalProgress * improve doc * remove option streamingUploadSizeless we agreed that this can be considered not a breaking change * change progress the to "of unknown" * revert * remove companion doc * add e2e test --- e2e/cypress/integration/dashboard-xhr.spec.ts | 12 ++ e2e/cypress/integration/reusable-tests.ts | 2 +- e2e/mock-server.mjs | 25 +++ e2e/start-companion-with-load-balancer.mjs | 53 ++--- package.json | 1 + .../companion-client/src/RequestClient.ts | 21 +- .../@uppy/companion/src/server/Uploader.js | 19 +- packages/@uppy/core/src/Uppy.test.ts | 30 +-- packages/@uppy/core/src/Uppy.ts | 196 ++++++++++-------- .../@uppy/progress-bar/src/ProgressBar.tsx | 9 +- packages/@uppy/status-bar/src/Components.tsx | 31 +-- packages/@uppy/status-bar/src/StatusBar.tsx | 35 +++- packages/@uppy/status-bar/src/StatusBarUI.tsx | 4 +- packages/@uppy/status-bar/src/locale.ts | 1 + packages/@uppy/utils/package.json | 1 - packages/@uppy/utils/src/FileProgress.ts | 6 +- .../@uppy/utils/src/emitSocketProgress.ts | 28 --- yarn.lock | 124 ++++++++++- 18 files changed, 402 insertions(+), 196 deletions(-) delete mode 100644 packages/@uppy/utils/src/emitSocketProgress.ts diff --git a/e2e/cypress/integration/dashboard-xhr.spec.ts b/e2e/cypress/integration/dashboard-xhr.spec.ts index 4a093c517a..9cb0752f35 100644 --- a/e2e/cypress/integration/dashboard-xhr.spec.ts +++ b/e2e/cypress/integration/dashboard-xhr.spec.ts @@ -1,5 +1,6 @@ import { interceptCompanionUrlMetaRequest, + interceptCompanionUrlRequest, runRemoteUrlImageUploadTest, runRemoteUnsplashUploadTest, } from './reusable-tests.ts' @@ -57,6 +58,17 @@ describe('Dashboard with XHR', () => { }) }) + it('should upload unknown size files', () => { + cy.get('[data-cy="Url"]').click() + cy.get('.uppy-Url-input').type('http://localhost:4678/unknown-size') + cy.get('.uppy-Url-importButton').click() + interceptCompanionUrlRequest() + cy.get('.uppy-StatusBar-actionBtn--upload').click() + cy.wait('@url').then(() => { + cy.get('.uppy-StatusBar-statusPrimary').should('contain', 'Complete') + }) + }) + it('should upload remote image with Unsplash plugin', () => { runRemoteUnsplashUploadTest() }) diff --git a/e2e/cypress/integration/reusable-tests.ts b/e2e/cypress/integration/reusable-tests.ts index d311b60e39..39031d8281 100644 --- a/e2e/cypress/integration/reusable-tests.ts +++ b/e2e/cypress/integration/reusable-tests.ts @@ -1,6 +1,6 @@ /* global cy */ -const interceptCompanionUrlRequest = () => +export const interceptCompanionUrlRequest = () => cy .intercept({ method: 'POST', url: 'http://localhost:3020/url/get' }) .as('url') diff --git a/e2e/mock-server.mjs b/e2e/mock-server.mjs index 02d84b77da..6678de6ff8 100644 --- a/e2e/mock-server.mjs +++ b/e2e/mock-server.mjs @@ -13,6 +13,31 @@ const requestListener = (req, res) => { } case '/file-no-headers': break + + case '/unknown-size': { + res.setHeader('Content-Type', 'text/html; charset=UTF-8'); + res.setHeader('Transfer-Encoding', 'chunked'); + const chunkSize = 1e5; + if (req.method === 'GET') { + let i = 0; + const interval = setInterval(() => { + if (i >= 10) { // 1MB + clearInterval(interval); + res.end(); + return; + } + res.write(Buffer.from(Array.from({ length: chunkSize }, () => '1').join(''))); + res.write('\n'); + i++; + }, 10); + } else if (req.method === 'HEAD') { + res.end(); + } else { + throw new Error('Unhandled method') + } + } + break; + default: res.writeHead(404).end('Unhandled request') } diff --git a/e2e/start-companion-with-load-balancer.mjs b/e2e/start-companion-with-load-balancer.mjs index c2d33d508b..eccec38f67 100755 --- a/e2e/start-companion-with-load-balancer.mjs +++ b/e2e/start-companion-with-load-balancer.mjs @@ -1,9 +1,10 @@ #!/usr/bin/env node -import { spawn } from 'node:child_process' import http from 'node:http' import httpProxy from 'http-proxy' import process from 'node:process' +import { execaNode } from 'execa'; + const numInstances = 3 const lbPort = 3020 @@ -49,41 +50,27 @@ function createLoadBalancer (baseUrls) { const isWindows = process.platform === 'win32' const isOSX = process.platform === 'darwin' -const startCompanion = ({ name, port }) => { - const cp = spawn(process.execPath, [ +const startCompanion = ({ name, port }) => execaNode('packages/@uppy/companion/src/standalone/start-server.js', { + nodeOptions: [ '-r', 'dotenv/config', // Watch mode support is limited to Windows and macOS at the time of writing. ...(isWindows || isOSX ? ['--watch-path', 'packages/@uppy/companion/src', '--watch'] : []), - './packages/@uppy/companion/src/standalone/start-server.js', - ], { - cwd: new URL('../', import.meta.url), - stdio: 'inherit', - env: { - // Note: these env variables will override anything set in .env - ...process.env, - COMPANION_PORT: port, - COMPANION_SECRET: 'development', // multi instance will not work without secret set - COMPANION_PREAUTH_SECRET: 'development', // multi instance will not work without secret set - COMPANION_ALLOW_LOCAL_URLS: 'true', - COMPANION_ENABLE_URL_ENDPOINT: 'true', - COMPANION_LOGGER_PROCESS_NAME: name, - COMPANION_CLIENT_ORIGINS: 'true', - }, - }) - // Adding a `then` property so the return value is awaitable: - return Object.defineProperty(cp, 'then', { - __proto__: null, - writable: true, - configurable: true, - value: Promise.prototype.then.bind(new Promise((resolve, reject) => { - cp.on('exit', (code) => { - if (code === 0) resolve(cp) - else reject(new Error(`Non-zero exit code: ${code}`)) - }) - cp.on('error', reject) - })), - }) -} + ], + cwd: new URL('../', import.meta.url), + stdio: 'inherit', + env: { + // Note: these env variables will override anything set in .env + ...process.env, + COMPANION_PORT: port, + COMPANION_SECRET: 'development', // multi instance will not work without secret set + COMPANION_PREAUTH_SECRET: 'development', // multi instance will not work without secret set + COMPANION_ALLOW_LOCAL_URLS: 'true', + COMPANION_ENABLE_URL_ENDPOINT: 'true', + COMPANION_LOGGER_PROCESS_NAME: name, + COMPANION_CLIENT_ORIGINS: 'true', + }, +}) + const hosts = Array.from({ length: numInstances }, (_, index) => { const port = companionStartPort + index diff --git a/package.json b/package.json index 786a7ad5e5..01240307ab 100644 --- a/package.json +++ b/package.json @@ -84,6 +84,7 @@ "eslint-plugin-react": "^7.22.0", "eslint-plugin-react-hooks": "^4.2.0", "eslint-plugin-unicorn": "^53.0.0", + "execa": "^9.5.1", "github-contributors-list": "^1.2.4", "glob": "^8.0.0", "jsdom": "^24.0.0", diff --git a/packages/@uppy/companion-client/src/RequestClient.ts b/packages/@uppy/companion-client/src/RequestClient.ts index 68a0992941..d8f3702e64 100644 --- a/packages/@uppy/companion-client/src/RequestClient.ts +++ b/packages/@uppy/companion-client/src/RequestClient.ts @@ -4,7 +4,6 @@ import pRetry, { AbortError } from 'p-retry' import fetchWithNetworkError from '@uppy/utils/lib/fetchWithNetworkError' import ErrorWithCause from '@uppy/utils/lib/ErrorWithCause' -import emitSocketProgress from '@uppy/utils/lib/emitSocketProgress' import getSocketHost from '@uppy/utils/lib/getSocketHost' import type Uppy from '@uppy/core' @@ -81,6 +80,26 @@ async function handleJSONResponse(res: Response): Promise { throw new HttpError({ statusCode: res.status, message: errMsg }) } +function emitSocketProgress( + uploader: { uppy: Uppy }, + progressData: { + progress: string // pre-formatted percentage number as a string + bytesTotal: number + bytesUploaded: number + }, + file: UppyFile, +): void { + const { progress, bytesUploaded, bytesTotal } = progressData + if (progress) { + uploader.uppy.log(`Upload progress: ${progress}`) + uploader.uppy.emit('upload-progress', file, { + uploadStarted: file.progress.uploadStarted ?? 0, + bytesUploaded, + bytesTotal, + }) + } +} + export default class RequestClient { static VERSION = packageJson.version diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index 25febdb309..9c2e3dd64b 100644 --- a/packages/@uppy/companion/src/server/Uploader.js +++ b/packages/@uppy/companion/src/server/Uploader.js @@ -220,10 +220,14 @@ class Uploader { if (this.readStream) this.readStream.destroy(err) } - async _uploadByProtocol(req) { + _getUploadProtocol() { // todo a default protocol should not be set. We should ensure that the user specifies their protocol. // after we drop old versions of uppy client we can remove this - const protocol = this.options.protocol || PROTOCOLS.multipart + return this.options.protocol || PROTOCOLS.multipart + } + + async _uploadByProtocol(req) { + const protocol = this._getUploadProtocol() switch (protocol) { case PROTOCOLS.multipart: @@ -264,8 +268,8 @@ class Uploader { this.readStream = fileStream } - _needDownloadFirst() { - return !this.options.size || !this.options.companionOptions.streamingUpload + _canStream() { + return this.options.companionOptions.streamingUpload } /** @@ -281,7 +285,8 @@ class Uploader { this.#uploadState = states.uploading this.readStream = stream - if (this._needDownloadFirst()) { + + if (!this._canStream()) { logger.debug('need to download the whole file first', 'controller.get.provider.size', this.shortToken) // Some streams need to be downloaded entirely first, because we don't know their size from the provider // This is true for zoom and drive (exported files) or some URL downloads. @@ -429,7 +434,7 @@ class Uploader { // If fully downloading before uploading, combine downloaded and uploaded bytes // This will make sure that the user sees half of the progress before upload starts (while downloading) let combinedBytes = bytesUploaded - if (this._needDownloadFirst()) { + if (!this._canStream()) { combinedBytes = Math.floor((combinedBytes + (this.downloadedBytes || 0)) / 2) } @@ -606,7 +611,7 @@ class Uploader { const response = await runRequest(url, reqOptions) - if (bytesUploaded !== this.size) { + if (this.size != null && bytesUploaded !== this.size) { const errMsg = `uploaded only ${bytesUploaded} of ${this.size} with status: ${response.statusCode}` logger.error(errMsg, 'upload.multipart.mismatch.error') throw new Error(errMsg) diff --git a/packages/@uppy/core/src/Uppy.test.ts b/packages/@uppy/core/src/Uppy.test.ts index 0a27e16e52..cb6b985a84 100644 --- a/packages/@uppy/core/src/Uppy.test.ts +++ b/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), }) }) @@ -1762,7 +1763,8 @@ describe('src/Core', () => { data: {}, }) - core.calculateTotalProgress() + // @ts-ignore + core[Symbol.for('uppy test: updateTotalProgress')]() const uploadPromise = core.upload() await Promise.all([ @@ -1774,7 +1776,6 @@ describe('src/Core', () => { bytesUploaded: 0, // null indicates unsized bytesTotal: null, - percentage: 0, }) // @ts-ignore @@ -1844,10 +1845,11 @@ describe('src/Core', () => { data: {}, }) - core.calculateTotalProgress() + // @ts-ignore + core[Symbol.for('uppy test: updateTotalProgress')]() - // foo.jpg at 35%, bar.jpg at 0% - expect(core.getState().totalProgress).toBe(18) + // foo.jpg at 35%, bar.jpg has unknown size and will not be counted + expect(core.getState().totalProgress).toBe(36) core.destroy() }) @@ -1893,8 +1895,8 @@ describe('src/Core', () => { bytesTotal: 17175, }) - core.calculateTotalProgress() - core.calculateProgress.flush() + // @ts-ignore + core[Symbol.for('uppy test: updateTotalProgress')]() expect(core.getState().totalProgress).toEqual(66) }) @@ -1937,8 +1939,8 @@ describe('src/Core', () => { bytesTotal: 17175, }) - core.calculateTotalProgress() - core.calculateProgress.flush() + // @ts-ignore + core[Symbol.for('uppy test: updateTotalProgress')]() expect(core.getState().totalProgress).toEqual(66) expect(core.getState().allowNewUpload).toEqual(true) diff --git a/packages/@uppy/core/src/Uppy.ts b/packages/@uppy/core/src/Uppy.ts index d20de5aaf1..5e1ddaf455 100644 --- a/packages/@uppy/core/src/Uppy.ts +++ b/packages/@uppy/core/src/Uppy.ts @@ -1279,7 +1279,7 @@ export class Uppy< } this.setState(stateUpdate) - this.calculateTotalProgress() + this.#updateTotalProgressThrottled() const removedFileIDs = Object.keys(removedFiles) removedFileIDs.forEach((fileID) => { @@ -1425,59 +1425,98 @@ 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 | 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) + : undefined, + } + + if (fileInState.progress.uploadStarted != null) { + this.setFileState(file.id, { + progress: { + ...fileInState.progress, + ...newProgress, + bytesUploaded: progress.bytesUploaded, + }, + }) + } else { this.setFileState(file.id, { progress: { ...fileInState.progress, - bytesUploaded: data.bytesUploaded, - bytesTotal: data.bytesTotal, - percentage: - canHavePercentage ? - Math.round((data.bytesUploaded / data.bytesTotal) * 100) - : 0, + ...newProgress, }, }) + } + + this.#updateTotalProgressThrottled() + } + + #updateTotalProgress() { + const totalProgress = this.#calculateTotalProgress() + let totalProgressPercent: number | null = null + if (totalProgress != null) { + totalProgressPercent = Math.round(totalProgress * 100) + if (totalProgressPercent > 100) totalProgressPercent = 100 + else if (totalProgressPercent < 0) totalProgressPercent = 0 + } + + this.emit('progress', totalProgressPercent ?? 0) + this.setState({ + totalProgress: totalProgressPercent ?? 0, + }) + } - this.calculateTotalProgress() - }, + // ___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 }, ) - calculateTotalProgress(): void { + // 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) => { + // note: also includes files that have completed uploading: + const filesInProgress = files.filter((file) => { return ( file.progress.uploadStarted || file.progress.preprocess || @@ -1485,54 +1524,48 @@ export class Uppy< ) }) - if (inProgress.length === 0) { - this.emit('progress', 0) - this.setState({ totalProgress: 0 }) - return + if (filesInProgress.length === 0) { + return 0 } - const sizedFiles = inProgress.filter( - (file) => file.progress.bytesTotal != null, - ) - const unsizedFiles = inProgress.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) - this.setState({ totalProgress }) - return + if (filesInProgress.every((file) => file.progress.uploadComplete)) { + // If every uploading file is complete, and we're still getting progress, it probably means + // there's a bug somewhere in some progress reporting code (maybe not even our code) + // and we're still getting progress, so let's just assume it means a 100% progress + return 1 } - let totalSize = sizedFiles.reduce((acc, file) => { - return (acc + (file.progress.bytesTotal ?? 0)) as number - }, 0) - const averageSize = totalSize / sizedFiles.length - totalSize += averageSize * unsizedFiles.length - - let uploadedSize = 0 - sizedFiles.forEach((file) => { - uploadedSize += file.progress.bytesUploaded as number - }) - unsizedFiles.forEach((file) => { - uploadedSize += (averageSize * (file.progress.percentage || 0)) / 100 - }) + const isSizedFile = (file: UppyFile) => + file.progress.bytesTotal != null && file.progress.bytesTotal !== 0 - let totalProgress = - totalSize === 0 ? 0 : Math.round((uploadedSize / totalSize) * 100) + const sizedFilesInProgress = filesInProgress.filter(isSizedFile) + const unsizedFilesInProgress = filesInProgress.filter( + (file) => !isSizedFile(file), + ) - // hot fix, because: - // uploadedSize ended up larger than totalSize, resulting in 1325% total - if (totalProgress > 100) { - totalProgress = 100 + if ( + sizedFilesInProgress.every((file) => file.progress.uploadComplete) && + unsizedFilesInProgress.length > 0 && + !unsizedFilesInProgress.every((file) => file.progress.uploadComplete) + ) { + // we are done with uploading all files of known size, however + // there is at least one file with unknown size still uploading, + // and we cannot say anything about their progress + // In any case, return null because it doesn't make any sense to show a progress + return null } - this.setState({ totalProgress }) - this.emit('progress', totalProgress) + const totalFilesSize = sizedFilesInProgress.reduce( + (acc, file) => acc + (file.progress.bytesTotal ?? 0), + 0, + ) + + const totalUploadedSize = sizedFilesInProgress.reduce( + (acc, file) => acc + (file.progress.bytesUploaded || 0), + 0, + ) + + return totalFilesSize === 0 ? 0 : totalUploadedSize / totalFilesSize } /** @@ -1618,7 +1651,6 @@ export class Uppy< progress: { uploadStarted: Date.now(), uploadComplete: false, - percentage: 0, bytesUploaded: 0, bytesTotal: file.size, } as FileProgressStarted, @@ -1631,7 +1663,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)) { @@ -1668,7 +1700,7 @@ export class Uppy< }) } - this.calculateTotalProgress() + this.#updateTotalProgressThrottled() }) this.on('preprocess-progress', (file, progress) => { @@ -1738,7 +1770,7 @@ export class Uppy< this.on('restored', () => { // Files may have changed--ensure progress is still accurate. - this.calculateTotalProgress() + this.#updateTotalProgressThrottled() }) // @ts-expect-error should fix itself when dashboard it typed (also this doesn't belong here) diff --git a/packages/@uppy/progress-bar/src/ProgressBar.tsx b/packages/@uppy/progress-bar/src/ProgressBar.tsx index f4a5d9ac69..42a2036ca7 100644 --- a/packages/@uppy/progress-bar/src/ProgressBar.tsx +++ b/packages/@uppy/progress-bar/src/ProgressBar.tsx @@ -40,10 +40,11 @@ export default class ProgressBar< } render(state: State): ComponentChild { - const progress = state.totalProgress || 0 + const { totalProgress } = state // before starting and after finish should be hidden if specified in the options const isHidden = - (progress === 0 || progress === 100) && this.opts.hideAfterFinish + (totalProgress === 0 || totalProgress === 100) && + this.opts.hideAfterFinish return (
-
{progress}
+
{totalProgress}
) } diff --git a/packages/@uppy/status-bar/src/Components.tsx b/packages/@uppy/status-bar/src/Components.tsx index fa096c95a0..747e7c74a1 100644 --- a/packages/@uppy/status-bar/src/Components.tsx +++ b/packages/@uppy/status-bar/src/Components.tsx @@ -265,8 +265,8 @@ interface ProgressDetailsProps { numUploads: number complete: number totalUploadedSize: number - totalSize: number - totalETA: number + totalSize: number | null + totalETA: number | null } function ProgressDetails(props: ProgressDetailsProps) { @@ -275,6 +275,8 @@ function ProgressDetails(props: ProgressDetailsProps) { const ifShowFilesUploadedOfTotal = numUploads > 1 + const totalUploadedSizeStr = prettierBytes(totalUploadedSize) + return (
{ifShowFilesUploadedOfTotal && @@ -289,16 +291,19 @@ function ProgressDetails(props: ProgressDetailsProps) { */} {ifShowFilesUploadedOfTotal && renderDot()} - {i18n('dataUploadedOfTotal', { - complete: prettierBytes(totalUploadedSize), - total: prettierBytes(totalSize), - })} + {totalSize != null ? + i18n('dataUploadedOfTotal', { + complete: totalUploadedSizeStr, + total: prettierBytes(totalSize), + }) + : i18n('dataUploadedOfUnknown', { complete: totalUploadedSizeStr })} {renderDot()} - {i18n('xTimeLeft', { - time: prettyETA(totalETA), - })} + {totalETA != null && + i18n('xTimeLeft', { + time: prettyETA(totalETA), + })}
) @@ -364,8 +369,8 @@ interface ProgressBarUploadingProps { numUploads: number complete: number totalUploadedSize: number - totalSize: number - totalETA: number + totalSize: number | null + totalETA: number | null startUpload: () => void } @@ -427,7 +432,9 @@ function ProgressBarUploading(props: ProgressBarUploadingProps) { : null}
- {supportsUploadProgress ? `${title}: ${totalProgress}%` : title} + {supportsUploadProgress && totalProgress !== 0 ? + `${title}: ${totalProgress}%` + : title}
{renderProgressDetails()} diff --git a/packages/@uppy/status-bar/src/StatusBar.tsx b/packages/@uppy/status-bar/src/StatusBar.tsx index 9083288378..2027abcd06 100644 --- a/packages/@uppy/status-bar/src/StatusBar.tsx +++ b/packages/@uppy/status-bar/src/StatusBar.tsx @@ -102,11 +102,15 @@ export default class StatusBar extends UIPlugin< #computeSmoothETA(totalBytes: { uploaded: number - total: number - remaining: number - }): number { - if (totalBytes.total === 0 || totalBytes.remaining === 0) { - return 0 + total: number | null // null means indeterminate + }) { + if (totalBytes.total == null || totalBytes.total === 0) { + return null + } + + const remaining = totalBytes.total - totalBytes.uploaded + if (remaining <= 0) { + return null } // When state is restored, lastUpdateTime is still nullish at this point. @@ -131,7 +135,7 @@ export default class StatusBar extends UIPlugin< currentSpeed : emaFilter(currentSpeed, this.#previousSpeed, speedFilterHalfLife, dt) this.#previousSpeed = filteredSpeed - const instantETA = totalBytes.remaining / filteredSpeed + const instantETA = remaining / filteredSpeed const updatedPreviousETA = Math.max(this.#previousETA! - dt, 0) const filteredETA = @@ -179,17 +183,30 @@ export default class StatusBar extends UIPlugin< const resumableUploads = !!capabilities.resumableUploads const supportsUploadProgress = capabilities.uploadProgress !== false - let totalSize = 0 + let totalSize: number | null = null let totalUploadedSize = 0 + // Only if all files have a known size, does it make sense to display a total size + if ( + startedFiles.every( + (f) => f.progress.bytesTotal != null && f.progress.bytesTotal !== 0, + ) + ) { + totalSize = 0 + startedFiles.forEach((file) => { + totalSize! += file.progress.bytesTotal || 0 + totalUploadedSize += file.progress.bytesUploaded || 0 + }) + } + + // however uploaded size we will always have startedFiles.forEach((file) => { - totalSize += file.progress.bytesTotal || 0 totalUploadedSize += file.progress.bytesUploaded || 0 }) + const totalETA = this.#computeSmoothETA({ uploaded: totalUploadedSize, total: totalSize, - remaining: totalSize - totalUploadedSize, }) return StatusBarUI({ diff --git a/packages/@uppy/status-bar/src/StatusBarUI.tsx b/packages/@uppy/status-bar/src/StatusBarUI.tsx index 6f5cc4480f..cbefede30b 100644 --- a/packages/@uppy/status-bar/src/StatusBarUI.tsx +++ b/packages/@uppy/status-bar/src/StatusBarUI.tsx @@ -54,8 +54,8 @@ export interface StatusBarUIProps { showProgressDetails?: boolean numUploads: number complete: number - totalSize: number - totalETA: number + totalSize: number | null + totalETA: number | null totalUploadedSize: number } diff --git a/packages/@uppy/status-bar/src/locale.ts b/packages/@uppy/status-bar/src/locale.ts index 33aadfd909..d37d89bd6f 100644 --- a/packages/@uppy/status-bar/src/locale.ts +++ b/packages/@uppy/status-bar/src/locale.ts @@ -27,6 +27,7 @@ export default { }, // When `showProgressDetails` is set, shows the amount of bytes that have been uploaded so far. dataUploadedOfTotal: '%{complete} of %{total}', + dataUploadedOfUnknown: '%{complete} of unknown', // When `showProgressDetails` is set, shows an estimation of how long the upload will take to complete. xTimeLeft: '%{time} left', // Used as the label for the button that starts an upload. diff --git a/packages/@uppy/utils/package.json b/packages/@uppy/utils/package.json index 643a0fa9e6..0b1fd3194a 100644 --- a/packages/@uppy/utils/package.json +++ b/packages/@uppy/utils/package.json @@ -27,7 +27,6 @@ "./lib/dataURItoBlob": "./lib/dataURItoBlob.js", "./lib/dataURItoFile": "./lib/dataURItoFile.js", "./lib/emaFilter": "./lib/emaFilter.js", - "./lib/emitSocketProgress": "./lib/emitSocketProgress.js", "./lib/findAllDOMElements": "./lib/findAllDOMElements.js", "./lib/findDOMElement": "./lib/findDOMElement.js", "./lib/generateFileID": "./lib/generateFileID.js", diff --git a/packages/@uppy/utils/src/FileProgress.ts b/packages/@uppy/utils/src/FileProgress.ts index e493abb201..0437e400d9 100644 --- a/packages/@uppy/utils/src/FileProgress.ts +++ b/packages/@uppy/utils/src/FileProgress.ts @@ -15,7 +15,11 @@ export type FileProcessingInfo = // TODO explore whether all of these properties need to be optional interface FileProgressBase { uploadComplete?: boolean - percentage?: number + percentage?: number // undefined if we don't know the percentage (e.g. for files with `bytesTotal` null) + // note that Companion will send `bytesTotal` 0 if unknown size (not `null`). + // this is not perfect because some files can actually have a size of 0, + // and then we might think those files have an unknown size + // todo we should change this in companion bytesTotal: number | null preprocess?: FileProcessingInfo postprocess?: FileProcessingInfo diff --git a/packages/@uppy/utils/src/emitSocketProgress.ts b/packages/@uppy/utils/src/emitSocketProgress.ts deleted file mode 100644 index 9caf3718e5..0000000000 --- a/packages/@uppy/utils/src/emitSocketProgress.ts +++ /dev/null @@ -1,28 +0,0 @@ -import throttle from 'lodash/throttle.js' -import type { UppyFile } from './UppyFile.ts' -import type { FileProgress } from './FileProgress.ts' - -function emitSocketProgress( - uploader: any, - progressData: { - progress: string // pre-formatted percentage - bytesTotal: number - bytesUploaded: number - }, - file: UppyFile, -): void { - const { progress, bytesUploaded, bytesTotal } = progressData - if (progress) { - uploader.uppy.log(`Upload progress: ${progress}`) - uploader.uppy.emit('upload-progress', file, { - uploadStarted: file.progress.uploadStarted ?? 0, - bytesUploaded, - bytesTotal, - } satisfies FileProgress) - } -} - -export default throttle(emitSocketProgress, 300, { - leading: true, - trailing: true, -}) diff --git a/yarn.lock b/yarn.lock index 0be0798828..b44966bd56 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6195,6 +6195,13 @@ __metadata: languageName: node linkType: hard +"@sec-ant/readable-stream@npm:^0.4.1": + version: 0.4.1 + resolution: "@sec-ant/readable-stream@npm:0.4.1" + checksum: 10/aac89581652ac85debe7c5303451c2ebf8bf25ca25db680e4b9b73168f6940616d9a4bbe3348981827b1159b14e2f2e6af4b7bd5735cac898c12d5c51909c102 + languageName: node + linkType: hard + "@sideway/address@npm:^4.1.5": version: 4.1.5 resolution: "@sideway/address@npm:4.1.5" @@ -6290,6 +6297,13 @@ __metadata: languageName: node linkType: hard +"@sindresorhus/merge-streams@npm:^4.0.0": + version: 4.0.0 + resolution: "@sindresorhus/merge-streams@npm:4.0.0" + checksum: 10/16551c787f5328c8ef05fd9831ade64369ccc992df78deb635ec6c44af217d2f1b43f8728c348cdc4e00585ff2fad6e00d8155199cbf6b154acc45fe65cbf0aa + languageName: node + linkType: hard + "@sinonjs/commons@npm:^3.0.0": version: 3.0.1 resolution: "@sinonjs/commons@npm:3.0.1" @@ -8150,6 +8164,7 @@ __metadata: eslint-plugin-react: "npm:^7.22.0" eslint-plugin-react-hooks: "npm:^4.2.0" eslint-plugin-unicorn: "npm:^53.0.0" + execa: "npm:^9.5.1" github-contributors-list: "npm:^1.2.4" glob: "npm:^8.0.0" jsdom: "npm:^24.0.0" @@ -15026,6 +15041,26 @@ __metadata: languageName: node linkType: hard +"execa@npm:^9.5.1": + version: 9.5.1 + resolution: "execa@npm:9.5.1" + dependencies: + "@sindresorhus/merge-streams": "npm:^4.0.0" + cross-spawn: "npm:^7.0.3" + figures: "npm:^6.1.0" + get-stream: "npm:^9.0.0" + human-signals: "npm:^8.0.0" + is-plain-obj: "npm:^4.1.0" + is-stream: "npm:^4.0.1" + npm-run-path: "npm:^6.0.0" + pretty-ms: "npm:^9.0.0" + signal-exit: "npm:^4.1.0" + strip-final-newline: "npm:^4.0.0" + yoctocolors: "npm:^2.0.0" + checksum: 10/aa030cdd43ffbf6a8825c16eec1515729553ce3655a8fa5165f0ddab2320957a9783effbeff37662e238e6f5d979d9732e3baa4bcaaeba4360856e627a214177 + languageName: node + linkType: hard + "executable@npm:^4.1.1": version: 4.1.1 resolution: "executable@npm:4.1.1" @@ -15593,6 +15628,15 @@ __metadata: languageName: node linkType: hard +"figures@npm:^6.1.0": + version: 6.1.0 + resolution: "figures@npm:6.1.0" + dependencies: + is-unicode-supported: "npm:^2.0.0" + checksum: 10/9822d13630bee8e6a9f2da866713adf13854b07e0bfde042defa8bba32d47a1c0b2afa627ce73837c674cf9a5e3edce7e879ea72cb9ea7960b2390432d8e1167 + languageName: node + linkType: hard + "file-entry-cache@npm:^6.0.1": version: 6.0.1 resolution: "file-entry-cache@npm:6.0.1" @@ -16224,6 +16268,16 @@ __metadata: languageName: node linkType: hard +"get-stream@npm:^9.0.0": + version: 9.0.1 + resolution: "get-stream@npm:9.0.1" + dependencies: + "@sec-ant/readable-stream": "npm:^0.4.1" + is-stream: "npm:^4.0.1" + checksum: 10/ce56e6db6bcd29ca9027b0546af035c3e93dcd154ca456b54c298901eb0e5b2ce799c5d727341a100c99e14c523f267f1205f46f153f7b75b1f4da6d98a21c5e + languageName: node + linkType: hard + "get-symbol-description@npm:^1.0.2": version: 1.0.2 resolution: "get-symbol-description@npm:1.0.2" @@ -17069,6 +17123,13 @@ __metadata: languageName: node linkType: hard +"human-signals@npm:^8.0.0": + version: 8.0.0 + resolution: "human-signals@npm:8.0.0" + checksum: 10/89acdc7081ac2a065e41cca7351c4b0fe2382e213b7372f90df6a554e340f31b49388a307adc1d6f4c60b2b4fe81eeff0bc1f44be6f5d844311cd92ccc7831c6 + languageName: node + linkType: hard + "humanize-ms@npm:^1.2.1": version: 1.2.1 resolution: "humanize-ms@npm:1.2.1" @@ -17925,7 +17986,7 @@ __metadata: languageName: node linkType: hard -"is-plain-obj@npm:^4.0.0": +"is-plain-obj@npm:^4.0.0, is-plain-obj@npm:^4.1.0": version: 4.1.0 resolution: "is-plain-obj@npm:4.1.0" checksum: 10/6dc45da70d04a81f35c9310971e78a6a3c7a63547ef782e3a07ee3674695081b6ca4e977fbb8efc48dae3375e0b34558d2bcd722aec9bddfa2d7db5b041be8ce @@ -18011,6 +18072,13 @@ __metadata: languageName: node linkType: hard +"is-stream@npm:^4.0.1": + version: 4.0.1 + resolution: "is-stream@npm:4.0.1" + checksum: 10/cbea3f1fc271b21ceb228819d0c12a0965a02b57f39423925f99530b4eb86935235f258f06310b67cd02b2d10b49e9a0998f5ececf110ab7d3760bae4055ad23 + languageName: node + linkType: hard + "is-string@npm:^1.0.5, is-string@npm:^1.0.7": version: 1.0.7 resolution: "is-string@npm:1.0.7" @@ -18052,6 +18120,13 @@ __metadata: languageName: node linkType: hard +"is-unicode-supported@npm:^2.0.0": + version: 2.1.0 + resolution: "is-unicode-supported@npm:2.1.0" + checksum: 10/f254e3da6b0ab1a57a94f7273a7798dd35d1d45b227759f600d0fa9d5649f9c07fa8d3c8a6360b0e376adf916d151ec24fc9a50c5295c58bae7ca54a76a063f9 + languageName: node + linkType: hard + "is-weakmap@npm:^2.0.2": version: 2.0.2 resolution: "is-weakmap@npm:2.0.2" @@ -22966,6 +23041,16 @@ __metadata: languageName: node linkType: hard +"npm-run-path@npm:^6.0.0": + version: 6.0.0 + resolution: "npm-run-path@npm:6.0.0" + dependencies: + path-key: "npm:^4.0.0" + unicorn-magic: "npm:^0.3.0" + checksum: 10/1a1b50aba6e6af7fd34a860ba2e252e245c4a59b316571a990356417c0cdf0414cabf735f7f52d9c330899cb56f0ab804a8e21fb12a66d53d7843e39ada4a3b6 + languageName: node + linkType: hard + "npmlog@npm:^6.0.0": version: 6.0.2 resolution: "npmlog@npm:6.0.2" @@ -23695,6 +23780,13 @@ __metadata: languageName: node linkType: hard +"parse-ms@npm:^4.0.0": + version: 4.0.0 + resolution: "parse-ms@npm:4.0.0" + checksum: 10/673c801d9f957ff79962d71ed5a24850163f4181a90dd30c4e3666b3a804f53b77f1f0556792e8b2adbb5d58757907d1aa51d7d7dc75997c2a56d72937cbc8b7 + languageName: node + linkType: hard + "parse-node-version@npm:^1.0.1": version: 1.0.1 resolution: "parse-node-version@npm:1.0.1" @@ -24704,6 +24796,15 @@ __metadata: languageName: node linkType: hard +"pretty-ms@npm:^9.0.0": + version: 9.1.0 + resolution: "pretty-ms@npm:9.1.0" + dependencies: + parse-ms: "npm:^4.0.0" + checksum: 10/3622a8999e4b2aa05ff64bf48c7e58143b3ede6e3434f8ce5588def90ebcf6af98edf79532344c4c9e14d5ad25deb3f0f5ca9f9b91e5d2d1ac26dad9cf428fc0 + languageName: node + linkType: hard + "proc-log@npm:^2.0.0, proc-log@npm:^2.0.1": version: 2.0.1 resolution: "proc-log@npm:2.0.1" @@ -27942,6 +28043,13 @@ __metadata: languageName: node linkType: hard +"strip-final-newline@npm:^4.0.0": + version: 4.0.0 + resolution: "strip-final-newline@npm:4.0.0" + checksum: 10/b5fe48f695d74863153a3b3155220e6e9bf51f4447832998c8edec38e6559b3af87a9fe5ac0df95570a78a26f5fa91701358842eab3c15480e27980b154a145f + languageName: node + linkType: hard + "strip-indent@npm:^3.0.0": version: 3.0.0 resolution: "strip-indent@npm:3.0.0" @@ -29271,6 +29379,13 @@ __metadata: languageName: node linkType: hard +"unicorn-magic@npm:^0.3.0": + version: 0.3.0 + resolution: "unicorn-magic@npm:0.3.0" + checksum: 10/bdd7d7c522f9456f32a0b77af23f8854f9a7db846088c3868ec213f9550683ab6a2bdf3803577eacbafddb4e06900974385841ccb75338d17346ccef45f9cb01 + languageName: node + linkType: hard + "unified-args@npm:^11.0.0": version: 11.0.1 resolution: "unified-args@npm:11.0.1" @@ -31063,6 +31178,13 @@ __metadata: languageName: node linkType: hard +"yoctocolors@npm:^2.0.0": + version: 2.1.1 + resolution: "yoctocolors@npm:2.1.1" + checksum: 10/563fbec88bce9716d1044bc98c96c329e1d7a7c503e6f1af68f1ff914adc3ba55ce953c871395e2efecad329f85f1632f51a99c362032940321ff80c42a6f74d + languageName: node + linkType: hard + "zone.js@npm:~0.14.3": version: 0.14.7 resolution: "zone.js@npm:0.14.7"