Track components that haven't opted into cacheability

- Resolve any ViewComponent::Base descendant as a dependency rather than
  only registered ones, so a digest can't be silently partial
- Resolve unregistered components back from their virtual path, requiring
  the path to round-trip to the same class
- Add regression tests for an untracked child changing without moving the
  parent's digest or the fragment digest

Author: erikaxel

docs/CHANGELOG.md

@@ -10,6 +10,14 @@ nav_order: 6
 
 ## main
 
+* Track every component in a fragment's render tree when the experimental caching feature is enabled, not only the components that included `ViewComponent::ExperimentallyCacheable`.
+
+    Dependency tracking used to be transitively opt-in: a parent that included the module got a digest covering only the children that also included it. The digest looked complete while it was silently partial, and the gap surfaced as stale HTML at an arbitrary later time, whenever an unrelated tracked component happened to change.
+
+    Only the component wrapped in the `<% cache %>` block needs the include now. Applications that never opt in are unaffected, since dependency tracking still short-circuits until the first component registers.
+
+    *Erik Axel Nielsen*
+
 ## 4.15.0
 
 * Add experimental caching support, opt-in per component via `include ViewComponent::ExperimentallyCacheable`.

docs/guide/caching.md

@@ -28,7 +28,7 @@ Editing `PostComponent`'s template, Ruby class, or sidecar files doesn't invalid
 
 ## Opting in
 
-Include `ViewComponent::ExperimentallyCacheable` in each component that should participate in caching:
+Include `ViewComponent::ExperimentallyCacheable` in the component rendered inside the `cache` block:
 
 ```ruby
 class PostComponent < ViewComponent::Base
@@ -42,6 +42,8 @@ end
 
 That's all that's needed for the `<% cache %>` block above to work. The component is registered with Rails' digest tree, and the fragment is invalidated when the component's template, Ruby class, sidecar files, superclasses, child components, or rendered partials change, including components and partials rendered from an inline template or a `#call` method.
 
+Once any component in the application has opted in, the whole render tree is tracked: the child components `PostComponent` renders, and the components *they* render, invalidate the fragment whether or not they include the module.
+
 ## Self-caching
 
 To have a component cache its own output without needing a `cache` block, use `cache_on` to declare methods used for the component's cache key.
