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

Fixes for hot reloading interval handling and refreshing apps within viewer pane #148

Merged
merged 5 commits into from
Dec 4, 2019

Conversation

rpkyle
Copy link
Contributor

@rpkyle rpkyle commented Nov 15, 2019

This PR proposes several modifications to address issues noted in #146 and #147:

  • a new private$in_viewer field tracks whether the app should be reloaded within RStudio's viewer pane
  • app_url now uses self$server$host and self$server$port
  • self$config$hot_reload incorrectly used hot_reload_watch_interval, this has been corrected to hot_reload_interval
  • permit_reload is always TRUE until one reload event has occurred; on subsequent cycles, reloads are permitted if the current (wall) time is at least hot_reload_watch_interval seconds greater than the time stored in private$last_reload

Resolves #146, fixes #147.

@rpkyle rpkyle added the bug label Nov 15, 2019
@rpkyle rpkyle self-assigned this Nov 15, 2019
R/dash.R Outdated
else {
# determine if the time since last reload end is equal to or longer than the requested check interval
# (Sys.time() - private$last_reload) provides time in seconds, same units as hot_reload_watch_interval
permit_reload <- (Sys.time() - private$last_reload) >= hot_reload_watch_interval
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will work. But a common way to do it with only one variable is to start with private$last_reload = 0 or some known distant past time (0 often works for that purpose because it typically means the unix epoch = 1/1/1970)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed in 0ce7a6d

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃 looks good, just one comment to consider if you want to tighten up the logic a bit.

@rpkyle rpkyle merged commit 68be6a9 into dev Dec 4, 2019
@alexcjohnson alexcjohnson deleted the 147-fix-reloading branch December 4, 2019 02:49
@rpkyle rpkyle mentioned this pull request Jan 3, 2020
rpkyle added a commit that referenced this pull request Jan 4, 2020
* Provide support for no_update in Dash for R (#111)

* Use dev_tools_prune_errors instead of pruned_errors (#113)

* Better handling for user-defined error conditions in debug mode (#116)

* Provide support for multiple outputs (#119)

* Provide support for hot reloading in Dash for R (#127)

* Implement support for clientside callbacks in Dash for R (#130)

* Add line number context to stack traces when srcrefs are available (#133)

* Update dash-renderer to 1.2.2 and fix dev tools UI display of stack traces (#137)

* Support for meta tags in Dash for R (#142)

* Fixes for hot reloading interval handling and refreshing apps within viewer pane (#148)

* Support for asynchronous loading/compression in Dash for R (#157)

* Support returning asset URLs via public method within Dash class (#160)

* Minor fix for get_asset_url + docs, add url_base_pathname (#161)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants