Add a comment; abstract out duplication in logic (#354)

ramnathv · Oct 7, 2019 · cb7e3d1 · cb7e3d1
1 parent ebda03a
commit cb7e3d1
Showing 1 changed file with 33 additions and 10 deletions.
diff --git a/inst/www/htmlwidgets.js b/inst/www/htmlwidgets.js
@@ -659,6 +659,7 @@
     invokePostRenderHandlers();
   }
 
+
   function has_jQuery3() {
     if (!window.jQuery) {
       return false;
@@ -668,24 +669,46 @@
     return $major_version >= 3;
   }
 
+  /*
+  / Shiny 1.4 bumped jQuery from 1.x to 3.x which means jQuery's
+  / on-ready handler (i.e., $(fn)) is now asyncronous (i.e., it now
+  / really means $(setTimeout(fn)).
+  / https://jquery.com/upgrade-guide/3.0/#breaking-change-document-ready-handlers-are-now-asynchronous
+  /
+  / Since Shiny uses $() to schedule initShiny, shiny>=1.4 calls initShiny
+  / one tick later than it did before, which means staticRender() is
+  / called renderValue() earlier than (advanced) widget authors might be expecting.
+  / https://github.com/rstudio/shiny/issues/2630
+  /
+  / For a concrete example, leaflet has some methods (e.g., updateBounds)
+  / which reference Shiny methods registered in initShiny (e.g., setInputValue).
+  / Since leaflet is privy to this life-cycle, it knows to use setTimeout() to
+  / delay execution of those methods (until Shiny methods are ready)
+  / https://github.com/rstudio/leaflet/blob/18ec981/javascript/src/index.js#L266-L268
+  /
+  / Ideally widget authors wouldn't need to use this setTimeout() hack that
+  / leaflet uses to call Shiny methods on a staticRender(). In the long run,
+  / the logic initShiny should be broken up so that method registration happens
+  / right away, but binding happens later.
+  */
+  function maybeStaticRenderLater() {
+    if (shinyMode && has_jQuery3()) {
+      window.jQuery(window.HTMLWidgets.staticRender);
+    } else {
+      window.HTMLWidgets.staticRender();
+    }
+  }
+
   if (document.addEventListener) {
     document.addEventListener("DOMContentLoaded", function() {
       document.removeEventListener("DOMContentLoaded", arguments.callee, false);
-      if (shinyMode && window.jQuery && has_jQuery3()) {
-        window.jQuery(window.HTMLWidgets.staticRender);
-      } else {
-        window.HTMLWidgets.staticRender();
-      }
+      maybeStaticRenderLater();
     }, false);
   } else if (document.attachEvent) {
     document.attachEvent("onreadystatechange", function() {
       if (document.readyState === "complete") {
         document.detachEvent("onreadystatechange", arguments.callee);
-        if (shinyMode && window.jQuery && has_jQuery3()) {
-          window.jQuery(window.HTMLWidgets.staticRender);
-        } else {
-          window.HTMLWidgets.staticRender();
-        }
+        maybeStaticRenderLater();
       }
     });
   }