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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up shiny and DT namespace #282

Merged
merged 2 commits into from Sep 26, 2019
Merged

Clean up shiny and DT namespace #282

merged 2 commits into from Sep 26, 2019

Conversation

schloerke
Copy link
Contributor

@schloerke schloerke commented Sep 25, 2019

Next version of shiny (1.4.0) is not (re)exporting the same functions as shiny v1.3.2. By importing shiny as a package, rather than all of the methods individually, it allows us a little bit of freedom to change the exported api.

We are planning on releasing to CRAN here shortly. If this change could be posted to CRAN soon, that would be great! (If not, please let us know. 馃槂 )

Thank you for your help!
- Shiny Team


My interpretation was the shiny methods were imported individually to avoid a namespace conflict with DT. It appears that DT is only used for its two methods, so the package can be namespaced on the function call directly.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 43.383% when pulling 35cab39 on schloerke:shiny-dt-namespace into 037e87d on sdcTools:master.

@bernhard-da
Copy link
Collaborator

hi @schloerke thx for the pr which i merge. the reason for explicitly importing all the shiny functions was because importing the entire package led to some conflicting warnings (in some old versions) that I can't reproduce any longer now :)

we'll probably add some little updates to the package and upload a new version to CRAN soon.

@bernhard-da bernhard-da merged commit bd27770 into sdcTools:master Sep 26, 2019
@schloerke schloerke deleted the shiny-dt-namespace branch September 26, 2019 18:15
@schloerke
Copy link
Contributor Author

Thank you!


Today I also learned (r-lib/roxygen2#617 (comment) ) about using raw namespace to avoid the conflict. This keeps it in line with the idea of "black listing" known conflicts, but importing everything else.

#' @rawNamespace import(shiny, except=c(dataTableOutput, renderDataTable))

@bernhard-da
Copy link
Collaborator

@schloerke that's really great information! thx

@schloerke
Copy link
Contributor Author

@bernhard-da shiny will be heading to CRAN today. Without this change on CRAN, your package will not be able to install with the latest version of shiny. Thank you for your help!

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

Successfully merging this pull request may close these issues.

None yet

3 participants