Don't assume all scripts are given as character variables.

[dmurdoch]

Also fixes the error message to only report missing files. Fixes #320.

[cpsievert]

Thanks @dmurdoch! If you don't mind, it'd also be great to have a unit test

[dmurdoch]

Sure, I'll see if I can add one.

[dmurdoch]

The last commit adds a test. It just tries to see if the function runs without error; I think other tests check that the results are okay.

[cpsievert]

Does this correctly handle the case of htmlDependency("foo", "1.0", src="", script=list(list(src="file.js")))? I think the tests should verify that the files were actually copied in order to verify this

[dmurdoch]

Yes, that's worth testing. I'll add that.

[cpsievert]

I think it'd be clearer what's going on here if we were to have a helper function "pluck" the files from each slot first, then put them together. Something like

pluck_src <- function(x) {
  if (is.character(x)) return(x)
  if (is.list(x) && "src" %in% names(x)) return(x$src) 
  if (is.list(x)) return(pluck_src(x))
  NULL
}

files <- c(
  pluck_src(dep$script),
  pluck_src(dep$stylesheet),
  pluck_src(dep$attachment),
)

[dmurdoch]

The function makes sense. I'll add that.

[dmurdoch]

Just committed changes as per suggestions.

[dmurdoch]

Just a note in case there are more comments: I'm going offline soon, and won't be available for a week or two. I'll address other comments when I get back. If they're urgently needed before that, someone else will have to take over.

[wch]

I'm finding it a bit challenging to understand what exactly this function is trying to do. I think it's in part because the shape of the data going in is pretty loose -- that's something that has long been a cause of issues when dealing with html dependency objects.

In this particular case, I'm wondering about the NULL return and the vapply(). If this vapply() is called on the object and one of the items returns NULL, that will result in an error. Will it ever get to the NULL at the end of the function? If not, it would make sense to have it throw an error with a helpful message.

[wch]

I just remembered that we actually have a spec for the HTML dependency data structure, in Shiny's TypeScript code. We can use this to reason about how this code should work.
https://github.com/rstudio/shiny/blob/474f1400/srcts/src/shiny/render.ts#L79-L128

[wch]

I made some changes to the pluck_src function and merged into main manually.

[wch]

This would much simpler and easier to implement if the htmlDependency function normalized the shape of the data structures on the way in. We do it here in TypeScript: https://github.com/rstudio/shiny/blob/474f1400/srcts/src/shiny/render.ts#L355-L467. Maybe someday. Unfortunately, there might code out there that will break because it assumes the older format (just a character vector, not a list like script = list(src="abc")))