diff --git a/docs/companion.md b/docs/companion.md index 83aa31c900..6e8f5537e1 100644 --- a/docs/companion.md +++ b/docs/companion.md @@ -329,6 +329,7 @@ const options = { logClientVersion: true, periodicPingUrls: [], streamingUpload: true, + streamingUploadSizeless: false, clientSocketConnectTimeout: 60000, metrics: true, }; @@ -640,6 +641,15 @@ enabled, it will lead to _faster uploads_ because companion will start uploading at the same time as downloading using `stream.pipe`. If `false`, files will be fully downloaded first, then uploaded. Defaults to `true`. +#### `streamingUploadSizeless` `COMPANION_STREAMING_UPLOAD_SIZELESS` + +A boolean flag to tell Companion whether to also upload files that have an +unknown size. Currently this is only supported for Tus uploads. Note that this +requires an optional extension on the Tus server if using Tus uploads. For form +multipart uploads it requres a server that can handle +`transfer-encoding: chunked`. Default is `false`. If set to `true`, +`streamingUpload` also has to be set to `true`. + #### `maxFileSize` `COMPANION_MAX_FILE_SIZE` If this value is set, companion will limit the maximum file size to process. If diff --git a/docs/framework-integrations/react.mdx b/docs/framework-integrations/react.mdx index 5d2267ea33..d596366b3f 100644 --- a/docs/framework-integrations/react.mdx +++ b/docs/framework-integrations/react.mdx @@ -74,7 +74,7 @@ times, this is needed if you are building a custom UI for Uppy in React. const [uppy] = useState(() => new Uppy()); const files = useUppyState(uppy, (state) => state.files); -const totalProgress = useUppyState(uppy, (state) => state.totalProgress); +const totalProgress = useUppyState(uppy, (state) => state.progress); // We can also get specific plugin state. // Note that the value on `plugins` depends on the `id` of the plugin. const metaFields = useUppyState( diff --git a/docs/uppy-core.mdx b/docs/uppy-core.mdx index d00a7160bd..4685d18d29 100644 --- a/docs/uppy-core.mdx +++ b/docs/uppy-core.mdx @@ -794,7 +794,7 @@ const state = { capabilities: { resumableUploads: false, }, - totalProgress: 0, + progress: null, meta: { ...this.opts.meta }, info: { isHidden: true, 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/examples/react-example/App.tsx b/examples/react-example/App.tsx index 3387e8045c..014687a14a 100644 --- a/examples/react-example/App.tsx +++ b/examples/react-example/App.tsx @@ -77,7 +77,7 @@ export default function App() { uppy, (state) => Object.keys(state.files).length, ) - const totalProgress = useUppyState(uppy, (state) => state.totalProgress) + const totalProgress = useUppyState(uppy, (state) => state.progress) // Also possible to get the state of all plugins. const plugins = useUppyState(uppy, (state) => state.plugins) diff --git a/examples/react-native-expo/App.js b/examples/react-native-expo/App.js index f380b4251a..3454bb9a2a 100644 --- a/examples/react-native-expo/App.js +++ b/examples/react-native-expo/App.js @@ -39,7 +39,7 @@ export default function App () { setState({ progress: progress.bytesUploaded, total: progress.bytesTotal, - totalProgress: uppy.state.totalProgress, + totalProgress: uppy.state.progress, uploadStarted: true, }) }) diff --git a/package.json b/package.json index b1fa51fec6..6cdfd8e627 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 236ce62527..a6d5a2fee2 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/config/companion.js b/packages/@uppy/companion/src/config/companion.js index b847f3df7a..cae8c77ed7 100644 --- a/packages/@uppy/companion/src/config/companion.js +++ b/packages/@uppy/companion/src/config/companion.js @@ -20,6 +20,7 @@ const defaultOptions = { allowLocalUrls: false, periodicPingUrls: [], streamingUpload: true, + streamingUploadSizeless: false, clientSocketConnectTimeout: 60000, metrics: true, } diff --git a/packages/@uppy/companion/src/server/Uploader.js b/packages/@uppy/companion/src/server/Uploader.js index 25febdb309..c1bf7ba250 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,12 @@ class Uploader { this.readStream = fileStream } - _needDownloadFirst() { - return !this.options.size || !this.options.companionOptions.streamingUpload + _canStream() { + return this.options.companionOptions.streamingUpload && ( + this.options.size + // only tus uploads can be streamed without size, TODO: add also others + || this.options.companionOptions.streamingUploadSizeless + ) } /** @@ -281,7 +289,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 +438,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 +615,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/companion/src/standalone/helper.js b/packages/@uppy/companion/src/standalone/helper.js index e6ccd9887b..f631b1f1ea 100644 --- a/packages/@uppy/companion/src/standalone/helper.js +++ b/packages/@uppy/companion/src/standalone/helper.js @@ -182,6 +182,7 @@ const getConfigFromEnv = () => { // cookieDomain is kind of a hack to support distributed systems. This should be improved but we never got so far. cookieDomain: process.env.COMPANION_COOKIE_DOMAIN, streamingUpload: process.env.COMPANION_STREAMING_UPLOAD ? process.env.COMPANION_STREAMING_UPLOAD === 'true' : undefined, + streamingUploadSizeless: process.env.COMPANION_STREAMING_UPLOAD_SIZELESS ? process.env.COMPANION_STREAMING_UPLOAD_SIZELESS === 'true' : undefined, maxFileSize: process.env.COMPANION_MAX_FILE_SIZE ? parseInt(process.env.COMPANION_MAX_FILE_SIZE, 10) : undefined, chunkSize: process.env.COMPANION_CHUNK_SIZE ? parseInt(process.env.COMPANION_CHUNK_SIZE, 10) : undefined, clientSocketConnectTimeout: process.env.COMPANION_CLIENT_SOCKET_CONNECT_TIMEOUT diff --git a/packages/@uppy/core/src/Uppy.test.ts b/packages/@uppy/core/src/Uppy.test.ts index 0a27e16e52..653c8230cd 100644 --- a/packages/@uppy/core/src/Uppy.test.ts +++ b/packages/@uppy/core/src/Uppy.test.ts @@ -282,6 +282,7 @@ describe('src/Core', () => { meta: {}, plugins: {}, totalProgress: 0, + progress: null, recoveredState: null, } @@ -316,6 +317,7 @@ describe('src/Core', () => { meta: {}, plugins: {}, totalProgress: 0, + progress: null, recoveredState: null, }) // new state @@ -335,6 +337,7 @@ describe('src/Core', () => { meta: {}, plugins: {}, totalProgress: 0, + progress: null, recoveredState: null, }) }) @@ -358,7 +361,7 @@ describe('src/Core', () => { const coreStateUpdateEventMock = vi.fn() core.on('cancel-all', coreCancelEventMock) core.on('state-update', coreStateUpdateEventMock) - core.setState({ foo: 'bar', totalProgress: 30 }) + core.setState({ foo: 'bar', totalProgress: 30, progress: 30 }) core.cancelAll() @@ -386,6 +389,7 @@ describe('src/Core', () => { meta: {}, plugins: {}, totalProgress: 0, + progress: null, recoveredState: null, }) }) @@ -577,6 +581,7 @@ describe('src/Core', () => { meta: {}, plugins: {}, totalProgress: 0, + progress: null, recoveredState: null, }) expect(plugin.mocks.uninstall.mock.calls.length).toEqual(1) @@ -1187,7 +1192,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', @@ -1377,6 +1382,7 @@ describe('src/Core', () => { expect(core.getFiles().length).toEqual(1) core.setState({ totalProgress: 50, + progress: 30, }) const file = core.getFile(fileId) @@ -1385,6 +1391,7 @@ describe('src/Core', () => { expect(core.getFiles().length).toEqual(0) expect(fileRemovedEventMock.mock.calls[0][0]).toEqual(file) expect(core.getState().totalProgress).toEqual(0) + expect(core.getState().progress).toEqual(0) }) }) @@ -1701,6 +1708,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 +1721,7 @@ describe('src/Core', () => { bytesUploaded: 12345, bytesTotal: 17175, uploadComplete: false, - uploadStarted: null, + uploadStarted: expect.any(Number), }) // @ts-ignore @@ -1720,14 +1730,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 +1770,8 @@ describe('src/Core', () => { data: {}, }) - core.calculateTotalProgress() + // @ts-ignore + core[Symbol.for('uppy test: updateTotalProgress')]() const uploadPromise = core.upload() await Promise.all([ @@ -1774,7 +1783,6 @@ describe('src/Core', () => { bytesUploaded: 0, // null indicates unsized bytesTotal: null, - percentage: 0, }) // @ts-ignore @@ -1790,6 +1798,7 @@ describe('src/Core', () => { }) expect(core.getState().totalProgress).toBe(36) + expect(core.getState().progress).toBe(36) // @ts-ignore finishUpload() @@ -1844,10 +1853,12 @@ 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) + expect(core.getState().progress).toBe(36) core.destroy() }) @@ -1893,10 +1904,11 @@ 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().progress).toBe(66) }) it('should emit the progress', () => { @@ -1937,10 +1949,11 @@ 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().progress).toEqual(66) expect(core.getState().allowNewUpload).toEqual(true) expect(core.getState().error).toEqual(null) expect(core.getState().recoveredState).toEqual(null) @@ -1965,6 +1978,7 @@ describe('src/Core', () => { core.clear() expect(core.getState()).toMatchObject({ totalProgress: 0, + progress: null, allowNewUpload: true, error: null, recoveredState: null, diff --git a/packages/@uppy/core/src/Uppy.ts b/packages/@uppy/core/src/Uppy.ts index f8e25e538e..9376155525 100644 --- a/packages/@uppy/core/src/Uppy.ts +++ b/packages/@uppy/core/src/Uppy.ts @@ -232,7 +232,8 @@ export interface State details?: string | Record | null }> plugins: Plugins - totalProgress: number + totalProgress: number // todo remove backward compat + progress: number | null companion?: Record } @@ -361,6 +362,7 @@ type OmitFirstArg = T extends [any, ...infer U] ? U : never const defaultUploadState = { totalProgress: 0, + progress: null, allowNewUpload: true, error: null, recoveredState: null, @@ -758,7 +760,11 @@ export class Uppy< isUploadInProgress: boolean isSomeGhost: boolean } { - const { files: filesObject, totalProgress, error } = this.getState() + const { + files: filesObject, + progress: totalProgress, + error, + } = this.getState() const files = Object.values(filesObject) const inProgressFiles: UppyFile[] = [] @@ -1270,7 +1276,7 @@ export class Uppy< } this.setState(stateUpdate) - this.calculateTotalProgress() + this.#updateTotalProgressThrottled() const removedFileIDs = Object.keys(removedFiles) removedFileIDs.forEach((fileID) => { @@ -1416,59 +1422,99 @@ 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.calculateTotalProgress() - }, + 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) // todo remove `?? 0` in next major + this.setState({ + totalProgress: totalProgressPercent ?? 0, // todo remove backward compat in next major + progress: totalProgressPercent, + }) + } + + // ___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 || @@ -1476,54 +1522,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 + const isSizedFile = (file: UppyFile) => + file.progress.bytesTotal != null && file.progress.bytesTotal !== 0 - let uploadedSize = 0 - sizedFiles.forEach((file) => { - uploadedSize += file.progress.bytesUploaded as number - }) - unsizedFiles.forEach((file) => { - uploadedSize += (averageSize * (file.progress.percentage || 0)) / 100 - }) - - 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 } /** @@ -1609,7 +1649,6 @@ export class Uppy< progress: { uploadStarted: Date.now(), uploadComplete: false, - percentage: 0, bytesUploaded: 0, bytesTotal: file.size, } as FileProgressStarted, @@ -1622,7 +1661,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)) { @@ -1659,7 +1698,7 @@ export class Uppy< }) } - this.calculateTotalProgress() + this.#updateTotalProgressThrottled() }) this.on('preprocess-progress', (file, progress) => { @@ -1729,7 +1768,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/dashboard/src/Dashboard.tsx b/packages/@uppy/dashboard/src/Dashboard.tsx index dc9a7f5768..137cd7af1a 100644 --- a/packages/@uppy/dashboard/src/Dashboard.tsx +++ b/packages/@uppy/dashboard/src/Dashboard.tsx @@ -1202,7 +1202,7 @@ export default class Dashboard extends UIPlugin< isAllComplete, isAllPaused, totalFileCount: Object.keys(files).length, - totalProgress: state.totalProgress, + totalProgress: state.progress, allowNewUpload, acquirers, theme, diff --git a/packages/@uppy/dashboard/src/components/Dashboard.tsx b/packages/@uppy/dashboard/src/components/Dashboard.tsx index 36d185e3f9..379247a618 100644 --- a/packages/@uppy/dashboard/src/components/Dashboard.tsx +++ b/packages/@uppy/dashboard/src/components/Dashboard.tsx @@ -44,7 +44,7 @@ type DashboardUIProps = { isAllComplete: boolean isAllPaused: boolean totalFileCount: number - totalProgress: number + totalProgress: number | null allowNewUpload: boolean acquirers: TargetWithRender[] theme: string diff --git a/packages/@uppy/progress-bar/src/ProgressBar.tsx b/packages/@uppy/progress-bar/src/ProgressBar.tsx index f4a5d9ac69..ebc3e28984 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 { progress } = state // before starting and after finish should be hidden if specified in the options const isHidden = - (progress === 0 || progress === 100) && this.opts.hideAfterFinish + (progress == null || progress === 0 || progress === 100) && + this.opts.hideAfterFinish return (
{ const { result, rerender } = renderHook(() => useUppyState(uppy, (state) => state.totalProgress), ) + const { result: result2, rerender: rerender2 } = renderHook(() => + useUppyState(uppy, (state) => state.progress), + ) expectTypeOf(result.current).toEqualTypeOf() + expectTypeOf(result2.current).toEqualTypeOf() expect(result.current).toBe(0) + expect(result2.current).toBe(null) + act(() => uppy.setState({ progress: 50 })) act(() => uppy.setState({ totalProgress: 50 })) rerender() + rerender2() expect(result.current).toBe(50) + expect(result2.current).toBe(50) rerender() + rerender2() expect(result.current).toBe(50) + expect(result2.current).toBe(50) }) it('does not re-render unnecessarily', () => { diff --git a/packages/@uppy/status-bar/src/Components.tsx b/packages/@uppy/status-bar/src/Components.tsx index fa096c95a0..680b78cce1 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), + }) + : totalUploadedSizeStr} {renderDot()} - {i18n('xTimeLeft', { - time: prettyETA(totalETA), - })} + {totalETA != null && + i18n('xTimeLeft', { + time: prettyETA(totalETA), + })}
) @@ -355,7 +360,7 @@ function UploadNewlyAddedFiles(props: UploadNewlyAddedFilesProps) { interface ProgressBarUploadingProps { i18n: I18n supportsUploadProgress: boolean - totalProgress: number + totalProgress: number | null showProgressDetails: boolean | undefined isUploadStarted: boolean isAllComplete: boolean @@ -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 != null ? + `${title}: ${totalProgress}%` + : title}
{renderProgressDetails()} diff --git a/packages/@uppy/status-bar/src/StatusBar.tsx b/packages/@uppy/status-bar/src/StatusBar.tsx index 9083288378..5c124eefdd 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 = @@ -155,7 +159,7 @@ export default class StatusBar extends UIPlugin< capabilities, files, allowNewUpload, - totalProgress, + progress: totalProgress, error, recoveredState, } = state @@ -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..2d1ca2687d 100644 --- a/packages/@uppy/status-bar/src/StatusBarUI.tsx +++ b/packages/@uppy/status-bar/src/StatusBarUI.tsx @@ -40,7 +40,7 @@ export interface StatusBarUIProps { hideRetryButton?: boolean recoveredState: State['recoveredState'] uploadState: (typeof statusBarStates)[keyof typeof statusBarStates] - totalProgress: number + totalProgress: number | null files: Record> supportsUploadProgress: boolean hideAfterFinish?: boolean @@ -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/utils/package.json b/packages/@uppy/utils/package.json index ee0d010dde..eb48ffcfb1 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 426223c54b..5226905b7b 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" @@ -8129,6 +8143,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" @@ -14974,6 +14989,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" @@ -15541,6 +15576,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" @@ -16172,6 +16216,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" @@ -17017,6 +17071,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" @@ -17873,7 +17934,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 @@ -17959,6 +18020,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" @@ -18000,6 +18068,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" @@ -22914,6 +22989,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" @@ -23643,6 +23728,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" @@ -24652,6 +24744,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" @@ -27890,6 +27991,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" @@ -29219,6 +29327,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" @@ -31009,6 +31124,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"