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

runExample() collides with shiny #127

Closed
krlmlr opened this issue Jun 4, 2017 · 8 comments
Closed

runExample() collides with shiny #127

krlmlr opened this issue Jun 4, 2017 · 8 comments

Comments

@krlmlr
Copy link

krlmlr commented Jun 4, 2017

and makes importing both difficult:

#' @rawNamespace import(shiny, except = runExample)
#' @rawNamespace import(shinyjs, except = runExample)

Would you consider renaming it to runJsExample()? This looks like a function used mostly interactively, so this change shouldn't introduce downstream problems.

@daattali
Copy link
Owner

daattali commented Jun 5, 2017

Another one of those dumb mistakes because I didn't imagine anybody would ever use this package. Not as bad as making a show() function that collides with S4 though :)

Yes, I'll change it in the upcoming release which I'm planning on making soon, thanks

@daattali
Copy link
Owner

there's actually a PR in shiny for this rstudio/shiny#1458

@daattali
Copy link
Owner

I won't be fixing it as of yet because I want to see if shiny will incorporate it

@daattali
Copy link
Owner

Looks like {shiny} decided to kill the PR for this, so every other package that also contains runExample() will run into the same issue. That function will probably always be used in interactive settings so I'll be ok living with this.

@daattali daattali closed this as not planned Won't fix, can't repro, duplicate, stale Jan 16, 2023
@daattali
Copy link
Owner

I see you still want this :) I'm curious, do you actually have a usecase where you are importing the runExample() function? And doing so from multiple packages? My intuition was that this function would only ever be used interactively but I might be wrong

@krlmlr
Copy link
Author

krlmlr commented Jan 21, 2023

Right now I can't

#' @import shiny
#' @import shinyjs

because of conflicts. The workaround is rather awkward, the alternative is to qualify shinyjs functions everywhere.

@daattali
Copy link
Owner

I see, that makes sense.

I really hate giving a non satisfactory answer, but in this case I think that's all I have right now. I thought of changing the function name to example but that's a {utils} function. I don't want to change to snake_case because then there'll be inconsistency within the package. runJsExample is an awkward name and implied that we're running javascript, which isn't really true. So out of all the non ideal solutions, I'm leaning to keeping the current, which does mean that {shinyjs} functions will have to be namespaced. If you think you have a good alternative, let me know.

FWIW I always import shiny and namespace any shinyjs functions, not because of this issue but just because shiny functions are way more abundant. I hate telling/forcing someone to "just do it my way" ("just import shiny and namespace shinyjs, because that's how I do it"), but I can't think of a better solution.

@daattali
Copy link
Owner

daattali commented Jun 1, 2024

@krlmlr looks like shiny did end up implementing this a few months ago rstudio/shiny#4005

I added a deprecation notice right now, urging users to upgrade to {shiny} version 1.8.1 so that they can use shiny::runExample(package="shinyjs").

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