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

Work around server reloading issues #297

Merged
merged 2 commits into from Jul 11, 2022
Merged

Work around server reloading issues #297

merged 2 commits into from Jul 11, 2022

Conversation

kamilzyla
Copy link
Collaborator

@kamilzyla kamilzyla commented Jun 15, 2022

Changes

Closes #157. This was a really tricky bug and I still don't fully understand why it happens. My comments in the issue detail the investigation; this one in particular provides a minimal example which explains the changes in this PR.

In short, it seems that two things must be true about the server passed to shinyApp() for reloading to work properly:

  1. It must be defined using curly braces (function(input, output) { module_server("app") } and not function(input, output) module_server("app")).
  2. It must be evaluated (defined, created) in a file sourced with keep.source = TRUE.

How to test

Initialize a fresh project and run it with shiny::runApp(). Add some changes in the server of main.R, e.g. change the "Hello!" string. It should be sufficient to touch app.R (update its timestamp) and refresh the page in the browser to see the changes.

@kamilzyla
Copy link
Collaborator Author

The issue persists, I will investigate further.

@kamilzyla kamilzyla force-pushed the fix-reloading branch 2 times, most recently from ca53fc3 to 3769783 Compare July 7, 2022 14:19
@kamilzyla kamilzyla changed the title Use curly braces to avoid server reloading issues Work around server reloading issues Jul 7, 2022
@kamilzyla kamilzyla marked this pull request as ready for review July 7, 2022 15:00
Copy link
Member

@jakubnowicki jakubnowicki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great that you found a way to make it work! Still, since it is a hacky solution, we need to make sure that we keep checking if a more elegant solution is available (e.g. after a new version of shiny lands).

inst/as_top_level.R Show resolved Hide resolved
@kamilzyla kamilzyla merged commit 6348f5e into main Jul 11, 2022
@kamilzyla kamilzyla deleted the fix-reloading branch July 11, 2022 10: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.

Investigate reloading issues
2 participants