From 91e547e512d640a34045cce0eca04d8f9a2a460c Mon Sep 17 00:00:00 2001 From: Erik Axel Nielsen Date: Wed, 2 Sep 2026 14:56:21 +0200 Subject: [PATCH] Expire memoized template digests when a component registers - Return early from CacheDigest.register when the entry is unchanged, so reloads and re-registrations don't churn - Clear ActionView's digest caches on a new registration, leaving resolver caches alone - Add a regression test asserting a fragment digest is the same whether the component registered before or after the template was digested --- docs/CHANGELOG.md | 4 +++ lib/view_component/cache_digest.rb | 15 +++++++++ ...perimentally_cacheable_integration_test.rb | 31 +++++++++++++++++++ .../test/experimentally_cacheable_test.rb | 31 +++++++++++++++++++ 4 files changed, 81 insertions(+) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 9834fc583..98af99c55 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -10,6 +10,10 @@ nav_order: 6 ## main +* Invalidate Action View's memoized template digests when a component registers with `ViewComponent::CacheDigest`, so a digest computed before the component loaded isn't served for the rest of the process. + + *Erik Axel Nielsen* + ## 4.15.0 * Add experimental caching support, opt-in per component via `include ViewComponent::ExperimentallyCacheable`. diff --git a/lib/view_component/cache_digest.rb b/lib/view_component/cache_digest.rb index 98694c8dc..b0718f676 100644 --- a/lib/view_component/cache_digest.rb +++ b/lib/view_component/cache_digest.rb @@ -75,8 +75,16 @@ def enabled? # @private def register(component) return unless component.virtual_path && component.name + return if registry[component.virtual_path] == component.name registry[component.virtual_path] = component.name + + # Components register as they load, and under lazy loading that happens + # after Action View has already digested and memoized some templates. + # Those digests were computed without this component's dependencies and + # would otherwise be served for the rest of the process, making the + # digest a function of load order rather than of source. + expire_digests end # The synthetic virtual path a component is digested under. @@ -223,6 +231,13 @@ def install! private + # Drop Action View's memoized template digests, leaving its resolver + # caches alone: no template changed, only the set of dependencies the + # Digestor can see. + def expire_digests + ActionView::LookupContext::DetailsKey.digest_caches.each(&:clear) + end + # Resolve a constant name to a component that opted into caching. # # Returns nil for anything else, including constants that don't exist. diff --git a/test/sandbox/test/experimentally_cacheable_integration_test.rb b/test/sandbox/test/experimentally_cacheable_integration_test.rb index 2c08015de..d078e8418 100644 --- a/test/sandbox/test/experimentally_cacheable_integration_test.rb +++ b/test/sandbox/test/experimentally_cacheable_integration_test.rb @@ -75,6 +75,28 @@ def test_cache_block_digest_is_unaffected_by_unrelated_components end end + # Components register as they're autoloaded, so under lazy loading a template + # can be digested before the components it renders have loaded. Action View + # memoizes digests for the life of the process, so that first digest sticks: + # without invalidation the same source digests differently depending on the + # order things happened to load in. + def test_digest_does_not_depend_on_when_the_component_registered + registered_first = with_registry("cacheable_component" => "CacheableComponent") do + clear_digest_cache + fragment_digest_for("integration_examples/cached_component") + end + + registered_late = with_registry({}) do + clear_digest_cache + fragment_digest_for("integration_examples/cached_component") + ViewComponent::CacheDigest.register(CacheableComponent) + + fragment_digest_for("integration_examples/cached_component") + end + + assert_equal registered_first, registered_late + end + def test_component_output_is_cached_between_requests get "/cached_component" assert_select(".cacheable", text: "cached") @@ -98,6 +120,15 @@ def fragment_digest_for(virtual_path) ) end + def with_registry(entries) + saved = ViewComponent::CacheDigest.registry.dup + ViewComponent::CacheDigest.registry.replace(entries) + yield + ensure + ViewComponent::CacheDigest.registry.replace(saved) + clear_digest_cache + end + def view_context ApplicationController.new.tap { |c| c.request = ActionDispatch::TestRequest.create }.view_context end diff --git a/test/sandbox/test/experimentally_cacheable_test.rb b/test/sandbox/test/experimentally_cacheable_test.rb index bb0566163..e4a4b728f 100644 --- a/test/sandbox/test/experimentally_cacheable_test.rb +++ b/test/sandbox/test/experimentally_cacheable_test.rb @@ -20,6 +20,32 @@ def test_registers_component_with_the_digest_registry ) end + # Registration runs on every class load, so an unchanged component must not + # throw away digests other templates are still using. + def test_registering_an_unchanged_component_leaves_memoized_digests_alone + clear_digest_cache + CacheableComponent.cache_digest + memoized = digest_cache_size + + assert_operator memoized, :>, 0 + + ViewComponent::CacheDigest.register(CacheableComponent) + + assert_equal memoized, digest_cache_size + end + + def test_registering_a_new_component_expires_memoized_digests + clear_digest_cache + CacheableComponent.cache_digest + + assert_operator digest_cache_size, :>, 0 + + ViewComponent::CacheDigest.registry.delete("cacheable_component") + ViewComponent::CacheDigest.register(CacheableComponent) + + assert_equal 0, digest_cache_size + end + def test_component_is_marked_cacheable assert_predicate CacheableComponent, :__vc_cacheable? refute_respond_to ErbComponent, :__vc_cacheable? @@ -487,6 +513,11 @@ def recompile(component) component.__vc_compile(force: true) end + # Every digest Action View has memoized, across all details keys. + def digest_cache_size + ActionView::LookupContext::DetailsKey.digest_caches.sum(&:size) + end + def build_template(source) ActionView::Template.new( source,