From 1d7676c7d272148c6e13140fec046f24b8c64e68 Mon Sep 17 00:00:00 2001 From: Artur Paikin Date: Thu, 4 Aug 2022 13:11:12 +0100 Subject: [PATCH 1/3] core validateRestrictions: return error directly vs the result/reason obj --- packages/@uppy/core/src/Uppy.js | 9 +-------- packages/@uppy/core/src/Uppy.test.js | 17 ++++------------- packages/@uppy/provider-views/src/Browser.jsx | 14 ++++++++++---- 3 files changed, 15 insertions(+), 25 deletions(-) diff --git a/packages/@uppy/core/src/Uppy.js b/packages/@uppy/core/src/Uppy.js index fce36139a6..3b828f4a6d 100644 --- a/packages/@uppy/core/src/Uppy.js +++ b/packages/@uppy/core/src/Uppy.js @@ -390,14 +390,7 @@ class Uppy { } validateRestrictions (file, files = this.getFiles()) { - // TODO: directly return the Restriction error in next major version. - // we create RestrictionError's just to discard immediately, which doesn't make sense. - try { - this.#restricter.validate(file, files) - return { result: true } - } catch (err) { - return { result: false, reason: err.message } - } + return this.#restricter.validate(file, files) } #checkRequiredMetaFieldsOnFile (file) { diff --git a/packages/@uppy/core/src/Uppy.test.js b/packages/@uppy/core/src/Uppy.test.js index 4b62fb2f4e..2fed522770 100644 --- a/packages/@uppy/core/src/Uppy.test.js +++ b/packages/@uppy/core/src/Uppy.test.js @@ -1795,20 +1795,11 @@ describe('src/Core', () => { size: 270733, } - const validateRestrictions1 = core.validateRestrictions(newFile) - const validateRestrictions2 = core2.validateRestrictions(newFile) - - expect(validateRestrictions1).toMatchObject( - { - result: false, - reason: 'This file is smaller than the allowed size of 293 KB', - }, + expect(() => core.validateRestrictions(newFile)).toThrow( + 'This file is smaller than the allowed size of 293 KB', ) - expect(validateRestrictions2).toMatchObject( - { - result: false, - reason: 'You can only upload: image/png', - }, + expect(() => core2.validateRestrictions(newFile)).toThrow( + 'You can only upload: image/png', ) }) diff --git a/packages/@uppy/provider-views/src/Browser.jsx b/packages/@uppy/provider-views/src/Browser.jsx index 72d0a03a97..b36abbb746 100644 --- a/packages/@uppy/provider-views/src/Browser.jsx +++ b/packages/@uppy/provider-views/src/Browser.jsx @@ -101,10 +101,16 @@ function Browser (props) { })} {files.map((file) => { - const validated = validateRestrictions( - remoteFileObjToLocal(file), - [...uppyFiles, ...currentSelection], - ) + let validated + try { + validateRestrictions( + remoteFileObjToLocal(file), + [...uppyFiles, ...currentSelection], + ) + validated = { result: true } + } catch (err) { + validated = { result: false, reason: err.message } + } return Item({ id: file.id, From 5d69fc74941f89d994fa5f2e2a02bebf66715b20 Mon Sep 17 00:00:00 2001 From: Artur Paikin Date: Thu, 4 Aug 2022 14:04:58 +0100 Subject: [PATCH 2/3] Refacrtor to actually use restrictionError instead of {reason, message} --- packages/@uppy/provider-views/src/Browser.jsx | 9 ++++----- .../src/Item/components/GridLi.jsx | 4 ++-- .../src/Item/components/ListLi.jsx | 4 ++-- .../@uppy/provider-views/src/SharedHandler.js | 18 ++++++++++++------ 4 files changed, 20 insertions(+), 15 deletions(-) diff --git a/packages/@uppy/provider-views/src/Browser.jsx b/packages/@uppy/provider-views/src/Browser.jsx index b36abbb746..30049387b3 100644 --- a/packages/@uppy/provider-views/src/Browser.jsx +++ b/packages/@uppy/provider-views/src/Browser.jsx @@ -101,15 +101,14 @@ function Browser (props) { })} {files.map((file) => { - let validated + let restrictionError try { validateRestrictions( remoteFileObjToLocal(file), [...uppyFiles, ...currentSelection], ) - validated = { result: true } } catch (err) { - validated = { result: false, reason: err.message } + restrictionError = err } return Item({ @@ -125,8 +124,8 @@ function Browser (props) { viewType, i18n, type: 'file', - isDisabled: !validated.result && !isChecked(file), - restrictionReason: validated.reason, + isDisabled: restrictionError && !isChecked(file), + restrictionError, }) })} diff --git a/packages/@uppy/provider-views/src/Item/components/GridLi.jsx b/packages/@uppy/provider-views/src/Item/components/GridLi.jsx index 395600255a..482352cec4 100644 --- a/packages/@uppy/provider-views/src/Item/components/GridLi.jsx +++ b/packages/@uppy/provider-views/src/Item/components/GridLi.jsx @@ -4,7 +4,7 @@ function GridListItem (props) { const { className, isDisabled, - restrictionReason, + restrictionError, isChecked, title, itemIconEl, @@ -18,7 +18,7 @@ function GridListItem (props) { return (
  • {!isCheckboxDisabled ? ( Date: Fri, 5 Aug 2022 23:37:46 +0100 Subject: [PATCH 3/3] Return error instead of throwing --- packages/@uppy/core/src/Uppy.js | 7 ++++++- packages/@uppy/core/src/Uppy.test.js | 11 +++++------ packages/@uppy/provider-views/src/Browser.jsx | 13 ++++--------- packages/@uppy/provider-views/src/SharedHandler.js | 13 ++++--------- 4 files changed, 19 insertions(+), 25 deletions(-) diff --git a/packages/@uppy/core/src/Uppy.js b/packages/@uppy/core/src/Uppy.js index 3b828f4a6d..ad90d11aa4 100644 --- a/packages/@uppy/core/src/Uppy.js +++ b/packages/@uppy/core/src/Uppy.js @@ -390,7 +390,12 @@ class Uppy { } validateRestrictions (file, files = this.getFiles()) { - return this.#restricter.validate(file, files) + try { + this.#restricter.validate(file, files) + } catch (err) { + return err + } + return null } #checkRequiredMetaFieldsOnFile (file) { diff --git a/packages/@uppy/core/src/Uppy.test.js b/packages/@uppy/core/src/Uppy.test.js index 2fed522770..1157c3f6a8 100644 --- a/packages/@uppy/core/src/Uppy.test.js +++ b/packages/@uppy/core/src/Uppy.test.js @@ -1795,12 +1795,11 @@ describe('src/Core', () => { size: 270733, } - expect(() => core.validateRestrictions(newFile)).toThrow( - 'This file is smaller than the allowed size of 293 KB', - ) - expect(() => core2.validateRestrictions(newFile)).toThrow( - 'You can only upload: image/png', - ) + const validateRestrictions1 = core.validateRestrictions(newFile) + const validateRestrictions2 = core2.validateRestrictions(newFile) + + expect(validateRestrictions1.message).toEqual('This file is smaller than the allowed size of 293 KB') + expect(validateRestrictions2.message).toEqual('You can only upload: image/png') }) it('should emit `restriction-failed` event when some rule is violated', () => { diff --git a/packages/@uppy/provider-views/src/Browser.jsx b/packages/@uppy/provider-views/src/Browser.jsx index 30049387b3..6c0e1b97e3 100644 --- a/packages/@uppy/provider-views/src/Browser.jsx +++ b/packages/@uppy/provider-views/src/Browser.jsx @@ -101,15 +101,10 @@ function Browser (props) { })} {files.map((file) => { - let restrictionError - try { - validateRestrictions( - remoteFileObjToLocal(file), - [...uppyFiles, ...currentSelection], - ) - } catch (err) { - restrictionError = err - } + const restrictionError = validateRestrictions( + remoteFileObjToLocal(file), + [...uppyFiles, ...currentSelection], + ) return Item({ id: file.id, diff --git a/packages/@uppy/provider-views/src/SharedHandler.js b/packages/@uppy/provider-views/src/SharedHandler.js index 5be4b2d43d..e7af174116 100644 --- a/packages/@uppy/provider-views/src/SharedHandler.js +++ b/packages/@uppy/provider-views/src/SharedHandler.js @@ -51,15 +51,10 @@ export default class SharedHandler { // reduce it to only contain files that pass restrictions for (const item of currentSelection) { const { uppy } = this.plugin - let restrictionError - try { - uppy.validateRestrictions( - remoteFileObjToLocal(item), - [...uppy.getFiles(), ...reducedCurrentSelection], - ) - } catch (err) { - restrictionError = err - } + const restrictionError = uppy.validateRestrictions( + remoteFileObjToLocal(item), + [...uppy.getFiles(), ...reducedCurrentSelection], + ) if (!restrictionError) { reducedCurrentSelection.push(item)