Breaking changes in extendShinyjs (function argument)

[adisarid]

Hi, Not much of an issue perhaps more of a question about a recent update to extendShinyjs. I noticed it yesterday when some of my apps stopped working after I updated the package. https://github.com/daattali/shinyjs/blob/2404d5f714c6cc9531136da28a6d612082c53584/R/extendShinyjs.R#L241

Now the function requires a new argument called functions but in the past did not require it, which is not backwards compatible (I also saw that this was reported in the NEWS.md file.)

I'm wondering if the function used to work without the argument, how come it's required now? Perhaps provide some default value to the argument that would make the function backwards compatible, and if not (for my curiosity's sake) - why not?

See also this post.

[daattali]

Version 2.0 introduced this breaking change. If you use my packages you'll know that I am VERY conservative about breaking changes, so I had to do this only because there was a big benefit. The functions argument was always around, but before I was trying to automatically read the javascript file using the V8 package. Two issues with this: 1. The V8 package is a very heavy dependency and is hard to install in some operating systems, 2. This automatic reading didn't work in all cases. When I built it 6 years ago i did not understand shiny as well as I do now and I didn't know it wouldn't work in all cases. I tried publishing this news as much as I could, on twitter, to my mailing list, and in a blog post. And I added in the NEWS an entry about it explicitly saying this is a breaking change. I was hoping that saying "the functions argument is needed" would be enough to make the user look at the documentation and use the functions argument. If there was something else I missed that would have helped, I'm open to doing more

[adisarid]

I understand that this argument is now mandatory in order to forgo the V8 dependency. I also noticed it in the NEWS.md (and twitter?) but was still baffled for a moment when my app broke (a short moment).

Anyway, I have a suggestion to a possibly lighter fallback option, just if the user did not provide the functions argument.

I see that in most use cases the function name will appear immediately after shinyjs. and be followed by = function. If that's always the case (is it?) we can use stringr which is a reasonable dependency (IMO).

jsCode <- "shinyjs.pageCol = function(params){$('body').css('background', params);}
shinyjs.some_other_function = function(params){$('body').css('background', params);}"

library(stringr)
str_remove_all(
  unlist(
    str_extract_all(jsCode, "(?<=shinyjs.)(.*)(?=function)")
    ), "=|\\s")

Would this work?

Perhaps also issue a warning "functions argument not provided to extendShinyjs, using default fallback to extract function names".

[daattali]

I explicitly didn't want to do something like this. It's a complicated issue, I really really did not take this lightly but this is the right thing to do. It's not only a matter of parsing the file to find the function names, if that's all it was then I would have left https://github.com/daattali/shinyjs/issues/57 as an open issue. The problem is that R itself , the server side, prior to running the app, may not always have access to the script. This is related to the other breaking change I wrote about in the NEWS, how now shinyjs behaves correctly when searching for the script. For example, if you're using custom resource paths, then it becomes very complex to try to find the script, that's something that the browser is going to do , not shiny. Previously, before this update, using custom resource paths would simply not work with shinyjs, which was a true bug. It wasn't often noticed because most users don't do that, but it was a bug. Now every app can work with shinyjs, and I don't want to add code that isn't going to always work. I much prefer adding an error message that can be solved by using one more parameter. I feel very strongly about not adding code that isn't 100% correct.

[adisarid]

I understand, thanks for explaining that.