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

After upgrading to Shiny 1.8.1 from 1.8.0, no data is visible on my shiny APP #4016

Closed
gunawebs opened this issue Mar 28, 2024 · 23 comments · Fixed by #4019
Closed

After upgrading to Shiny 1.8.1 from 1.8.0, no data is visible on my shiny APP #4016

gunawebs opened this issue Mar 28, 2024 · 23 comments · Fixed by #4019

Comments

@gunawebs
Copy link

After upgrading to Shiny 1.8.1 from 1.8.0, no data is visible on my shiny APP

runApp() does not throw any error, but NO data is shown on the screen

Was some backward compatibility broken in this release?

@gunawebs
Copy link
Author

Note: As soon as I uninstalled 1.8.1 and installed 1.8.0 from : https://cran.r-project.org/src/contrib/Archive/shiny/shiny_1.8.0.tar.gz

Things worked JUST FINE

@jcheng5
Copy link
Member

jcheng5 commented Mar 28, 2024

Yikes! Certainly not intentionally!

When the problem occurs, is there any error in your browser’s JavaScript console?

@gunawebs
Copy link
Author

gunawebs commented Mar 28, 2024

Few possible points that may help you debug:

  1. Mine is a huge shiny app
  2. id= maybe missing for some elements
  3. many navbarPages may have the same value for id (shiny did not mind this till now)

@gunawebs
Copy link
Author

Similarly a few tabsetpanels also may have the same id.

I cannot think of anything else that could have broken this so badly.

Note: Earlier same behaviour was observed if there were multiple datatables or UI elements with same id. Is it possible that same strictness is now being enforced on tabsetpanels and navbarpages?

Just trying to think what could have happened :-)

@gunawebs
Copy link
Author

@jcheng5 please see if above comments help you debug

@jcheng5
Copy link
Member

jcheng5 commented Mar 28, 2024

Yes we’ve gotten stricter with duplicate IDs, but I would’ve expected messages in the browser’s JavaScript console in that case.

@gadenbuie @cpsievert any ideas?

@gunawebs
Copy link
Author

gunawebs commented Mar 28, 2024

As of now I have reverted to 1.8.0, So wont be able to replicate. Since things work perfectly fine on 1.8.0
Will try on my QA box tomorrow. with 1.8.1

But any easy way for me to spot the root cause and fix? I can then try those on QA box.

Thanks!

@gunawebs
Copy link
Author

OK, one thing is: I might have closed browser’s JavaScript console too soon. And was using large extension monitors. So there is a non zero chance that I might have missed this. Having said that, this may be breaking stuff for many shiny users. Since many would've copied code for tabsetpanels like me. But I guess there is nothing much you can do? (or is there a global flag to switch off duplicate checks added in 1.8.1? Just for testing?)

@gadenbuie
Copy link
Member

@gunawebs Could you show us a small snippet of code to give us a sense of how you're using navbarPage() and tabsetPanel()? In particular, the parts that you suspect might be related?

@cpsievert
Copy link
Collaborator

cpsievert commented Mar 28, 2024

Another thing that'll be helpful for diagnosing the issue: call shiny::devmode(TRUE) before launching the app. If duplicate IDs is the issue, you should see a popup appear in the app with the relevant IDs. Something like this:

error-dialog

@wyu
Copy link

wyu commented Mar 28, 2024

I had the same problem on my modulized app. It turns the "sidebar" in two of my modules had the same id. Wrapping them with ns(), as I should have done, fixed the problem. Thanks to the comment from Joe!

Wen

@gunawebs
Copy link
Author

hello @cpsievert @jcheng5 @gadenbuie

I repeated the test on my QA box:

Here is my error on the screen, when in dev mode
image

Here is my error on javascript console
image

BUT There is no ID called: sidebarItemExpanded anywhere in my code
I searched all over.

So I am badly stuck

Also attached a sippet of code. this may or may not be the error.

image

@gunawebs
Copy link
Author

OK, I may have zero'ed in on the error:

Commenting the code as below, fixed the error
image

Basically it did NOT like the dashboardPage(under tabPanel) within navbarPage

