From 346c2fcd43bb425fb36ff4a7e40bae183c9a0d19 Mon Sep 17 00:00:00 2001
From: Artur Paikin <artur@arturpaikin.com>
Date: Mon, 8 Aug 2022 11:58:43 +0200
Subject: [PATCH] core validateRestrictions: return error directly vs the
 result/reason obj (#3951)

* core validateRestrictions: return error directly vs the result/reason obj

* Refacrtor to actually use restrictionError instead of {reason, message}

* Return error instead of throwing
---
 packages/@uppy/core/src/Uppy.js                    |  6 ++----
 packages/@uppy/core/src/Uppy.test.js               | 14 ++------------
 packages/@uppy/provider-views/src/Browser.jsx      |  6 +++---
 .../provider-views/src/Item/components/GridLi.jsx  |  4 ++--
 .../provider-views/src/Item/components/ListLi.jsx  |  4 ++--
 packages/@uppy/provider-views/src/SharedHandler.js |  7 ++++---
 6 files changed, 15 insertions(+), 26 deletions(-)

diff --git a/packages/@uppy/core/src/Uppy.js b/packages/@uppy/core/src/Uppy.js
index fb61521376..cf38723b3e 100644
--- a/packages/@uppy/core/src/Uppy.js
+++ b/packages/@uppy/core/src/Uppy.js
@@ -390,14 +390,12 @@ 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 err
     }
+    return null
   }
 
   #checkRequiredMetaFieldsOnFile (file) {
diff --git a/packages/@uppy/core/src/Uppy.test.js b/packages/@uppy/core/src/Uppy.test.js
index 31dc6073cb..a0574db994 100644
--- a/packages/@uppy/core/src/Uppy.test.js
+++ b/packages/@uppy/core/src/Uppy.test.js
@@ -1804,18 +1804,8 @@ describe('src/Core', () => {
       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(validateRestrictions2).toMatchObject(
-        {
-          result: false,
-          reason: 'You can only upload: image/png',
-        },
-      )
+      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 72d0a03a97..6c0e1b97e3 100644
--- a/packages/@uppy/provider-views/src/Browser.jsx
+++ b/packages/@uppy/provider-views/src/Browser.jsx
@@ -101,7 +101,7 @@ function Browser (props) {
               })}
 
               {files.map((file) => {
-                const validated = validateRestrictions(
+                const restrictionError = validateRestrictions(
                   remoteFileObjToLocal(file),
                   [...uppyFiles, ...currentSelection],
                 )
@@ -119,8 +119,8 @@ function Browser (props) {
                   viewType,
                   i18n,
                   type: 'file',
-                  isDisabled: !validated.result && !isChecked(file),
-                  restrictionReason: validated.reason,
+                  isDisabled: restrictionError && !isChecked(file),
+                  restrictionError,
                 })
               })}
             </ul>
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 (
     <li
       className={className}
-      title={isDisabled ? restrictionReason : null}
+      title={isDisabled ? restrictionError?.message : null}
     >
       <input
         type="checkbox"
diff --git a/packages/@uppy/provider-views/src/Item/components/ListLi.jsx b/packages/@uppy/provider-views/src/Item/components/ListLi.jsx
index c88726f1b7..46271613b8 100644
--- a/packages/@uppy/provider-views/src/Item/components/ListLi.jsx
+++ b/packages/@uppy/provider-views/src/Item/components/ListLi.jsx
@@ -11,7 +11,7 @@ function ListItem (props) {
   const {
     className,
     isDisabled,
-    restrictionReason,
+    restrictionError,
     isCheckboxDisabled,
     isChecked,
     toggleCheckbox,
@@ -28,7 +28,7 @@ function ListItem (props) {
   return (
     <li
       className={className}
-      title={isDisabled ? restrictionReason : null}
+      title={isDisabled ? restrictionError?.message : null}
     >
       {!isCheckboxDisabled ? (
         <input
diff --git a/packages/@uppy/provider-views/src/SharedHandler.js b/packages/@uppy/provider-views/src/SharedHandler.js
index 4a72a8b313..e7af174116 100644
--- a/packages/@uppy/provider-views/src/SharedHandler.js
+++ b/packages/@uppy/provider-views/src/SharedHandler.js
@@ -51,14 +51,15 @@ export default class SharedHandler {
       // reduce it to only contain files that pass restrictions
       for (const item of currentSelection) {
         const { uppy } = this.plugin
-        const validatedRestrictions = uppy.validateRestrictions(
+        const restrictionError = uppy.validateRestrictions(
           remoteFileObjToLocal(item),
           [...uppy.getFiles(), ...reducedCurrentSelection],
         )
-        if (validatedRestrictions.result) {
+
+        if (!restrictionError) {
           reducedCurrentSelection.push(item)
         } else {
-          uppy.info({ message: validatedRestrictions.reason }, 'error', uppy.opts.infoTimeout)
+          uppy.info({ message: restrictionError.message }, 'error', uppy.opts.infoTimeout)
         }
       }
       this.plugin.setPluginState({ currentSelection: reducedCurrentSelection })