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

freezeReactiveValue() and indirect dependencies #2946

Closed
fweber144 opened this issue Jun 29, 2020 · 10 comments
Closed

freezeReactiveValue() and indirect dependencies #2946

fweber144 opened this issue Jun 29, 2020 · 10 comments

Comments

@fweber144
Copy link

fweber144 commented Jun 29, 2020

In my opinion, freezeReactiveValue() has a crucial drawback (and I guess this also applies to freezeReactiveVal() though I haven't tested it yet):
Only reactive expressions depending directly on the object being freezed will quit with a silent error as done with req(FALSE). Reactive expressions depending indirectly on the object being freezed won't realize that they should also quit with a silent error. Consider the following reproducible example which is a slightly modified version of the example from ?freezeReactiveValue:

library(shiny)

ui <- fluidPage(
  selectInput("data", "Data Set", c("mtcars", "pressure")),
  checkboxGroupInput("cols", "Columns (select 2)", character(0)),
  plotOutput("plot")
)

server <- function(input, output, session) {
  observe({
    data <- get(input$data)
    # Sets a flag on input$cols to essentially do req(FALSE) if input$cols
    # is accessed. Without this, an error will momentarily show whenever a
    # new data set is selected.
    freezeReactiveValue(input, "cols")
    updateCheckboxGroupInput(session, "cols", choices = names(data))
  }, priority = 1)
  
  ### Additional code not from ?freezeReactiveValue:
  cols_reactive <- reactive({
    input$cols
  })
  ### 
  
  output$plot <- renderPlot({
    # When a new data set is selected, input$cols will have been invalidated
    # above, and this will essentially do the same as req(FALSE), causing
    # this observer to stop and raise a silent exception.
    ### Modified code:
    cols <- cols_reactive() # Original code: cols <- input$cols
    ### 
    data <- get(input$data)
    
    if (length(cols) == 2) {
      plot(data[[ cols[1] ]], data[[ cols[2] ]])
    }
  })
}

shinyApp(ui, server)

The difference to the original example from ?freezeReactiveValue is that output$plot does not depend directly on input$cols, but only indirectly via cols_reactive(). Thus, after having selected two columns from mtcars and then switching to pressure, freezeReactiveValue() does not help in avoiding the (temporary) error. This temporary error might not be a problem in this simple example, but I have an app where I need to avoid it. Of course it would be possible to manually notify all indirect dependencies to quit with a silent error, but this is a tedious and error-prone task and in my opinion, it breaks the concept of reactivity.

I guess this drawback is already known, so I'm posting this as a feature request. I guess it will be quite complicated to fix this (so that also reactive expressions depending indirectly on the object being freezed will quit with a silent error), but it would be great if it could be managed somehow.

Edit 2020-09-16 by @jcheng5: I added priority = 1 to the example to make it clear that even with the observers executing in the most desirable order, this is still a problem.

@jcheng5
Copy link
Member

jcheng5 commented Jun 29, 2020

Thanks for the report! I actually don’t believe I’ve come across this before. I think it’d be easy enough to invalidate on freeze, we’d have to think carefully about the downstream effects though. (Especially the edge case where the value of the input before and after the freeze is identical, which may already be a problem—I think at least it used to be but maybe we fixed it?)

@jcheng5
Copy link
Member

jcheng5 commented Jul 17, 2020

Maybe invalidating on freeze and also invalidating on thaw, plus automatic thawing when the session is flushed next? See also #1791

@fweber144
Copy link
Author

Wouldn't this mean that the value is invalidated when the session is flushed next? Why isn't it enough to invalidate on freeze? Sorry if these are stupid questions, I'm not a Shiny expert.

@jcheng5
Copy link
Member

jcheng5 commented Jul 18, 2020

Invalidating during freeze will cause the dependent output to show blank instead of an error (that's good). However, if the updateCheckboxGroupInput call doesn't actually cause the value to change (i.e. the existing and new values are identical) then the dependent output will stay blank (that's bad) without another invalidation.

I'll take a crack at this on Monday.

@jcheng5
Copy link
Member

jcheng5 commented Jul 20, 2020

BTW, the scenario I referenced in my previous comment can be seen in #1791, the second example app I posted.

@fweber144
Copy link
Author

Yes, I've read through #1791 and also ran the code given there, but I didn't really understand how it was related to the issue here. But your comment

However, if the updateCheckboxGroupInput call doesn't actually cause the value to change (i.e. the existing and new values are identical) then the dependent output will stay blank (that's bad) without another invalidation.

made it clear that #1791 would also arise when invalidating on freeze, so that another invalidation (upon thaw) is needed.

@fweber144
Copy link
Author

By

automatic thawing when the session is flushed next

(see your post above), you would ensure that the value is not invalidated anymore when the session is flushed next? Then this was another misunderstanding by myself when I asked

Wouldn't this mean that the value is invalidated when the session is flushed next?

jcheng5 added a commit that referenced this issue Sep 15, 2020
1. freezeReactiveValue(input, "x") is called, inside a renderUI
   or in an observer that then calls updateXXXInput
2. Some reactive output tries to access input$x, this takes a
   reactive dependency but throws a (silent) error
3. When the flush cycle ends, it automatically thaws

What's *supposed* to happen next is the client receives the new
UI or updateXXXInput message, which causes input$x to change,
which causes the reactive output to invalidate and re-run, this
time without input$x being frozen.

This works, except when the renderUI or updateXXXInput just so
happens to set input$x to the same value it already is. In this
case, the client would detect the duplicate value and not send
it to the server. Therefore, the reactive output would not be
invalidated, and effectively be "stalled" until the next time it
is invalidated for some other reason.

With this change, freezeReactiveValue(input, "x") has a new side
effect, which is telling the client that the very next update to
input$x should not undergo duplicate checking.
@jcheng5
Copy link
Member

jcheng5 commented Sep 15, 2020

#3055 doesn't fix this problem, but it makes it so the only fix that's needed is invalidate on freeze (I believe).

jcheng5 added a commit that referenced this issue Sep 18, 2020
1. freezeReactiveValue(input, "x") is called, inside a renderUI
   or in an observer that then calls updateXXXInput
2. Some reactive output tries to access input$x, this takes a
   reactive dependency but throws a (silent) error
3. When the flush cycle ends, it automatically thaws

What's *supposed* to happen next is the client receives the new
UI or updateXXXInput message, which causes input$x to change,
which causes the reactive output to invalidate and re-run, this
time without input$x being frozen.

This works, except when the renderUI or updateXXXInput just so
happens to set input$x to the same value it already is. In this
case, the client would detect the duplicate value and not send
it to the server. Therefore, the reactive output would not be
invalidated, and effectively be "stalled" until the next time it
is invalidated for some other reason.

With this change, freezeReactiveValue(input, "x") has a new side
effect, which is telling the client that the very next update to
input$x should not undergo duplicate checking.
jcheng5 added a commit that referenced this issue Oct 6, 2020
1. freezeReactiveValue(input, "x") is called, inside a renderUI
   or in an observer that then calls updateXXXInput
2. Some reactive output tries to access input$x, this takes a
   reactive dependency but throws a (silent) error
3. When the flush cycle ends, it automatically thaws

What's *supposed* to happen next is the client receives the new
UI or updateXXXInput message, which causes input$x to change,
which causes the reactive output to invalidate and re-run, this
time without input$x being frozen.

This works, except when the renderUI or updateXXXInput just so
happens to set input$x to the same value it already is. In this
case, the client would detect the duplicate value and not send
it to the server. Therefore, the reactive output would not be
invalidated, and effectively be "stalled" until the next time it
is invalidated for some other reason.

With this change, freezeReactiveValue(input, "x") has a new side
effect, which is telling the client that the very next update to
input$x should not undergo duplicate checking.
@cpsievert
Copy link
Collaborator

cpsievert commented Dec 8, 2020

Seems like this and #1791 can be closed now?

@fweber144
Copy link
Author

That's true, at least the issue reported here does not occur anymore when using shiny version 1.5.0.9005 (from GitHub). Thanks a lot! I haven't checked #1791, though.

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

No branches or pull requests

3 participants