@jcheng5
Copy link
Member

jcheng5 commented Mar 29, 2024

If you have two dashboardPage calls, then yes, this problem will occur. dashboardPage, navbarPage, and really any function from our team that is either xxxPage or page_xxx is designed to be the outermost function call for your UI.

(Team, seems like this is a scenario we hadn't thought of--apps that are working "correctly" in Shiny <=1.8.0 despite having duplicate IDs, because the app doesn't actually use them.)

@gunawebs
Copy link
Author

Thanks @jcheng5

So looks like 2 gotchas:

  1. dashboardPage(under tabPanel) within navbarPage works fine in 1.8.0
    So presume it got stricter in 1.8.1, and will just break everything now.

  2. Duplicate, but unused, ids weren't a problem till 1.8.0, But 1.8.1 will break it.

Correct?

Thanks!

@jcheng5
Copy link
Member

jcheng5 commented Mar 29, 2024

@gunawebs, what other page functions are you using in your UI, and how many times each? Here we can see navbarPage x 1 and dashboardPage x 1, but are there others?

I ask because, while we never expected nor tested for putting dashboardPage inside navbarPage, I wouldn't expect that to give you the specific error you got; I'd expect that if there were two or more dashboardPage calls in your UI, anywhere.

@gunawebs
Copy link
Author

Great question @jcheng5

This was the error:
image

As you can see it said: 3 errors.

And there were exactly 3 combination of:

navbarPage x dashboardPage in my code.

I commented all three of them. And things worked fine after that.

cpsievert added a commit that referenced this issue Mar 29, 2024
cpsievert added a commit that referenced this issue Mar 29, 2024
cpsievert added a commit that referenced this issue Mar 29, 2024
…mode (#4019)

* Close #4016. Warn instead of error when duplicate binding IDs are found in non-devmode

* Get rid of unreachable ShinyClientError()

* `yarn build` (GitHub Actions)

* Update srcts/src/shiny/bind.ts

Co-authored-by: Garrick Aden-Buie <garrick@adenbuie.com>

* `yarn build` (GitHub Actions)

* Move logic to where error gets thrown not constructed

* `yarn build` (GitHub Actions)

* Update NEWS

---------

Co-authored-by: cpsievert <cpsievert@users.noreply.github.com>
Co-authored-by: Garrick Aden-Buie <garrick@adenbuie.com>
@cpsievert
Copy link
Collaborator

Thanks for the reports everyone. We're going to do a patch release early next week so that the error only gets thrown when opt-ing into shiny::devmode(TRUE). Otherwise, you'll just get a warning in the browser's JS console.

@gunawebs
Copy link
Author

@cpsievert thanks a ton.

quick question, by any chance would this take care of the dashboard issue too?
In other words can we now have dashboardPage within navbarPage?

@cpsievert
Copy link
Collaborator

quick question, by any chance would this take care of the dashboard issue too?

I believe so, yes. If you like to try it before we release: remotes::install_github("rstudio/shiny")

In other words can we now have dashboardPage within navbarPage?

Putting a page within any other page is not generally recommended. Are you needing that usage because {shinydashboard} doesn't allow for navigation in the header (just the sidebar)? If so, you probably should consider moving from {shinydashboard} to {bslib}, which offers dashboards with navigation in the page header https://rstudio.github.io/bslib/articles/dashboards/index.html#multi-page

@gunawebs
Copy link
Author

gunawebs commented Apr 2, 2024

Thanks @cpsievert will try right away

@gunawebs
Copy link
Author

gunawebs commented Apr 2, 2024

@cpsievert Thanks so so much. I have tested with the pre release version. And things work fine.

a. Could you please let me know the expected date of the patch to be released on CRAN
b. Regarding your question: "Are you needing that usage because {shinydashboard} doesn't allow for navigation in the header (just the sidebar)". Absolutely correct. Mine is a multi layered app, with deep hierarchy of tabs and dashboards.
c. Will surely check bslib out.

Thanks!

@cpsievert
Copy link
Collaborator

New release is now up on CRAN

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.

5 participants