ramnathv / htmlwidgets

HTML Widgets for R
http://htmlwidgets.org
Other
792 stars 205 forks source link

`shouldEval()` probably shouldn't recurse over list-like structures that aren't actually lists #466

Closed DavisVaughan closed 1 year ago

DavisVaughan commented 1 year ago

I received this bug report https://github.com/r-lib/clock/issues/335

df <- data.frame(cals = clock::year_quarter_day(year = 2000:2001, quarter = 2))

DT::datatable(df)
#> Error:
#> ! Names must be a character vector.

Ultimately I think this is an htmlwidgets issue.

I followed it all the way down to htmlwidgets:::shouldEval() which recurses over the input if is.list() is TRUE on it. shouldEval() is first given something like list(data = df), so it seems like it is reasonable to do 1 layer of recursion on the elements of that list, but then it does another layer of recursion because df is a data frame and is.list() returns TRUE for data frames. It actually then does another layer of recursion because df$cals is clock's year-quarter-day class built on vctrs::new_rcrd(), which is also technically built on a list structure, even though it isn't a list.

One solution for this that we typically use is to replace is.list(x) with inherits(x, "list"). This ensures that you only recurse over things that are either:

I feel like that may work here.