From 56bf3e58376931dde131a6e44820f5dbc58e90f4 Mon Sep 17 00:00:00 2001 From: Carson Date: Mon, 7 Oct 2019 09:54:03 -0500 Subject: [PATCH 1/4] Use to schedule staticRender() iff jQuery 3 or higher is present in shinyMode --- inst/www/htmlwidgets.js | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/inst/www/htmlwidgets.js b/inst/www/htmlwidgets.js index 64c7e8d7..789394a7 100644 --- a/inst/www/htmlwidgets.js +++ b/inst/www/htmlwidgets.js @@ -659,8 +659,17 @@ invokePostRenderHandlers(); } + function has_jQuery3() { + if (!window.jQuery) { + return false; + } + var $version = window.jQuery.fn.jquery; + var $major_version = parseInt($version.split(".")[0]); + return $major_version >= 3; + } + // Wait until after the document has loaded to render the widgets. - if (shinyMode && window.jQuery) { + if (shinyMode && window.jQuery && has_jQuery3()) { /* / Shiny 1.4.0 bumps jQuery from 1.x to 3.x, which means jQuery's / on-ready handler (i.e., $(fn)) is now asyncronous (i.e., it now From 6d0c24b68d7e98f762dfdb0bffa3dc6e93c64c61 Mon Sep 17 00:00:00 2001 From: Carson Date: Mon, 7 Oct 2019 12:04:49 -0500 Subject: [PATCH 2/4] Bump version; update NEWS --- DESCRIPTION | 2 +- inst/NEWS | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index d40a2387..cd1383d3 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,7 +1,7 @@ Package: htmlwidgets Type: Package Title: HTML Widgets for R -Version: 1.5.0.9000 +Version: 1.5.1 Authors@R: c( person("Ramnath", "Vaidyanathan", role = c("aut", "cph")), person("Yihui", "Xie", role = c("aut")), diff --git a/inst/NEWS b/inst/NEWS index 07374296..5d8b2f7d 100644 --- a/inst/NEWS +++ b/inst/NEWS @@ -1,3 +1,8 @@ +htmlwidgets 1.5.1 +------------------------------------------------------- + +* Fixed an issue with dynamically rendered widgets (i.e., using `shiny::uiOutput()` to render a widget) with any version of shiny prior to 1.4. This issue was introduced by htmlwidgets 1.5. (#351) + htmlwidgets 1.5 ----------------------------------------------------------------------- From ebda03ae23936b5be8f5d1576b5521e839265d1d Mon Sep 17 00:00:00 2001 From: Joe Cheng Date: Mon, 7 Oct 2019 11:52:52 -0700 Subject: [PATCH 3/4] Prevent staticRender from being called unconditionally when htmlwidgets.js is loaded after page load --- inst/www/htmlwidgets.js | 42 +++++++++++------------------------------ 1 file changed, 11 insertions(+), 31 deletions(-) diff --git a/inst/www/htmlwidgets.js b/inst/www/htmlwidgets.js index 789394a7..648d4fcd 100644 --- a/inst/www/htmlwidgets.js +++ b/inst/www/htmlwidgets.js @@ -668,44 +668,24 @@ return $major_version >= 3; } - // Wait until after the document has loaded to render the widgets. - if (shinyMode && window.jQuery && has_jQuery3()) { - /* - / Shiny 1.4.0 bumps 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 - / - / In order to ensure the order of execution of on-ready handlers - / remains consistent with how it's been in the past, a static render - / in Shiny is now scheduled via $(). - / - / This is not a long term solution: it just preserves the current order - / of execution. Part of that ordering is to ensure initShiny executes - / _after_ staticRender, which is both right and wrong: - / * It's wrong because, when initShiny executes, it registers methods - / like Shiny.onInputChange that widget authors would expect to be available - / during a staticRender. - / * It's also 'right' because initShiny currently (as of v1.4.0) wants - / to execute after user code so that it can bind to any dynamically - / created elements. - / - / A longer term solution might be to make changes to Shiny so that - / these methods are available before the binding takes place, and then - / the ordering would be: register methods -> static render -> bind. - */ - window.jQuery(function() { - window.HTMLWidgets.staticRender(); - }); - } else if (document.addEventListener) { + if (document.addEventListener) { document.addEventListener("DOMContentLoaded", function() { document.removeEventListener("DOMContentLoaded", arguments.callee, false); - window.HTMLWidgets.staticRender(); + if (shinyMode && window.jQuery && has_jQuery3()) { + window.jQuery(window.HTMLWidgets.staticRender); + } else { + window.HTMLWidgets.staticRender(); + } }, false); } else if (document.attachEvent) { document.attachEvent("onreadystatechange", function() { if (document.readyState === "complete") { document.detachEvent("onreadystatechange", arguments.callee); - window.HTMLWidgets.staticRender(); + if (shinyMode && window.jQuery && has_jQuery3()) { + window.jQuery(window.HTMLWidgets.staticRender); + } else { + window.HTMLWidgets.staticRender(); + } } }); } From cb7e3d1ad8c55560109edf91ccc179b4cb6a497e Mon Sep 17 00:00:00 2001 From: Carson Sievert Date: Mon, 7 Oct 2019 16:22:14 -0500 Subject: [PATCH 4/4] Add a comment; abstract out duplication in logic (#354) --- inst/www/htmlwidgets.js | 43 +++++++++++++++++++++++++++++++---------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/inst/www/htmlwidgets.js b/inst/www/htmlwidgets.js index 648d4fcd..6f3d672d 100644 --- 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(); } }); }