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

Revisit dependencies with no direct usage #2524

Closed
jennybc opened this issue Jul 10, 2023 · 3 comments · Fixed by #2526
Closed

Revisit dependencies with no direct usage #2524

jennybc opened this issue Jul 10, 2023 · 3 comments · Fixed by #2526

Comments

@jennybc
Copy link
Member

jennybc commented Jul 10, 2023

At the very least, update how we justify Imports that have no obvious direct usage within devtools. Determine if some of these @importFroms should be updated:

#' @importFrom lifecycle deprecated
#' @importFrom miniUI miniPage
#' @importFrom profvis profvis
#' @importFrom urlchecker url_check

to instead use the technique described here:

https://r-pkgs.org/dependencies-in-practice.html#how-to-not-use-a-package-in-imports

This came up after a recent incident that reminded us that merely loading devtools causes shiny to be loaded (via miniUI), during the "package version as character" episode.

@gadenbuie
Copy link

devtools is most likely taking a dependency on miniUI (and therefore shiny) to avoid a dialog that pops up when running an addin in RStudio if the package isn't installed. The dialog is somewhat aggressive in that it prevents any addins from running if the miniUI package isn't installed. rstudio/rstudio#12078 argues for the removal of this check, but isn't currently on a roadmap for release in the RStudio IDE.

@hadley
Copy link
Member

hadley commented Jul 11, 2023

I think we just need to use the new technique that doesn't immediately load these namespaces.

@jennybc
Copy link
Member Author

jennybc commented Jul 12, 2023

devtools actually makes use of lifecycle::deprecated(), so it makes sense to @importFrom.

OTOH these are expedient @importFroms, so we can convert them to use the new technique:

  • miniUI::miniPage()
  • profvis::profvis()
  • urlchecker::url_check()

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 a pull request may close this issue.

3 participants