Don't fetch widget dependencies if no yaml file exists #277
Conversation
|
LGTM. @ramnathv ? |
|
Looks like we lose bindingDep too in this case--did you intend that? |
Yes, if we were to include I guess this would introduce breaking changes for widgets that don't have no yaml and no external dependencies (just the binding), but that seems like a fairly small set of packages, and a fix would be easy (declare your binding in the yaml). |
|
Apologies for a delayed response on this. @cpsievert on the issue of including the binding dependency, can you throw some light on some use cases. Are there any reasons for wanting to include the binding dependency at a position other than right at the end or at the top? |
|
I've added a better comment about the early return in 1c1ace4 I need this for a few different things in plotly. For instance, our TypedArray polyfill and MathJaX extras (if included) need to appear before the plotly.js bundle/binding. Furthermore, I'd like to determine whether these (and other) dependencies are really needed at print time, rather than always including them. |
|
Thanks @cpsievert. This makes sense. Can we also add some documentation around this? You can use the motivating use case for this feature to explain how to use it. |
|
Sure, feel free to suggest a place to document. Before we move forward though, I'm not quite sure this is working in a shiny context. Here is a MRE devtools::install_github('ropensci/plotly')
library(shiny)
library(plotly)
ui <- fluidPage(
plotlyOutput("p")
)
server <- function(input, output, ...) {
output$p <- renderPlotly({
qplot(1:10)
})
}
shinyApp(ui, server)Nothing appears, and if you look in |
|
FWIW, it seems as though |
|
Huh, adding the yaml back (as done in plotly/plotly.R@45933a5) causes |
|
Having a closer look at Perhaps it'd be possible to dynamically register htmlwidget bindings in shiny, but that will likely require significant revisions in |
|
Why can't the bindingDep be the first thing that's loaded in your case? I.e. don't lose the bindingDep in the no-yaml case. When the binding script executes it should generally just be registering closures, not actually using d3 or whatever...? |
This could cause an issue if you have two plotly plots on the same Shiny page that have different dynamic requirements. How bad is it if those extras are loaded after the plotly bundle? |
Sorry, I should've mentioned that it can (at least it seems to work fine for me at the moment). I'll close the issue (at least for now) since I'm not sure if these proposed changes are a good idea anymore.
Bad, especially for MathJaX, but perhaps I could export some helpers to insert dependencies in the UI (e.g., |
As @ramnathv points out in #259 (comment), this makes
getDependency()only return the htmlwidgets.js dependency if no yaml config file exists.This should fix #179 and also makes #259 obsolete