From b3073e76e7c2794013a9f022f00e838d8009b260 Mon Sep 17 00:00:00 2001 From: Artur Paikin Date: Fri, 24 May 2019 20:34:24 +0300 Subject: [PATCH 1/2] pass newFile with all uppy meta info and detected type to onBeforeFileAdded --- packages/@uppy/core/src/index.js | 28 ++++++++++++---------------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/packages/@uppy/core/src/index.js b/packages/@uppy/core/src/index.js index 6361173089..35c2191433 100644 --- a/packages/@uppy/core/src/index.js +++ b/packages/@uppy/core/src/index.js @@ -402,21 +402,6 @@ class Uppy { onError(new Error('Cannot add new files: already uploading.')) } - const onBeforeFileAddedResult = this.opts.onBeforeFileAdded(file, files) - - if (onBeforeFileAddedResult === false) { - this.log('Not adding file because onBeforeFileAdded returned false') - return - } - - if (typeof onBeforeFileAddedResult === 'object' && onBeforeFileAddedResult) { - // warning after the change in 0.24 - if (onBeforeFileAddedResult.then) { - throw new TypeError('onBeforeFileAdded() returned a Promise, but this is no longer supported. It must be synchronous.') - } - file = onBeforeFileAddedResult - } - const fileType = getFileType(file) let fileName if (file.name) { @@ -437,7 +422,7 @@ class Uppy { // `null` means the size is unknown. const size = isFinite(file.data.size) ? file.data.size : null - const newFile = { + let newFile = { source: file.source || '', id: fileID, name: fileName, @@ -458,6 +443,17 @@ class Uppy { preview: file.preview } + const onBeforeFileAddedResult = this.opts.onBeforeFileAdded(newFile, files) + + if (onBeforeFileAddedResult === false) { + this.log('Not adding file because onBeforeFileAdded returned false') + return + } + + if (typeof onBeforeFileAddedResult === 'object' && onBeforeFileAddedResult) { + newFile = onBeforeFileAddedResult + } + try { this._checkRestrictions(newFile) } catch (err) { From 2caedeca66717bd6ee2e31c6a6f95273ca5443e4 Mon Sep 17 00:00:00 2001 From: Artur Paikin Date: Fri, 24 May 2019 20:40:38 +0300 Subject: [PATCH 2/2] Run _checkRestrictions and _checkMinNumberOfFiles before onBeforeFileAdded and onBeforeUpload callbacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses #1545, but I’m not sure this is the right move. Is it better to check restrictions before or after the callbacks? --- packages/@uppy/core/src/index.js | 43 +++++++++++++++++--------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/packages/@uppy/core/src/index.js b/packages/@uppy/core/src/index.js index 35c2191433..ea18732b6d 100644 --- a/packages/@uppy/core/src/index.js +++ b/packages/@uppy/core/src/index.js @@ -443,6 +443,13 @@ class Uppy { preview: file.preview } + try { + this._checkRestrictions(newFile) + } catch (err) { + this.emit('restriction-failed', newFile, err) + onError(err) + } + const onBeforeFileAddedResult = this.opts.onBeforeFileAdded(newFile, files) if (onBeforeFileAddedResult === false) { @@ -454,13 +461,6 @@ class Uppy { newFile = onBeforeFileAddedResult } - try { - this._checkRestrictions(newFile) - } catch (err) { - this.emit('restriction-failed', newFile, err) - onError(err) - } - this.setState({ files: Object.assign({}, files, { [fileID]: newFile @@ -1240,6 +1240,21 @@ class Uppy { } let files = this.getState().files + + const handleError = (err) => { + const message = typeof err === 'object' ? err.message : err + const details = typeof err === 'object' ? err.details : null + this.log(`${message} ${details}`) + this.info({ message: message, details: details }, 'error', 4000) + return Promise.reject(typeof err === 'object' ? err : new Error(err)) + } + + try { + this._checkMinNumberOfFiles(files) + } catch (err) { + return handleError(err) + } + const onBeforeUploadResult = this.opts.onBeforeUpload(files) if (onBeforeUploadResult === false) { @@ -1247,16 +1262,10 @@ class Uppy { } if (onBeforeUploadResult && typeof onBeforeUploadResult === 'object') { - // warning after the change in 0.24 - if (onBeforeUploadResult.then) { - throw new TypeError('onBeforeUpload() returned a Promise, but this is no longer supported. It must be synchronous.') - } - files = onBeforeUploadResult } return Promise.resolve() - .then(() => this._checkMinNumberOfFiles(files)) .then(() => { const { currentUploads } = this.getState() // get a list of files that are currently assigned to uploads @@ -1274,13 +1283,7 @@ class Uppy { const uploadID = this._createUpload(waitingFileIDs) return this._runUpload(uploadID) }) - .catch((err) => { - const message = typeof err === 'object' ? err.message : err - const details = typeof err === 'object' ? err.details : null - this.log(`${message} ${details}`) - this.info({ message: message, details: details }, 'error', 4000) - return Promise.reject(typeof err === 'object' ? err : new Error(err)) - }) + .catch(handleError) } }