Warn instead of error when duplicate binding IDs are found in non-dev…

…mode (#4019) * Close #4016. Warn instead of error when duplicate binding IDs are found in non-devmode * Get rid of unreachable ShinyClientError() * `yarn build` (GitHub Actions) * Update srcts/src/shiny/bind.ts Co-authored-by: Garrick Aden-Buie <[email protected]> * `yarn build` (GitHub Actions) * Move logic to where error gets thrown not constructed * `yarn build` (GitHub Actions) * Update NEWS --------- Co-authored-by: cpsievert <[email protected]> Co-authored-by: Garrick Aden-Buie <[email protected]>
rstudio · Mar 29, 2024 · edd1db7 · edd1db7
1 parent 47526a7
commit edd1db7
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 22 deletions.
diff --git a/NEWS.md b/NEWS.md
@@ -1,5 +1,7 @@
 # shiny (development version)
 
+* In v1.8.1, shiny.js starting throwing an error when input/output bindings have duplicate IDs. This error is now only thrown when `shiny::devmode(TRUE)` is enabled, so the issue is still made discoverable through the JS error console, but avoids unnecessarily breaking apps that happen to work with duplicate IDs. (#4019)
+
 # shiny 1.8.1
 
 ## New features and improvements

diff --git a/inst/www/shared/shiny.js b/inst/www/shared/shiny.js
@@ -18604,12 +18604,6 @@
       };
     }
     function addBinding(id, bindingType) {
-      if (id === "") {
-        throw new ShinyClientError({
-          headline: "Empty ".concat(bindingType, " ID found"),
-          message: "Binding IDs must not be empty."
-        });
-      }
       var existingBinding = bindings.get(id);
       if (existingBinding) {
         existingBinding.push(bindingType);
@@ -18848,13 +18842,19 @@
               currentInputs = bindInputs(shinyCtx, scope);
               bindingValidity = bindingsRegistry.checkValidity();
               if (!(bindingValidity.status === "error")) {
-                _context2.next = 6;
+                _context2.next = 10;
+                break;
+              }
+              if (!Shiny.inDevMode()) {
+                _context2.next = 9;
                 break;
               }
               throw bindingValidity.error;
-            case 6:
+            case 9:
+              console.warn("[shiny] " + bindingValidity.error.message);
+            case 10:
               return _context2.abrupt("return", currentInputs);
-            case 7:
+            case 11:
             case "end":
               return _context2.stop();
           }

diff --git a/inst/www/shared/shiny.js.map b/inst/www/shared/shiny.js.map
diff --git a/inst/www/shared/shiny.min.js b/inst/www/shared/shiny.min.js
diff --git a/inst/www/shared/shiny.min.js.map b/inst/www/shared/shiny.min.js.map
diff --git a/srcts/src/shiny/bind.ts b/srcts/src/shiny/bind.ts
@@ -136,13 +136,6 @@ const bindingsRegistry = (() => {
    * @param bindingType Binding type, either "input" or "output"
    */
   function addBinding(id: string, bindingType: BindingTypes): void {
-    if (id === "") {
-      throw new ShinyClientError({
-        headline: `Empty ${bindingType} ID found`,
-        message: "Binding IDs must not be empty.",
-      });
-    }
-
     const existingBinding = bindings.get(id);
 
     if (existingBinding) {
@@ -427,7 +420,12 @@ async function _bindAll(
   // re-run, and then see the next collision, etc.
   const bindingValidity = bindingsRegistry.checkValidity();
   if (bindingValidity.status === "error") {
-    throw bindingValidity.error;
+    // Only throw if we're in dev mode. Otherwise, just log a warning.
+    if (Shiny.inDevMode()) {
+      throw bindingValidity.error;
+    } else {
+      console.warn("[shiny] " + bindingValidity.error.message);
+    }
   }
 
   return currentInputs;