First draft of auto-reload functionality

[klmr]

Work in progress implementation of #234.

The first draft includes auto-reloading for subsequent box::use calls:

• box::enable_autoreload() enables auto-reloading for box::use calls only. This solution should work e.g. for Shiny applications. But it won’t work well inside scripts and R Markdown documents, since box::use is required to trigger a reload.
• box::disable_autoreload() disables auto-reloading.
• box::enable_autoreload accepts include and exclude arguments to limit which modules get auto-reloaded:
  • box::enable_autoreload(include = c(./a, my/mod, foo/bar))
  • or a single module, box::enable_autoreload(include = foo/bar)
  • same for exclude
• box::autoreload_include(...) and box::autoreload_exclude(...) add includes or excludes after auto-reloading has been enabled
• only “top-level” changes are tracked, not module dependencies; this means that, if ./a imports ./b and b.r changes, then box::use(./a) won’t reload either a.r or b.r, since only the (unchanged) timestamp of a.r is checked.

Well, that’s about it for now.

Still to be done:

• [x] implement tests
• [ ] implement reloading for module dependency changes
• [x] implement auto-reloading for qualified module access (mod$x)
• [x] implement auto-reloading for access of attached module objects
• [ ] finalise API design
• [ ] handle module namespace hooks (how?) — the current draft only runs .on_load

/cc @grst for feedback

[grst]

Nice! Looks really clean, although I feel I don't know enough low-level R to properly review this.

I think I can follow the code for tracking the file changes, I don't get where this actually hooks into the box::use call.

[klmr]

@grst The relevant changes are in loaded.r. is_mod_loaded and register_mod are invoked during the actual module loading, which is one of the steps invoked for each module loaded by box::use:

https://github.com/klmr/box/blob/ac3f6910c9472ca598b558ab28e84e4d3f8e82e3/R/use.r#L372-L392

[klmr]

@grst I’d be grateful if you could test-drive the pull request. The implementation is pretty crude, I’m sure there are situations which I’ve overlooked in my tests, and real-world usage is necessary to improve the implementation.

To install, open R and run

install.packages('https://github.com/klmr/box/files/7192969/box_1.1.9000.tar.gz', type = 'source', repos = NULL)

… or download the archive and run the command locally.

As far as I’m aware, the code is feature complete — except no unload hook is being run. I haven’t decided yet whether to do this. Manual reload does, but honestly if a module relies on that hook then dynamic reloading isn’t going to work properly anyway in all situations.

The API is also still subject to change: I’m not happy with the current names, they’re too long for my taste. I might shorten enable_autoload to just autoload, and I would like to change on_access to something more explanatory, or shorter, or ideally both.

[grst]

I'll give it a try this week!

[klmr]

@grst You may want to hold off on testing, Iʼve already found a bug in combination with Shiny, and I havenʼt got a fix yet.

[klmr]

@grst OK, fixed that bug. New archive:

https://github.com/klmr/box/files/7204838/box_1.1.9000.tar.gz

[grst]

I currently get the following error when starting my shiny app:

