Dev mode aware client and duplicate input/output ID handling updates (#…

…3956) * Add field with devmode status to the initial config object sent over on connection * Add indicator of "devmode" status to the client via an injected script tag on load. This is modeled after what is done for showcase mode. * Add logic to flag all duplicated IDs when in devmode * Only show error console in devmode. * Remove left-over devmode status in code * `yarn build` (GitHub Actions) * Build shiny.js * `devtools::document()` (GitHub Actions) * `yarn build` (GitHub Actions) --------- Co-authored-by: nstrayer <[email protected]> Co-authored-by: Winston Chang <[email protected]> Co-authored-by: wch <[email protected]>
rstudio · Jan 24, 2024 · f26b133 · f26b133
1 parent 370ba1f
commit f26b133
Show file tree

Hide file tree

Showing 11 changed files with 74 additions and 13 deletions.
diff --git a/DESCRIPTION b/DESCRIPTION
@@ -204,7 +204,7 @@ Collate:
     'version_selectize.R'
     'version_strftime.R'
     'viewer.R'
-RoxygenNote: 7.3.0
+RoxygenNote: 7.3.1
 Encoding: UTF-8
 Roxygen: list(markdown = TRUE)
 RdMacros: lifecycle

diff --git a/R/shinyui.R b/R/shinyui.R
@@ -69,6 +69,21 @@ renderPage <- function(ui, showcase=0, testMode=FALSE) {
       )
   }
 
+  if (in_devmode()) {
+    # If we're in dev mode, add a simple script to the head that injects a
+    # global variable for the client to use to detect dev mode.
+    shiny_deps[[length(shiny_deps) + 1]] <-
+      htmlDependency(
+        "shiny-devmode",
+        get_package_version("shiny"),
+        src = "www/shared",
+        package = "shiny",
+        head="<script>window.__SHINY_DEV_MODE__ = true;</script>",
+        all_files = FALSE
+      )
+  }
+
+
   html <- renderDocument(ui, shiny_deps, processDep = createWebDependency)
   enc2utf8(paste(collapse = "\n", html))
 }

diff --git a/inst/www/shared/shiny.js b/inst/www/shared/shiny.js
@@ -18574,9 +18574,10 @@
         idTypes.forEach(function(type) {
           return counts[type] += 1;
         });
-        if (Object.values(counts).some(function(count) {
-          return count > 1;
-        })) {
+        if (counts.input === 1 && counts.output === 1 && !Shiny.inDevMode()) {
+          return;
+        }
+        if (counts.input > 0 || counts.output > 0) {
           duplicateIds.set(id, counts);
         }
       });