@@ -187,7 +189,7 @@ The same works in a template, where the branch is often the more natural place f
 <%= render component.new(post: @post) %>
 ```
 
-Declared components must include `ViewComponent::ExperimentallyCacheable` themselves, since a component that hasn't opted in has no digest to depend on.
+Declared components don't need to include `ViewComponent::ExperimentallyCacheable` themselves. A component that overrides `virtual_path` does, since it's otherwise digested under a path that doesn't lead back to it.
 
 ## Caveats
 

lib/view_component/cache_digest.rb

@@ -21,9 +21,11 @@ module ViewComponent
   #    not just its template.
   #
   # This module fixes both, reusing Rails' own `ActionView::Digestor` rather than
-  # reimplementing static analysis. Components opt in individually by including
-  # `ViewComponent::ExperimentallyCacheable`; until at least one component does,
-  # every hook here short-circuits.
+  # reimplementing static analysis. Until at least one component opts in by
+  # including `ViewComponent::ExperimentallyCacheable`, every hook here
+  # short-circuits. Once one has, every component reachable from a digested
+  # template is tracked, whether or not it included the module: a digest that
+  # covered only part of the render tree would look exactly like a complete one.
   #
   # @private
   module CacheDigest
@@ -90,17 +92,26 @@ def virtual_path_for(component)
 
       # Resolve a synthetic virtual path back to the component that owns it.
       #
+      # Components that opted in are looked up in the registry. Everything else
+      # is derived from the path, which `ViewComponent::Base` builds by
+      # underscoring the class name. The derived constant has to underscore back
+      # to the same path, so a component that overrides `virtual_path` — and
+      # would therefore be digested under a path that isn't its own — is left
+      # unresolved rather than confused with another component.
+      #
       # @return [Class, nil]
       def component_for(virtual_path)
         return unless virtual_path.start_with?("#{VIRTUAL_PATH_PREFIX}/")
 
-        name = registry[virtual_path.delete_prefix("#{VIRTUAL_PATH_PREFIX}/")]
-        return unless name
+        path = virtual_path.delete_prefix("#{VIRTUAL_PATH_PREFIX}/")
+        name = registry[path]
+        return constantize_component(name) if name
 
-        constantize_component(name)
+        component = constantize_component(path.camelize)
+        component if component&.virtual_path == path
       end
 
-      # Scan a template's source for renders of cacheable components.
+      # Scan a template's source for renders of components.
       #
       # Called for every template Rails digests, so it exits early when the
       # feature is unused.
@@ -113,7 +124,7 @@ def dependencies_in(template)
       end
 
       # Scan arbitrary source (a template or a component's Ruby file) for
-      # renders of cacheable components.
+      # renders of components.
       #
       # @return [Array<String>] synthetic virtual paths
       def component_paths_in(source)
@@ -223,15 +234,24 @@ def install!
 
       private
 
-      # Resolve a constant name to a component that opted into caching.
+      # Resolve a constant name to a component.
+      #
+      # Any component counts, not only those that included
+      # `ExperimentallyCacheable`. Tracking only the ones that opted in makes
+      # dependency tracking transitive: a parent's digest covers the children
+      # that happen to have included the module and silently omits the rest,
+      # which is indistinguishable from a complete digest until the untracked
+      # child changes and stale HTML stays on. Applications that never opt in
+      # are unaffected either way, because every hook here short-circuits while
+      # the registry is empty.
       #
       # Returns nil for anything else, including constants that don't exist.
       # Autoloading here is safe: the template is about to render this constant
       # anyway.
       def constantize_component(constant_name)
         component = constant_name.safe_constantize
         return unless component.is_a?(Class)
-        return unless component.respond_to?(:__vc_cacheable?) && component.__vc_cacheable?
+        return unless component < ViewComponent::Base
 
         component
       rescue

test/sandbox/app/components/cacheable_untracked_parent_component.html.erb

@@ -0,0 +1 @@
+<div class="cacheable-untracked-parent"><%= render UntrackedChildComponent.new %></div>

test/sandbox/app/components/cacheable_untracked_parent_component.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+# Renders a child that never opted in, so changes to the child must still
+# invalidate the parent.
+class CacheableUntrackedParentComponent < ViewComponent::Base
+  include ViewComponent::ExperimentallyCacheable
+end

test/sandbox/app/components/untracked_child_component.html.erb

@@ -0,0 +1 @@
+<span class="untracked-child">untracked</span>

test/sandbox/app/components/untracked_child_component.rb

@@ -0,0 +1,6 @@
+# frozen_string_literal: true
+
+# Deliberately does not include `ViewComponent::ExperimentallyCacheable`, so
+# nothing registers it with the digest tree.
+class UntrackedChildComponent < ViewComponent::Base
+end

test/sandbox/app/views/integration_examples/cached_untracked_component.html.erb

@@ -0,0 +1,4 @@
+<% cache "cached-untracked-component-fragment" do %>
+  <%= render CacheableComponent.new(title: "cached") %>
+  <%= render UntrackedChildComponent.new %>
+<% end %>

test/sandbox/config/routes.rb

@@ -28,6 +28,7 @@
   get :cached_partial, to: "integration_examples#cached_partial"
   get :cached_component, to: "integration_examples#cached_component"
   get :cached_nested_component, to: "integration_examples#cached_nested_component"
+  get :cached_untracked_component, to: "integration_examples#cached_untracked_component"
   get :inherited_sidecar, to: "integration_examples#inherited_sidecar"
   get :inherited_from_uncompilable_component, to: "integration_examples#inherited_from_uncompilable_component"
   get :unsafe_component, to: "integration_examples#unsafe_component"

test/sandbox/test/experimentally_cacheable_integration_test.rb

@@ -65,6 +65,36 @@ def test_cache_block_is_invalidated_when_a_nested_component_changes
     end
   end
 
+  # The failure this guards against is silent: the digest looks complete while
+  # covering only the children that happened to opt in, so an edit to an
+  # untracked one sits invisible until an unrelated tracked component changes.
+  def test_cache_block_is_invalidated_when_an_untracked_component_changes
+    get "/cached_untracked_component"
+    assert_select(".untracked-child", text: "untracked")
+
+    before = fragment_digest_for("integration_examples/cached_untracked_component")
+
+    modify_file "app/components/untracked_child_component.html.erb", "<span class=\"untracked-child\">changed</span>\n" do
+      clear_digest_cache
+
+      refute_equal before, fragment_digest_for("integration_examples/cached_untracked_component")
+    end
+  end
+
+  def test_cached_markup_of_an_untracked_component_is_not_served_stale
+    get "/cached_untracked_component"
+    assert_select(".untracked-child", text: "untracked")
+
+    modify_file "app/components/untracked_child_component.html.erb", "<span class=\"untracked-child\">changed</span>\n" do
+      clear_digest_cache
+      with_new_cache do
+        get "/cached_untracked_component"
+
+        assert_select(".untracked-child", text: "changed")
+      end
+    end
+  end
+
   def test_cache_block_digest_is_unaffected_by_unrelated_components
     before = fragment_digest_for("integration_examples/cached_component")