fix: Use bootstrap version from template package if not defined local…

…ly (r-lib#2443)

Fixes r-lib#2440. Closes r-lib#2441.

NEWS.md

@@ -4,6 +4,7 @@
 * Preview links now work once again (#2435).
 * `build_home()` no longer renders Github issue and pull request templates (@hsloot, #2362)
 * It is now easier to preview parts of the website locally interactively. `build_reference_index()` and friends will call `init_site()` internally instead of erroring (@olivroy, #2329).
+* Fixed an issue introduced in 2.0.8 where pkgdown was not using the Bootstrap version specified in a template package (@gadenbuie, #2443).
 
 # pkgdown 2.0.8
 

R/package.R

@@ -24,12 +24,28 @@ as_pkgdown <- function(pkg = ".", override = list()) {
   meta <- read_meta(pkg)
   meta <- modify_list(meta, override)
 
-  bs_version <- get_bootstrap_version(list(meta = meta))
+  # A local Bootstrap version, when provided, may drive the template choice
+  config_path <- pkgdown_config_path(pkg)
+  config_path <- if (!is.null(config_path)) fs::path_rel(config_path, pkg)
+  bs_version_local <- get_bootstrap_version(
+    template = meta$template,
+    config_path = config_path
+  )
 
   template_config <- find_template_config(
     package = meta$template$package,
-    bs_version = bs_version
+    bs_version = bs_version_local
   )
+
+  bs_version_template <-
+    if (is.null(bs_version_local)) {
+      get_bootstrap_version(
+        template = template_config$template,
+        config_path = config_path,
+        package = meta$template$package
+      )
+    }
+
   meta <- modify_list(template_config, meta)
 
   # Ensure the URL has no trailing slash
@@ -40,6 +56,12 @@ as_pkgdown <- function(pkg = ".", override = list()) {
   package <- desc$get_field("Package")
   version <- desc$get_field("Version")
 
+  # Check the final Bootstrap version, possibly filled in by template pkg
+  bs_version <- check_bootstrap_version(
+    bs_version_local %||% bs_version_template,
+    pkg
+  )
+
   development <- meta_development(meta, version, bs_version)
 
   if (is.null(meta$destination)) {
@@ -98,26 +120,41 @@ read_desc <- function(path = ".") {
   desc::description$new(path)
 }
 
-get_bootstrap_version <- function(pkg) {
-  template_bootstrap <- pkg$meta[["template"]]$bootstrap
-  template_bslib <- pkg$meta[["template"]]$bslib$version
+get_bootstrap_version <- function(template, config_path = NULL, package = NULL) {
+  if (is.null(template)) {
+    return(NULL)
+  }
+
+  template_bootstrap <- template[["bootstrap"]]
+  template_bslib <- template[["bslib"]][["version"]]
 
   if (!is.null(template_bootstrap) && !is.null(template_bslib)) {
+    instructions <-
+      if (!is.null(package)) {
+        paste0(
+          "Update the pkgdown config in {.pkg ", package, "}, ",
+          "or set a Bootstrap version in your {.file ",
+          if (is.null(config_path)) "_pkgdown.yml" else config_path,
+          "}."
+        )
+      } else if (!is.null(config_path)) {
+        paste("Remove one of them from {.file", config_path, "}")
+      }
+
     cli::cli_abort(
       c(
         sprintf(
           "Both {.field %s} and {.field %s} are set.",
-          pkgdown_field(pkg, c("template", "bootstrap")),
-          pkgdown_field(pkg, c("template", "bslib", "version"))
+          pkgdown_field(list(), c("template", "bootstrap")),
+          pkgdown_field(list(), c("template", "bslib", "version"))
         ),
-        x = "Remove one of them from {.file {pkgdown_config_relpath(pkg)}}"
+        i = instructions
       ),
       call = caller_env()
     )
   }
 
-  version <- template_bootstrap %||% template_bslib
-  check_bootstrap_version(version, pkg)
+  template_bootstrap %||% template_bslib
 }
 
 check_bootstrap_version <- function(version, pkg) {

R/pkgdown.R

@@ -52,3 +52,26 @@ local_pkgdown_site <- function(path = NULL, meta = NULL, env = parent.frame()) {
 
   pkg
 }
+
+local_pkgdown_template_pkg <- function(path = NULL, meta = NULL, env = parent.frame()) {
+  if (is.null(path)) {
+    path <- withr::local_tempdir(.local_envir = env)
+    desc <- desc::desc("!new")
+    desc$set("Package", "templatepackage")
+    desc$set("Title", "A test template package")
+    desc$write(file = file.path(path, "DESCRIPTION"))
+  }
+
+  if (!is.null(meta)) {
+    path_pkgdown_yml <- fs::path(path, "inst", "pkgdown", "_pkgdown.yml")
+    fs::dir_create(fs::path_dir(path_pkgdown_yml))
+    yaml::write_yaml(meta, path_pkgdown_yml)
+  }
+
+  rlang::check_installed("pkgload")
+  pkgload::load_all(path)
+  withr::defer(pkgload::unload("templatepackage"), envir = env)
+
+  path
+}
+

tests/testthat/_snaps/templates.md

@@ -0,0 +1,18 @@
+# Invalid bootstrap version spec in template package
+
+    Code
+      local_pkgdown_site(meta = list(template = list(package = "templatepackage")))
+    Condition
+      Error in `as_pkgdown()`:
+      ! Both template.bootstrap and template.bslib.version are set.
+      i Update the pkgdown config in templatepackage, or set a Bootstrap version in your '_pkgdown.yml'.
+
+# Invalid bootstrap version spec in _pkgdown.yml
+
+    Code
+      local_pkgdown_site(meta = list(template = list(bootstrap = 4, bslib = list(
+        version = 5))))
+    Condition
+      Error in `as_pkgdown()`:
+      ! Both template.bootstrap and template.bslib.version are set.
+

tests/testthat/test-templates.R

@@ -92,3 +92,57 @@ test_that("BS5 templates have main + aside", {
   }
 })
 
+
+# Bootstrap version resolution --------------------------------------------
+test_that("Bootstrap version in template package under `template.bootstrap`", {
+  path_template_package <- local_pkgdown_template_pkg(
+    meta = list(template = list(bootstrap = 5))
+  )
+
+  pkg <- local_pkgdown_site(meta = list(template = list(package = "templatepackage")))
+
+  expect_equal(pkg$bs_version, 5)
+})
+
+test_that("Bootstrap version in template package under `template.bslib.version`", {
+  path_template_package <- local_pkgdown_template_pkg(
+    meta = list(template = list(bslib = list(version = 5)))
+  )
+
+  pkg <- local_pkgdown_site(meta = list(template = list(package = "templatepackage")))
+
+  expect_equal(pkg$bs_version, 5)
+})
+
+test_that("Invalid bootstrap version spec in template package", {
+  path_template_package <- local_pkgdown_template_pkg(
+    meta = list(template = list(bootstrap = 4, bslib = list(version = 5)))
+  )
+
+  expect_snapshot(
+    error = TRUE,
+    local_pkgdown_site(meta = list(template = list(package = "templatepackage")))
+  )
+})
+
+test_that("Invalid bootstrap version spec in _pkgdown.yml", {
+  expect_snapshot(
+    error = TRUE,
+    local_pkgdown_site(meta = list(template = list(
+      bootstrap = 4, bslib = list(version = 5)
+    )))
+  )
+})
+
+test_that("Valid local Bootstrap version masks invalid template package", {
+  path_template_package <- local_pkgdown_template_pkg(
+    meta = list(template = list(bootstrap = 4, bslib = list(version = 5)))
+  )
+
+  expect_no_error(
+    local_pkgdown_site(meta = list(template = list(
+      package = "templatepackage",
+      bootstrap = 5
+    )))
+  )
+})