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

extendShinyjs: missing documentation of special id treatment #229

Closed
DavidBarke opened this issue Jan 4, 2021 · 3 comments
Closed

extendShinyjs: missing documentation of special id treatment #229

DavidBarke opened this issue Jan 4, 2021 · 3 comments

Comments

@DavidBarke
Copy link
Contributor

DavidBarke commented Jan 4, 2021

Hello Dean,

if I call a extendShinyjs function from R with argument id, it gets pasted with the namespace:

# R side
js$fun(id = "id")
// JS side
shinyjs.fun = function(params) {
  params.id // is ns(id)
};

I could pass asis = TRUE to prevent this.

This might be either a bug due to unexpected behavior or should be documented better. The documentation for extendShinyjs already includes an example with argument id:

#' For example, calling \code{js$foo("bar", 5)} in R will call \code{shinyjs.foo(["bar", 5])}
#' in JS, while calling \code{js$foo(num = 5, id = "bar")} in R will call
#' \code{shinyjs.foo({num : 5, id : "bar"})} in JS. This means that the
#' \code{shinyjs.foo} function needs to be able to deal with both types of
#' parameters.

Maybe that would be a appropriate location to include this information?

Best reagrds
David

@daattali
Copy link
Owner

daattali commented Jan 4, 2021

Thanks for bringing to my attention. I wanted the id parameter in shinyjs functions to automatically get namespaced so that it would be in-line with how shiny works - in shiny whenever you do anything in the server, you don't need to specify a namespace with an ID, it's inferred. However, I was only thinking about the built in shinyjs functions when I did that. It makes sense for those functions because I control what those functions are and I know that id for those functions always refers to an elemtn ID from the UI and that the namespace would be correct.

However, I didn't have in mind custom extendShinyjs functions, and the fact that they can also have an id parameter. I'm actually not entirely sure what should be the behaviour in that case. Custom functions can operate on anything, and id doesn't necessarily correspond to a namespaced ID of an element from the shiny UI. I'm not sure if this behaviour should remain (and be documented), or if it should be reverted.

@DavidBarke
Copy link
Contributor Author

Not namespacing id would probably be what most users expect. However, given the fact that you added extendShinyjs more than five years ago, I can't imagine to be the first one trying to call a js function from within a module with argument id. Maybe others just noticed this issue without reporting it.

Reverting this behaviour shouldn't be too difficult when #230 is merged, as one would just need to test in jsFuncHelper whether this function name is in the shinyjs function names or not.

@daattali
Copy link
Owner

daattali commented Jan 9, 2021

I'm generally very opposed to modifying behaviour in a breaking way, but I don't think many people use id in custom js functions inside modules. I think it'd be a good idea to remove automatic namespacing from all custom functions.

DavidBarke added a commit to DavidBarke/shinyjs that referenced this issue Jan 11, 2021
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