Provide support for hot reloading in Dash for R #127

rpkyle · 2019-09-18T16:38:06Z

This PR proposes to support frontend and backend hot reloading in Dash for R, in a manner analogous to that currently provided in Dash for Python (plotly/dash#66, plotly/dash#362, plotly/dash-renderer#73).

The primary modifications required include

Identifying the local path to the running Dash app, and storing this within the Dash object
Tracking file changes, both within the specified assets folder, but also within the app directory root itself
Generating a reload hash and a route to supply it on demand to the frontend
Formatting a JSON response to the frontend, including a modified file manifest
Handle partial reloading for CSS modifications only as in Dash for Python
Providing a custom Fiery logger which silences routing and request traffic on demand using the dev_tools_silence_routes_logging parameter, as in Dash for Python
Providing support for restarting the Fiery server when hard_reload is set (e.g. if non-CSS assets or the app itself is modified), and running source to reload the app code

✨ It also proposes a new (logical) viewer parameter, which can be used to load Dash apps within RStudio for a more pleasant development experience. This feature requires specifying 127.0.0.1 or localhost for the host parameter; this is a restriction imposed by RStudio's API.

https://www.rdocumentation.org/packages/rstudioapi/versions/0.10/topics/viewer

Additional work is required:

To format and return a JSON response on file modification, and to ensure that reloading for modifications to CSS only behaves as in Dash for Python (soft reload, style should change without losing state)
I'll also need to consider implementation of dev_tools_hot_reload_watch_interval; this is used to control how frequently walk_assets_directory is invoked.
🚨 We'll need a minimal test suite to verify that this feature behaves as desired.
Add in relevant R help content for (internal, package-level) documentation
Update CHANGELOG.md for dev branch

Closes #125.

@alexcjohnson @StalkComrade

This reverts commit 4db6184.

add more trace

🔧 fix the ci

points to dashr debug

rpkyle · 2019-10-30T19:05:22Z

Test a hard-reload scenario - same as above but the callback output should reset. I guess that would mean putting the app itself into a file and editing that file similarly to the css edit...

fixed in 9979346

rpkyle · 2019-10-30T20:13:27Z

Update CHANGELOG.md for dev branch

fixed in da30cb2

CHANGELOG.md

…hot-reloading

Co-Authored-By: alexcjohnson <[email protected]>

rpkyle · 2019-10-30T21:53:04Z

Add in relevant R help content for (internal, package-level) documentation

fixed in 79a4996

rpkyle · 2019-10-30T22:32:19Z

@alexcjohnson I think this one is ready to go 🚢 . I also added in documentation for clientsideFunction, which was previously missing.

alexcjohnson · 2019-11-01T14:13:45Z

R/dash.R

      router$add_route(route, "dashR-endpoints")
      server$attach(router)
-
+      


fixed in aa1ad07

alexcjohnson · 2019-11-01T14:29:27Z

R/dash.R

+      self$server$port <- getServerParam(as.integer(port), "numeric", 8050)
+
+      dev_tools_ui <- getServerParam(dev_tools_ui, "logical", NULL)
+      dev_tools_props_check <- getServerParam(dev_tools_props_check, "logical", NULL)


Do you need an as.logical here (and all the "logical" items), for the getenv case? In fact, perhaps getServerParam could be written to do this coercion, at least if you start with a string?

This looks like a good solution to me - one kind of funny thing is users can provide all the run_server args as strings - app$run_server(port="1234") as well as app$run_server(port=1234) - but a nice side effect is we won't accept run_server args that don't end up with the correct type.

BTW it would be cleaner to set debug first, then use that as the default for some of these other args - rather than needing the confusing is.null(x) && debug || isTRUE(x) clauses.

Good point. I'll make that change, you're right that the handling for logicals-as-strings is missing this critical detail.

As for the confusing clauses, following up on our offline conversation, will use debug as their default since it is always TRUE or FALSE.

fixed in 6131677

alexcjohnson · 2019-11-01T19:45:29Z

R/dash.R

-      if (is.null(dev_tools_ui) && debug || isTRUE(dev_tools_ui)) {
-        self$config$ui <- TRUE
+      getServerParam <- function(value, type, default) {
+        if (!is.null(value) && toupper(value) %in% c("TRUE", "FALSE"))


Suggested change

if (!is.null(value) && toupper(value) %in% c("TRUE", "FALSE"))

if (type == "logical" && !is.null(value) && toupper(value) %in% c("TRUE", "FALSE"))

Maybe also add a clause

if (type == "numeric") value <- as.numeric(value)

for the hot reload intervals; as.integer probably has to stay where it is since that doesn't appear to map onto a type...

You can verify that an integer is of mode numeric and storage mode integer, typeof is probably a better approach but mode and storage.mode are other options:

> foo <- 1L > mode(foo) [1] "numeric" > storage.mode(foo) [1] "integer"

The col_info function I wrote a while back attempts to present a summary with all the type/mode information I generally want for vectors and data.frames:

https://github.com/rpkyle/cscmisc/blob/master/R/col_info.R

e.g.

> cscmisc::col_info(foo) mode class storage.mode is.factor foo numeric integer integer FALSE

fixed in ba7c015

rpkyle · 2019-11-01T20:30:09Z

@alexcjohnson As a follow-up to our offline conversation, I've tried to make handling of (what ought to be) numeric dev tools parameters a bit more robust in 2e8b90c.

alexcjohnson

🔥 💃 🔥

* 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)

Ryan Patrick Kyle added 16 commits September 9, 2019 10:52

✨ logic to track state and set hashes

c9dd851

⌚ use integer modtime, not hash

90d4f1a

🙈 fix bindings

4db6184

Revert ":see_no_evil: fix bindings"

1daa216

This reverts commit 4db6184.

🙈 use self$config

78577de

insert reload_hash initial route

9f98709

✨ track modified files

d996f53

✨ add custom logger

704a8e6

🔨 update logger

63266de

🙈 remove browser()

4611541

🔨 refactor file tracking

8391448

🙈 fixes for asset tracking

3b0c9b3

✨ hot reload app.R

23e80af

🙈 hot reload can be NULL

8b34a1f

🔨 properly detect changes recursively

35ec809

use modtimeFromPath recursively

0328055

rpkyle added enhancement feature request Community request for a package enhancement labels Sep 18, 2019

rpkyle added this to the Dash v1.4.0 milestone Sep 18, 2019

rpkyle requested a review from alexcjohnson September 18, 2019 16:38

rpkyle self-assigned this Sep 18, 2019

Ryan Patrick Kyle and others added 4 commits September 18, 2019 13:04

🙈 allow reloading without assets

2fd459a

38f7094

add more trace

961418a

🔧 fix the ci

401860d

points to dashr debug

Marc-Andre-Rivet added dash-stage-in_review dash-type-enhancement and removed enhancement labels Sep 20, 2019

rpkyle added the parity Modifications to improve parity across Dash implementations label Sep 22, 2019

Marc-Andre-Rivet removed this from the Dash v1.4.0 milestone Oct 8, 2019

Ryan Patrick Kyle added 2 commits October 30, 2019 13:15

🌳 remove dead branch

c928f9f

🚨 add hard reload test, rename to clarify

9979346

Ryan Patrick Kyle and others added 3 commits October 30, 2019 15:33

🚚 rename files to make app 🏃 happy

4d374ac

Update CHANGELOG.md

d7039e1

Update CHANGELOG.md

da30cb2

alexcjohnson reviewed Oct 30, 2019

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Ryan Patrick Kyle and others added 3 commits October 30, 2019 16:36

✏️ fix path to work in CI and locally

5228dd4

Merge branch '125-hot-reloading' of github.com:plotly/dashR into 125-…

b2fd947

…hot-reloading

Update CHANGELOG.md

1461ee7

Co-Authored-By: alexcjohnson <[email protected]>

rpkyle force-pushed the 125-hot-reloading branch 2 times, most recently from cb02497 to df4c737 Compare October 31, 2019 07:47

rpkyle requested a review from alexcjohnson October 31, 2019 15:42

alexcjohnson reviewed Nov 1, 2019

View reviewed changes

alexcjohnson approved these changes Nov 1, 2019

View reviewed changes

📚 add documentation and some fixes

24c254a

rpkyle force-pushed the 125-hot-reloading branch from 6b81790 to 24c254a Compare November 1, 2019 22:54

rpkyle merged commit c947c73 into dev Nov 1, 2019

Marc-Andre-Rivet removed the dash-stage-in_review label Nov 1, 2019

rpkyle deleted the 125-hot-reloading branch November 1, 2019 23:17

This was referenced Nov 15, 2019

Dash for R apps reload inconsistently in RStudio's viewer pane #146

Closed

Hot reloading interval control for non-asset files in app directory requires improvement #147

Closed

rpkyle mentioned this pull request Jan 3, 2020

Dash for R v0.2.0 #162

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Provide support for hot reloading in Dash for R #127

Provide support for hot reloading in Dash for R #127

rpkyle commented Sep 18, 2019 •

edited

Loading

rpkyle commented Oct 30, 2019

rpkyle commented Oct 30, 2019

rpkyle commented Oct 30, 2019

rpkyle commented Oct 30, 2019

alexcjohnson Nov 1, 2019

rpkyle Nov 1, 2019

alexcjohnson Nov 1, 2019

rpkyle Nov 1, 2019 •

edited

Loading

rpkyle Nov 1, 2019

alexcjohnson Nov 1, 2019

rpkyle Nov 1, 2019 •

edited

Loading

rpkyle Nov 1, 2019

rpkyle commented Nov 1, 2019

alexcjohnson left a comment

		router$add_route(route, "dashR-endpoints")
		server$attach(router)

	if (!is.null(value) && toupper(value) %in% c("TRUE", "FALSE"))
	if (type == "logical" && !is.null(value) && toupper(value) %in% c("TRUE", "FALSE"))

Provide support for hot reloading in Dash for R #127

Provide support for hot reloading in Dash for R #127

Conversation

rpkyle commented Sep 18, 2019 • edited Loading

rpkyle commented Oct 30, 2019

rpkyle commented Oct 30, 2019

rpkyle commented Oct 30, 2019

rpkyle commented Oct 30, 2019

alexcjohnson Nov 1, 2019

Choose a reason for hiding this comment

rpkyle Nov 1, 2019

Choose a reason for hiding this comment

alexcjohnson Nov 1, 2019

Choose a reason for hiding this comment

rpkyle Nov 1, 2019 • edited Loading

Choose a reason for hiding this comment

rpkyle Nov 1, 2019

Choose a reason for hiding this comment

alexcjohnson Nov 1, 2019

Choose a reason for hiding this comment

rpkyle Nov 1, 2019 • edited Loading

Choose a reason for hiding this comment

rpkyle Nov 1, 2019

Choose a reason for hiding this comment

rpkyle commented Nov 1, 2019

alexcjohnson left a comment

Choose a reason for hiding this comment

rpkyle commented Sep 18, 2019 •

edited

Loading

rpkyle Nov 1, 2019 •

edited

Loading

rpkyle Nov 1, 2019 •

edited

Loading