Raise when a caller sets a slot on a self-caching component

Slot content set by the caller is no more part of the cache key than a
block is, so it carries the same risk of serving one caller's content to
another.

Checked before rendering via @__vc_set_slots, which callers populate
through `with_*` setters. Slots a component fills in for itself with a
`default_*` method resolve lazily during the render, so they aren't
counted and are cached normally.

Author: joelhawksley

docs/api.md

@@ -467,9 +467,9 @@ Content for slot SLOT_NAME has already been provided.
 
 ### `ContentPassedToCachedComponentError`
 
-Content was passed to COMPONENT, which caches its own output because it declares `cache_on`.
+COMPONENT declares `cache_on`, so it caches its own output, but its caller passed it content.
 
-Content provided by the caller isn't part of the cache key, so caching it would risk serving one caller's content to another.
+Content and slots set by the caller aren't part of the cache key, so caching them would risk serving one caller's content to another.
 
 To fix this issue, either remove `cache_on` from COMPONENT, or move the content into the component and derive it from the values declared in `cache_on`.
 

docs/guide/caching.md

@@ -158,20 +158,39 @@ Declared components must include `ViewComponent::ExperimentallyCacheable` themse
 
 ## Caveats
 
-**Self-caching components can't take content.** Content passed by the caller isn't part of the cache key, so caching it would risk serving one caller's content to another. Passing a block or `with_content` to a component that declares `cache_on` raises:
+**Self-caching components can't take content from their callers.** Content passed by the caller isn't part of the cache key, so caching it would risk serving one caller's content to another. Passing a block, `with_content`, or a slot to a component that declares `cache_on` raises:
 
 ```erb
 <%# Raises ContentPassedToCachedComponentError %>
 <%= render PostComponent.new(post: @post) do %>
   Hello
 <% end %>
+
+<%# Also raises %>
+<%= render PostComponent.new(post: @post) do |component| %>
+  <% component.with_header { "Hello" } %>
+<% end %>
 ```
 
 The error is raised whether or not caching is enabled, so the conflict surfaces in development and test rather than only in production.
 
-To cache a component that takes content, move the content into the component and derive it from values declared in `cache_on`. Components that don't declare `cache_on` are unaffected: they still accept content, and a `<% cache %>` block around them still invalidates correctly.
+Slots a component fills in for itself with a `default_*` method are part of its own output, not the caller's, so those are cached normally:
+
+```ruby
+class PostComponent < ViewComponent::Base
+  include ViewComponent::ExperimentallyCacheable
+
+  renders_one :header
+
+  cache_on :post
+
+  def default_header
+    post.title # cached, because the component decides it
+  end
+end
+```
 
-**Slot content set by the caller isn't part of the key either**, and isn't currently detected. Declare the values it depends on in `cache_on`.
+To cache a component that takes content, move the content into the component and derive it from values declared in `cache_on`. Components that don't declare `cache_on` are unaffected: they still accept content and slots, and a `<% cache %>` block around them still invalidates correctly.
 
 **`cache_on` methods run before the component renders**, so they can only depend on the component's own state, not on `helpers` or the view context. A cache key that depends on the view context is usually a sign the value should be passed to the component instead.
 

lib/view_component/errors.rb

@@ -244,8 +244,8 @@ def initialize(component_name, method_name)
 
   class ContentPassedToCachedComponentError < StandardError
     MESSAGE =
-      "Content was passed to COMPONENT, which caches its own output because it declares `cache_on`.\n\n" \
-      "Content provided by the caller isn't part of the cache key, so caching it would risk " \
+      "COMPONENT declares `cache_on`, so it caches its own output, but its caller passed it content.\n\n" \
+      "Content and slots set by the caller aren't part of the cache key, so caching them would risk " \
       "serving one caller's content to another.\n\n" \
       "To fix this issue, either remove `cache_on` from COMPONENT, or move the content into the " \
       "component and derive it from the values declared in `cache_on`.".freeze

lib/view_component/experimentally_cacheable.rb

@@ -122,7 +122,7 @@ def render_in(view_context, **, &block)
       # it would serve one caller's content to another. Raised whether or not
       # caching is currently enabled, so the conflict surfaces in development
       # and test rather than only in production.
-      if block || __vc_content_set_by_with_content_defined?
+      if block || __vc_content_set_by_with_content_defined? || __vc_slots_set_by_caller?
         raise ContentPassedToCachedComponentError.new(self.class.name)
       end
 
@@ -167,6 +167,13 @@ def cache_key(view_context = nil)
 
     private
 
+    # Slots set by the caller via `with_*`. Checked before rendering, so slots
+    # a component fills in for itself with a `default_*` method — which resolve
+    # lazily during the render — aren't counted.
+    def __vc_slots_set_by_caller?
+      defined?(@__vc_set_slots) && @__vc_set_slots.present?
+    end
+
     def __vc_cache_enabled?(view_context)
       return false unless defined?(Rails) && Rails.respond_to?(:cache) && Rails.cache
 

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

@@ -0,0 +1,4 @@
+<div class="cacheable-slot">
+  <span class="header"><%= header %></span>
+  <span class="title"><%= title %></span>
+</div>

test/sandbox/app/components/cacheable_slot_component.rb

@@ -0,0 +1,23 @@
+# frozen_string_literal: true
+
+# Declares both a slot and `cache_on`, so callers setting the slot must be
+# rejected while a default-filled slot must not be.
+class CacheableSlotComponent < ViewComponent::Base
+  include ViewComponent::ExperimentallyCacheable
+
+  renders_one :header
+
+  cache_on :title
+
+  def initialize(title:)
+    @title = title
+  end
+
+  def default_header
+    "default header"
+  end
+
+  private
+
+  attr_reader :title
+end

test/sandbox/test/experimentally_cacheable_test.rb

@@ -342,6 +342,33 @@ def test_passing_with_content_to_a_cached_component_raises
     end
   end
 
+  def test_setting_a_slot_on_a_cached_component_raises
+    error = assert_raises(ViewComponent::ContentPassedToCachedComponentError) do
+      render_inline(CacheableSlotComponent.new(title: "a").tap { |c| c.with_header { "set" } })
+    end
+
+    assert_includes error.message, "CacheableSlotComponent"
+  end
+
+  # A slot the component fills in for itself isn't caller-provided, so it's
+  # part of the component's own output and safe to cache.
+  def test_a_default_filled_slot_does_not_raise
+    render_inline(CacheableSlotComponent.new(title: "a"))
+
+    assert_selector(".header", text: "default header")
+    assert_selector(".title", text: "a")
+  end
+
+  def test_default_filled_slots_are_cached
+    with_caching do
+      render_inline(CacheableSlotComponent.new(title: "cached"))
+
+      refute_nil Rails.cache.read(
+        CacheableSlotComponent.new(title: "cached").cache_key(vc_test_controller.view_context)
+      )
+    end
+  end
+
   # Raised regardless of whether caching is on, so the conflict is caught in
   # development and test rather than only in production.
   def test_content_raises_even_when_caching_is_enabled