> shiny::runApp()
Loading required package: shiny
Error in box::use(shiny[...], shinydashboard[...], ./modules/overview,  : 
  object 'c_strict_extract' not found
(inside “`$.box$ns`(ns, ".__module__.")”)

EDIT: I installed the latest version of this branch using remotes::install_github("klmr/box@feature/auto-reload")

[klmr]

EDIT: I installed the latest version of this branch using remotes::install_github("klmr/box@feature/auto-reload")

Unfortunately that won’t work since I’m not checking generated code (NAMESPACE and man files) into development branches. However, the ‘remote’ (as well as ‘devtools’ and ‘pak’) packages require these files. I’m surprised the installation “worked” at all, but it seems the compiled files are missing. Please try installing the .tar.gz file instead, it contains the generated files.

[grst]

I see!

If I use the archive 7204838/box_1.1.9000.tar.gz from above, I can run the app, but autoreload doesnt work as expected.

For instance, in my app.R I have

box::use(
  shiny[...],
  shinydashboard[...],
  . / modules / overview,
  . / modules / gene_expression,
  . / modules / correlation,
  . / config[bar],
  ggpubr[mean_ci]
)
box::enable_autoreload()

ui <- [...]

server <- function(input, output) {
  callModule(overview$server, "overview")
  callModule(gene_expression$server, "gene_expression")
  callModule(correlation$server, "correlation")
}

shinyApp(ui = ui, server = server)

If I change something in server() of modules/gene_expression.r and click "Reload App", this doesn't change in the app. If I restart the rsession and restart the app, it does.

[grst]

Actually, by using box::enable_autoreload(on_access=TRUE) instead, it works :tada:

Is that expected? I thought with shiny, we wouldn't even need the on_access feature.

[klmr]

Ah yes, the current default mode of box::enable_autoreload() is only triggered when a box::use declaration is re-executed (that’s because this is vastly more efficient than capturing every single access).

This works if your box::use call is inside the server function, for instance, because reloading the Shiny application (via “Reload app” or by refreshing the browser window) re-executes the server function (even without restarting the session).

When it’s at file scope as in your case, you’ll need to pass on_access = TRUE to box::enable_autoreload(). This is also needed if you want to get the effect of auto-reload without reloading your Shiny application.

As mentioned, the only reason this isn’t the default currently is that it’s less efficient. However, the whole point of testing is to figure out what would be most convenient for the user: I’m more than happy to change the default behaviour if we find that that’s more appropriate. So please let me know what you think, and whether you have any ideas for a better syntax, or other configuration options you find worth having.

---

Incidentally, box::enable_autoreload() isn’t supposed to go into the code (that kind of defeats the purpose), but I understand that it’s convenient. I just execute it inside the console after starting the R session.

[DavidJesse21]

EDIT: Sorry about the previous reply, I didn't realize you already mentioned that the specific version I have downloaded has a bug and that you provided a more recent version. I have downloaded the more recent archive file and now everything works as I was expecting :)

[klmr]

How useful is it to be able to configure which modules are auto-reloaded (via box::autoreload_include and box::autorerload_exclude)? I am considering dropping this functionality, and/or removing the box::*_autoreload functions in favour of having configurable R options:

• If getOption('box.autoreload') is TRUE (inside interactive sessions only), modules are auto-reloaded. Otherwise they aren’t.
• Optionally, getOption('box.autoreload_mode') controls how auto-reloading works; the value could be either 'on_use' (corresponds to setting on_access = FALSE) or 'all' (corresponds to setting on_access = TRUE).

… as with the box.path option, these would also be settable via environment variables (R_BOX_AUTORELOAD and R_BOX_AUTORELOAD_MODE).

Using options rather than functions to configure auto-reloading is less powerful: there’s currently no provision for excluding modules from auto-reload, and no way to enable or disable auto-reload mode inside a running R session — the ‘box’ package would have to be unloaded and reloaded before changed options would take effect.

On the flip side, I think it would simplify documentation and usage.

[grst]

So far, I didn't run into any situation where I'd have had to exclude certain files from autoreloading. However, one situation where I believe this could be useful is if there is a short-running function that is called many times. In that case the overhead of checking if the file was modified could become significant.

But then again, I could simply not use the on_access feature in that case.

Lastly, I find the proposed naming of the options slightly confusing: At the first glance I thought on_use equals on_access=TRUE, because it reloads "on use" of the function (but ofc it refers to box::use instead). How about on_use and on_access to make the distinction clearer?

[DavZim]

slightly off-topic...

I love {box} and its functionality and the auto-reload/invalidate-cache functionality would help in my use-case a lot (I currently use box inside of shiny applications). This PR would allow me to auto-reload the modules I import when the app is run (eg I change something in the module, then I would need to either refresh the session to invalidate the cache so that the latest version is loaded or use the following workaround).

A current workaround is taken from rhino where the following two-liner invalidates the whole box-cache:

loaded_mods <- loadNamespace("box")$loaded_mods
rm(list = ls(loaded_mods), envir = loaded_mods)