ramnathv / htmlwidgets

HTML Widgets for R
http://htmlwidgets.org
Other
786 stars 207 forks source link

Problem with htmlwidgets, manipulateWidget, and crosstalk #300

Open dmurdoch opened 6 years ago

dmurdoch commented 6 years ago

In some cases (not sure exactly what conditions, but I have a reproducible example below), the jquery.js dependency gets loaded before htmlwidgets.js. When this happens, all of the document.ready handlers are run before the htmlwidgets.js initialization happens.

The problem is that the combineWidgets() function in manipulateWidget creates the objects from crosstalk during htmlwidgets initialization, after the crosstalk initialization is already complete. So the linking that crosstalk is supposed to do never happens.

A workaround is to install a custom dependency_resolver function that re-orders the dependencies in the whole document to put htmlwidgets ahead of jquery. There's probably a better solution...

This document illustrates the problem:

---
output:
  html_document
---

```{r}
knitr::opts_chunk$set(echo = TRUE, fig.height = 3.5)

library(rgl)
library(crosstalk)
library(manipulateWidget)

sd <- SharedData$new(mtcars)
ids <- plot3d(sd$origData(), col = mtcars$cyl, type = "s")
rglsd <- rglShared(ids["data"], key = sd$key(), group = sd$groupName())

f <- filter_checkbox("cylinderselector", 
                        "Cylinders", sd, ~ cyl,inline = TRUE)
combineWidgets(
  rglwidget(shared = rglsd),
  f)

(Sorry, github ate the final ```)

A workaround is to install this function as a dependency_resolver parameter to the html_document output format:

crosstalkPatch <- function(deps) {
  deps <- htmltools::resolveDependencies(deps)
  widgets <- sapply(deps, function(d) d$name == "htmlwidgets")
  jquery <- sapply(deps, function(d) d$name == "jquery")
  if (any(widgets) && any(jquery) && min(which(jquery)) < min(which(widgets))) {
    before <- seq_along(deps) < min(which(jquery))
    deps <- c(deps[before], deps[widgets], deps[!widgets && !before])    
  }
  deps
}

This assumes that the htmlwidgets package itself has no dependencies that need to be loaded earlier. I've temporarily put this function in the development version of rgl, so to use it, set the yaml header to

---
output:
  html_document:
    dependency_resolver: !expr rgl::crosstalkPatch
---

Any other suggestions? (I've only seen the problem when using crosstalk with rgl, so it's possible a fix to rgl could solve everything, but it looks like a more general problem.)

jcheng5 commented 6 years ago

It's interesting (and surprising) that reordering the dependencies fixes the problem, but that feels pretty fragile--the underlying problem, I now realize, is that crosstalk inputs don't ever expect to be dynamically injected into a page other than with Shiny. I suggested to @dmurdoch over email that calling HTMLWidgets.staticRender() from combineWidgets's HTML might fix it, but I forgot that crosstalk inputs aren't HTML widgets at all--they're an ad-hoc static version of Shiny inputs whereas HTML widgets are a static version of Shiny outputs.

I would still like to explore the possibility of refactoring combineWidgets to emit its children's HTML when its own HTML is emitted, rather than doing it in JS, as it would let us rely on the usual mental model for loading HTML widgets and crosstalk widgets, and let crosstalk off the hook for now. Though at the very least it seems crosstalk should export bind() for when it's really necessary to do this after page load.

dmurdoch commented 6 years ago

I added several alert()s into the crosstalk and htmlwidgets setup code to determine the order of startup calculations. Everything crosstalk related happens prior to anything htmlwidgets related in the original code. After reordering the dependency list, the installation of document.ready handlers happens before htmlwidgets starts handling DOMContentLoaded events, but the document.ready handlers are fired afterwards, so they work fine.

A possible fix for this is for htmlwidgets to make use of jQuery to handle things. It already has a couple of different ways of triggering "on loaded" events. I don't know how hard it would be to allow a user to ask it to use jQuery instead. I think it's good that htmlwidgets doesn't by default have any Javascript dependencies, but it may be useful to be able to ask for one, if it would be included anyways (because of crosstalk in my example, but maybe for other reasons).

I don't know for sure that this would work, but the documentation for jQuery suggests that it is designed to handle a case where an object created in one document.ready handler can install a later one. Possibly changes to crosstalk would also be necessary to support this.

I'd try putting together a PR, but my Javascript skills are pretty limited.

dmurdoch commented 6 years ago

I submitted a PR to export bind() from crosstalk (which is sufficient so that rgl can work around the issue), but it failed the Travis tests. I don't understand why it failed (because I don't understand the log messages).

dmurdoch commented 6 years ago

I've been looking into changes to combineWidgets to let it work with crosstalk, and it doesn't look promising to refactor it. It might be possible if the htmlwidgets custom widget HTML function *_html was passed a full copy of the widget data, but without that, as far as I can see htmlwidgets doesn't provide a way to create objects that need initialization before the renderValue call.

I'll look into ways for combineWidgets to detect crosstalk and call bind() if necessary.