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

Infinite loop with conditionalPanel #4126

Open
kamilzyla opened this issue Sep 13, 2024 · 1 comment · May be fixed by #4127
Open

Infinite loop with conditionalPanel #4126

kamilzyla opened this issue Sep 13, 2024 · 1 comment · May be fixed by #4127

Comments

@kamilzyla
Copy link

kamilzyla commented Sep 13, 2024

Example app

This app prints dots indefinitely:

shinyApp(
  ui = div(
    id = "box",
    conditionalPanel(condition = "undefined"),
    tags$script(HTML("
      $('#box').on('hide', () => {
        Shiny.setInputValue('dot', Math.random());
      });
    "))
  ),
  server = function(input, output) {
    observeEvent(input$dot, cat("."))
  }
)

Details

The falsy condition in conditionalPanel causes it to trigger a hide event, upon which we update the dot input. This causes the condition to be evaluated again and another event to be fired, resulting in an endless loop.

It works because this check ("Is the desired state different from the current state?") always evaluates to true if the condition passed to conditionalPanel is not a boolean. Replacing undefined with 42 and hide with show also results in an endless loop.

Fix

My suggestion is to fix the problem by coercing the condition to a bool in line 617, like so:

const show = !!condFunc(nsScope);

A workaround for user code is to ensure that condition evaluates to a boolean (e.g. with !!).

Notes

Checked with Shiny 1.9.1, but looking at git history the problem has been around for a while.

Perhaps this is not something many people will encounter, but in our case this bug was causing performance issues and it was very hard to find it (the conditionalPanel() and the event listener were pieces of two unrelated functionalities in a complex app).

@gadenbuie
Copy link
Member

Thanks @kamilzyla, I can reproduce the issue even on shinylive.io.

We definitely want to avoid the constant stream of events and I think you're right in your fix. Note that in theory we've typed scopeExprToFunc() (which creates the condFunc()) as returning boolean, so in theory the fix could be applied in there. That said, I think scopeExprToFunc() would be better typed as unknown because of its general application; alternatively we could force it to return a Boolean value and rename it scopeExprToPredicateFunc(). I'm going to PR the type change to unknown.

Thanks again for finding this issue and reporting it with a clear reprex and suggestion!

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

Successfully merging a pull request may close this issue.

2 participants