Skip to content

Fix bs_dependency_defer() cache collisions for factory-built closures - #1348

Open
AleKoure wants to merge 2 commits into
rstudio:mainfrom
AleKoure:main
Open

Fix bs_dependency_defer() cache collisions for factory-built closures#1348
AleKoure wants to merge 2 commits into
rstudio:mainfrom
AleKoure:main

Conversation

@AleKoure

Copy link
Copy Markdown

Closes #1330

Summary

bs_dependency_defer() memoises dependency functions using a shared cache. The
memoise key includes the function's formals and body, but not its enclosing
environment.

As a result, closures created by the same factory can collide when they differ
only in captured values. After the first dependency is cached, subsequent
dependencies may incorrectly receive the first dependency's compiled output.

This PR adds an optional cache_key argument:

bs_dependency_defer(func, memoise = TRUE, cache_key = NULL)

When supplied, cache_key is folded into the memoise key, allowing callers to
distinguish factory-generated dependency functions. Existing behavior is
unchanged when cache_key = NULL.

When memoise = FALSE, cache_key has no effect and a warning is emitted.

Reproducible example

library(bslib)

mk_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"
      )
    },
    cache_key = name
  )
}

mk_dep("red", ".x { color: red }")()$name
#> [1] "test-red"

mk_dep("blue", ".x { color: blue }")()$name
#> [1] "test-blue"

Without distinct cache keys, both factory-generated closures have identical
formals and bodies and can resolve to the same cached dependency.

Documentation

The bs_dependency_defer() documentation now describes when cache_key is
needed. The custom-components vignette also notes that helper-generated
dependency functions should use a stable, distinct cache key.

Testing

Added unit tests covering:

  • distinct factory-generated dependencies with cache_key;
  • the existing collision without cache_key;
  • memoise = FALSE;
  • the warning when cache_key is combined with memoise = FALSE;
  • unchanged caching behavior for functions that are already distinct.

devtools::check() completes with 0 errors, 0 warnings, and 0 notes.

Checklist

  • Relevant issue exists and the contribution was welcomed by a maintainer.
  • Unit tests were added under tests/testthat.
  • Documentation was updated.
  • devtools::check() passes.
  • Code follows the project style.
  • NEWS.md includes an entry for the change.

Alexandros Kouretsis and others added 2 commits August 19, 2026 12:22
…sions

`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 rstudio#1330

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…encies

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 rstudio#1330

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@CLAassistant

CLAassistant commented Aug 19, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

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.

bs_dependency_defer()  factory-built closures collide on the memoise cache

2 participants