Suggestion: Explicitly discourage use of includeScript() and includeCSS()

[daattali]

These functions come from {htmltools} but they are commonly used in shiny so I think posting in this repo is appropriate.

The problem I see with these functions is that they don't use the conventional system of including static resources, which can lead to confusion to anyone who isn't intimately familiar with the differences of includeCSS to link(rel = "stylesheet"). Using include functions simply looks for the given file in the file system, completely ignoring any resource paths that may have been set. I've seen people be very confused about why these functions need to be given a www/ prefix while images and any scripts or CSS loaded with the script and link tags don't need it. Similarly, it also means that it cannot load files under a resourcePath that's been added. And another small issue is that these functions result in inline css/js instead of linking to a file, but that's a minor issue.

[cpsievert]

To this point, it's probably worth mentioning bslib::bs_add_rules() as a better/modern alternative to includeCSS() as well as inline tags$link()/tags$style()

[jcheng5]

I see in the docs that the includeXXX functions' path parameter is documented as "The path of the file to be included"--this should probably be tweaked to make clear that it's "path on disk" or "filesystem path", not "URL path".

If people are calling includeScript/includeCSS on files within www/, but not including www/, we could also emit a helpful error message when includeScript/includeCSS is given a relative file path that doesn't exist, and prepending www/ points to a file that does exist.

BTW, I didn't expect the average Shiny app author to use, or even know about, addResourcePath; that was intended for package authors writing Shiny extension packages (like shinyjs)--and the same goes for addResourcePath's replacement, htmlDependency. The people you've seen who are confused about this, are they app authors or package authors? And if they're app authors, do you know why they're calling addResourcePath instead of just using www/--is it because they've using a package to deliver their app (à la {golem})?

(And as usual, thanks for the feedback!)

[daattali]

I think includeCSS/includeScript do technically work with URLs as well. You can pass in https://stackpath.bootstrapcdn.com/bootstrap/4.4.1/css/bootstrap.min.css and it would work. But any @import statements probaly won't. If a local path is given, I think the idea of showing a warning message if the path doesn't exist is good. But now that I think about it, maybe you're right and your original idea that most shiny developers shouldn't be exposed to understanding resource paths and they would just use www/ is more practical. So maybe you shouldn't discourage those functions.

I've been seeing it popping up more and more recently, not only in packages, but it might be because they look at how proper shiny apps are built and see it used inside other packages. I've been seeing people use both addResourcePath() and htmlDependency() and includeCSS() all in the same file, suggesting they just don't understand what the difference between them is. That's a larger problem though that wouldn't be solved with a single sentence in the docs, maybe an article in the shiny dev center could address it.

BTW it's good to know that htmlDependency is meant to be the replacement. I still use addResourcePath as I find it simpler to use and understand, and I was never sure if I should be switching to htmlDependency.

[jcheng5]

htmlDependency is nice because you can have multiple packages that all want to (automatically) introduce dependencies, for example font-awesome or d3, but possibly at different versions of those dependencies. With addResourcePath and link tags, htmltools doesn’t have enough information to know there is a collision, much less what should be done about it. With htmlDependency, htmltools can use the dependency name and version to ensure that one and only one copy of the dependency is loaded.

(Again, mostly useful for package developers, who can’t know what dependencies are going to be brought in by other packages used by the app author.)

[daattali]

(This is probably a discussion better suited elsewhere, but I wanted to add my two cents)

I do see the benefit of htmlDependency() in many scenarios and I use it when dealing with reusable dependencies (like the ones you mention). But with dependencies that are very specific to the current application, I've been continuing to use addResourcePath() because I do find that it results in more clear code, and I personally find it easier troubleshoot and especially to extend UI when everything is explicitly in HTML nodes rather than in the meta data (eg in R attributes)

[jcheng5]

I don’t usually bother with htmlDependency for custom CSS and JS that’s specific for an app. But I also don’t usually use addResourcePath, rather just use the www dir (or alternatively, includeXXX). You should feel free to use whatever of these approaches you feel like, for custom CSS/JS the trade offs are not significant.

[daattali]

Coincidentally, I just ran into a bug that has been present in my own code for years and went by undetected until it started causing errors recently. It was caused precisely for this reason, because I did not understand the includeScript/Css functions at the time :) daattali/addinslist#117