rstudio / reactlog

Shiny Reactivity Visualizer
http://rstudio.github.io/reactlog
Other
121 stars 9 forks source link

Should `reactiveValues` be grouped? #54

Open schloerke opened 4 years ago

schloerke commented 4 years ago

Currently each key is a separate node. names(obj) is a separate node.

Should there be a master reactiveValues node (always in a ready state) that has a child nodes of keys and child nodes for as.list or names?

earnaud commented 4 years ago

Are you talking about reactiveValues representation? or about a RV for all reactlog's nodes? I am currently hesitating to open a new issue to ask whether it would be possible to display the name of reactivevalues instead of non-understandable reactiveValue8965$interesting-element (whereas name$interesting-element would allow to better understand the graph).

schloerke commented 4 years ago

In the current setup, each node is a reactive element. I would like to display the reactiveValues objects to be connected to each other somehow. This could be done by having an ancestor element (ex: "input") which could point to "input$val".

The issue is input does not actually exist. It is only the group label.

Other functions such as reactiveValuesToList(rv) and names(rv)are also dependent upon the keys and values inside the reactiveValues object.

I am thinking of displaying them in a group. Example:

image

This is also my current vision for "super nodes" (or a group of reactive elements that can be collapsed or expanded). "Super nodes" would be useful for things like

It may end up that reactiveValues are just displayed as an uncollapsed "super node"


@earnaud Labels are not natively implemented for reactive values. I believe we'll need a function in shiny for it to work properly. This would also cause a wrinkle for reactlog. Currently, input (a reactiveValues) uses a label and it printed as you'd expect.

For now, you can use...

reactiveValuesWithLabel <- function(..., .label = paste0('reactiveValues', shiny:::p_randomInt(1000, 10000)) {

  args <- list(...)
  if ((length(args) > 0) && (is.null(names(args)) || any(names(args) == "")))
    stop("All arguments passed to reactiveValues() must be named.")

  # ### Update start
  values <- shiny:::.createReactiveValues(shiny:::ReactiveValues$new(label = .label))
  # ### Update end

  # Use .subset2() instead of [[, to avoid method dispatch
  .subset2(values, 'impl')$mset(args)
  values
}

disclaimer: this uses internal methods which are subject to change

earnaud commented 4 years ago

Thanks for your enlightments, they are more than helpful to my work :D I will try to give a look at how I could help in return for your troubles. In my concerns, maybe that passing a specially-named argument to reactiveValues() (thus modifying the function specification) could do the trick.

schloerke commented 4 years ago

I completely agree that a label should be able to be passed in. But given the current implementation of shiny reactiveValues where all ... values are set to key/value pairs, the function definition will not change to avoid any unexpected behavior for anyone.

A reactiveValuesWithLabel (or some another fitting function name) could be added to shiny.