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

Author: erikaxel

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`.

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.

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

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,