@@ -22355,6 +22356,9 @@
   _defineProperty19(ShinyErrorMessage, "styles", [i(_templateObject5 || (_templateObject5 = _taggedTemplateLiteral(['\n      :host {\n        color: var(--red-11);\n        display: block;\n        font-size: var(--font-md);\n\n        position: relative;\n        --icon-size: var(--font-lg)\n\n        /* Reset box sizing */\n        box-sizing: border-box;\n      }\n\n      .container {\n        display: flex;\n        gap: var(--space-2);\n      }\n\n      .contents {\n        width: 40ch;\n        display: flex;\n        flex-direction: column;\n        gap: var(--space-1);\n        padding-block-start: 0;\n        padding-block-end: var(--space-3);\n        overflow: auto;\n      }\n\n      :host(:last-of-type) .contents {\n\n        padding-block-end: var(--space-1);\n      }\n\n      .contents > h3 {\n        font-size: 1em;\n        font-weight: 500;\n        color: var(--red-12);\n      }\n\n      .contents > * {\n        margin-block: 0;\n      }\n\n      .error-message {\n        font-family: "Courier New", Courier, monospace;\n      }\n\n      .decoration-container {\n        flex-shrink: 0;\n        position: relative;\n\n        --line-w: 2px;\n        --dot-size: 11px;\n      }\n\n      :host(:hover) .decoration-container {\n        --scale: 1.25;\n      }\n\n      .vertical-line {\n        margin-inline: auto;\n        width: var(--line-w);\n        height: 100%;\n\n        background-color: var(--red-10);\n      }\n\n      :host(:first-of-type) .vertical-line {\n        height: calc(100% - var(--dot-size));\n        margin-top: var(--dot-size);\n      }\n\n      .dot {\n        position: absolute;\n        width: var(--dot-size);\n        height: var(--dot-size);\n        top: calc(-1px +  var(--dot-size) / 2);\n        left: calc(50% - var(--dot-size) / 2);\n        border-radius: 100%;\n        transform: scale(var(--scale, 1));\n\n        color: var(--red-6);\n        background-color: var(--red-10);\n      }\n\n      .actions {\n        transform: scaleX(0);\n        transition: transform calc(var(--animation-speed) / 2) ease-in-out;\n        display: flex;\n        justify-content: center;\n        flex-direction: column;\n      }\n\n      /* Delay transition on mouseout so the buttons don\'t jump away if the user\n      overshoots them with their mouse */\n      :host(:not(:hover)) .actions {\n        transition-delay: 0.15s;\n      }\n\n      :host(:hover) .actions {\n        transform: scaleX(1);\n      }\n\n      ', "\n\n      .copy-button {\n        padding: 0;\n        width: var(--space-8);\n        height: var(--space-8);\n        position: relative;\n        --pad: var(--space-2);\n      }\n\n      .copy-button-inner {\n        position: relative;\n        width: 100%;\n        height: 100%;\n        border-radius: inherit;\n        transition: transform 0.5s;\n        transform-style: preserve-3d;\n      }\n\n      /* Animate flipping to the other side when the .copy-success class is\n      added to the host */\n      :host(.copy-success) .copy-button-inner {\n        transform: rotateY(180deg);\n      }\n\n      /* Position the front and back side */\n      .copy-button .front,\n      .copy-button .back {\n        --side: calc(100% - 2 * var(--pad));\n        position: absolute;\n        inset: var(--pad);\n        height: var(--side);\n        width: var(--side);\n        -webkit-backface-visibility: hidden; /* Safari */\n        backface-visibility: hidden;\n      }\n\n      .copy-button:hover .copy-button-inner {\n        background-color: var(--gray-2);\n      }\n\n      /* Style the back side */\n      .copy-button .back {\n        --pad: var(--space-1);\n        color: var(--green-8);\n        transform: rotateY(180deg);\n      }\n    "])), buttonStyles)]);
   customElements.define("shiny-error-message", ShinyErrorMessage);
   function showErrorInClientConsole(e4) {
+    if (!Shiny.inDevMode()) {
+      return;
+    }
     var errorMsg = null;
     var headline = "Error on client while running Shiny app";
     if (typeof e4 === "string") {
@@ -25127,6 +25131,11 @@
     windowShiny2.renderContent = renderContent;
     windowShiny2.renderHtmlAsync = renderHtmlAsync;
     windowShiny2.renderHtml = renderHtml2;
+    windowShiny2.inDevMode = function() {
+      if ("__SHINY_DEV_MODE__" in window)
+        return Boolean(window.__SHINY_DEV_MODE__);
+      return false;
+    };
     (0, import_jquery40.default)(function() {
       setTimeout(/* @__PURE__ */ _asyncToGenerator15(/* @__PURE__ */ _regeneratorRuntime15().mark(function _callee() {
         return _regeneratorRuntime15().wrap(function _callee$(_context) {

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/components/errorConsole.ts b/srcts/src/components/errorConsole.ts
@@ -490,12 +490,17 @@ customElements.define("shiny-error-message", ShinyErrorMessage);
 
 /**
  * Function to show an error message to user in shiny-error-message web
- * component
+ * component. Only shows the error if we're in development mode.
  * @param e - Error object to show to user. This is whatever is caught in
  * a try-catch statement so it may be a string or it may be a proper Error
  * object.
  */
 export function showErrorInClientConsole(e: unknown): void {
+  if (!Shiny.inDevMode()) {
+    // If we're in production, don't show the error to the user
+    return;
+  }
+
   let errorMsg: string | null = null;
   let headline = "Error on client while running Shiny app";
 

diff --git a/srcts/src/shiny/bind.ts b/srcts/src/shiny/bind.ts
@@ -89,7 +89,17 @@ const bindingsRegistry = (() => {
 
       idTypes.forEach((type) => (counts[type] += 1));
 
-      if (Object.values(counts).some((count) => count > 1)) {
+      // If there's a single duplication of ids across both binding types, then
+      // when we're not in devmode, we allow this to pass because a good amount of
+      // existing applications use this pattern even though its invalid. Eventually
+      // this behavior should be removed.
+      if (counts.input === 1 && counts.output === 1 && !Shiny.inDevMode()) {
+        return;
+      }
+
+      // If we have duplicated IDs, then add them to the set of duplicated IDs
+      // to be reported to the user.
+      if (counts.input > 0 || counts.output > 0) {
         duplicateIds.set(id, counts);
       }
     });

diff --git a/srcts/src/shiny/index.ts b/srcts/src/shiny/index.ts
@@ -68,6 +68,14 @@ interface Shiny {
   // Eventually deprecate
   // For old-style custom messages - should deprecate and migrate to new
   oncustommessage?: Handler;
+
+  /**
+   * Method to check if Shiny is running in development mode. By packaging as a
+   * method, we can we can avoid needing to look for the `__SHINY_DEV_MODE__`
+   * variable in the global scope.
+   * @returns `true` if Shiny is running in development mode, `false` otherwise.
+   */
+  inDevMode: () => boolean;
 }
 
 let windowShiny: Shiny;
@@ -108,6 +116,13 @@ function setShiny(windowShiny_: Shiny): void {
   windowShiny.renderHtmlAsync = renderHtmlAsync;
   windowShiny.renderHtml = renderHtml;
 
+  windowShiny.inDevMode = () => {
+    if ("__SHINY_DEV_MODE__" in window)
+      return Boolean(window.__SHINY_DEV_MODE__);
+
+    return false;
+  };
+
   $(function () {
     // Init Shiny a little later than document ready, so user code can
     // run first (i.e. to register bindings)

diff --git a/srcts/types/src/components/errorConsole.d.ts b/srcts/types/src/components/errorConsole.d.ts
@@ -12,7 +12,7 @@ export declare class ShinyErrorMessage extends LitElement {
 }
 /**
  * Function to show an error message to user in shiny-error-message web
- * component
+ * component. Only shows the error if we're in development mode.
  * @param e - Error object to show to user. This is whatever is caught in
  * a try-catch statement so it may be a string or it may be a proper Error
  * object.

diff --git a/srcts/types/src/shiny/index.d.ts b/srcts/types/src/shiny/index.d.ts
@@ -47,6 +47,13 @@ interface Shiny {
     unbindAll?: typeof shinyUnbindAll;
     initializeInputs?: typeof shinyInitializeInputs;
     oncustommessage?: Handler;
+    /**
+     * Method to check if Shiny is running in development mode. By packaging as a
+     * method, we can we can avoid needing to look for the `__SHINY_DEV_MODE__`
+     * variable in the global scope.
+     * @returns `true` if Shiny is running in development mode, `false` otherwise.
+     */
+    inDevMode: () => boolean;
 }
 declare let windowShiny: Shiny;
 declare function setShiny(windowShiny_: Shiny): void;