Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Shiny Autoreload Work #528

Closed
wants to merge 4 commits into from
Closed

Conversation

TymekDev
Copy link
Contributor

@TymekDev TymekDev commented Jan 9, 2024

⚠️ Note: these changes include using :::. I guess it needs a workaround for CRAN to accept it.

Changes

Implements automatic reloading from #339.

With shiny.autoreload working I don't think we need to modify shiny.autoreload.pattern. Running rhino::build_js(watch = TRUE) and rhino::build_sass(watch = TRUE) outputs minified .js and .css files respectively. Shiny by default tracks .js and .css changes, so it will reload when either JS or CSS is built.

How to test

  1. Install my forked version: pak::pak("TymekDev/rhino@autoreload")
  2. (Optional) Initialize a new application: rhino::init()
  3. Enable autoreload: options(shiny.autoreload = TRUE)
  4. Run the app: shiny::runApp() (rhino::app won't work!)

This will work in new and old apps alike. The only thing that changed is the main module is not passed directly. Instead, an environment where it was loaded with box::use is passed. This allows box::reload to work.

This change adds an autoreload callback registration if shiny.autoreload
option is enabled. The callback uses box::reload as outlined in [1].

Currently only the default mode (i.e. without legacy_entrypoint set)
supports autoreloading with {box}.

[1]: klmr/box#351
@codecov-commenter
Copy link

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (6926a37) 17.82% compared to head (e5d1a17) 17.56%.

❗ Current head e5d1a17 differs from pull request most recent head 735b984. Consider uploading reports for the commit 735b984 to get more accurate results

Files Patch % Lines
R/app.R 0.00% 10 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #528      +/-   ##
==========================================
- Coverage   17.82%   17.56%   -0.27%     
==========================================
  Files           9        9              
  Lines         404      410       +6     
==========================================
  Hits           72       72              
- Misses        332      338       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@TymekDev
Copy link
Contributor Author

I have been using my forked version ever since opening this PR. I couldn't help but notice it is spamming warnings at times about modules not being found for removal:

Warning in rm(list = info$source_path, envir = loaded_mods) :
  object '/Users/tymek/work/<<snip>>.R' not found
Warning in rm(list = info$source_path, envir = loaded_mods) :
  object '/Users/tymek/work/<<snip>>.R' not found
Warning in rm(list = info$source_path, envir = loaded_mods) :
  object '/Users/tymek/work/<<snip>>.R' not found

It does not seem to affect anything. Leaving it here for transparency.

@kamilzyla
Copy link
Collaborator

Superseded by #559.

@kamilzyla kamilzyla closed this Feb 20, 2024
@TymekDev TymekDev deleted the autoreload branch February 27, 2024 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants