From 733bdf77154072d3cbc1a39d475534a5fe64b106 Mon Sep 17 00:00:00 2001 From: Alexandros Kouretsis Date: Fri, 24 Jul 2026 15:53:35 +0300 Subject: [PATCH 1/2] fix(bs_dependency_defer): add `cache_key` to avoid shared-cache collisions `bs_dependency_defer()` memoises `func` against a shared cache keyed on its formals, body, and call arguments -- but not its enclosing environment. Closures produced by a factory (the pattern from the "Dynamically themeable component" article) differ only in captured variables, so they hash to the same key and every dependency after the first is served the first one's compiled output. Add an optional `cache_key` argument that is folded into the memoise key so each factory-built closure gets its own cache entry. Behavior is unchanged when `cache_key` is NULL, and the argument is ignored (with a warning) when `memoise = FALSE`. Fixes #1330 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- NEWS.md | 4 ++ R/bs-dependencies.R | 24 ++++++++++- man/bs_dependency.Rd | 8 +++- tests/testthat/test-bs-dependencies.R | 61 +++++++++++++++++++++++++++ 4 files changed, 94 insertions(+), 3 deletions(-) create mode 100644 tests/testthat/test-bs-dependencies.R diff --git a/NEWS.md b/NEWS.md index c565404a3..e2339d81a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -8,6 +8,10 @@ * Navsets created with an `id` (e.g. `navset_tab(id = "tabs")`) now use that `id` as their `data-tabsetid`, so their tab panes get stable `tab-tabs-1` style DOM ids instead of ones derived from a random integer. This makes the rendered markup reproducible across renders and easier to target from custom CSS and JavaScript. Navsets without an `id`, and `nav_menu()` dropdowns, keep the random ID. (#1342) +## Bug fixes + +* `bs_dependency_defer()` gains a `cache_key` argument. Set it to a unique value when you create several similar dependencies, so each one stays distinct in the cache. Previously, dependencies built the same way could be mistaken for one another, and every one after the first was served a copy of the first. (#1330) + # bslib 0.12.0 ## New features diff --git a/R/bs-dependencies.R b/R/bs-dependencies.R index 2ddd47e47..20b020136 100644 --- a/R/bs-dependencies.R +++ b/R/bs-dependencies.R @@ -298,6 +298,11 @@ bs_dependency <- function( #' that you may want to avoid memoisation if `func` relies on side-effects #' (e.g., files on-disk) that need to change for each themable widget #' instance. +#' @param cache_key A unique identifier for this dependency. Set this to a +#' distinct value (e.g. the widget name) whenever you create several +#' dependencies from similar `func`s. Without it, they can be mistaken for +#' one another in the cache, and every dependency after the first is served a +#' copy of the first one. Ignored when `memoise = FALSE`. #' #' @export #' @@ -351,7 +356,12 @@ bs_dependency <- function( #' myWidgetDependency() #' ) #' } -bs_dependency_defer <- function(func, memoise = TRUE) { +bs_dependency_defer <- function(func, memoise = TRUE, cache_key = NULL) { + if (!is.null(cache_key) && !memoise) { + rlang::warn("`cache_key` is ignored when `memoise = FALSE`.") + cache_key <- NULL + } + # func() most likely calls stuff like sass_file() and bs_dependency() -> # sass_partial() -> sass() (e.g., see example section above) Even though # sass() calls can be cached, there is still considerable overhead involved @@ -364,7 +374,17 @@ bs_dependency_defer <- function(func, memoise = TRUE) { # and then it is used once. This is not how memoized functions are normally # used, but in this case it works because the caching object is re-used, and # it still provides very significant improvement in performance. - mfunc <- memoise::memoise(func, cache = .dependency_cache) + memoise_func <- func + if (!is.null(cache_key)) { + # memoise() keys on formals, body, and args, not the enclosing + # environment, so fold cache_key in as a formal to tell otherwise + # identical closures apart. + formals(memoise_func) <- c( + formals(memoise_func), + list(.bslib_cache_key = cache_key) + ) + } + mfunc <- memoise::memoise(memoise_func, cache = .dependency_cache) } else { mfunc <- func } diff --git a/man/bs_dependency.Rd b/man/bs_dependency.Rd index 16363c1e2..c1429f084 100644 --- a/man/bs_dependency.Rd +++ b/man/bs_dependency.Rd @@ -15,7 +15,7 @@ bs_dependency( .sass_args = list() ) -bs_dependency_defer(func, memoise = TRUE) +bs_dependency_defer(func, memoise = TRUE, cache_key = NULL) } \arguments{ \item{input}{Sass rules to compile, using \code{theme}.} @@ -46,6 +46,12 @@ benefits when many instances of the same themable widget are rendered. Note that you may want to avoid memoisation if \code{func} relies on side-effects (e.g., files on-disk) that need to change for each themable widget instance.} + +\item{cache_key}{A unique identifier for this dependency. Set this to a +distinct value (e.g. the widget name) whenever you create several +dependencies from similar \code{func}s. Without it, they can be mistaken for +one another in the cache, and every dependency after the first is served a +copy of the first one. Ignored when \code{memoise = FALSE}.} } \value{ \code{bs_dependency()} returns an \code{\link[htmltools:htmlDependency]{htmltools::htmlDependency()}} and diff --git a/tests/testthat/test-bs-dependencies.R b/tests/testthat/test-bs-dependencies.R new file mode 100644 index 000000000..100ca2d07 --- /dev/null +++ b/tests/testthat/test-bs-dependencies.R @@ -0,0 +1,61 @@ +# Mirrors the "Dynamically themeable component" factory pattern from #1330: +# each call captures a different `name`/`css`, but the deferred closures are +# otherwise byte-identical and so collide on the shared memoise cache unless +# given a distinct `cache_key`. +defer_test_dep <- function(name, css, ...) { + bs_dependency_defer( + function(theme) { + if (!is_bs_theme(theme)) { + theme <- bs_theme(version = 5) + } + bs_dependency( + input = css, + theme = theme, + name = paste0("test-", name), + version = "0.0.0" + ) + }, + ... + ) +} + +test_that("bs_dependency_defer() gives factory-built closures distinct cache entries via `cache_key` (#1330)", { + red <- defer_test_dep("red", ".x { color: red }", cache_key = "red")() + blue <- defer_test_dep("blue", ".x { color: blue }", cache_key = "blue")() + + expect_identical(red$name, "test-red") + expect_identical(blue$name, "test-blue") +}) + +test_that("bs_dependency_defer() collides factory-built closures without `cache_key` (#1330)", { + red <- defer_test_dep("red", ".x { color: red }")() + blue <- defer_test_dep("blue", ".x { color: blue }")() + + # The deferred closures differ only in captured variables, so they share a + # memoise key: `blue` is served `red`'s cached dependency instead of its own. + expect_identical(red$name, "test-red") + expect_identical(blue$name, "test-red") +}) + +test_that("bs_dependency_defer(memoise = FALSE) recomputes on every call (#1330)", { + red <- defer_test_dep("red", ".x { color: red }", memoise = FALSE)() + blue <- defer_test_dep("blue", ".x { color: blue }", memoise = FALSE)() + + expect_identical(red$name, "test-red") + expect_identical(blue$name, "test-blue") +}) + +test_that("bs_dependency_defer() warns when `cache_key` is set with `memoise = FALSE`", { + expect_warning( + bs_dependency_defer(function(theme) NULL, memoise = FALSE, cache_key = "x"), + "cache_key.*ignored" + ) +}) + +test_that("bs_dependency_defer() keeps distinct functions separate without `cache_key`", { + dep_a <- bs_dependency_defer(function(theme) list(name = "a", version = "0.0.0"))() + dep_b <- bs_dependency_defer(function(theme) list(name = "b", version = "0.0.0"))() + + expect_identical(dep_a$name, "a") + expect_identical(dep_b$name, "b") +}) From 4681e3d20a94b4973dece202d10d89fa8b21eff6 Mon Sep 17 00:00:00 2001 From: Alexandros Kouretsis Date: Fri, 24 Jul 2026 18:51:50 +0300 Subject: [PATCH 2/2] docs(custom-components): note `cache_key` for helper-generated dependencies Add a callout to the "Dynamically themeable component" article explaining that `bs_dependency_defer()` needs a distinct `cache_key` when the dependency function is returned by another function, so each dependency stays separate in the cache. Also trim an overly verbose comment in the new tests. Re #1330 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/testthat/test-bs-dependencies.R | 5 +---- vignettes/custom-components/index.Rmd | 6 ++++++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/tests/testthat/test-bs-dependencies.R b/tests/testthat/test-bs-dependencies.R index 100ca2d07..a0f9dfc19 100644 --- a/tests/testthat/test-bs-dependencies.R +++ b/tests/testthat/test-bs-dependencies.R @@ -1,7 +1,4 @@ -# Mirrors the "Dynamically themeable component" factory pattern from #1330: -# each call captures a different `name`/`css`, but the deferred closures are -# otherwise byte-identical and so collide on the shared memoise cache unless -# given a distinct `cache_key`. +# A factory of deferred deps that differ only in captured variables (#1330). defer_test_dep <- function(name, css, ...) { bs_dependency_defer( function(theme) { diff --git a/vignettes/custom-components/index.Rmd b/vignettes/custom-components/index.Rmd index 3258426f2..4a7247be8 100644 --- a/vignettes/custom-components/index.Rmd +++ b/vignettes/custom-components/index.Rmd @@ -109,6 +109,12 @@ person <- function(name, title, company) { Note that when `theme` is a `bs_theme()` object, then `person.scss` is compiled with Bootstrap Sass variables and mixins included via `bs_dependency()` (which returns the compiled CSS as an `htmlDependency()`). Otherwise, if `theme` is *not* a `bs_theme()` object, then `person()` is being used in a context where `{bslib}` is not relevant, so a pre-compiled CSS file is returned instead. Pre-complied CSS isn't necessarily a requirement, but it's a good idea for increasing performance and reducing software dependencies for end users. +::: {.callout .callout-note} +

Generating dependencies from a helper function

+ +Pass a distinct `cache_key`, such as the component `name`, when your dependency function is returned by another function. This keeps each dependency separate in the cache, instead of every one after the first inheriting the first's styles. +::: + ## HTML widgets For `{htmlwidgets}` that can be themed via CSS, we recommend supplying a `bs_dependency_defer()` to the `dependencies` argument of `createWidget()` (similar to the `person()` component from the last section), which will make the widget dynamically themeable. For widgets that can *not* be themed via CSS, the best option may be to query the active theme inside a `preRenderHook()` via `bs_current_theme()`, and then translate any relevant information to the widget's instance data, for example: