esquisse icon indicating copy to clipboard operation
esquisse copied to clipboard

[Feature Request] - Support reactives in addition to reactiveValues

Open kieran-mace opened this issue 3 years ago • 3 comments

In addition to this functionality, that uses reactiveValues and observeEvent:

  data_rv <- reactiveValues(data = iris, name = "iris")
  
  observeEvent(input$data, {
    if (input$data == "iris") {
      data_rv$data <- iris
      data_rv$name <- "iris"
    } else {
      data_rv$data <- mtcars
      data_rv$name <- "mtcars"
    }
  })
  
  esquisse_server(
    id = "esquisse", 
    data_rv = data_rv
  )

also support the use of reactives:

  data_reactive <- reactive({
    if (input$data == "iris") {
      data <- iris
    } else {
      data <- mtcars
    }
    return(data)
  })
  
  esquisse_server(
    id = "esquisse", 
    data_rv = data_reactive()
  )

according to the developer of shiny reactive is far preferred over the combination of reactiveValue + observeEvent

kieran-mace avatar May 14 '21 03:05 kieran-mace

reactive is for calculating values without side effects observe is for producing side effects

kieran-mace avatar May 14 '21 03:05 kieran-mace

Yes might be possible to support a reactive function there. But with your example esquisse_server won't have the name of the dataset to generate the code.

Currently reactiveValue does what I need, that's sufficient for me.

Victor

pvictor avatar May 15 '21 09:05 pvictor

I had a similar issue. One way to circumvent this is to resolve the reactive and assign the dataset to the global environment with <<-.

  observeEvent(my_reactive_data(), {
    req(my_reactive_data())
    esq_data <<- my_reactive_data() # assign to global env
    data_r$data <- esq_data
    data_r$name <- "esq_data"
  })
  
  results <- esquisse_server(
    id = "esquisse",
    data_rv = data_r
  )

Not sure if its safe or preferred. I'd be interested to see alternative solutions.

HTH!

teofiln avatar May 19 '21 01:05 teofiln

It's true that the current implementation using reactiveValues works, but I do also think that it's awkward/not intuitive. It would be more clear and more in-line with most of the other shiny extensions if instead there would be two separate variables "data" and "name" and both of them would take a reactive. This will also organically solve another issue: using one parameter to group together two parameters is also not a great practice. It will also solve #227

I'm usually extremely conservative with breaking API changes, but I feel strongly enough about this that I would even argue it's worth changing the data argument so that it only expects a dataframe (either static or reactive) and adding a name argument (that accepts a string or a reactive string). To help users that have old code that uses reactiveValues, the data argument could be checked to see if it contains an object of class reactiveValues and if it does then give a very informative error message on how to update the code.

daattali avatar Nov 04 '22 20:11 daattali

An example of the difference it would make in code:

Current:

ui <- fluidPage(
  selectInput("dataset", "dataset", c("iris", "mtcars")),
  esquisse::esquisse_ui("test")
)

server <- function(input, output, session) {
  rv <- reactiveValues(data = data.frame(), name = "")
  observeEvent(input$dataset, {
    rv$data <- get(input$dataset)
    rv$name <- input$dataset
  })
  esquisse::esquisse_server("test", rv)
}

shinyApp(ui, server)

Proposed:

ui <- fluidPage(
  selectInput("dataset", "dataset", c("iris", "mtcars")),
  esquisse::esquisse_ui("test")
)

server <- function(input, output, session) {
  data <- reactive({
    get(input$dataset)
  })
  name <- reactive({
    input$data
  })
  esquisse::esquisse_server("test", data = data, name = name)
}

shinyApp(ui, server)

And it would also be nice to be able to do without reactive values:

ui <- fluidPage(
  esquisse::esquisse_ui("test")
)

server <- function(input, output, session) {
  esquisse::esquisse_server("test", data = iris, name = "iris")
}

shinyApp(ui, server)

daattali avatar Nov 04 '22 20:11 daattali

Thanks to all for suggesting this evolution and to @kieran-mace for initiating it. Now you can use of these three methods to pass data to module :

library(esquisse)
library(shiny)

ui <- fluidPage(
  esquisse_ui("test")
)

server <- function(input, output, session) {
  
  ## with reactivalues
  # rv <- reactiveValues(data = iris, name = "iris")
  # esquisse_server("test", rv, import_from = NULL)
  

  ## with reactive()
  esquisse_server("test", reactive(iris), name = reactive("iris"), import_from = NULL)
  
  ## with data.frame
  # esquisse_server("test", iris, name = "iris")
}

shinyApp(ui, server)

pvictor avatar Nov 24 '22 17:11 pvictor