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

Documentation request: Need to call useShinyjs within the app session #182

Closed
ambevill opened this issue Apr 18, 2019 · 10 comments
Closed

Comments

@ambevill
Copy link

I wrote a package myapp that uses shinyjs. myapp defines the UI, including calling shinyjs::useShinyjs(). The session using myapp only needs to pass the UI to shiny::runApp().

The problem is that the session never actually calls useShinyjs when myapp is loaded, so the shinyjs javascript functions aren't available from the shiny server. I guess the call to shiny::addResourcePath isn't persistent within the session.

Based on other open issues, it seems like useShinyjs is probably unavoidable. But the documentation should mention that useShinyjs should be called again within the session running the app, even if it was called in the myapp package.

@daattali
Copy link
Owner

daattali commented Apr 18, 2019 via email

@ambevill
Copy link
Author

Thanks for your patience. The issue has to do with packaged apps, so I had to throw together a demo package. Here's a MWE:

devtools::install_github('ambevill/shinyjs_ui_in_package')

# useShinyjs was called when the package was built, but hasn't been called
# yet in this session. So shiny::addResourcePath doesn't get called in this
# session, and the disable() command doesn't work:
shinyjs.ui.in.package::run_with_ui_static()

# but if I call useShinyjs in this session, the app works as expected:
shinyjs::useShinyjs()
shinyjs.ui.in.package::run_with_ui_static()

MWE package repo is here.

My recommendation is to add a note about this use case in the shinyjs README.md and the doc here.

P.S. Thanks for your hard work supporting this package---I've used it in all of my recent shiny apps.

@daattali
Copy link
Owner

I see what you mean! That is a great reprex, thank you for your effort to make it so clear and for your debugging to see what the problem is. You're right, if you just call shiny::addResourcePath("shinyjs", system.file("srcjs", package = "shinyjs")) before running your app, then it will also work (obviously I'm not suggesting this as a solution!). I've never run across this usecase, of creating a UI function that uses shinyjs in a package and then using that function elsewhere.

So the takeaway here is that the UI function needs to be actively called, you can't just use a previously executed UI as a variable, it must be called as a function, right? I think that would be a more advisable suggestion than to say "you should call useShinyjs() in the end user app". What do you think?

@ambevill
Copy link
Author

Sounds good! And I think in general it should be clear that useShinyjs() has the side-effect of calling shiny::addResourcePath. (My initial assumption was that it just inserted headers into the HTML.)

For what it's worth, I haven't really seen an elegant way to automatically call shiny::addResourcePath at the right time, every time. The shinyBS package calls it when library(shinyBS) is called, but this means that users have to call library(shinyBS) instead of the package-preferred shinyBS::xyz syntax. Info here. My point being that I wish I could recommend a code solution to elegantly handle all use cases, but I don't know what to recommend.

@ambevill
Copy link
Author

Update: it looks like the best practice (according to shiny and shinyBS devs) is to call shiny::addResourcePath using a .onLoad hook. So one alternative is to skip the documentation updates and simply implement this.

@daattali
Copy link
Owner

I've been thinking that .onLoad might be the right approach but wanted to really think about it more to ensure there weren't any weird cases I wasn't thinking about. If shiny approves that method, then that makes me feel much more comfortable with that. I do want to do a lot of testing around this so I'll get around to it hopefully within the week. Thanks a lot for your help

@daattali
Copy link
Owner

Unfortunately that suggestion doesn't fix the issue. I tried placing the addResourcePath() call in shinyjs's .onLoad. The result is that the first time you call run_with_ui_static() it doesn't work properly because its javascript runs before shinyjs's javascript loads. If I close the app and then run it again, the second time around it does work fine because now the resource has been added in time. I wonder if this actually works for shinyBS or anyone else.

I do think that the safest thing to do is tell users that useShinyjs() must literally be called in the app, either by the end user or by the exported function.

@ambevill
Copy link
Author

🤦‍♂️ my bad, of course the .onLoad hook doesn't solve the MWE I made because the session doesn't call shinyjs:: except inside the server function. So I guess package developers like me should probably use the run_with_ui_function approach from the MWE. In which case calling aRP from .onLoad or from useShinyjs would work equally well.

@daattali
Copy link
Owner

Yes, exactly. I still appreciate your effort here, this was a usecase I haven't thought about and haven't run into so it was important to go through it. Feel free to add a short note in the docs

@daattali
Copy link
Owner

Fixed with 5795e52

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

2 participants