Given the disruption caused by #467 and the innumerable ways that people may have unknowingly been relying on htmlwidget's previously eager recursion into any list-like object, this PR restores previous behavior.
Both #466 and https://github.com/rstudio/DT/issues/1092 provide examples why shouldEval() shouldn't recurse into arbitrary objects -- while uncommon, it's possible to run into cases where an object is infinitely recursive.
In https://github.com/rstudio/leaflet/issues/891, issues arise from using something akin to structure(list(), class = "custom_options")). This is a common way to create a list-like options argument. The problem is that structure() doesn't include the "list" class when the class argument is provided, and therefore the code introduced in #467 would not recurse into these objects. We could do one level of recursion for list-like, but not "list" inherting, objects, but...
https://github.com/react-R/reactR/issues/82 shows that a custom, non-list classed object isn't a reliable signal of how far a package author expects htmlwidgets to recurse. In reactR's case, AFAICT, there's an expectation that JS() could be used on htmltools tag() attributes, which could be at any part of the HTML tree, and where the outer shiny.tag doesn't include a "list" class.
This PR will at least fix #477 and likely many other places we have yet to discover.
While it seems reasonable to carve out an exception for POSIXlt as in #473, both that PR and #466 indicate that there are likely many object classes that could require an exception. It's not clear to me that htmlwidgets should go down the path of carving out specific exceptions, especially considering that shouldEval() is called on an object that is in the process of moving from R to JSON, where these objects need to be serialized as common JSON types anyway.
Given the disruption caused by #467 and the innumerable ways that people may have unknowingly been relying on htmlwidget's previously eager recursion into any list-like object, this PR restores previous behavior.
Both #466 and https://github.com/rstudio/DT/issues/1092 provide examples why
shouldEval()shouldn't recurse into arbitrary objects -- while uncommon, it's possible to run into cases where an object is infinitely recursive.On the other hand, https://github.com/rstudio/leaflet/issues/891 and https://github.com/react-R/reactR/issues/82 show two cases where eager recursion was a feature, not a bug:
structure(list(), class = "custom_options")). This is a common way to create a list-like options argument. The problem is thatstructure()doesn't include the"list"class when theclassargument is provided, and therefore the code introduced in #467 would not recurse into these objects. We could do one level of recursion for list-like, but not"list"inherting, objects, but...JS()could be used on htmltoolstag()attributes, which could be at any part of the HTML tree, and where the outershiny.tagdoesn't include a"list"class.This PR will at least fix #477 and likely many other places we have yet to discover.
While it seems reasonable to carve out an exception for
POSIXltas in #473, both that PR and #466 indicate that there are likely many object classes that could require an exception. It's not clear to me that htmlwidgets should go down the path of carving out specific exceptions, especially considering thatshouldEval()is called on an object that is in the process of moving from R to JSON, where these objects need to be serialized as common JSON types anyway.