Disabling autoload required with shiny 1.5.0

[riccardoporreca]

With the recent release of shiny version 1.5.0, autoload (rstudio/shiny#2659, rstudio/shiny#2547) is enabled by default (?shiny::loadSupport).

• For a packaged golem application this is not the desired behavior, especially when deployed via a top-level app.R (e.g. golem::add_rstudioconnect_file() and friends).
• In particular, autoload causes scripts under R/ to be sourced, which is unnecessary and potentially breaking if the code assumes objects imported from other packages through the NAMESPACE.

See miraisolutions/Covid19@984ecc5 miraisolutions/Covid19#116 for a concrete such example.

The issue can be reproduced locally (with shiny 1.5.0) with a minimal example as follows

golem::create_golem("testAutoload")`

# as a dummy example, use the NAMESPACE-imported favicon to define an object
write("fav_icon <- favicon()", "R/break_autoload.R")
golem::add_rstudioconnect_file()
shiny::runApp()
# => could not find function "favicon"

Autoloading should be disabled by including the special file R/_disable_autoload.R when using golem::add_rstudioconnect_file() and friends (if not even done upon golem::create_golem()), perhaps with some informative content like in miraisolutions/Covid19@984ecc5.

[ColinFay]

You're completely right that this should be done when the user has shiny > 1.5.0 installed (just as the new version of the modules, #455).

Would you draft a PR for this options disabling?

Thanks

[riccardoporreca]

@ColinFay, sure happy to draft a PR!

[ColinFay]

Ok, so I think it's safe to assume that an R session / project will contain one golem app and there is very small chance that one would need to either use this option or not—as soon as you are in a golem app, this options should be turned off.

So I suppose changing this option in with_golem_options() for example is a safe call.

[riccardoporreca]

@ColinFay, sorry for the long absence (hectic times over the last months)... I can finally get back to this pretty soon

[ColinFay]

I still don't have a perfect answer for this one. The only sane way I was able to think of was creating a .Rprofile with each golem project, containing options(shiny.autoload.r=FALSE): it seems a little bit cumbersome to add a _disable_autoload.R file to every project, and might be more confusing than helpful?

Basically the idea would be to do

write("options(shiny.autoload.r=FALSE)", ".Rprofile", append = TRUE)

With any project.

Any opinion on that?

[riccardoporreca]

I like the .Rprofile as opposed to _disable_autoload.R, as the latter "pollutes" the R/ package directory. In fact, I am also realizing that R CMD build complains about such file

* excluding invalid files
Subdirectory 'R' contains invalid file names:
  ‘_d_isable_autoload.R’

Le me try to collect my current understanding of the issue in view of possible alternatives.

• The issues is solely related to using the app.R file created with golem::add_rstudioconnect_file() and used by shiny::runApp(), which seems to do the autoload as first thing before considering the content of app.R. For this reason, setting options(shiny.autoload.r=FALSE) alongside the golem_options does not seem viable.
• To the best of my understanding, this only affects deployments to shinyapps.io or RS Connect, whereas autoload should not affect any other cases based on explicitly running pkg::run_app(), e.g. in a Docker container.

In other words, we indeed have these two alternatives:

1. Using _disable_autoload.R. This is guaranteed to work in all cases, but it is confusing / not appropriate for a proper R package like a golem app.
2. Setting the option in the .Rprofile looks a cleaner solution, provided the .Rprofile is considered by RSConnect or shinyapps.io at the R session startup before the app is launched: I could quickly confirm this is the case for shinyapp.io using the minimal example at the top; perhaps worth checking with connect.

We seem to have a good preference for (2). Since the issue is limited to the RSConnect or shinyapps.io, I guess this could be done by the corresponding golem::add_*_file().

One final consideration. As mentioned in the top comment, autoload does not make much sense for packages, in particular since the source files under R/ might depend on imported symbols from the NAMESPACE. It might be a sensible choice for shiny to default to FALSE instead of TRUE if a NAMESPACE file (probably alongside DESCRIPTION) is found, as a clear indication of a package. Do you think we could propose this to the shiny maintainers?

[ColinFay]

I think your diagnosis is correct @riccardoporreca

As I see it, that would probably make sense to have a .Rprofile in every {golem} based project regardless of whether or not it's deployed through shinyapps io or not 🤔

For your other question (about ignoring if there is a NAMESPACE), I'm all in favor of asking.

Wanna open an issue on the {shiny} repo?

[riccardoporreca]

@ColinFay, about creating the .Rprofile for every {golem} vs. only for shinyapps deployment: The first option might create the false impression that a packaged shiny app requires something in the .Rprofile for it to work, whereas we know it does not require anything else than pkg::run_app(). On the other end, it is also nice to have {golem} defining a setup that is guaranteed to work no matter what the way of deploying (or launching it) will eventually be.
Your call in the end :).

I will open an issue in {shiny} about the NAMESPACE logic, thanks for your input!

[ColinFay]

I think I'll go for a default .Rprofile, leaving us space for future uses.
Also, I suppose a portion of people do not display dot files on their RStudio, making it invisible for most :)

I'll take care of implementing that, thank you @riccardoporreca for your help!

[ColinFay]

Close via #574

Thanks again @riccardoporreca for all your help on that.

[hadley]

I'd strongly discourage using a project .Rprofile for this because it a project .Rprofile is loaded instead of a user .Rprofile, not as well as.

[ColinFay]

Hey @hadley,

Thanks for the feedback.

The way I went was to set the .Rprofile as :

# Sourcing user .Rprofile if it exists 
home_profile <- file.path(
  Sys.getenv("HOME"), 
  ".Rprofile"
)
if (file.exists(home_profile)){
  source(home_profile)
}
rm(home_profile)
# Setting shiny.autoload.r to FALSE 
options(shiny.autoload.r = FALSE)

Allowing the user .Rprofile to be sourced too :)

[hadley]

We have gone down this path in the past and have never managed to get 100% correct behaviour in all cases. I’d not recommend it.

[ColinFay]

Yeah I don't fully enjoy it either, but the R/_disable_autoload.R file seems a bit messy to have in the package structure 🤔

Any other idea to make this work smoothly?

I suppose we could have this file added only when creating the deployment for RStudio products (this issue seems to arise only in that case : as long as we keep the package load and launch system, it will work.

[ColinFay]

Ok after giving it some thoughts and an internal discussion with @VincentGuyader, I've switched to adding a function called disable_autoload() that will create the file.

This function is called whenever an RStudio deployment file is created.