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

asis parameter of shinyjs functions doesn't work for reset() #146

Closed
daattali opened this issue Nov 13, 2017 · 4 comments
Closed

asis parameter of shinyjs functions doesn't work for reset() #146

daattali opened this issue Nov 13, 2017 · 4 comments

Comments

@daattali
Copy link
Owner

related to #118 but only one function doesn't work

@daattali
Copy link
Owner Author

daattali commented Jan 7, 2018

Closing because I'm removing the asis feature anyway

@daattali daattali closed this as completed Jan 7, 2018
@daattali
Copy link
Owner Author

The asis parameter is alive again (f5f57c6) so reopening this issue

@daattali daattali reopened this Apr 16, 2018
@sdrumwr
Copy link

sdrumwr commented Nov 29, 2018

For your consideration: Anyone using shiny modules HAS to be aware of the namespace ns()! This is one of the core tenets of using modules and what allows module re-use! You have to write ALL your module code with this in mind. Otherwise you are circumventing the intended use (and could just source() separate .R files if you are just looking to break a large app into pieces). For those of us who want to actually re-use modules as intended, the "assumption" of namespace is not good. I realize you don't want to break all the code out there that already uses shinyjs/modules (even if they are doing it wrong;), but for concept/consistency it really should be on the programmer to pass the correct ns() in their references. This was the main reason I dropped shinyjs for a while b/c it wouldn't work as expected (with modules) and it wasn't readily apparent why. I realize the "asis" is meant to provide both options, but just having to think about shinyjs module code differently than other module code is not ideal. Anyway, I can see the need for backward-compatibility so maybe "asis" is what we have for now. But, I would seriously consider deprecating the way this works in favor of aligning with the way all module code should be written and a programmer doesn't have to take special note to NOT include ns() (or use "asis") when dealing with shinyjs. [Which is a GREAT package BTW. Much needed and appreciated!]

@daattali
Copy link
Owner Author

@sdrumwr I feel like your comment is more appropriate for #118 than this issue?

I certainly understand your point about the fact that people who use modules need to know how modules and namespaces work. However, I don't see why the current behaviour is so bad. With shiny modules, the ns() function is used in the UI, but when you write server code, you don't need to explicitly pass a namespace - shiny automatically adds the correct namespace when you use shiny functions in the server. Shinyjs by default works the same way as shiny does - namespaces are automatically determined from the session. But if someone wants extra flexibility, they can use asis, which is true that it goes against the idea of modules, but you have to explicitly want to use it. You cannot mess up and do weird things without explicitly wanting to do so. By default things should work as intended.

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