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

feat: remove the use of usethis::create_project from create_golem #1176

Open
ColinFay opened this issue Sep 6, 2024 · 5 comments
Open

feat: remove the use of usethis::create_project from create_golem #1176

ColinFay opened this issue Sep 6, 2024 · 5 comments

Comments

@ColinFay
Copy link
Member

ColinFay commented Sep 6, 2024

When creating a golem, we don't need to use usethis::create_projetct, as we then copy a skeleton for a package.

@ilyaZar
Copy link
Contributor

ilyaZar commented Sep 6, 2024

hey @ColinFay thanks for reworking the internals the readability is so much better each time I look at the code !

FWIW, I remember (and also just checked !) that on dev usethis::create_project() is bootstrapped in usethis_create_project() once and used in create_golem() which is exported in the {golem} package once .

Now, neither dev nor master contain usethis in the imports field of the DESCRIPTION file. It is only in suggests.

I wonder, how this is (was, as this FR is about to eliminate it anyway) a wise choice in general given that imports lists strict requirements and suggests optional, but create_golem() is clearly a core functionality at the user level ...

I guess the answer is obvious and I just have a flawed understanding of R packaging.

Anyways, just wanted to leave a note here. There are quite a lot of other usethis-bootstrapped type functions that may rely on usethis but make their way up to the exported/user level this way.

@ColinFay
Copy link
Member Author

ColinFay commented Sep 6, 2024

Hey @ilyaZar,

That's indeed a great question, so let me explain how it's handled in golem and the rationale behind this weird way of doing.

When developping an app, you'll need a lot of dev dependencies — usethis, devtools, desc, fs... which are not necessary at deployment time. For example, there is no need for usethis in the deployed app.

Historically, usethis used to be a golem dep, but it made deploying the app very cumbersome — you had to install a lot of deps on the server that you don't need.

In order to make this process easier, I've moved all the dev dependencies to Suggests, and when you try to use a function that needs them, we check that the package is installed, and if it's not, we ask the user to install it.

This is for example what you'll get from calling document_and_reload() (which you only call in dev, inside run_dev), which calls roxygen2_roxygenise():

check_roxygen2_installed <- function() {
  rlang::check_installed(
    "roxygen2",
    reason = "to document the package."
  )
}

roxygen2_roxygenise <- function(
  package.dir = ".",
  roclets = NULL,
  load_code = NULL,
  clean = FALSE
) {
  check_roxygen2_installed()
  roxygen2::roxygenise(
    package.dir = package.dir,
    roclets = roclets,
    load_code = load_code,
    clean = clean
  )
}

rlang::check_installed will suggest installing the package if it can't find it.

So to sum up, here is the theoretical workflow on a fresh machine.

  • You install golem, it doesn't install roxygen2
  • You call golem::run_dev(), and you'll get a prompt that says something like 'roxygen2 is required to document the package. Do you want to install it?'.
  • You install it, and everything works just fine.
  • When deploying on the server, you'll install your app as a package that depends on golem, BUT roxygen2 will not be installed, making the install process lighter.

You can also find some information about this here https://golemverse.org/news/golem-0.4.0-release-on-cran/

Let me know if that makes things clearer to you.

@ilyaZar
Copy link
Contributor

ilyaZar commented Sep 6, 2024

Ah ok nice trick ! !

So it works basically because a shiny app never uses the dev facilities provided by {golem}, hence it cannot fail this route.

The imports field causes the reverse dep installs when deploying shiny apps, hiding usethis in suggests does not, which solves the massive install problem when deploying.

Still the user can install them when he/she is developing.

Also CRAN is happy as it seems it does not check all possible function calls to be in imports, it allows function calls from packages that are in suggests whenever they are properly hidden in the bootstrap-type fashion, even if calls "creep" up to exported functions ?

@ColinFay
Copy link
Member Author

ColinFay commented Sep 6, 2024

Yeah my understanding is that CRAN installs all the deps even the Suggests, but by definition it has access to it so that's ok 😅

@ilyaZar
Copy link
Contributor

ilyaZar commented Sep 7, 2024

allrighty ! thanks for taking time to explain !

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