From 6f6902ab37bdc967e1099ad0fef0f172cf65f3b9 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 20 Aug 2026 11:06:06 -0600 Subject: [PATCH 01/12] Add experimental, opt-in component caching Closes #234. Integrates components into Rails' template digest tree rather than reimplementing static analysis, so a `<% cache %>` block wrapping a component is invalidated when the component changes. Opt in per component with `include ViewComponent::ExperimentallyCacheable`. Adding `cache_on` caches the component's own rendered output, and `.cache_digest` exposes the digest outside a request. The API is experimental and may change or be removed in a non-major release. Co-authored-by: Reegan Viljoen Co-authored-by: Godfrey Chan Co-authored-by: JWShuff Co-authored-by: timburgan Co-authored-by: Patrick Arnett --- docs/CHANGELOG.md | 24 ++ docs/api.md | 65 +++- docs/guide/caching.md | 174 ++++++++++ lib/view_component.rb | 2 + lib/view_component/cache_digest.rb | 180 ++++++++++ .../cache_digest/dependency_tracking.rb | 35 ++ lib/view_component/cache_digest/resolver.rb | 140 ++++++++ lib/view_component/errors.rb | 22 ++ .../experimentally_cacheable.rb | 191 ++++++++++ .../cacheable_child_component.html.erb | 1 + .../components/cacheable_child_component.rb | 5 + .../components/cacheable_component.html.erb | 1 + .../app/components/cacheable_component.rb | 15 + ...ble_explicit_dependency_component.html.erb | 1 + ...cacheable_explicit_dependency_component.rb | 17 + .../cacheable_parent_component.html.erb | 1 + .../components/cacheable_parent_component.rb | 6 + .../cacheable_subclass_component.rb | 6 + .../cached_component.html.erb | 3 + .../cached_nested_component.html.erb | 3 + test/sandbox/config/routes.rb | 2 + ...perimentally_cacheable_integration_test.rb | 104 ++++++ .../test/experimentally_cacheable_test.rb | 327 ++++++++++++++++++ test/test_helper.rb | 21 ++ 24 files changed, 1332 insertions(+), 14 deletions(-) create mode 100644 docs/guide/caching.md create mode 100644 lib/view_component/cache_digest.rb create mode 100644 lib/view_component/cache_digest/dependency_tracking.rb create mode 100644 lib/view_component/cache_digest/resolver.rb create mode 100644 lib/view_component/experimentally_cacheable.rb create mode 100644 test/sandbox/app/components/cacheable_child_component.html.erb create mode 100644 test/sandbox/app/components/cacheable_child_component.rb create mode 100644 test/sandbox/app/components/cacheable_component.html.erb create mode 100644 test/sandbox/app/components/cacheable_component.rb create mode 100644 test/sandbox/app/components/cacheable_explicit_dependency_component.html.erb create mode 100644 test/sandbox/app/components/cacheable_explicit_dependency_component.rb create mode 100644 test/sandbox/app/components/cacheable_parent_component.html.erb create mode 100644 test/sandbox/app/components/cacheable_parent_component.rb create mode 100644 test/sandbox/app/components/cacheable_subclass_component.rb create mode 100644 test/sandbox/app/views/integration_examples/cached_component.html.erb create mode 100644 test/sandbox/app/views/integration_examples/cached_nested_component.html.erb create mode 100644 test/sandbox/test/experimentally_cacheable_integration_test.rb create mode 100644 test/sandbox/test/experimentally_cacheable_test.rb diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 9f95415fb..1ae03eff0 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -10,6 +10,30 @@ nav_order: 6 ## main +* Add experimental caching support, opt-in per component via `include ViewComponent::ExperimentallyCacheable`. + + Components have never participated in Rails' template digests, so a `<% cache %>` block wrapping `render MyComponent.new` was never invalidated when the component changed ([#234](https://github.com/ViewComponent/view_component/issues/234), open since 2020). + + Including the module registers the component with Rails' own `ActionView::Digestor`, so fragment caches are invalidated when the component's template, Ruby class, sidecar files, superclasses, child components, or rendered partials change. Adding `cache_on` caches the component's own rendered output, and `.cache_digest` exposes the digest for use outside a request. + + ```ruby + class MessageComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + cache_on :message + + def initialize(message:) + @message = message + end + end + ``` + + **This API is experimental and may change or be removed in a non-major release.** It's shipping opt-in and per-component precisely so we can iterate on it in response to real-world use. **Please try it and tell us what breaks, what's missing, and what feels wrong in [#234](https://github.com/ViewComponent/view_component/issues/234).** We're especially interested in feedback on: whether `cache_on` is the right shape for declaring cache keys, how the feature behaves with slots and content blocks, and whether the `# Template Dependency:` escape hatch is sufficient for dynamic renders. See [the caching guide](https://viewcomponent.org/guide/caching.html) for details and known caveats. + + This work builds directly on prior art from the community: the `cache_on` API and the case for component-local caching come from [#2126](https://github.com/ViewComponent/view_component/pull/2126) by *Reegan Viljoen*; the approach of integrating with Rails' digest tree rather than reimplementing it comes from [`view_component-cache_digest`](https://github.com/tildeio/view_component-cache_digest) by *Godfrey Chan*; the invalidation cases it's tested against were contributed by *JWShuff* and *timburgan*, drawing on [`view_component-fragment_caching`](https://github.com/patrickarnett/view_component-fragment_caching) by *Patrick Arnett*. The issue was opened and researched by *ozzyaaron*, *pinzonjulian*, and *Derek Kniffin*, and the digest workaround that surfaced the superclass gap came from *cannikin* and *rnestler*. + + *Reegan Viljoen*, *Godfrey Chan*, *JWShuff*, *timburgan*, *Patrick Arnett*, *ozzyaaron*, *pinzonjulian*, *Derek Kniffin*, *cannikin*, *rnestler*, *Joel Hawksley* + ## 4.13.0 * Add support for Turbo-streaming ViewComponents. diff --git a/docs/api.md b/docs/api.md index 94aebc8d2..0be2d06c2 100644 --- a/docs/api.md +++ b/docs/api.md @@ -10,6 +10,15 @@ nav_order: 3 ## Class methods +### `.after_compile` + +Hook called by the compiler after a component is compiled. + +Extensions can override this class method to run logic after +compilation (e.g., generate helpers, register metadata, etc.). + +By default, this is a no-op. + ### `.config` → [ActiveSupport::OrderedOptions] Returns the current config. @@ -18,11 +27,6 @@ Returns the current config. The file path of the component Ruby file. -### `.new(...)` - -Redefine `new` so we can pre-allocate instance variables to optimize -for Ruby object shapes. - ### `.sidecar_files(extensions)` Find sidecar files for the given extensions. @@ -64,6 +68,11 @@ with_collection_parameter :item ## Instance methods +### `#around_render` → [void] + +Called around rendering the component. Override to wrap the rendering of a +component in custom instrumentation, etc. + ### `#before_render` → [void] Called before rendering the component. Override to perform operations that @@ -86,11 +95,21 @@ that inhibits encapsulation & reuse, often making testing difficult. Returns the value of attribute current_template. +### `#format` + +Rails expects us to define `format` on all renderables, +but we do not know the `format` of a ViewComponent until runtime. + ### `#helpers` → [ActionView::Base] A proxy through which to access helpers. Use sparingly as doing so introduces coupling that inhibits encapsulation & reuse, often making testing difficult. +### `#initialize` → [Base] + +Including `Rails.application.routes.url_helpers` defines an initializer that accepts (...), +so we have to define our own empty initializer to overwrite it. + ### `#output_postamble` → [String] Optional content to be returned after the rendered template. @@ -103,7 +122,7 @@ Optional content to be returned before the rendered template. Override to determine whether the ViewComponent should render. -### `#render_in(view_context, &block)` → [String] +### `#render_in(view_context, **, &block)` → [String] Entrypoint for rendering components. @@ -311,13 +330,6 @@ render_preview(:default) assert_text("Hello, World!") ``` -Note: `#rendered_preview` expects a preview to be defined with the same class -name as the calling test, but with `Test` replaced with `Preview`: - -MyComponentTest -> MyComponentPreview etc. - -In RSpec, `Preview` is appended to `described_class`. - ### `#rendered_content` → [ActionView::OutputBuffer] Returns the result of a render_inline call. @@ -365,6 +377,11 @@ test "component does not render in Firefox" do end ``` +### `#vc_test_view_context` → [ActionView::Base] + +Returns the view context used to render components in tests. Note that the view context +is reset after each call to `render_inline`. + ### `#with_controller_class(klass)` Set the controller to be used while executing the given block, @@ -386,7 +403,7 @@ with_format(:json) do end ``` -### `#with_request_url(full_path, host: nil, method: nil)` +### `#with_request_url(full_path, host: nil, method: nil, protocol: nil)` Set the URL of the current request (such as when using request-dependent path helpers): @@ -412,6 +429,14 @@ with_request_url("/users/42", method: "POST") do end ``` +To specify a protocol, pass the protocol param: + +```ruby +with_request_url("/users/42", protocol: :https) do + render_inline(MyComponent.new) +end +``` + ### `#with_variant(*variants)` Set the Action Pack request variant for the given block: @@ -430,6 +455,12 @@ A method called 'SETTER_METHOD_NAME' already exists and would be overwritten by Please choose a different setter name. +### `CacheDigestTemplateError` + +The synthetic cache digest template for COMPONENT was rendered. + +This template exists only so Rails can compute a cache digest for the component and is never meant to be rendered. Render the component itself instead. + ### `ContentAlreadySetForPolymorphicSlotError` Content for slot SLOT_NAME has already been provided. @@ -549,3 +580,9 @@ It's sometimes possible to fix this issue by moving code dependent on `#translat COMPONENT declares a slot named SLOT_NAME, which is an uncountable word To fix this issue, choose a different name. + +### `UndefinedCacheKeyMethodError` + +`cache_on` declared `METHOD` on COMPONENT, but no such method is defined. + +To fix this issue, define `METHOD` or remove it from `cache_on`. diff --git a/docs/guide/caching.md b/docs/guide/caching.md new file mode 100644 index 000000000..1e39bd06b --- /dev/null +++ b/docs/guide/caching.md @@ -0,0 +1,174 @@ +--- +layout: default +title: Caching +parent: How-to guide +--- + +# Caching + +Experimental +{: .label .label-yellow } + +Since 4.14.0 +{: .label } + +**This API is experimental.** It may change or be removed in a non-major release. +Please share feedback in [#234](https://github.com/ViewComponent/view_component/issues/234). + +Rails computes a digest for every template from its source and from the templates +it renders. That digest is mixed into the key of every `<% cache %>` block in the +template, so editing a partial invalidates the caches of everything that renders it. + +Components are invisible to that mechanism, which means this doesn't work: + +```erb +<% cache @post do %> + <%= render PostComponent.new(post: @post) %> +<% end %> +``` + +Editing `PostComponent`'s template, Ruby class, or sidecar files doesn't invalidate +the fragment, so the stale markup is served until the cache is cleared by hand. + +## Opting in + +Include `ViewComponent::ExperimentallyCacheable` in each component that should +participate in caching: + +```ruby +class PostComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + def initialize(post:) + @post = post + end +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 any of +the following change: + +| Change | Invalidates | +|---|---| +| The component's template | ✅ | +| The component's Ruby class | ✅ | +| A sidecar file (such as an i18n `.yml`) | ✅ | +| A superclass's template or Ruby class | ✅ | +| A child component rendered by the template | ✅ | +| A partial rendered by the template | ✅ | + +Components that don't include the module are unaffected, and applications that +never opt in pay no cost. + +## Caching a component's own output + +Use `cache_on` to have the component cache its own rendered output. Each argument +names a method whose value identifies a rendering of the component: + +```ruby +class PostComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + cache_on :post + + def initialize(post:) + @post = post + end + + private + + attr_reader :post +end +``` + +Rendering the component now reads from and writes to `Rails.cache`, with no +`<% cache %>` block at the call site: + +```erb +<%= render PostComponent.new(post: @post) %> +``` + +Private methods are allowed, so the values that form the key don't have to be +part of the component's public interface. + +The cache key combines: + +- the component's virtual path +- its digest, computed by Rails' `ActionView::Digestor` +- the requested format and variant +- the current `I18n.locale` +- the values returned by the `cache_on` methods + +Caching is skipped unless `perform_caching` is enabled on the controller, matching +the behavior of Rails' `cache` helper. Override `#cache_key` for full control. + +## Reading a component's digest + +`.cache_digest` returns the digest of everything the component renders from. It +works outside a request, where no view context exists: + +```ruby +PostComponent.cache_digest # => "a1b2c3..." +``` + +Use it to build cache keys by hand, or to key a cache in a background job: + +```erb +<% cache [@post, PostComponent.cache_digest] do %> + <%= render PostComponent.new(post: @post) %> +<% end %> +``` + +## Declaring dependencies static analysis can't see + +Dependencies are discovered by scanning template source, so dynamic renders are +invisible: + +```erb +<%= render @component %> +``` + +Declare these with Rails' `# Template Dependency:` comment, in either the Ruby +file or the template: + +```ruby +class PostComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + # Template Dependency: posts/byline +end +``` + +## Caveats + +**Content blocks aren't cached.** Content passed as a block isn't part of the +cache key, so caching it would risk serving one caller's content to another: + +```erb +<%# Not cached: the block's content isn't in the key %> +<%= render PostComponent.new(post: @post) do %> + Hello +<% end %> +``` + +To cache a component that takes content, include the values that determine that +content in `cache_on`, and set the content from within the component rather than +from the call site. + +**Slots have the same constraint.** Slot content set by the caller isn't part of +the key unless declared in `cache_on`. + +**`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. + +**Inline templates and `#call` methods** are digested through the component's Ruby +file, so edits to them invalidate correctly. Partials and components they render +aren't discovered, because there's no template source to scan; use +`# Template Dependency:` for those. + +**Included modules aren't tracked.** A component's superclasses are, but a module +included into a component isn't. Use `# Template Dependency:` or bump +`config.action_controller.perform_caching` cache versions on deploy. diff --git a/lib/view_component.rb b/lib/view_component.rb index b47469426..bb7ba1d42 100644 --- a/lib/view_component.rb +++ b/lib/view_component.rb @@ -8,10 +8,12 @@ module ViewComponent extend ActiveSupport::Autoload autoload :Base + autoload :CacheDigest autoload :Compiler autoload :CompileCache autoload :Config autoload :Deprecation + autoload :ExperimentallyCacheable autoload :InlineTemplate autoload :Instrumentation autoload :Preview diff --git a/lib/view_component/cache_digest.rb b/lib/view_component/cache_digest.rb new file mode 100644 index 000000000..53ee88aa9 --- /dev/null +++ b/lib/view_component/cache_digest.rb @@ -0,0 +1,180 @@ +# frozen_string_literal: true + +require "active_support/dependencies/autoload" +require "action_view/digestor" + +module ViewComponent + # Integrates ViewComponents into Rails' template digest tree. + # + # Rails computes a digest for every template from its source and the templates + # it renders. That digest is mixed into the key of every `<% cache %>` block in + # the template, so editing a partial busts the caches of everything that + # renders it. + # + # Components are invisible to that mechanism for two reasons: + # + # 1. **Discovery** — `ActionView::DependencyTracker` doesn't recognize + # `render SomeComponent.new(...)` as a dependency. + # 2. **Resolution** — component templates live outside the view paths, and a + # component's rendered output depends on its Ruby class and sidecar files, + # 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. + # + # @private + module CacheDigest + extend ActiveSupport::Autoload + + autoload :DependencyTracking + autoload :Resolver + + # Prefix for the synthetic virtual paths components are digested under. + # + # Namespaced under `view_component/` so it can't collide with an + # application partial. + VIRTUAL_PATH_PREFIX = "view_component/cache_digest" + + # Matches `render FooComponent`, `render(Foo::BarComponent.new(...))`, + # `render FooComponent.with_collection(...)`, etc. + # + # Deliberately a plain source scan rather than a tracker-specific hook: it + # behaves identically for the ERB tracker, the Prism-based Ruby tracker, and + # third-party Haml/Slim trackers. + RENDER_CALL = / + \brender(?:_to_string)?\b # render or render_to_string + \s*\(?\s* # optional opening paren + (? + (?:::)?[A-Z]\w* # a constant + (?:::[A-Z]\w*)* # optionally namespaced + ) + /x + + class << self + # Virtual paths of components that have opted into caching, mapped to + # their class names. + # + # Class *names* rather than class objects so the registry survives + # autoloader reloads without pinning stale constants in memory. + # + # @return [Hash{String => String}] + def registry + @registry ||= {} + end + + # @return [Boolean] whether any component has opted in. + def enabled? + !registry.empty? + end + + # @private + def register(component) + return unless component.virtual_path && component.name + + registry[component.virtual_path] = component.name + end + + # The synthetic virtual path a component is digested under. + # + # @return [String, nil] + def virtual_path_for(component) + return unless component.respond_to?(:virtual_path) && component.virtual_path + + "#{VIRTUAL_PATH_PREFIX}/#{component.virtual_path}" + end + + # Resolve a synthetic virtual path back to the component that owns it. + # + # @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 + + constantize_component(name) + end + + # Scan a template's source for renders of cacheable components. + # + # Called for every template Rails digests, so it exits early when the + # feature is unused. + # + # @return [Array] synthetic virtual paths + def dependencies_in(template) + return [] unless enabled? + + source = template.source + return [] unless source.is_a?(String) && source.include?("render") + + source.scan(RENDER_CALL).flatten.uniq.filter_map do |constant_name| + component = constantize_component(constant_name) + virtual_path_for(component) if component + end + end + + # Compute the digest of a component using Rails' digest tree. + # + # @param component [Class] a component that includes `ExperimentallyCacheable` + # @param finder [ActionView::LookupContext] + # @param format [Symbol] + # @return [String] + def digest(component, finder: default_finder, format: :html) + virtual_path = virtual_path_for(component) + return "" unless virtual_path + + ActionView::Digestor.digest(name: virtual_path, format: format, finder: finder) + end + + # A lookup context for digesting components outside a request, where no + # view context (and therefore no finder) exists. + # + # @return [ActionView::LookupContext] + def default_finder + # Not memoized across reloads: view paths change when the app reloads. + ActionView::LookupContext.new(ActionController::Base.view_paths) + end + + # Wire the tracker and resolver into Action View. + # + # Idempotent, and called the first time a component includes + # `ExperimentallyCacheable`. Both hooks short-circuit while the registry + # is empty, so applications that never opt in are unaffected. + # + # @private + def install! + return if @installed + + @installed = true + + DependencyTracking.install! + + ActiveSupport.on_load(:action_controller_base) do + resolver = ViewComponent::CacheDigest::Resolver.instance + + append_view_path(resolver) unless view_paths.include?(resolver) + end + end + + private + + # Resolve a constant name to a component that opted into caching. + # + # 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? + + component + rescue + # Never let digest computation break rendering. + nil + end + end + end +end diff --git a/lib/view_component/cache_digest/dependency_tracking.rb b/lib/view_component/cache_digest/dependency_tracking.rb new file mode 100644 index 000000000..a50900909 --- /dev/null +++ b/lib/view_component/cache_digest/dependency_tracking.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require "action_view/dependency_tracker" + +module ViewComponent + module CacheDigest + # Teaches `ActionView::DependencyTracker` to see components. + # + # Prepended to the tracker's singleton class rather than to a specific + # tracker implementation (`ERBTracker`, `RubyTracker`, or the trackers + # registered by the Haml and Slim gems). `find_dependencies` is the single + # seam every tracker flows through, so hooking it here works regardless of + # which handler a template uses and doesn't depend on tracker internals. + # + # @private + module DependencyTracking + def find_dependencies(name, template, view_paths = nil) + super + CacheDigest.dependencies_in(template) + rescue + # A broken digest is preferable to a broken render. Falling back to the + # dependencies Rails found on its own means the component simply isn't + # tracked, which is the pre-existing behavior. + super + end + + # @private + def self.install! + tracker = ActionView::DependencyTracker.singleton_class + return if tracker.include?(self) + + tracker.prepend(self) + end + end + end +end diff --git a/lib/view_component/cache_digest/resolver.rb b/lib/view_component/cache_digest/resolver.rb new file mode 100644 index 000000000..c83792ffc --- /dev/null +++ b/lib/view_component/cache_digest/resolver.rb @@ -0,0 +1,140 @@ +# frozen_string_literal: true + +module ViewComponent + module CacheDigest + # Synthesizes the templates Rails' `ActionView::Digestor` digests components from. + # + # Once `DependencyTracking` reports `view_component/cache_digest/foo_component` + # as a dependency, the Digestor tries to find a template at that path. No such + # file exists: a component's rendered output depends on its template *and* its + # Ruby class, its sidecar files, and its superclasses. + # + # This resolver answers with a synthetic template whose source encodes all of + # those inputs. The template is never compiled or rendered; the Digestor only + # reads `#source` to hash it and to scan it for further dependencies. + # + # @private + class Resolver < ActionView::Resolver + # Extensions whose contents are hashed into the synthetic source. + SIDECAR_EXTENSIONS = %w[yml yaml].freeze + + # `# Template Dependency: foo/bar` comments in a component's Ruby file, the + # escape hatch for dependencies static analysis can't see. + EXPLICIT_DEPENDENCY = /#\s*Template Dependency:\s*(\S+)/ + + class << self + def instance + @instance ||= new + end + end + + def find_templates(name, prefix, partial, details, locals = []) + virtual_path = [prefix.presence, name].compact.join("/") + component = CacheDigest.component_for(virtual_path) + return [] unless component + + [build_template(component, virtual_path, details)] + rescue + # Never let digest resolution break rendering. Returning no template + # makes the Digestor treat this as a missing node, which degrades to + # the behavior components have without this feature. + [] + end + + def to_s + "ViewComponent::CacheDigest::Resolver" + end + alias_method :to_path, :to_s + + def eql?(other) + self.class.equal?(other.class) + end + alias_method :==, :eql? + + private + + def build_template(component, virtual_path, details) + ActionView::Template.new( + source_for(component), + "view_component cache digest for #{component.name}", + ActionView::Template.handler_for_extension(:erb), + locals: [], + format: Array(details[:formats]).first || :html, + virtual_path: virtual_path + ) + end + + # The synthetic source. Every section exists to change this string when + # something the component renders from changes. + def source_for(component) + parts = [] + + # Safety net: this template should never be rendered, only digested. + parts << "<% raise ViewComponent::CacheDigestTemplateError.new(#{component.name.inspect}) %>" + + # Content hashes of the Ruby files and sidecar files backing the + # component and its component superclasses. Hashing rather than + # inlining keeps the source small and avoids embedding Ruby that a + # tracker might misread as a render call. + source_files(component).each do |path| + parts << "<%# Resolved Dependency: #{path} #{file_digest(path)} %>" + end + + # Dependencies declared with `# Template Dependency:` in the component's + # Ruby file. Re-emitted so the Digestor resolves them as tree nodes. + explicit_dependencies(component).each do |dependency| + parts << "<%# Template Dependency: #{dependency} %>" + end + + # Template sources verbatim, so trackers can discover the partials and + # components they render. + template_sources(component).each do |source| + parts << source + end + + parts.join("\n") + end + + # The component and any component superclasses, nearest first. Including + # ancestors means editing `ApplicationComponent` invalidates every + # component that inherits from it. + def component_ancestors(component) + component.ancestors.select do |ancestor| + ancestor.is_a?(Class) && + ancestor <= ViewComponent::Base && + ancestor != ViewComponent::Base + end + end + + def source_files(component) + component_ancestors(component).flat_map { |ancestor| + [ancestor.identifier, *ancestor.sidecar_files(SIDECAR_EXTENSIONS)] + }.compact.uniq.select { |path| ::File.exist?(path) } + end + + def template_files(component) + component_ancestors(component) + .flat_map { |ancestor| ancestor.sidecar_files(ActionView::Template::Handlers.extensions) } + .uniq + .select { |path| ::File.exist?(path) } + end + + def template_sources(component) + template_files(component).map { |path| ::File.read(path) } + end + + def explicit_dependencies(component) + component_ancestors(component).flat_map { |ancestor| + path = ancestor.identifier + next [] unless path && ::File.exist?(path) + + ::File.read(path).scan(EXPLICIT_DEPENDENCY).flatten + }.uniq + end + + def file_digest(path) + ActiveSupport::Digest.hexdigest(::File.read(path)) + end + end + end +end diff --git a/lib/view_component/errors.rb b/lib/view_component/errors.rb index 9b50254c0..73c072626 100644 --- a/lib/view_component/errors.rb +++ b/lib/view_component/errors.rb @@ -219,4 +219,26 @@ def initialize(setter_method_name, setter_name) super(MESSAGE.gsub("SETTER_METHOD_NAME", setter_method_name.to_s).gsub("SETTER_NAME", setter_name.to_s)) end end + + class CacheDigestTemplateError < StandardError + MESSAGE = + "The synthetic cache digest template for COMPONENT was rendered.\n\n" \ + "This template exists only so Rails can compute a cache digest for the " \ + "component and is never meant to be rendered. Render the component " \ + "itself instead.".freeze + + def initialize(component_name) + super(MESSAGE.gsub("COMPONENT", component_name.to_s)) + end + end + + class UndefinedCacheKeyMethodError < StandardError + MESSAGE = + "`cache_on` declared `METHOD` on COMPONENT, but no such method is defined.\n\n" \ + "To fix this issue, define `METHOD` or remove it from `cache_on`.".freeze + + def initialize(component_name, method_name) + super(MESSAGE.gsub("COMPONENT", component_name.to_s).gsub("METHOD", method_name.to_s)) + end + end end diff --git a/lib/view_component/experimentally_cacheable.rb b/lib/view_component/experimentally_cacheable.rb new file mode 100644 index 000000000..d308df01a --- /dev/null +++ b/lib/view_component/experimentally_cacheable.rb @@ -0,0 +1,191 @@ +# frozen_string_literal: true + +require "view_component/cache_digest" + +module ViewComponent + # Experimental caching support for ViewComponents. + # + # **This API is experimental.** It may change or be removed in a non-major + # release. Please share feedback in + # https://github.com/ViewComponent/view_component/issues/234. + # + # Including this module does two things: + # + # 1. Registers the component with Rails' template digest tree, so a + # `<% cache %>` block wrapping the component in a view is invalidated when + # the component's template, Ruby class, sidecar files, or child components + # change. + # 2. Enables the `cache_on` macro, which caches the component's own rendered + # output. + # + # ```ruby + # class MessageComponent < ViewComponent::Base + # include ViewComponent::ExperimentallyCacheable + # + # cache_on :message + # + # def initialize(message:) + # @message = message + # end + # end + # ``` + module ExperimentallyCacheable + extend ActiveSupport::Concern + + included do + ViewComponent::CacheDigest.install! + ViewComponent::CacheDigest.register(self) + end + + class_methods do + # Declare the values that identify a rendering of this component. + # + # Each argument names a method on the component whose value is mixed into + # the cache key, alongside a digest of the component's source. Private + # methods are allowed. + # + # ```ruby + # cache_on :message, :current_user + # ``` + # + # Calling `cache_on` opts the component into caching its own output. + # Without it, including this module only registers the component with + # Rails' digest tree. + # + # These methods are called before the component renders, so they can only + # depend on the component's own state, not on `helpers` or the view + # context. + # + # @param methods [Array] Methods whose values form the cache key. + # @return [void] + def cache_on(*methods) + @__vc_cache_on = __vc_cache_on | methods.map(&:to_sym) + end + + # @private + def __vc_cache_on + @__vc_cache_on ||= superclass.respond_to?(:__vc_cache_on) ? superclass.__vc_cache_on : [] + end + + # @private + def __vc_cacheable? + true + end + + # Whether this component caches its own rendered output. + # + # @return [Boolean] + def __vc_caches_output? + __vc_cache_on.any? + end + + # A digest of everything this component renders from: its template, its + # Ruby class, its sidecar files, its superclasses, and the components and + # partials it renders. + # + # Computed with Rails' own `ActionView::Digestor`, so it's the same digest + # used to invalidate `<% cache %>` blocks. + # + # Usable outside a request, where no view context exists: + # + # ```ruby + # MessageComponent.cache_digest + # ``` + # + # @param finder [ActionView::LookupContext] Defaults to a lookup context + # built from `ActionController::Base.view_paths`. + # @param format [Symbol] + # @return [String] + def cache_digest(finder: nil, format: :html) + ViewComponent::CacheDigest.digest( + self, + finder: finder || ViewComponent::CacheDigest.default_finder, + format: format + ) + end + + # @private + def inherited(child) + super + ViewComponent::CacheDigest.register(child) + end + end + + # Renders the component, reading from and writing to the Rails cache when + # `cache_on` has been declared. + # + # @private + def render_in(view_context, **, &block) + return super unless __vc_cache_enabled?(view_context, block) + + store = Rails.cache + key = cache_key(view_context) + + if (cached = store.read(key)) + # Safe to mark as HTML-safe: the cached string was produced by this same + # rendering pipeline, which escapes output before it's written. + return cached.html_safe # rubocop:disable Rails/OutputSafety + end + + super.tap do |output| + store.write(key, output.to_s) + end + end + + # The cache key for this rendering of the component. + # + # Combines the component's identity, its source digest, the requested + # format and variant, the current locale, and the values declared with + # `cache_on`. Override for full control. + # + # @param view_context [ActionView::Base] + # @return [String] + def cache_key(view_context = nil) + lookup_context = view_context&.lookup_context + + ActiveSupport::Cache.expand_cache_key( + [ + "view_component", + self.class.virtual_path, + self.class.cache_digest(finder: lookup_context, format: __vc_cache_format(lookup_context)), + __vc_cache_variant(lookup_context), + I18n.locale, + *__vc_cache_on_values + ].compact + ) + end + + private + + def __vc_cache_enabled?(view_context, block) + return false unless self.class.__vc_caches_output? + + # Content passed as a block isn't part of the cache key, so caching it + # would serve one caller's content to another. + return false if block + + return false unless defined?(Rails) && Rails.respond_to?(:cache) && Rails.cache + + controller = view_context.try(:controller) + controller.respond_to?(:perform_caching) && controller.perform_caching + end + + def __vc_cache_on_values + self.class.__vc_cache_on.map do |method_name| + unless respond_to?(method_name, true) + raise UndefinedCacheKeyMethodError.new(self.class.name, method_name) + end + + send(method_name) + end + end + + def __vc_cache_format(lookup_context) + Array(lookup_context&.formats).first || :html + end + + def __vc_cache_variant(lookup_context) + Array(lookup_context&.variants).first + end + end +end diff --git a/test/sandbox/app/components/cacheable_child_component.html.erb b/test/sandbox/app/components/cacheable_child_component.html.erb new file mode 100644 index 000000000..6d8b781ef --- /dev/null +++ b/test/sandbox/app/components/cacheable_child_component.html.erb @@ -0,0 +1 @@ +child diff --git a/test/sandbox/app/components/cacheable_child_component.rb b/test/sandbox/app/components/cacheable_child_component.rb new file mode 100644 index 000000000..d0af57230 --- /dev/null +++ b/test/sandbox/app/components/cacheable_child_component.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class CacheableChildComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable +end diff --git a/test/sandbox/app/components/cacheable_component.html.erb b/test/sandbox/app/components/cacheable_component.html.erb new file mode 100644 index 000000000..f247befc9 --- /dev/null +++ b/test/sandbox/app/components/cacheable_component.html.erb @@ -0,0 +1 @@ +
<%= title %>
diff --git a/test/sandbox/app/components/cacheable_component.rb b/test/sandbox/app/components/cacheable_component.rb new file mode 100644 index 000000000..27b40e079 --- /dev/null +++ b/test/sandbox/app/components/cacheable_component.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CacheableComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + cache_on :title + + def initialize(title:) + @title = title + end + + private + + attr_reader :title +end diff --git a/test/sandbox/app/components/cacheable_explicit_dependency_component.html.erb b/test/sandbox/app/components/cacheable_explicit_dependency_component.html.erb new file mode 100644 index 000000000..656d5ab5f --- /dev/null +++ b/test/sandbox/app/components/cacheable_explicit_dependency_component.html.erb @@ -0,0 +1 @@ +
<%= render partial %>
diff --git a/test/sandbox/app/components/cacheable_explicit_dependency_component.rb b/test/sandbox/app/components/cacheable_explicit_dependency_component.rb new file mode 100644 index 000000000..8ece5e067 --- /dev/null +++ b/test/sandbox/app/components/cacheable_explicit_dependency_component.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# Renders a partial that static analysis can't see, declared with the +# `# Template Dependency:` escape hatch. +class CacheableExplicitDependencyComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + # Template Dependency: integration_examples/erb_partial + + def initialize(partial: "integration_examples/erb_partial") + @partial = partial + end + + private + + attr_reader :partial +end diff --git a/test/sandbox/app/components/cacheable_parent_component.html.erb b/test/sandbox/app/components/cacheable_parent_component.html.erb new file mode 100644 index 000000000..f6f9acb63 --- /dev/null +++ b/test/sandbox/app/components/cacheable_parent_component.html.erb @@ -0,0 +1 @@ +
<%= render CacheableChildComponent.new %>
diff --git a/test/sandbox/app/components/cacheable_parent_component.rb b/test/sandbox/app/components/cacheable_parent_component.rb new file mode 100644 index 000000000..190a87402 --- /dev/null +++ b/test/sandbox/app/components/cacheable_parent_component.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +# Renders a child component, so changes to the child must invalidate the parent. +class CacheableParentComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable +end diff --git a/test/sandbox/app/components/cacheable_subclass_component.rb b/test/sandbox/app/components/cacheable_subclass_component.rb new file mode 100644 index 000000000..291a1bd07 --- /dev/null +++ b/test/sandbox/app/components/cacheable_subclass_component.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +# Inherits its template from CacheableComponent, so changes to the parent must +# invalidate this component's digest. +class CacheableSubclassComponent < CacheableComponent +end diff --git a/test/sandbox/app/views/integration_examples/cached_component.html.erb b/test/sandbox/app/views/integration_examples/cached_component.html.erb new file mode 100644 index 000000000..170e0a62b --- /dev/null +++ b/test/sandbox/app/views/integration_examples/cached_component.html.erb @@ -0,0 +1,3 @@ +<% cache "cached-component-fragment" do %> + <%= render CacheableComponent.new(title: "cached") %> +<% end %> diff --git a/test/sandbox/app/views/integration_examples/cached_nested_component.html.erb b/test/sandbox/app/views/integration_examples/cached_nested_component.html.erb new file mode 100644 index 000000000..b3d4f790d --- /dev/null +++ b/test/sandbox/app/views/integration_examples/cached_nested_component.html.erb @@ -0,0 +1,3 @@ +<% cache "cached-nested-component-fragment" do %> + <%= render CacheableParentComponent.new %> +<% end %> diff --git a/test/sandbox/config/routes.rb b/test/sandbox/config/routes.rb index 2d8fa470b..71b76dd95 100644 --- a/test/sandbox/config/routes.rb +++ b/test/sandbox/config/routes.rb @@ -26,6 +26,8 @@ get :link_to_helper, to: "integration_examples#link_to_helper" get :cached_capture, to: "integration_examples#cached_capture" 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 :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" diff --git a/test/sandbox/test/experimentally_cacheable_integration_test.rb b/test/sandbox/test/experimentally_cacheable_integration_test.rb new file mode 100644 index 000000000..2c08015de --- /dev/null +++ b/test/sandbox/test/experimentally_cacheable_integration_test.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +require "test_helper" + +# Proves the scenario from https://github.com/ViewComponent/view_component/issues/234: +# a `<% cache %>` block in a view that renders a component is invalidated when +# the component changes. +class ExperimentallyCacheableIntegrationTest < ActionDispatch::IntegrationTest + def setup + Rails.cache.clear + clear_digest_cache + ActionController::Base.perform_caching = true + end + + def teardown + ActionController::Base.perform_caching = false + Rails.cache.clear + clear_digest_cache + end + + def test_renders_a_component_inside_a_cache_block + get "/cached_component" + + assert_response :success + assert_select(".cacheable", text: "cached") + end + + def test_cache_block_is_invalidated_when_the_component_template_changes + get "/cached_component" + assert_select(".cacheable", text: "cached") + + modify_file "app/components/cacheable_component.html.erb", "
changed
\n" do + clear_digest_cache + with_new_cache do + get "/cached_component" + assert_select(".cacheable", text: "changed") + end + end + end + + def test_cache_block_is_invalidated_when_the_component_ruby_file_changes + get "/cached_component" + assert_select(".cacheable", text: "cached") + + before = fragment_digest_for("integration_examples/cached_component") + + original = File.read(Rails.root.join("app/components/cacheable_component.rb")) + modify_file "app/components/cacheable_component.rb", original + "\n# a comment\n" do + clear_digest_cache + + refute_equal before, fragment_digest_for("integration_examples/cached_component") + end + end + + def test_cache_block_is_invalidated_when_a_nested_component_changes + get "/cached_nested_component" + assert_select(".cacheable-child", text: "child") + + before = fragment_digest_for("integration_examples/cached_nested_component") + + modify_file "app/components/cacheable_child_component.html.erb", "changed\n" do + clear_digest_cache + + refute_equal before, fragment_digest_for("integration_examples/cached_nested_component") + end + end + + def test_cache_block_digest_is_unaffected_by_unrelated_components + before = fragment_digest_for("integration_examples/cached_component") + + modify_file "app/components/erb_component.html.erb", "
unrelated change
\n" do + clear_digest_cache + + assert_equal before, fragment_digest_for("integration_examples/cached_component") + end + end + + def test_component_output_is_cached_between_requests + get "/cached_component" + assert_select(".cacheable", text: "cached") + + # The component's own `cache_on` entry is written on first render. + component = CacheableComponent.new(title: "cached") + refute_nil Rails.cache.read(component.cache_key(view_context)) + end + + private + + # The digest Rails mixes into every `<% cache %>` key in the given template. + # + # Built from a fresh lookup context each time: an existing one holds its own + # digest cache, which a real request would never reuse across a code reload. + def fragment_digest_for(virtual_path) + ActionView::Digestor.digest( + name: virtual_path, + format: :html, + finder: ActionView::LookupContext.new(ActionController::Base.view_paths) + ) + end + + def view_context + ApplicationController.new.tap { |c| c.request = ActionDispatch::TestRequest.create }.view_context + end +end diff --git a/test/sandbox/test/experimentally_cacheable_test.rb b/test/sandbox/test/experimentally_cacheable_test.rb new file mode 100644 index 000000000..cfe7c49db --- /dev/null +++ b/test/sandbox/test/experimentally_cacheable_test.rb @@ -0,0 +1,327 @@ +# frozen_string_literal: true + +require "test_helper" + +class ExperimentallyCacheableTest < ViewComponent::TestCase + def setup + super + Rails.cache.clear + end + + def teardown + Rails.cache.clear + super + end + + def test_registers_component_with_the_digest_registry + assert_equal( + "CacheableComponent", + ViewComponent::CacheDigest.registry["cacheable_component"] + ) + end + + def test_component_is_marked_cacheable + assert_predicate CacheableComponent, :__vc_cacheable? + refute_respond_to ErbComponent, :__vc_cacheable? + end + + def test_cache_on_declares_key_methods + assert_equal [:title], CacheableComponent.__vc_cache_on + end + + def test_component_without_cache_on_does_not_cache_output + refute_predicate CacheableParentComponent, :__vc_caches_output? + assert_predicate CacheableComponent, :__vc_caches_output? + end + + def test_cache_digest_is_computable_outside_a_request + digest = CacheableComponent.cache_digest + + assert_kind_of String, digest + refute_empty digest + end + + def test_cache_digest_changes_when_the_template_changes + assert_digest_changes( + "app/components/cacheable_component.html.erb", + "
changed
\n" + ) { CacheableComponent.cache_digest } + end + + def test_cache_digest_changes_when_the_ruby_file_changes + original = File.read(Rails.root.join("app/components/cacheable_component.rb")) + + assert_digest_changes( + "app/components/cacheable_component.rb", + original + "\n# a comment\n" + ) { CacheableComponent.cache_digest } + end + + def test_cache_digest_changes_when_a_child_component_template_changes + assert_digest_changes( + "app/components/cacheable_child_component.html.erb", + "changed\n" + ) { CacheableParentComponent.cache_digest } + end + + def test_cache_digest_changes_when_a_child_component_ruby_file_changes + original = File.read(Rails.root.join("app/components/cacheable_child_component.rb")) + + assert_digest_changes( + "app/components/cacheable_child_component.rb", + original + "\n# a comment\n" + ) { CacheableParentComponent.cache_digest } + end + + def test_cache_digest_changes_when_a_superclass_template_changes + assert_digest_changes( + "app/components/cacheable_component.html.erb", + "
changed
\n" + ) { CacheableSubclassComponent.cache_digest } + end + + def test_cache_digest_changes_when_a_superclass_ruby_file_changes + original = File.read(Rails.root.join("app/components/cacheable_component.rb")) + + assert_digest_changes( + "app/components/cacheable_component.rb", + original + "\n# a comment\n" + ) { CacheableSubclassComponent.cache_digest } + end + + def test_cache_digest_changes_when_an_explicitly_declared_dependency_changes + assert_digest_changes( + "app/views/integration_examples/_erb_partial.html.erb", + "
changed partial
\n" + ) { CacheableExplicitDependencyComponent.cache_digest } + end + + def test_cache_digest_is_unaffected_by_unrelated_changes + clear_digest_cache + before = CacheableComponent.cache_digest + + modify_file "app/components/erb_component.html.erb", "
unrelated
\n" do + clear_digest_cache + + assert_equal before, CacheableComponent.cache_digest + end + end + + def test_digests_of_different_components_differ + refute_equal CacheableComponent.cache_digest, CacheableParentComponent.cache_digest + end + + def test_cache_key_includes_cache_on_values + a = CacheableComponent.new(title: "a").cache_key + b = CacheableComponent.new(title: "b").cache_key + + refute_equal a, b + end + + def test_cache_key_includes_the_digest + key = CacheableComponent.new(title: "a").cache_key + + assert_includes key, CacheableComponent.cache_digest + end + + def test_undefined_cache_on_method_raises + component = Class.new(CacheableComponent) do + cache_on :nonexistent + end + + error = assert_raises(ViewComponent::UndefinedCacheKeyMethodError) do + component.new(title: "a").cache_key + end + + assert_includes error.message, "nonexistent" + end + + def test_renders_normally_when_caching_is_disabled + render_inline(CacheableComponent.new(title: "hello")) + + assert_selector(".cacheable", text: "hello") + end + + def test_nothing_is_written_to_the_cache_when_caching_is_disabled + render_inline(CacheableComponent.new(title: "hello")) + + assert_nil Rails.cache.read(CacheableComponent.new(title: "hello").cache_key(vc_test_controller.view_context)) + end + + def test_output_is_served_from_the_cache_on_a_second_render + with_caching do + render_inline(CacheableComponent.new(title: "first")) + assert_selector(".cacheable", text: "first") + + # Change the template underneath a warm cache. The digest is memoized, so + # a second render of the same key must return the cached markup. + modify_file "app/components/cacheable_component.html.erb", "
ignored
\n" do + render_inline(CacheableComponent.new(title: "first")) + + assert_selector(".cacheable", text: "first") + end + end + end + + def test_different_cache_on_values_produce_different_output + with_caching do + render_inline(CacheableComponent.new(title: "one")) + assert_selector(".cacheable", text: "one") + + render_inline(CacheableComponent.new(title: "two")) + assert_selector(".cacheable", text: "two") + end + end + + def test_cached_output_is_html_safe + with_caching do + render_inline(CacheableComponent.new(title: "bold")) + first = page.native.to_html + + render_inline(CacheableComponent.new(title: "bold")) + + assert_equal first, page.native.to_html + assert_no_selector("b") + end + end + + def test_components_without_cache_on_are_not_output_cached + with_caching do + render_inline(CacheableParentComponent.new) + assert_selector(".cacheable-child", text: "child") + + modify_file "app/components/cacheable_child_component.html.erb", "changed\n" do + with_new_cache do + render_inline(CacheableParentComponent.new) + + assert_selector(".cacheable-child", text: "changed") + end + end + end + end + + def test_content_blocks_are_not_cached + component = Class.new(CacheableComponent) do + def self.name + "BlockCacheableComponent" + end + end + + with_caching do + # A block's content isn't part of the cache key, so caching is skipped + # rather than risk serving one caller's content to another. + instance = component.new(title: "a") + + refute instance.send(:__vc_cache_enabled?, vc_test_controller.view_context, proc { "content" }) + assert instance.send(:__vc_cache_enabled?, vc_test_controller.view_context, nil) + end + end + + def test_anonymous_components_are_not_registered + before = ViewComponent::CacheDigest.registry.dup + + Class.new(CacheableComponent) + + assert_equal before, ViewComponent::CacheDigest.registry + end + + def test_virtual_path_for_returns_nil_without_a_virtual_path + assert_nil ViewComponent::CacheDigest.virtual_path_for(Class.new(CacheableComponent)) + end + + def test_component_for_ignores_paths_outside_the_prefix + assert_nil ViewComponent::CacheDigest.component_for("integration_examples/cached_component") + end + + def test_component_for_returns_nil_for_unregistered_paths + assert_nil ViewComponent::CacheDigest.component_for("view_component/cache_digest/nope") + end + + def test_dependencies_are_not_scanned_for_sources_without_render + assert_empty ViewComponent::CacheDigest.dependencies_in(build_template("no calls here")) + end + + def test_dependencies_are_found_for_component_renders + assert_equal( + ["view_component/cache_digest/cacheable_component"], + ViewComponent::CacheDigest.dependencies_in(build_template("<%= render CacheableComponent.new(title: 'a') %>")) + ) + end + + def test_dependencies_ignore_components_that_did_not_opt_in + assert_empty ViewComponent::CacheDigest.dependencies_in(build_template("<%= render ErbComponent.new(message: 'a') %>")) + end + + def test_resolver_is_identified_by_class + resolver = ViewComponent::CacheDigest::Resolver.instance + + assert_equal "ViewComponent::CacheDigest::Resolver", resolver.to_s + assert_equal "ViewComponent::CacheDigest::Resolver", resolver.to_path + assert_equal resolver, ViewComponent::CacheDigest::Resolver.new + end + + def test_resolver_returns_no_template_when_synthesis_fails + resolver = ViewComponent::CacheDigest::Resolver.instance + + ViewComponent::CacheDigest.stub(:component_for, ->(_) { raise "boom" }) do + assert_empty resolver.find_templates("cacheable_component", "view_component/cache_digest", true, {}) + end + end + + def test_dependency_tracking_falls_back_when_scanning_fails + template = build_template("<%= render CacheableComponent.new(title: 'a') %>") + + ViewComponent::CacheDigest.stub(:dependencies_in, ->(_) { raise "boom" }) do + refute_includes( + ActionView::DependencyTracker.find_dependencies("some/template", template, []), + "view_component/cache_digest/cacheable_component" + ) + end + end + + def test_constantizing_swallows_unexpected_errors + Object.const_set(:BoomComponent, Class.new do + def self.__vc_cacheable? + raise ArgumentError + end + end) + + assert_nil ViewComponent::CacheDigest.send(:constantize_component, "BoomComponent") + ensure + Object.send(:remove_const, :BoomComponent) + end + + def test_install_is_idempotent + resolver_count = ActionController::Base.view_paths.count { |path| path.is_a?(ViewComponent::CacheDigest::Resolver) } + + ViewComponent::CacheDigest.install! + + assert_equal( + resolver_count, + ActionController::Base.view_paths.count { |path| path.is_a?(ViewComponent::CacheDigest::Resolver) } + ) + end + + private + + def build_template(source) + ActionView::Template.new( + source, + "test template", + ActionView::Template.handler_for_extension(:erb), + locals: [], + format: :html, + virtual_path: "test/template" + ) + end + + def with_caching + old_value = ActionController::Base.perform_caching + ActionController::Base.perform_caching = true + Rails.cache.clear + yield + ensure + ActionController::Base.perform_caching = old_value + Rails.cache.clear + end +end diff --git a/test/test_helper.rb b/test/test_helper.rb index 803843772..5284e86e7 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -168,6 +168,27 @@ def modify_file(file, content) end end +# Rails caches template digests globally and clears them from the reloader on +# each code reload. Tests that change files on disk have to do the same. +def clear_digest_cache + ActionView::LookupContext::DetailsKey.clear +end + +# Modify a file and assert the block's return value changes while it's modified, +# clearing Rails' digest cache the way a code reload would. +def assert_digest_changes(file, content) + clear_digest_cache + before = yield + + modify_file(file, content) do + clear_digest_cache + refute_equal before, yield, "Expected the digest to change when #{file} changed" + end + + clear_digest_cache + assert_equal before, yield, "Expected the digest to be restored when #{file} was restored" +end + def with_default_preview_layout(layout, &block) with_previews_option(:default_layout, layout, &block) end From 9787698b07bee86aa6b6ebe73b5e60f3d7bbb343 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 20 Aug 2026 11:15:19 -0600 Subject: [PATCH 02/12] Track dependencies expressed in Ruby, not just in templates Resolves both limitations documented by view_component-cache_digest. Inheritance: superclass templates and Ruby files were already tracked, since the resolver holds the component class and can walk its ancestors. Ruby files: inline template sources are now embedded in the synthetic digest template, and component renders in Ruby (`#call` methods, helper methods) are emitted as explicit dependencies. Previously these were invisible because Action View's trackers only read template files. The one remaining case is a partial referenced by string path from a `#call` method, which still needs `# Template Dependency:`. Discovering it would mean reimplementing Rails' partial static analysis for Ruby. --- docs/CHANGELOG.md | 2 +- docs/guide/caching.md | 25 ++++++---- lib/view_component/cache_digest.rb | 9 +++- lib/view_component/cache_digest/resolver.rb | 36 ++++++++++---- .../components/cacheable_call_component.rb | 10 ++++ .../cacheable_call_partial_component.rb | 9 ++++ .../cacheable_inline_partial_component.rb | 9 ++++ .../cacheable_inline_template_component.rb | 10 ++++ .../test/experimentally_cacheable_test.rb | 47 +++++++++++++++++++ 9 files changed, 138 insertions(+), 19 deletions(-) create mode 100644 test/sandbox/app/components/cacheable_call_component.rb create mode 100644 test/sandbox/app/components/cacheable_call_partial_component.rb create mode 100644 test/sandbox/app/components/cacheable_inline_partial_component.rb create mode 100644 test/sandbox/app/components/cacheable_inline_template_component.rb diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 1ae03eff0..224f600c7 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -14,7 +14,7 @@ nav_order: 6 Components have never participated in Rails' template digests, so a `<% cache %>` block wrapping `render MyComponent.new` was never invalidated when the component changed ([#234](https://github.com/ViewComponent/view_component/issues/234), open since 2020). - Including the module registers the component with Rails' own `ActionView::Digestor`, so fragment caches are invalidated when the component's template, Ruby class, sidecar files, superclasses, child components, or rendered partials change. Adding `cache_on` caches the component's own rendered output, and `.cache_digest` exposes the digest for use outside a request. + Including the module registers the component with Rails' own `ActionView::Digestor`, so fragment caches are invalidated when the component's template, Ruby class, sidecar files, superclasses, child components, or rendered partials change — including children rendered from inline templates and `#call` methods. Adding `cache_on` caches the component's own rendered output, and `.cache_digest` exposes the digest for use outside a request. ```ruby class MessageComponent < ViewComponent::Base diff --git a/docs/guide/caching.md b/docs/guide/caching.md index 1e39bd06b..f9837659e 100644 --- a/docs/guide/caching.md +++ b/docs/guide/caching.md @@ -57,6 +57,9 @@ the following change: | A superclass's template or Ruby class | ✅ | | A child component rendered by the template | ✅ | | A partial rendered by the template | ✅ | +| A child component rendered by an inline template or `#call` method | ✅ | +| A partial rendered by an inline template | ✅ | +| A partial rendered by a `#call` method | ❌ [use the escape hatch](#declaring-dependencies-static-analysis-cant-see) | Components that don't include the module are unaffected, and applications that never opt in pay no cost. @@ -122,13 +125,22 @@ Use it to build cache keys by hand, or to key a cache in a background job: ## Declaring dependencies static analysis can't see -Dependencies are discovered by scanning template source, so dynamic renders are -invisible: +Dependencies are discovered by scanning template and Ruby source, so dynamic +renders are invisible: ```erb <%= render @component %> ``` +The same applies to a partial referenced by string path from a `#call` method, +which is the one dependency kind that isn't discovered automatically: + +```ruby +def call + render "posts/byline" # not tracked +end +``` + Declare these with Rails' `# Template Dependency:` comment, in either the Ruby file or the template: @@ -164,11 +176,6 @@ 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. -**Inline templates and `#call` methods** are digested through the component's Ruby -file, so edits to them invalidate correctly. Partials and components they render -aren't discovered, because there's no template source to scan; use -`# Template Dependency:` for those. - **Included modules aren't tracked.** A component's superclasses are, but a module -included into a component isn't. Use `# Template Dependency:` or bump -`config.action_controller.perform_caching` cache versions on deploy. +included into a component isn't, since a module has no template or source file of +its own to hash. Use `# Template Dependency:` for those. diff --git a/lib/view_component/cache_digest.rb b/lib/view_component/cache_digest.rb index 53ee88aa9..fe1d68bf6 100644 --- a/lib/view_component/cache_digest.rb +++ b/lib/view_component/cache_digest.rb @@ -106,7 +106,14 @@ def component_for(virtual_path) def dependencies_in(template) return [] unless enabled? - source = template.source + component_paths_in(template.source) + end + + # Scan arbitrary source (a template or a component's Ruby file) for + # renders of cacheable components. + # + # @return [Array] synthetic virtual paths + def component_paths_in(source) return [] unless source.is_a?(String) && source.include?("render") source.scan(RENDER_CALL).flatten.uniq.filter_map do |constant_name| diff --git a/lib/view_component/cache_digest/resolver.rb b/lib/view_component/cache_digest/resolver.rb index c83792ffc..0c13a9f07 100644 --- a/lib/view_component/cache_digest/resolver.rb +++ b/lib/view_component/cache_digest/resolver.rb @@ -81,8 +81,10 @@ def source_for(component) end # Dependencies declared with `# Template Dependency:` in the component's - # Ruby file. Re-emitted so the Digestor resolves them as tree nodes. - explicit_dependencies(component).each do |dependency| + # Ruby file, plus components rendered from Ruby rather than from a + # template (`#call` methods, helper methods). Re-emitted so the Digestor + # resolves them as tree nodes. + (explicit_dependencies(component) | ruby_component_dependencies(component)).each do |dependency| parts << "<%# Template Dependency: #{dependency} %>" end @@ -119,17 +121,35 @@ def template_files(component) .select { |path| ::File.exist?(path) } end + # Sidecar template files plus inline templates, which live in the Ruby + # file and so are invisible to Action View's trackers. def template_sources(component) - template_files(component).map { |path| ::File.read(path) } + sources = template_files(component).map { |path| ::File.read(path) } + + component_ancestors(component).each do |ancestor| + inline_template = ancestor.__vc_inline_template + sources << inline_template.source if inline_template + end + + sources.uniq end def explicit_dependencies(component) - component_ancestors(component).flat_map { |ancestor| - path = ancestor.identifier - next [] unless path && ::File.exist?(path) + ruby_sources(component).flat_map { |source| source.scan(EXPLICIT_DEPENDENCY).flatten }.uniq + end + + # Components rendered from Ruby code rather than from a template. Action + # View's trackers only read templates, so a `#call` method that renders + # another component would otherwise go unnoticed. + def ruby_component_dependencies(component) + ruby_sources(component).flat_map { |source| CacheDigest.component_paths_in(source) }.uniq + end - ::File.read(path).scan(EXPLICIT_DEPENDENCY).flatten - }.uniq + def ruby_sources(component) + component_ancestors(component).filter_map { |ancestor| + path = ancestor.identifier + ::File.read(path) if path && ::File.exist?(path) + } end def file_digest(path) diff --git a/test/sandbox/app/components/cacheable_call_component.rb b/test/sandbox/app/components/cacheable_call_component.rb new file mode 100644 index 000000000..ea1602814 --- /dev/null +++ b/test/sandbox/app/components/cacheable_call_component.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +# Renders a child component from a `call` method rather than a template. +class CacheableCallComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + def call + render CacheableChildComponent.new + end +end diff --git a/test/sandbox/app/components/cacheable_call_partial_component.rb b/test/sandbox/app/components/cacheable_call_partial_component.rb new file mode 100644 index 000000000..819f34439 --- /dev/null +++ b/test/sandbox/app/components/cacheable_call_partial_component.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class CacheableCallPartialComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + def call + render "integration_examples/erb_partial" + end +end diff --git a/test/sandbox/app/components/cacheable_inline_partial_component.rb b/test/sandbox/app/components/cacheable_inline_partial_component.rb new file mode 100644 index 000000000..db0759fc9 --- /dev/null +++ b/test/sandbox/app/components/cacheable_inline_partial_component.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class CacheableInlinePartialComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + erb_template <<~ERB +
<%= render "integration_examples/erb_partial" %>
+ ERB +end diff --git a/test/sandbox/app/components/cacheable_inline_template_component.rb b/test/sandbox/app/components/cacheable_inline_template_component.rb new file mode 100644 index 000000000..91bad5925 --- /dev/null +++ b/test/sandbox/app/components/cacheable_inline_template_component.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +# Renders a child component from an inline template rather than a sidecar file. +class CacheableInlineTemplateComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + erb_template <<~ERB +
<%= render CacheableChildComponent.new %>
+ ERB +end diff --git a/test/sandbox/test/experimentally_cacheable_test.rb b/test/sandbox/test/experimentally_cacheable_test.rb index cfe7c49db..1ff66fa99 100644 --- a/test/sandbox/test/experimentally_cacheable_test.rb +++ b/test/sandbox/test/experimentally_cacheable_test.rb @@ -96,6 +96,53 @@ def test_cache_digest_changes_when_an_explicitly_declared_dependency_changes ) { CacheableExplicitDependencyComponent.cache_digest } end + # Components rendered from an inline template are invisible to Action View's + # trackers, which only read template files. + def test_cache_digest_changes_when_a_child_of_an_inline_template_changes + assert_digest_changes( + "app/components/cacheable_child_component.html.erb", + "changed\n" + ) { CacheableInlineTemplateComponent.cache_digest } + end + + def test_cache_digest_changes_when_a_partial_of_an_inline_template_changes + assert_digest_changes( + "app/views/integration_examples/_erb_partial.html.erb", + "
changed partial
\n" + ) { CacheableInlinePartialComponent.cache_digest } + end + + # Components rendered from a `#call` method live in Ruby, not in a template. + def test_cache_digest_changes_when_a_child_of_a_call_method_changes + assert_digest_changes( + "app/components/cacheable_child_component.html.erb", + "changed\n" + ) { CacheableCallComponent.cache_digest } + end + + def test_cache_digest_changes_when_a_call_method_child_ruby_file_changes + original = File.read(Rails.root.join("app/components/cacheable_child_component.rb")) + + assert_digest_changes( + "app/components/cacheable_child_component.rb", + original + "\n# a comment\n" + ) { CacheableCallComponent.cache_digest } + end + + # The one dependency kind still requiring the escape hatch: a partial + # referenced by a string path from Ruby code. Discovering it would mean + # reimplementing Rails' partial static analysis for Ruby files. + def test_partials_rendered_from_call_methods_are_not_tracked + clear_digest_cache + before = CacheableCallPartialComponent.cache_digest + + modify_file "app/views/integration_examples/_erb_partial.html.erb", "
changed
\n" do + clear_digest_cache + + assert_equal before, CacheableCallPartialComponent.cache_digest + end + end + def test_cache_digest_is_unaffected_by_unrelated_changes clear_digest_cache before = CacheableComponent.cache_digest From 116ee51928ca15138c596a7d7188ff2623e8a34e Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 20 Aug 2026 11:19:35 -0600 Subject: [PATCH 03/12] Unwrap caching guide prose and replace invalidation table with a sentence --- docs/guide/caching.md | 75 +++++++++++-------------------------------- 1 file changed, 18 insertions(+), 57 deletions(-) diff --git a/docs/guide/caching.md b/docs/guide/caching.md index f9837659e..bc7e6fb26 100644 --- a/docs/guide/caching.md +++ b/docs/guide/caching.md @@ -12,12 +12,9 @@ Experimental Since 4.14.0 {: .label } -**This API is experimental.** It may change or be removed in a non-major release. -Please share feedback in [#234](https://github.com/ViewComponent/view_component/issues/234). +**This API is experimental.** It may change or be removed in a non-major release. Please share feedback in [#234](https://github.com/ViewComponent/view_component/issues/234). -Rails computes a digest for every template from its source and from the templates -it renders. That digest is mixed into the key of every `<% cache %>` block in the -template, so editing a partial invalidates the caches of everything that renders it. +Rails computes a digest for every template from its source and from the templates it renders. That digest is mixed into the key of every `<% cache %>` block in the template, so editing a partial invalidates the caches of everything that renders it. Components are invisible to that mechanism, which means this doesn't work: @@ -27,13 +24,11 @@ Components are invisible to that mechanism, which means this doesn't work: <% end %> ``` -Editing `PostComponent`'s template, Ruby class, or sidecar files doesn't invalidate -the fragment, so the stale markup is served until the cache is cleared by hand. +Editing `PostComponent`'s template, Ruby class, or sidecar files doesn't invalidate the fragment, so the stale markup is served until the cache is cleared by hand. ## Opting in -Include `ViewComponent::ExperimentallyCacheable` in each component that should -participate in caching: +Include `ViewComponent::ExperimentallyCacheable` in each component that should participate in caching: ```ruby class PostComponent < ViewComponent::Base @@ -45,29 +40,11 @@ class PostComponent < ViewComponent::Base 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 any of -the following change: - -| Change | Invalidates | -|---|---| -| The component's template | ✅ | -| The component's Ruby class | ✅ | -| A sidecar file (such as an i18n `.yml`) | ✅ | -| A superclass's template or Ruby class | ✅ | -| A child component rendered by the template | ✅ | -| A partial rendered by the template | ✅ | -| A child component rendered by an inline template or `#call` method | ✅ | -| A partial rendered by an inline template | ✅ | -| A partial rendered by a `#call` method | ❌ [use the escape hatch](#declaring-dependencies-static-analysis-cant-see) | - -Components that don't include the module are unaffected, and applications that -never opt in pay no cost. +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 children and partials rendered from an inline template and children rendered from a `#call` method. The one exception is a partial referenced by string path from a `#call` method, which needs [the escape hatch](#declaring-dependencies-static-analysis-cant-see). ## Caching a component's own output -Use `cache_on` to have the component cache its own rendered output. Each argument -names a method whose value identifies a rendering of the component: +Use `cache_on` to have the component cache its own rendered output. Each argument names a method whose value identifies a rendering of the component: ```ruby class PostComponent < ViewComponent::Base @@ -85,15 +62,13 @@ class PostComponent < ViewComponent::Base end ``` -Rendering the component now reads from and writes to `Rails.cache`, with no -`<% cache %>` block at the call site: +Rendering the component now reads from and writes to `Rails.cache`, with no `<% cache %>` block at the call site: ```erb <%= render PostComponent.new(post: @post) %> ``` -Private methods are allowed, so the values that form the key don't have to be -part of the component's public interface. +Private methods are allowed, so the values that form the key don't have to be part of the component's public interface. The cache key combines: @@ -103,13 +78,11 @@ The cache key combines: - the current `I18n.locale` - the values returned by the `cache_on` methods -Caching is skipped unless `perform_caching` is enabled on the controller, matching -the behavior of Rails' `cache` helper. Override `#cache_key` for full control. +Caching is skipped unless `perform_caching` is enabled on the controller, matching the behavior of Rails' `cache` helper. Override `#cache_key` for full control. ## Reading a component's digest -`.cache_digest` returns the digest of everything the component renders from. It -works outside a request, where no view context exists: +`.cache_digest` returns the digest of everything the component renders from. It works outside a request, where no view context exists: ```ruby PostComponent.cache_digest # => "a1b2c3..." @@ -125,15 +98,13 @@ Use it to build cache keys by hand, or to key a cache in a background job: ## Declaring dependencies static analysis can't see -Dependencies are discovered by scanning template and Ruby source, so dynamic -renders are invisible: +Dependencies are discovered by scanning template and Ruby source, so dynamic renders are invisible: ```erb <%= render @component %> ``` -The same applies to a partial referenced by string path from a `#call` method, -which is the one dependency kind that isn't discovered automatically: +The same applies to a partial referenced by string path from a `#call` method, which is the one dependency kind that isn't discovered automatically: ```ruby def call @@ -141,8 +112,7 @@ def call end ``` -Declare these with Rails' `# Template Dependency:` comment, in either the Ruby -file or the template: +Declare these with Rails' `# Template Dependency:` comment, in either the Ruby file or the template: ```ruby class PostComponent < ViewComponent::Base @@ -154,8 +124,7 @@ end ## Caveats -**Content blocks aren't cached.** Content passed as a block isn't part of the -cache key, so caching it would risk serving one caller's content to another: +**Content blocks aren't cached.** Content passed as a block isn't part of the cache key, so caching it would risk serving one caller's content to another: ```erb <%# Not cached: the block's content isn't in the key %> @@ -164,18 +133,10 @@ cache key, so caching it would risk serving one caller's content to another: <% end %> ``` -To cache a component that takes content, include the values that determine that -content in `cache_on`, and set the content from within the component rather than -from the call site. +To cache a component that takes content, include the values that determine that content in `cache_on`, and set the content from within the component rather than from the call site. -**Slots have the same constraint.** Slot content set by the caller isn't part of -the key unless declared in `cache_on`. +**Slots have the same constraint.** Slot content set by the caller isn't part of the key unless declared in `cache_on`. -**`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. +**`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. -**Included modules aren't tracked.** A component's superclasses are, but a module -included into a component isn't, since a module has no template or source file of -its own to hash. Use `# Template Dependency:` for those. +**Included modules aren't tracked.** A component's superclasses are, but a module included into a component isn't, since a module has no template or source file of its own to hash. Use `# Template Dependency:` for those. From f0c5feacc41aa2a4f170f8166260cce06bab6f37 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 20 Aug 2026 11:22:28 -0600 Subject: [PATCH 04/12] Clarify that cache_on removes the need for a wrapping cache block --- docs/guide/caching.md | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/docs/guide/caching.md b/docs/guide/caching.md index bc7e6fb26..f4ceb0438 100644 --- a/docs/guide/caching.md +++ b/docs/guide/caching.md @@ -44,7 +44,9 @@ That's all that's needed for the `<% cache %>` block above to work. The componen ## Caching a component's own output -Use `cache_on` to have the component cache its own rendered output. Each argument names a method whose value identifies a rendering of the component: +Everything above is about making a `<% cache %>` block that wraps a component behave correctly. `cache_on` does something different: the component caches itself, so **no `<% cache %>` block is needed anywhere**. + +Declare the methods whose values identify a rendering of the component: ```ruby class PostComponent < ViewComponent::Base @@ -62,12 +64,22 @@ class PostComponent < ViewComponent::Base end ``` -Rendering the component now reads from and writes to `Rails.cache`, with no `<% cache %>` block at the call site: +Every call site is now cached, with nothing to wrap and nothing to remember: ```erb <%= render PostComponent.new(post: @post) %> ``` +That single line is equivalent to writing this at every call site: + +```erb +<% cache [@post, PostComponent.cache_digest] do %> + <%= render PostComponent.new(post: @post) %> +<% end %> +``` + +Moving caching into the component means it can't be forgotten at one of a dozen call sites, and the cache key lives next to the state it's derived from. + Private methods are allowed, so the values that form the key don't have to be part of the component's public interface. The cache key combines: @@ -88,12 +100,12 @@ Caching is skipped unless `perform_caching` is enabled on the controller, matchi PostComponent.cache_digest # => "a1b2c3..." ``` -Use it to build cache keys by hand, or to key a cache in a background job: +Use it when a cache needs to be tied to a component's source but is written somewhere the component isn't rendered, such as a background job: -```erb -<% cache [@post, PostComponent.cache_digest] do %> - <%= render PostComponent.new(post: @post) %> -<% end %> +```ruby +Rails.cache.fetch(["post-summary", post, PostComponent.cache_digest]) do + expensive_summary_for(post) +end ``` ## Declaring dependencies static analysis can't see From ae85e83d62b5a520297adbefd92d53786f9b15fc Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 20 Aug 2026 11:36:31 -0600 Subject: [PATCH 05/12] Track partials rendered by string path from Ruby Removes the last dependency kind that needed the escape hatch. Runs Rails' own render parser -- the one RubyTracker uses on compiled templates -- over the component's Ruby source, rather than adding a second implementation of the same analysis. Results are narrowed to paths that appear verbatim in the source. That keeps string literals and discards the speculative entries the parser infers from dynamic renders (`render @thing` becomes `things/_thing`, `render FooComponent.new` becomes `news/_new`), which would resolve to nothing and only add "Couldn't find template for digesting" log noise. Components rendered from Ruby are already found precisely elsewhere. --- docs/CHANGELOG.md | 2 +- docs/guide/caching.md | 26 +++---- lib/view_component/cache_digest.rb | 40 ++++++++++ lib/view_component/cache_digest/resolver.rb | 27 ++++--- .../test/experimentally_cacheable_test.rb | 76 ++++++++++++++++--- 5 files changed, 134 insertions(+), 37 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 224f600c7..64a0aa242 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -14,7 +14,7 @@ nav_order: 6 Components have never participated in Rails' template digests, so a `<% cache %>` block wrapping `render MyComponent.new` was never invalidated when the component changed ([#234](https://github.com/ViewComponent/view_component/issues/234), open since 2020). - Including the module registers the component with Rails' own `ActionView::Digestor`, so fragment caches are invalidated when the component's template, Ruby class, sidecar files, superclasses, child components, or rendered partials change — including children rendered from inline templates and `#call` methods. Adding `cache_on` caches the component's own rendered output, and `.cache_digest` exposes the digest for use outside a request. + Including the module registers the component with Rails' own `ActionView::Digestor`, so fragment caches are invalidated when the component's template, Ruby class, sidecar files, superclasses, child components, or rendered partials change — including components and partials rendered from inline templates and `#call` methods. Adding `cache_on` caches the component's own rendered output, and `.cache_digest` exposes the digest for use outside a request. ```ruby class MessageComponent < ViewComponent::Base diff --git a/docs/guide/caching.md b/docs/guide/caching.md index f4ceb0438..981aafabf 100644 --- a/docs/guide/caching.md +++ b/docs/guide/caching.md @@ -16,7 +16,7 @@ Since 4.14.0 Rails computes a digest for every template from its source and from the templates it renders. That digest is mixed into the key of every `<% cache %>` block in the template, so editing a partial invalidates the caches of everything that renders it. -Components are invisible to that mechanism, which means this doesn't work: +Components are invisible to that mechanism. ```erb <% cache @post do %> @@ -40,13 +40,11 @@ class PostComponent < ViewComponent::Base 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 children and partials rendered from an inline template and children rendered from a `#call` method. The one exception is a partial referenced by string path from a `#call` method, which needs [the escape hatch](#declaring-dependencies-static-analysis-cant-see). +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. -## Caching a component's own output +## Self-caching -Everything above is about making a `<% cache %>` block that wraps a component behave correctly. `cache_on` does something different: the component caches itself, so **no `<% cache %>` block is needed anywhere**. - -Declare the methods whose values identify a rendering of the component: +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. ```ruby class PostComponent < ViewComponent::Base @@ -64,13 +62,13 @@ class PostComponent < ViewComponent::Base end ``` -Every call site is now cached, with nothing to wrap and nothing to remember: +Every call site is now cached automatically: ```erb <%= render PostComponent.new(post: @post) %> ``` -That single line is equivalent to writing this at every call site: +Which is equivalent to writing: ```erb <% cache [@post, PostComponent.cache_digest] do %> @@ -78,10 +76,6 @@ That single line is equivalent to writing this at every call site: <% end %> ``` -Moving caching into the component means it can't be forgotten at one of a dozen call sites, and the cache key lives next to the state it's derived from. - -Private methods are allowed, so the values that form the key don't have to be part of the component's public interface. - The cache key combines: - the component's virtual path @@ -90,7 +84,7 @@ The cache key combines: - the current `I18n.locale` - the values returned by the `cache_on` methods -Caching is skipped unless `perform_caching` is enabled on the controller, matching the behavior of Rails' `cache` helper. Override `#cache_key` for full control. +Caching is skipped unless `perform_caching` is enabled on the controller, matching the behavior of Rails' `cache` helper. ## Reading a component's digest @@ -110,17 +104,15 @@ end ## Declaring dependencies static analysis can't see -Dependencies are discovered by scanning template and Ruby source, so dynamic renders are invisible: +Dependencies are discovered by scanning template and Ruby source for literal references, so renders resolved at runtime are invisible: ```erb <%= render @component %> ``` -The same applies to a partial referenced by string path from a `#call` method, which is the one dependency kind that isn't discovered automatically: - ```ruby def call - render "posts/byline" # not tracked + render "posts/#{@post.style}" # interpolated, so not tracked end ``` diff --git a/lib/view_component/cache_digest.rb b/lib/view_component/cache_digest.rb index fe1d68bf6..a23429c39 100644 --- a/lib/view_component/cache_digest.rb +++ b/lib/view_component/cache_digest.rb @@ -2,6 +2,7 @@ require "active_support/dependencies/autoload" require "action_view/digestor" +require "action_view/render_parser" module ViewComponent # Integrates ViewComponents into Rails' template digest tree. @@ -122,6 +123,45 @@ def component_paths_in(source) end end + # Scan a component's Ruby source for partials referenced by string path, + # such as `render "posts/byline"` inside a `#call` method. + # + # Uses Rails' own render parser — the same one `RubyTracker` runs over + # compiled templates — rather than a second implementation of the same + # analysis. Its results are then narrowed to paths that appear verbatim in + # the source, which keeps string literals and discards the speculative + # `things/_thing` entries the parser infers from dynamic renders like + # `render @thing` or `render FooComponent.new`. Those would resolve to + # nothing and only add log noise; components rendered from Ruby are + # already found precisely by `component_paths_in`. + # + # @param source [String] Ruby source + # @param name [String] virtual path the source is being digested under + # @return [Array] partial virtual paths + def partial_paths_in(source, name) + return [] unless source.is_a?(String) && source.include?("render") + + render_parser.new(name, source).render_calls.uniq.select do |path| + source.include?(path) || source.include?(path.sub(%r{(\A|/)_}, '\1')) + end + rescue + # Never let digest computation break rendering. + [] + end + + # Rails 7.1 exposes the parser as a class; 7.2+ as a module with a + # `Default` implementation chosen from Prism or Ripper. + # + # @return [Class] + def render_parser + @render_parser ||= + if ActionView::RenderParser.is_a?(Class) + ActionView::RenderParser + else + ActionView::RenderParser::Default + end + end + # Compute the digest of a component using Rails' digest tree. # # @param component [Class] a component that includes `ExperimentallyCacheable` diff --git a/lib/view_component/cache_digest/resolver.rb b/lib/view_component/cache_digest/resolver.rb index 0c13a9f07..303c177c7 100644 --- a/lib/view_component/cache_digest/resolver.rb +++ b/lib/view_component/cache_digest/resolver.rb @@ -80,11 +80,11 @@ def source_for(component) parts << "<%# Resolved Dependency: #{path} #{file_digest(path)} %>" end - # Dependencies declared with `# Template Dependency:` in the component's - # Ruby file, plus components rendered from Ruby rather than from a - # template (`#call` methods, helper methods). Re-emitted so the Digestor - # resolves them as tree nodes. - (explicit_dependencies(component) | ruby_component_dependencies(component)).each do |dependency| + # Everything the component renders from Ruby rather than from a + # template: `# Template Dependency:` declarations, components rendered + # from `#call` methods, and partials referenced by string path. + # Re-emitted so the Digestor resolves them as tree nodes. + ruby_dependencies(component).each do |dependency| parts << "<%# Template Dependency: #{dependency} %>" end @@ -138,11 +138,18 @@ def explicit_dependencies(component) ruby_sources(component).flat_map { |source| source.scan(EXPLICIT_DEPENDENCY).flatten }.uniq end - # Components rendered from Ruby code rather than from a template. Action - # View's trackers only read templates, so a `#call` method that renders - # another component would otherwise go unnoticed. - def ruby_component_dependencies(component) - ruby_sources(component).flat_map { |source| CacheDigest.component_paths_in(source) }.uniq + # Everything a component renders from Ruby code rather than from a + # template. Action View's trackers only read templates, so a `#call` + # method that renders another component or a partial would otherwise go + # unnoticed. + def ruby_dependencies(component) + virtual_path = CacheDigest.virtual_path_for(component) + + explicit_dependencies(component) | + ruby_sources(component).flat_map { |source| + CacheDigest.component_paths_in(source) | + CacheDigest.partial_paths_in(source, virtual_path) + }.uniq end def ruby_sources(component) diff --git a/test/sandbox/test/experimentally_cacheable_test.rb b/test/sandbox/test/experimentally_cacheable_test.rb index 1ff66fa99..d3daa80df 100644 --- a/test/sandbox/test/experimentally_cacheable_test.rb +++ b/test/sandbox/test/experimentally_cacheable_test.rb @@ -129,17 +129,66 @@ def test_cache_digest_changes_when_a_call_method_child_ruby_file_changes ) { CacheableCallComponent.cache_digest } end - # The one dependency kind still requiring the escape hatch: a partial - # referenced by a string path from Ruby code. Discovering it would mean - # reimplementing Rails' partial static analysis for Ruby files. - def test_partials_rendered_from_call_methods_are_not_tracked - clear_digest_cache - before = CacheableCallPartialComponent.cache_digest + # Partials referenced by string path from Ruby are found with Rails' own + # render parser, the same one `RubyTracker` runs over compiled templates. + def test_cache_digest_changes_when_a_partial_of_a_call_method_changes + assert_digest_changes( + "app/views/integration_examples/_erb_partial.html.erb", + "
changed partial
\n" + ) { CacheableCallPartialComponent.cache_digest } + end - modify_file "app/views/integration_examples/_erb_partial.html.erb", "
changed
\n" do - clear_digest_cache + def test_partial_paths_are_extracted_from_ruby_source + source = <<~RUBY + def call + render "posts/byline" + end + RUBY - assert_equal before, CacheableCallPartialComponent.cache_digest + assert_equal( + ["posts/_byline"], + ViewComponent::CacheDigest.partial_paths_in(source, "view_component/cache_digest/post_component") + ) + end + + # The parser speculatively infers `things/_thing` from dynamic renders. Those + # resolve to nothing, so they're discarded rather than emitted as noise. + def test_speculative_partial_paths_are_discarded + source = <<~RUBY + def call + render @thing + render OtherComponent.new + render "bare_name" + end + RUBY + + assert_empty( + ViewComponent::CacheDigest.partial_paths_in(source, "view_component/cache_digest/post_component") + ) + end + + def test_partial_paths_are_not_extracted_from_sources_without_render + assert_empty ViewComponent::CacheDigest.partial_paths_in("def call; end", "a/b") + end + + def test_partial_path_extraction_swallows_parser_errors + ViewComponent::CacheDigest.stub(:render_parser, ->(*) { raise "boom" }) do + assert_empty ViewComponent::CacheDigest.partial_paths_in("render \"a/b\"", "a/b") + end + end + + # Rails 7.1 exposes the parser as a class, 7.2+ as a module with `Default`. + def test_render_parser_supports_both_action_view_shapes + with_reset_render_parser do + ActionView::RenderParser.stub(:is_a?, true) do + assert_equal ActionView::RenderParser, ViewComponent::CacheDigest.render_parser + end + end + + with_reset_render_parser do + ActionView::RenderParser.stub(:is_a?, false) do + assert_equal ActionView::RenderParser::Default, ViewComponent::CacheDigest.render_parser + end end end @@ -351,6 +400,15 @@ def test_install_is_idempotent private + def with_reset_render_parser + cache_digest = ViewComponent::CacheDigest + original = cache_digest.instance_variable_get(:@render_parser) + cache_digest.instance_variable_set(:@render_parser, nil) + yield + ensure + cache_digest.instance_variable_set(:@render_parser, original) + end + def build_template(source) ActionView::Template.new( source, From f12c6d18f238f5ceecaf358b08add9cfd62ffca0 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 20 Aug 2026 11:42:48 -0600 Subject: [PATCH 06/12] Let Template Dependency name a component class Rails' escape hatch takes a template path, but the path a component is digested under is an internal detail. Requiring `# Template Dependency: view_component/cache_digest/post_component` in application code would freeze that path as public API for an experimental feature. Component class names are now translated to the path the Digestor can resolve, so a dynamically rendered component is declared as `# Template Dependency: PostComponent`. The raw class name is replaced rather than added, since Rails would otherwise resolve it as a template path, find nothing, and log about it. Works in both Ruby files and templates. Document with examples for runtime-chosen components. --- docs/guide/caching.md | 32 ++++++++++++++++++- lib/view_component/cache_digest.rb | 23 +++++++++++++ .../cache_digest/dependency_tracking.rb | 13 +++++++- lib/view_component/cache_digest/resolver.rb | 16 +++++++--- .../cacheable_dynamic_constant_component.rb | 17 ++++++++++ .../test/experimentally_cacheable_test.rb | 32 +++++++++++++++++++ 6 files changed, 126 insertions(+), 7 deletions(-) create mode 100644 test/sandbox/app/components/cacheable_dynamic_constant_component.rb diff --git a/docs/guide/caching.md b/docs/guide/caching.md index 981aafabf..5322b737a 100644 --- a/docs/guide/caching.md +++ b/docs/guide/caching.md @@ -116,7 +116,7 @@ def call end ``` -Declare these with Rails' `# Template Dependency:` comment, in either the Ruby file or the template: +Declare these with Rails' `# Template Dependency:` comment, in either the Ruby file or the template. Partials are named by path: ```ruby class PostComponent < ViewComponent::Base @@ -126,6 +126,36 @@ class PostComponent < ViewComponent::Base end ``` +Components are named by class, listing each one the component might render: + +```ruby +class PostComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + # Template Dependency: PostSummaryComponent + # Template Dependency: PostDetailComponent + + def call + render(@detailed ? PostDetailComponent : PostSummaryComponent).new(post: @post) + end +end +``` + +The same works in a template, where the branch is often the more natural place for it: + +```erb +<% if params[:style] == "summary" %> + <%# Template Dependency: PostSummaryComponent %> + <% component = PostSummaryComponent %> +<% else %> + <%# Template Dependency: PostDetailComponent %> + <% component = PostDetailComponent %> +<% end %> +<%= 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. + ## Caveats **Content blocks aren't cached.** Content passed as a block isn't part of the cache key, so caching it would risk serving one caller's content to another: diff --git a/lib/view_component/cache_digest.rb b/lib/view_component/cache_digest.rb index a23429c39..d9738d980 100644 --- a/lib/view_component/cache_digest.rb +++ b/lib/view_component/cache_digest.rb @@ -53,6 +53,9 @@ module CacheDigest ) /x + # Rails' escape hatch for dependencies static analysis can't see. + EXPLICIT_DEPENDENCY = /#\s*Template Dependency:\s*(\S+)/ + class << self # Virtual paths of components that have opted into caching, mapped to # their class names. @@ -162,6 +165,26 @@ def render_parser end end + # Resolve `# Template Dependency: SomeComponent` declarations. + # + # Rails' escape hatch takes a template path, but the path a component is + # digested under is an internal detail. Naming the class instead keeps + # that detail out of application code, so `SomeComponent` is translated + # to the path the Digestor can resolve. + # + # @return [Array] pairs of declared name and + # synthetic virtual path + def explicit_component_dependencies(source) + return [] unless source.is_a?(String) && source.include?("Template Dependency:") + + source.scan(EXPLICIT_DEPENDENCY).flatten.uniq.filter_map do |declared| + next unless /\A(?:::)?[A-Z]/.match?(declared) + + component = constantize_component(declared) + [declared, virtual_path_for(component)] if component + end + end + # Compute the digest of a component using Rails' digest tree. # # @param component [Class] a component that includes `ExperimentallyCacheable` diff --git a/lib/view_component/cache_digest/dependency_tracking.rb b/lib/view_component/cache_digest/dependency_tracking.rb index a50900909..737616dde 100644 --- a/lib/view_component/cache_digest/dependency_tracking.rb +++ b/lib/view_component/cache_digest/dependency_tracking.rb @@ -15,7 +15,18 @@ module CacheDigest # @private module DependencyTracking def find_dependencies(name, template, view_paths = nil) - super + CacheDigest.dependencies_in(template) + dependencies = super + source = template.source + + # `# Template Dependency: SomeComponent` names a class, which Rails + # would resolve as a template path and never find. Swap it for the path + # the component is digested under, so the declaration resolves instead + # of becoming a missing node. + CacheDigest.explicit_component_dependencies(source).each do |declared, virtual_path| + dependencies = dependencies - [declared] + [virtual_path] + end + + dependencies + CacheDigest.dependencies_in(template) rescue # A broken digest is preferable to a broken render. Falling back to the # dependencies Rails found on its own means the component simply isn't diff --git a/lib/view_component/cache_digest/resolver.rb b/lib/view_component/cache_digest/resolver.rb index 303c177c7..b8f88f1e3 100644 --- a/lib/view_component/cache_digest/resolver.rb +++ b/lib/view_component/cache_digest/resolver.rb @@ -18,10 +18,6 @@ class Resolver < ActionView::Resolver # Extensions whose contents are hashed into the synthetic source. SIDECAR_EXTENSIONS = %w[yml yaml].freeze - # `# Template Dependency: foo/bar` comments in a component's Ruby file, the - # escape hatch for dependencies static analysis can't see. - EXPLICIT_DEPENDENCY = /#\s*Template Dependency:\s*(\S+)/ - class << self def instance @instance ||= new @@ -135,7 +131,17 @@ def template_sources(component) end def explicit_dependencies(component) - ruby_sources(component).flat_map { |source| source.scan(EXPLICIT_DEPENDENCY).flatten }.uniq + ruby_sources(component).flat_map { |source| + declared = source.scan(CacheDigest::EXPLICIT_DEPENDENCY).flatten + + # Component class names are translated to the path they're digested + # under; anything else is a template path already. + CacheDigest.explicit_component_dependencies(source).each do |name, virtual_path| + declared = declared - [name] + [virtual_path] + end + + declared + }.uniq end # Everything a component renders from Ruby code rather than from a diff --git a/test/sandbox/app/components/cacheable_dynamic_constant_component.rb b/test/sandbox/app/components/cacheable_dynamic_constant_component.rb new file mode 100644 index 000000000..246c04fd1 --- /dev/null +++ b/test/sandbox/app/components/cacheable_dynamic_constant_component.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +# Same as CacheableDynamicComponent, but naming the component class directly +# rather than its internal digest path. +class CacheableDynamicConstantComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + # Template Dependency: CacheableChildComponent + + def initialize(component: CacheableChildComponent) + @component = component + end + + def call + render @component.new + end +end diff --git a/test/sandbox/test/experimentally_cacheable_test.rb b/test/sandbox/test/experimentally_cacheable_test.rb index d3daa80df..e06b42be8 100644 --- a/test/sandbox/test/experimentally_cacheable_test.rb +++ b/test/sandbox/test/experimentally_cacheable_test.rb @@ -96,6 +96,38 @@ def test_cache_digest_changes_when_an_explicitly_declared_dependency_changes ) { CacheableExplicitDependencyComponent.cache_digest } end + # A component rendered dynamically can be declared by class name, without + # naming the internal path it's digested under. + def test_cache_digest_changes_when_a_component_declared_by_class_name_changes + assert_digest_changes( + "app/components/cacheable_child_component.html.erb", + "changed\n" + ) { CacheableDynamicConstantComponent.cache_digest } + end + + def test_declared_component_class_names_resolve_to_the_component + template = build_template("<%# Template Dependency: CacheableChildComponent %>") + dependencies = ActionView::DependencyTracker.find_dependencies("test/template", template, []) + + assert_includes dependencies, "view_component/cache_digest/cacheable_child_component" + # The raw class name would resolve to nothing, so it's replaced rather than added. + refute_includes dependencies, "CacheableChildComponent" + end + + def test_declared_template_paths_are_left_alone + template = build_template("<%# Template Dependency: integration_examples/erb_partial %>") + dependencies = ActionView::DependencyTracker.find_dependencies("test/template", template, []) + + assert_includes dependencies, "integration_examples/erb_partial" + end + + def test_declared_names_that_are_not_cacheable_components_are_left_alone + assert_empty ViewComponent::CacheDigest.explicit_component_dependencies( + "# Template Dependency: ErbComponent" + ) + assert_empty ViewComponent::CacheDigest.explicit_component_dependencies("no declarations here") + end + # Components rendered from an inline template are invisible to Action View's # trackers, which only read template files. def test_cache_digest_changes_when_a_child_of_an_inline_template_changes From 66dcc783d86a5caf22ed1a3b2b3e0d1aa8bd1bf0 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 20 Aug 2026 11:53:11 -0600 Subject: [PATCH 07/12] Raise when content is passed to a self-caching component Caller-provided content isn't part of the cache key, so caching it risks serving one caller's content to another. Previously the cache was silently skipped, which is hard to notice: the component just quietly stops being cached. Passing a block or `with_content` to a component that declares `cache_on` now raises ContentPassedToCachedComponentError. Raised whether or not caching is enabled, so the conflict surfaces in development and test rather than only in production. Components without `cache_on` are unaffected and still accept content. --- docs/api.md | 8 +++++ docs/guide/caching.md | 10 +++--- lib/view_component/errors.rb | 13 +++++++ .../experimentally_cacheable.rb | 20 ++++++----- .../test/experimentally_cacheable_test.rb | 35 +++++++++++++------ 5 files changed, 63 insertions(+), 23 deletions(-) diff --git a/docs/api.md b/docs/api.md index 0be2d06c2..49b775ab3 100644 --- a/docs/api.md +++ b/docs/api.md @@ -465,6 +465,14 @@ This template exists only so Rails can compute a cache digest for the component 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`. + +Content provided by the caller isn't part of the cache key, so caching it 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`. + ### `ContentSlotNameError` COMPONENT declares a slot named content, which is a reserved word in ViewComponent. diff --git a/docs/guide/caching.md b/docs/guide/caching.md index 5322b737a..3c6770d23 100644 --- a/docs/guide/caching.md +++ b/docs/guide/caching.md @@ -158,18 +158,20 @@ Declared components must include `ViewComponent::ExperimentallyCacheable` themse ## Caveats -**Content blocks aren't cached.** Content passed as a block isn't part of the cache key, so caching it would risk serving one caller's content to another: +**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: ```erb -<%# Not cached: the block's content isn't in the key %> +<%# Raises ContentPassedToCachedComponentError %> <%= render PostComponent.new(post: @post) do %> Hello <% end %> ``` -To cache a component that takes content, include the values that determine that content in `cache_on`, and set the content from within the component rather than from the call site. +The error is raised whether or not caching is enabled, so the conflict surfaces in development and test rather than only in production. -**Slots have the same constraint.** Slot content set by the caller isn't part of the key unless declared 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 a `<% cache %>` block around them still invalidates correctly. + +**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`. **`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. diff --git a/lib/view_component/errors.rb b/lib/view_component/errors.rb index 73c072626..41ad5cf3e 100644 --- a/lib/view_component/errors.rb +++ b/lib/view_component/errors.rb @@ -241,4 +241,17 @@ def initialize(component_name, method_name) super(MESSAGE.gsub("COMPONENT", component_name.to_s).gsub("METHOD", method_name.to_s)) end end + + 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 " \ + "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 + + def initialize(component_name) + super(MESSAGE.gsub("COMPONENT", component_name.to_s)) + end + end end diff --git a/lib/view_component/experimentally_cacheable.rb b/lib/view_component/experimentally_cacheable.rb index d308df01a..c0c7887c7 100644 --- a/lib/view_component/experimentally_cacheable.rb +++ b/lib/view_component/experimentally_cacheable.rb @@ -116,7 +116,17 @@ def inherited(child) # # @private def render_in(view_context, **, &block) - return super unless __vc_cache_enabled?(view_context, block) + return super unless self.class.__vc_caches_output? + + # Content provided by the caller isn't part of the cache key, so caching + # 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? + raise ContentPassedToCachedComponentError.new(self.class.name) + end + + return super unless __vc_cache_enabled?(view_context) store = Rails.cache key = cache_key(view_context) @@ -157,13 +167,7 @@ def cache_key(view_context = nil) private - def __vc_cache_enabled?(view_context, block) - return false unless self.class.__vc_caches_output? - - # Content passed as a block isn't part of the cache key, so caching it - # would serve one caller's content to another. - return false if block - + def __vc_cache_enabled?(view_context) return false unless defined?(Rails) && Rails.respond_to?(:cache) && Rails.cache controller = view_context.try(:controller) diff --git a/test/sandbox/test/experimentally_cacheable_test.rb b/test/sandbox/test/experimentally_cacheable_test.rb index e06b42be8..2a79beced 100644 --- a/test/sandbox/test/experimentally_cacheable_test.rb +++ b/test/sandbox/test/experimentally_cacheable_test.rb @@ -328,23 +328,36 @@ def test_components_without_cache_on_are_not_output_cached end end - def test_content_blocks_are_not_cached - component = Class.new(CacheableComponent) do - def self.name - "BlockCacheableComponent" - end + def test_passing_a_block_to_a_cached_component_raises + error = assert_raises(ViewComponent::ContentPassedToCachedComponentError) do + render_inline(CacheableComponent.new(title: "a")) { "content" } end - with_caching do - # A block's content isn't part of the cache key, so caching is skipped - # rather than risk serving one caller's content to another. - instance = component.new(title: "a") + assert_includes error.message, "CacheableComponent" + end + + def test_passing_with_content_to_a_cached_component_raises + assert_raises(ViewComponent::ContentPassedToCachedComponentError) do + render_inline(CacheableComponent.new(title: "a").with_content("content")) + end + end - refute instance.send(:__vc_cache_enabled?, vc_test_controller.view_context, proc { "content" }) - assert instance.send(:__vc_cache_enabled?, vc_test_controller.view_context, nil) + # 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 + with_caching do + assert_raises(ViewComponent::ContentPassedToCachedComponentError) do + render_inline(CacheableComponent.new(title: "a")) { "content" } + end end end + def test_components_without_cache_on_still_accept_content + render_inline(CacheableParentComponent.new) { "content" } + + assert_selector(".cacheable-child", text: "child") + end + def test_anonymous_components_are_not_registered before = ViewComponent::CacheDigest.registry.dup From e86cfda777bd8c0f3cda95381a3aeb91174f606c Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 20 Aug 2026 11:56:33 -0600 Subject: [PATCH 08/12] 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. --- docs/api.md | 4 +-- docs/guide/caching.md | 25 ++++++++++++++--- lib/view_component/errors.rb | 4 +-- .../experimentally_cacheable.rb | 9 ++++++- .../cacheable_slot_component.html.erb | 4 +++ .../components/cacheable_slot_component.rb | 23 ++++++++++++++++ .../test/experimentally_cacheable_test.rb | 27 +++++++++++++++++++ 7 files changed, 88 insertions(+), 8 deletions(-) create mode 100644 test/sandbox/app/components/cacheable_slot_component.html.erb create mode 100644 test/sandbox/app/components/cacheable_slot_component.rb diff --git a/docs/api.md b/docs/api.md index 49b775ab3..b36d09576 100644 --- a/docs/api.md +++ b/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`. diff --git a/docs/guide/caching.md b/docs/guide/caching.md index 3c6770d23..c7f9131a5 100644 --- a/docs/guide/caching.md +++ b/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. diff --git a/lib/view_component/errors.rb b/lib/view_component/errors.rb index 41ad5cf3e..2322693ce 100644 --- a/lib/view_component/errors.rb +++ b/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 diff --git a/lib/view_component/experimentally_cacheable.rb b/lib/view_component/experimentally_cacheable.rb index c0c7887c7..65d5878e3 100644 --- a/lib/view_component/experimentally_cacheable.rb +++ b/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 diff --git a/test/sandbox/app/components/cacheable_slot_component.html.erb b/test/sandbox/app/components/cacheable_slot_component.html.erb new file mode 100644 index 000000000..26cb7f88d --- /dev/null +++ b/test/sandbox/app/components/cacheable_slot_component.html.erb @@ -0,0 +1,4 @@ +
+ <%= header %> + <%= title %> +
diff --git a/test/sandbox/app/components/cacheable_slot_component.rb b/test/sandbox/app/components/cacheable_slot_component.rb new file mode 100644 index 000000000..8949cad35 --- /dev/null +++ b/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 diff --git a/test/sandbox/test/experimentally_cacheable_test.rb b/test/sandbox/test/experimentally_cacheable_test.rb index 2a79beced..42830b6b3 100644 --- a/test/sandbox/test/experimentally_cacheable_test.rb +++ b/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 From 1892d6fb9340db94be3264f5db74241baa95ec69 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 20 Aug 2026 13:48:47 -0600 Subject: [PATCH 09/12] Document the content restriction and inheritance gotcha with cache_on --- docs/guide/caching.md | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/docs/guide/caching.md b/docs/guide/caching.md index c7f9131a5..7410ad892 100644 --- a/docs/guide/caching.md +++ b/docs/guide/caching.md @@ -86,6 +86,19 @@ The cache key combines: Caching is skipped unless `perform_caching` is enabled on the controller, matching the behavior of Rails' `cache` helper. +A component that declares `cache_on` can't accept content from its callers. A block, `with_content`, or a slot set by the caller isn't part of the cache key, so passing one raises `ContentPassedToCachedComponentError`: + +```erb +<%# Raises: the block's content isn't in the cache key %> +<%= render PostComponent.new(post: @post) do %> + 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. See [Caveats](#caveats) for how to restructure a component that needs to take content. + +`cache_on` is inherited, so declaring it on a base class opts every subclass into self-caching, and into that restriction. Declare it on the components that should cache themselves rather than on `ApplicationComponent`. + ## Reading a component's digest `.cache_digest` returns the digest of everything the component renders from. It works outside a request, where no view context exists: @@ -158,22 +171,15 @@ Declared components must include `ViewComponent::ExperimentallyCacheable` themse ## Caveats -**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: +**Self-caching components can't take content from their callers.** Besides a block, this covers `with_content` and slots set by the caller: ```erb -<%# Raises ContentPassedToCachedComponentError %> -<%= render PostComponent.new(post: @post) do %> - Hello -<% end %> - -<%# Also raises %> +<%# Also raises ContentPassedToCachedComponentError %> <%= 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. - 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 From c0acadf6afff338208c2b7f5a8f2c3a03fdaed26 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Thu, 20 Aug 2026 14:48:48 -0600 Subject: [PATCH 10/12] Fix CI failures on Rails 7.1 and main Four issues, all surfaced by jobs that can't run on this machine: - The render parser shape test hardcoded ActionView::RenderParser::Default, which exists only on Rails 7.2-8.1. Both 7.1 and main ship the parser as a class. The selection logic is now a pure function tested with doubles, so both shapes are covered on every version, and the result is resolved once into RENDER_PARSER at load time. - A test-ordering leak: `with_new_cache` compiles components against whatever is on disk, then restores the previous compile cache, so a component compiled while its template was modified kept rendering the modified output after the file was restored. Force a recompile. - Ractor readiness: replaced the memoized @render_parser and @instance with load-time constants, and dropped the @installed flag since both steps of `install!` are already indi `install!` are already indi `install!` are already indi `install!` are already indi `install!` are already indi ing whitespace in the caching guide. --- .audition-baseline.json | 1 + docs/guide/caching.md | 2 +- lib/view_component/cache_digest.rb | 33 +++++++------- lib/view_component/cache_digest/resolver.rb | 7 ++- .../test/experimentally_cacheable_test.rb | 44 +++++++++++-------- 5 files changed, 48 insertions(+), 39 deletions(-) diff --git a/.audition-baseline.json b/.audition-baseline.json index 82f873106..5b1e04999 100644 --- a/.audition-baseline.json +++ b/.audition-baseline.json @@ -1,5 +1,6 @@ { "class-level-state|lib/view_component/base.rb": 9, + "class-level-state|lib/view_component/cache_digest.rb": 1, "class-level-state|lib/view_component/preview.rb": 2, "class-variables|lib/view_component/base.rb": 2, "runtime-class-state|/Users/joelhawksley/.local/share/mise/installs/ruby/4.0.5/lib/ruby/4.0.0/delegate.rb": 1, diff --git a/docs/guide/caching.md b/docs/guide/caching.md index 7410ad892..fd7c724ce 100644 --- a/docs/guide/caching.md +++ b/docs/guide/caching.md @@ -16,7 +16,7 @@ Since 4.14.0 Rails computes a digest for every template from its source and from the templates it renders. That digest is mixed into the key of every `<% cache %>` block in the template, so editing a partial invalidates the caches of everything that renders it. -Components are invisible to that mechanism. +Components are invisible to that mechanism. ```erb <% cache @post do %> diff --git a/lib/view_component/cache_digest.rb b/lib/view_component/cache_digest.rb index d9738d980..98694c8dc 100644 --- a/lib/view_component/cache_digest.rb +++ b/lib/view_component/cache_digest.rb @@ -55,7 +55,6 @@ module CacheDigest # Rails' escape hatch for dependencies static analysis can't see. EXPLICIT_DEPENDENCY = /#\s*Template Dependency:\s*(\S+)/ - class << self # Virtual paths of components that have opted into caching, mapped to # their class names. @@ -144,7 +143,7 @@ def component_paths_in(source) def partial_paths_in(source, name) return [] unless source.is_a?(String) && source.include?("render") - render_parser.new(name, source).render_calls.uniq.select do |path| + RENDER_PARSER.new(name, source).render_calls.uniq.select do |path| source.include?(path) || source.include?(path.sub(%r{(\A|/)_}, '\1')) end rescue @@ -152,17 +151,14 @@ def partial_paths_in(source, name) [] end - # Rails 7.1 exposes the parser as a class; 7.2+ as a module with a - # `Default` implementation chosen from Prism or Ripper. + # Action View has shipped its render parser as a class (Rails 7.1, and + # again on main) and as a module holding a `Default` implementation + # chosen from Prism or Ripper (Rails 7.2 through 8.1). # + # @param parser [Class, Module] `ActionView::RenderParser` # @return [Class] - def render_parser - @render_parser ||= - if ActionView::RenderParser.is_a?(Class) - ActionView::RenderParser - else - ActionView::RenderParser::Default - end + def resolve_render_parser(parser) + parser.is_a?(Class) ? parser : parser::Default end # Resolve `# Template Dependency: SomeComponent` declarations. @@ -209,16 +205,13 @@ def default_finder # Wire the tracker and resolver into Action View. # - # Idempotent, and called the first time a component includes - # `ExperimentallyCacheable`. Both hooks short-circuit while the registry - # is empty, so applications that never opt in are unaffected. + # Called each time a component includes `ExperimentallyCacheable`. Both + # steps below are individually idempotent, so no "already installed" flag + # is kept. Both hooks short-circuit while the registry is empty, so + # applications that never opt in are unaffected. # # @private def install! - return if @installed - - @installed = true - DependencyTracking.install! ActiveSupport.on_load(:action_controller_base) do @@ -246,5 +239,9 @@ def constantize_component(constant_name) nil end end + + # Resolved once at load time rather than memoized, so no class-level state + # is written after boot. + RENDER_PARSER = resolve_render_parser(ActionView::RenderParser) end end diff --git a/lib/view_component/cache_digest/resolver.rb b/lib/view_component/cache_digest/resolver.rb index b8f88f1e3..eca6198bf 100644 --- a/lib/view_component/cache_digest/resolver.rb +++ b/lib/view_component/cache_digest/resolver.rb @@ -20,7 +20,7 @@ class Resolver < ActionView::Resolver class << self def instance - @instance ||= new + INSTANCE end end @@ -168,6 +168,11 @@ def ruby_sources(component) def file_digest(path) ActiveSupport::Digest.hexdigest(::File.read(path)) end + + # Built once at load time rather than memoized, so no class-level state + # is written after boot. The resolver is stateless: it reads from disk on + # every call so it can't go stale when a component changes. + INSTANCE = new end end end diff --git a/test/sandbox/test/experimentally_cacheable_test.rb b/test/sandbox/test/experimentally_cacheable_test.rb index 42830b6b3..bb0566163 100644 --- a/test/sandbox/test/experimentally_cacheable_test.rb +++ b/test/sandbox/test/experimentally_cacheable_test.rb @@ -204,24 +204,28 @@ def test_partial_paths_are_not_extracted_from_sources_without_render end def test_partial_path_extraction_swallows_parser_errors - ViewComponent::CacheDigest.stub(:render_parser, ->(*) { raise "boom" }) do + ViewComponent::CacheDigest::RENDER_PARSER.stub(:new, ->(*) { raise "boom" }) do assert_empty ViewComponent::CacheDigest.partial_paths_in("render \"a/b\"", "a/b") end end - # Rails 7.1 exposes the parser as a class, 7.2+ as a module with `Default`. + # Action View has shipped the parser as a class (7.1, main) and as a module + # with a `Default` implementation (7.2 through 8.1). Exercised with doubles so + # both shapes are covered whichever version is running. def test_render_parser_supports_both_action_view_shapes - with_reset_render_parser do - ActionView::RenderParser.stub(:is_a?, true) do - assert_equal ActionView::RenderParser, ViewComponent::CacheDigest.render_parser - end - end + parser_class = Class.new - with_reset_render_parser do - ActionView::RenderParser.stub(:is_a?, false) do - assert_equal ActionView::RenderParser::Default, ViewComponent::CacheDigest.render_parser - end - end + assert_equal parser_class, ViewComponent::CacheDigest.resolve_render_parser(parser_class) + + default = Class.new + parser_module = Module.new + parser_module.const_set(:Default, default) + + assert_equal default, ViewComponent::CacheDigest.resolve_render_parser(parser_module) + end + + def test_render_parser_is_resolved_for_the_running_action_view + assert_respond_to ViewComponent::CacheDigest::RENDER_PARSER, :new end def test_cache_digest_is_unaffected_by_unrelated_changes @@ -326,6 +330,8 @@ def test_components_without_cache_on_are_not_output_cached end end end + ensure + recompile(CacheableChildComponent) end def test_passing_a_block_to_a_cached_component_raises @@ -472,13 +478,13 @@ def test_install_is_idempotent private - def with_reset_render_parser - cache_digest = ViewComponent::CacheDigest - original = cache_digest.instance_variable_get(:@render_parser) - cache_digest.instance_variable_set(:@render_parser, nil) - yield - ensure - cache_digest.instance_variable_set(:@render_parser, original) + # `with_new_cache` compiles components against whatever is on disk, then + # restores the previous compile cache on exit. A component compiled while its + # template was modified is therefore still registered as compiled, and keeps + # rendering the modified output after the file is restored. Force it back. + def recompile(component) + ViewComponent::CompileCache.cache.delete(component) + component.__vc_compile(force: true) end def build_template(source) From 1681063705f3ead3ca3340196cb476e9aaf59bc2 Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 21 Aug 2026 09:35:23 -0600 Subject: [PATCH 11/12] Add failing tests for review feedback on #2685 Reproduces the four issues @reeganviljoen reported: 1. Format is passed to `cache_digest` but never enters the cache key, so formats whose digests match share one entry. 2. The key array is compacted as a whole, so a nil in one `cache_on` position is indistinguishable from a nil in another. 3. There is no way to skip caching for a single render. 4. A block passed to `cache_on` is silently dropped, and a proc raises NoMethodError from `to_sym`. Two passing control tests are included so the fixes can't regress normal caching or the nil-vs-empty-string distinction. --- .../conditional_cache_probe_component.rb | 27 ++++ .../format_sensitive_cacheable_component.rb | 19 +++ .../positional_cache_key_component.rb | 22 ++++ .../experimentally_cacheable_review_test.rb | 117 ++++++++++++++++++ 4 files changed, 185 insertions(+) create mode 100644 test/sandbox/app/components/conditional_cache_probe_component.rb create mode 100644 test/sandbox/app/components/format_sensitive_cacheable_component.rb create mode 100644 test/sandbox/app/components/positional_cache_key_component.rb create mode 100644 test/sandbox/test/experimentally_cacheable_review_test.rb diff --git a/test/sandbox/app/components/conditional_cache_probe_component.rb b/test/sandbox/app/components/conditional_cache_probe_component.rb new file mode 100644 index 000000000..74a0f6e29 --- /dev/null +++ b/test/sandbox/app/components/conditional_cache_probe_component.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +# Counts renders so cache hits are observable. +class ConditionalCacheProbeComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + cache_on :identity + + class_attribute :render_count, default: 0 + + def initialize(cacheable:) + @cacheable = cacheable + end + + def call + self.class.render_count += 1 + "render-#{self.class.render_count}".html_safe + end + + private + + attr_reader :cacheable + + def identity + "constant" + end +end diff --git a/test/sandbox/app/components/format_sensitive_cacheable_component.rb b/test/sandbox/app/components/format_sensitive_cacheable_component.rb new file mode 100644 index 000000000..f37bb016a --- /dev/null +++ b/test/sandbox/app/components/format_sensitive_cacheable_component.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +# Renders the requested format, so a cache entry shared across formats is +# visible in the output. +class FormatSensitiveCacheableComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + cache_on :identity + + def call + view_context.lookup_context.formats.first.to_s.html_safe # rubocop:disable Rails/OutputSafety + end + + private + + def identity + "same-component" + end +end diff --git a/test/sandbox/app/components/positional_cache_key_component.rb b/test/sandbox/app/components/positional_cache_key_component.rb new file mode 100644 index 000000000..7ef549109 --- /dev/null +++ b/test/sandbox/app/components/positional_cache_key_component.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +# Two `cache_on` values where either may be nil, so positional collisions are +# visible in the key. +class PositionalCacheKeyComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + cache_on :first, :second + + def initialize(first:, second:) + @first = first + @second = second + end + + def call + "#{first}-#{second}".html_safe # rubocop:disable Rails/OutputSafety + end + + private + + attr_reader :first, :second +end diff --git a/test/sandbox/test/experimentally_cacheable_review_test.rb b/test/sandbox/test/experimentally_cacheable_review_test.rb new file mode 100644 index 000000000..0a61b2364 --- /dev/null +++ b/test/sandbox/test/experimentally_cacheable_review_test.rb @@ -0,0 +1,117 @@ +# frozen_string_literal: true + +require "test_helper" + +# Failing tests for the issues @reeganviljoen reported reviewing #2685. +# https://github.com/ViewComponent/view_component/pull/2685 +class ExperimentallyCacheableReviewTest < ViewComponent::TestCase + def setup + super + Rails.cache.clear + ConditionalCacheProbeComponent.render_count = 0 + end + + def teardown + Rails.cache.clear + super + end + + # Issue 1: format is passed to `cache_digest` but never enters the key itself. + # Two formats whose digests match therefore share one cache entry. + def test_formats_do_not_share_a_cache_entry + with_caching do + with_format(:html) do + render_inline(FormatSensitiveCacheableComponent.new) + assert_text "html" + end + + with_format(:json) do + render_inline(FormatSensitiveCacheableComponent.new) + + assert_text "json" + end + end + end + + def test_cache_key_differs_by_format + html_key = with_format(:html) { FormatSensitiveCacheableComponent.new.cache_key(vc_test_controller.view_context) } + json_key = with_format(:json) { FormatSensitiveCacheableComponent.new.cache_key(vc_test_controller.view_context) } + + refute_equal html_key, json_key + end + + # Issue 2: the key array is compacted as a whole, so a nil in one position is + # indistinguishable from a nil in another. + def test_nil_cache_on_values_do_not_collide_positionally + left = PositionalCacheKeyComponent.new(first: nil, second: "same") + right = PositionalCacheKeyComponent.new(first: "same", second: nil) + + refute_equal left.cache_key, right.cache_key + end + + def test_nil_and_empty_cache_on_values_do_not_collide + nils = PositionalCacheKeyComponent.new(first: nil, second: nil) + empties = PositionalCacheKeyComponent.new(first: "", second: "") + + refute_equal nils.cache_key, empties.cache_key + end + + # Issue 3: there's no way to skip caching for a single render. A false or nil + # value from a `cache_on` method becomes an ordinary key component (or is + # dropped from the key) rather than disabling the cache. + def test_caching_can_be_disabled_per_render + with_caching do + render_inline(ConditionalCacheProbeComponent.new(cacheable: false)) + render_inline(ConditionalCacheProbeComponent.new(cacheable: false)) + + assert_equal 2, ConditionalCacheProbeComponent.render_count + end + end + + def test_caching_still_applies_when_the_condition_is_met + with_caching do + render_inline(ConditionalCacheProbeComponent.new(cacheable: true)) + render_inline(ConditionalCacheProbeComponent.new(cacheable: true)) + + assert_equal 1, ConditionalCacheProbeComponent.render_count + end + end + + # Issue 4: a block passed to `cache_on` is silently dropped, leaving the + # component uncached with no indication why. + def test_cache_on_rejects_a_block + assert_raises(ArgumentError) do + Class.new(ViewComponent::Base) do + include ViewComponent::ExperimentallyCacheable + + cache_on { :identity } + end + end + end + + # ...and a proc raises NoMethodError from `to_sym` rather than saying what's + # wrong. + def test_cache_on_rejects_a_proc_with_a_useful_error + error = assert_raises(ArgumentError) do + Class.new(ViewComponent::Base) do + include ViewComponent::ExperimentallyCacheable + + cache_on -> { :identity } + end + end + + assert_match(/cache_on/, error.message) + end + + private + + def with_caching + old_value = ActionController::Base.perform_caching + ActionController::Base.perform_caching = true + Rails.cache.clear + yield + ensure + ActionController::Base.perform_caching = old_value + Rails.cache.clear + end +end From c6b64df6675942bc1124c67459c57593cbb7b55b Mon Sep 17 00:00:00 2001 From: Joel Hawksley Date: Fri, 21 Aug 2026 10:00:33 -0600 Subject: [PATCH 12/12] Fix cache key correctness issues found in review All four reported by @reeganviljoen on #2685: - Format was passed to cache_digest but never entered the key. A component rendering every format from one template has the same digest for each, so formats shared a cache entry. Format is now a key component in its own right. - The key array was compacted as a whole, so [nil, "same"] and ["same", nil] collapsed to the same key. Nils are now substituted rather than removed, using a sentinel so they also stay distinct from empty strings, which expand_cache_key renders identically to nil. - Added if:/unless: to cache_on for conditional caching. Both take a method name or a proc. Deliberately separate from the key values: a cache_on method returning nil or false should contribute that value to the key, not silently disable caching. - cache_on now raises ArgumentError for a block, a non-symbol argument, or an unknown option. A block was silently dropped, leaving the component uncached with no indication why, and a proc raised NoMethodError from to_sym. --- docs/CHANGELOG.md | 6 +- docs/guide/caching.md | 20 ++++ .../experimentally_cacheable.rb | 101 ++++++++++++++++-- .../conditional_cache_probe_component.rb | 6 +- .../experimentally_cacheable_review_test.rb | 79 ++++++++++++++ 5 files changed, 198 insertions(+), 14 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index ba00a2ab4..534c6f1b8 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -14,13 +14,13 @@ nav_order: 6 Components have never participated in Rails' template digests, so a `<% cache %>` block wrapping `render MyComponent.new` was never invalidated when the component changed ([#234](https://github.com/ViewComponent/view_component/issues/234), open since 2020). - Including the module registers the component with Rails' own `ActionView::Digestor`, so fragment caches are invalidated when the component's template, Ruby class, sidecar files, superclasses, child components, or rendered partials change — including components and partials rendered from inline templates and `#call` methods. Adding `cache_on` caches the component's own rendered output, and `.cache_digest` exposes the digest for use outside a request. + Including the module registers the component with Rails' own `ActionView::Digestor`, so fragment caches are invalidated when the component's template, Ruby class, sidecar files, superclasses, child components, or rendered partials change — including components and partials rendered from inline templates and `#call` methods. Adding `cache_on` caches the component's own rendered output, optionally guarded by `if:`/`unless:`, and `.cache_digest` exposes the digest for use outside a request. ```ruby class MessageComponent < ViewComponent::Base include ViewComponent::ExperimentallyCacheable - cache_on :message + cache_on :message, unless: -> { message.draft? } def initialize(message:) @message = message @@ -30,7 +30,7 @@ nav_order: 6 **This API is experimental and may change or be removed in a non-major release.** It's shipping opt-in and per-component precisely so we can iterate on it in response to real-world use. **Please try it and tell us what breaks, what's missing, and what feels wrong in [#234](https://github.com/ViewComponent/view_component/issues/234).** We're especially interested in feedback on: whether `cache_on` is the right shape for declaring cache keys, how the feature behaves with slots and content blocks, and whether the `# Template Dependency:` escape hatch is sufficient for dynamic renders. See [the caching guide](https://viewcomponent.org/guide/caching.html) for details and known caveats. - This work builds directly on prior art from the community: the `cache_on` API and the case for component-local caching come from [#2126](https://github.com/ViewComponent/view_component/pull/2126) by *Reegan Viljoen*; the approach of integrating with Rails' digest tree rather than reimplementing it comes from [`view_component-cache_digest`](https://github.com/tildeio/view_component-cache_digest) by *Godfrey Chan*; the invalidation cases it's tested against were contributed by *JWShuff* and *timburgan*, drawing on [`view_component-fragment_caching`](https://github.com/patrickarnett/view_component-fragment_caching) by *Patrick Arnett*. The issue was opened and researched by *ozzyaaron*, *pinzonjulian*, and *Derek Kniffin*, and the digest workaround that surfaced the superclass gap came from *cannikin* and *rnestler*. + This work builds directly on prior art from the community: the `cache_on` API and the case for component-local caching come from [#2126](https://github.com/ViewComponent/view_component/pull/2126) by *Reegan Viljoen*; the approach of integrating with Rails' digest tree rather than reimplementing it comes from [`view_component-cache_digest`](https://github.com/tildeio/view_component-cache_digest) by *Godfrey Chan*; the invalidation cases it's tested against were contributed by *JWShuff* and *timburgan*, drawing on [`view_component-fragment_caching`](https://github.com/patrickarnett/view_component-fragment_caching) by *Patrick Arnett*. The issue was opened and researched by *ozzyaaron*, *pinzonjulian*, and *Derek Kniffin*, and the digest workaround that surfaced the superclass gap came from *cannikin* and *rnestler*. Cache-key correctness issues — formats sharing an entry, positional `nil` collisions, conditional caching, and silently ignored `cache_on` blocks — were found and reported by *Reegan Viljoen*. *Reegan Viljoen*, *Godfrey Chan*, *JWShuff*, *timburgan*, *Patrick Arnett*, *ozzyaaron*, *pinzonjulian*, *Derek Kniffin*, *cannikin*, *rnestler*, *Joel Hawksley* diff --git a/docs/guide/caching.md b/docs/guide/caching.md index fd7c724ce..65c811053 100644 --- a/docs/guide/caching.md +++ b/docs/guide/caching.md @@ -86,6 +86,26 @@ The cache key combines: Caching is skipped unless `perform_caching` is enabled on the controller, matching the behavior of Rails' `cache` helper. +To cache only some renders, pass `if:` or `unless:`. Both accept a method name or a proc evaluated on the component: + +```ruby +class PostComponent < ViewComponent::Base + include ViewComponent::ExperimentallyCacheable + + cache_on :post, unless: -> { post.draft? } + + def initialize(post:) + @post = post + end + + private + + attr_reader :post +end +``` + +Drafts now render every time, while published posts are cached. Note that this controls whether the cache is *used*, not what goes into the key: a `cache_on` method returning `nil` or `false` still contributes that value to the key rather than disabling caching. + A component that declares `cache_on` can't accept content from its callers. A block, `with_content`, or a slot set by the caller isn't part of the cache key, so passing one raises `ContentPassedToCachedComponentError`: ```erb diff --git a/lib/view_component/experimentally_cacheable.rb b/lib/view_component/experimentally_cacheable.rb index 65d5878e3..804d23873 100644 --- a/lib/view_component/experimentally_cacheable.rb +++ b/lib/view_component/experimentally_cacheable.rb @@ -32,6 +32,11 @@ module ViewComponent module ExperimentallyCacheable extend ActiveSupport::Concern + # Stands in for `nil` in the cache key. Without it, `expand_cache_key` + # renders `nil` and `""` identically, so two components differing only in + # that respect would share an entry. + NIL_CACHE_VALUE = :__vc_nil + included do ViewComponent::CacheDigest.install! ViewComponent::CacheDigest.register(self) @@ -52,14 +57,46 @@ module ExperimentallyCacheable # Without it, including this module only registers the component with # Rails' digest tree. # + # Pass `if:` or `unless:` to cache only some renders. Both accept a method + # name or a proc evaluated on the component: + # + # ```ruby + # cache_on :message, if: :persisted? + # cache_on :message, unless: -> { message.draft? } + # ``` + # # These methods are called before the component renders, so they can only # depend on the component's own state, not on `helpers` or the view # context. # # @param methods [Array] Methods whose values form the cache key. + # @param options [Hash] `:if` and/or `:unless` conditions. # @return [void] - def cache_on(*methods) + def cache_on(*methods, **options, &block) + if block + raise ArgumentError, + "`cache_on` doesn't accept a block. Name the methods whose values form the cache key, " \ + "such as `cache_on :message`." + end + + methods.each do |method| + next if method.is_a?(Symbol) || method.is_a?(String) + + raise ArgumentError, + "`cache_on` expects method names as symbols, got #{method.class}. " \ + "Define a method for the value and name it, such as `cache_on :message`." + end + + unknown = options.keys - %i[if unless] + if unknown.any? + raise ArgumentError, + "`cache_on` received unknown #{"option".pluralize(unknown.count)} " \ + "#{unknown.map(&:inspect).to_sentence}. Supported options are `:if` and `:unless`." + end + @__vc_cache_on = __vc_cache_on | methods.map(&:to_sym) + @__vc_cache_if = options[:if] if options.key?(:if) + @__vc_cache_unless = options[:unless] if options.key?(:unless) end # @private @@ -67,6 +104,20 @@ def __vc_cache_on @__vc_cache_on ||= superclass.respond_to?(:__vc_cache_on) ? superclass.__vc_cache_on : [] end + # @private + def __vc_cache_if + return @__vc_cache_if if defined?(@__vc_cache_if) + + superclass.__vc_cache_if if superclass.respond_to?(:__vc_cache_if) + end + + # @private + def __vc_cache_unless + return @__vc_cache_unless if defined?(@__vc_cache_unless) + + superclass.__vc_cache_unless if superclass.respond_to?(:__vc_cache_unless) + end + # @private def __vc_cacheable? true @@ -152,16 +203,25 @@ def render_in(view_context, **, &block) # @return [String] def cache_key(view_context = nil) lookup_context = view_context&.lookup_context + format = __vc_cache_format(lookup_context) + parts = [ + "view_component", + self.class.virtual_path, + self.class.cache_digest(finder: lookup_context, format: format), + # Included in its own right, not just as a digest input: components that + # render every format from one template have the same digest for each. + format, + __vc_cache_variant(lookup_context), + I18n.locale, + *__vc_cache_on_values + ] + + # Positions are significant, so nils are substituted rather than removed. + # Compacting the array would let a nil in one position collapse into a nil + # in another. ActiveSupport::Cache.expand_cache_key( - [ - "view_component", - self.class.virtual_path, - self.class.cache_digest(finder: lookup_context, format: __vc_cache_format(lookup_context)), - __vc_cache_variant(lookup_context), - I18n.locale, - *__vc_cache_on_values - ].compact + parts.map { |part| part.nil? ? NIL_CACHE_VALUE : part } ) end @@ -176,11 +236,34 @@ def __vc_slots_set_by_caller? def __vc_cache_enabled?(view_context) return false unless defined?(Rails) && Rails.respond_to?(:cache) && Rails.cache + return false unless __vc_cache_conditions_met? controller = view_context.try(:controller) controller.respond_to?(:perform_caching) && controller.perform_caching end + def __vc_cache_conditions_met? + if (condition = self.class.__vc_cache_if) + return false unless __vc_evaluate_cache_condition(condition) + end + + if (condition = self.class.__vc_cache_unless) + return false if __vc_evaluate_cache_condition(condition) + end + + true + end + + def __vc_evaluate_cache_condition(condition) + return instance_exec(&condition) if condition.respond_to?(:to_proc) && !condition.is_a?(Symbol) + + unless respond_to?(condition, true) + raise UndefinedCacheKeyMethodError.new(self.class.name, condition) + end + + send(condition) + end + def __vc_cache_on_values self.class.__vc_cache_on.map do |method_name| unless respond_to?(method_name, true) diff --git a/test/sandbox/app/components/conditional_cache_probe_component.rb b/test/sandbox/app/components/conditional_cache_probe_component.rb index 74a0f6e29..edab74b29 100644 --- a/test/sandbox/app/components/conditional_cache_probe_component.rb +++ b/test/sandbox/app/components/conditional_cache_probe_component.rb @@ -4,7 +4,7 @@ class ConditionalCacheProbeComponent < ViewComponent::Base include ViewComponent::ExperimentallyCacheable - cache_on :identity + cache_on :identity, if: :cacheable? class_attribute :render_count, default: 0 @@ -19,7 +19,9 @@ def call private - attr_reader :cacheable + def cacheable? + @cacheable + end def identity "constant" diff --git a/test/sandbox/test/experimentally_cacheable_review_test.rb b/test/sandbox/test/experimentally_cacheable_review_test.rb index 0a61b2364..cfdcbde13 100644 --- a/test/sandbox/test/experimentally_cacheable_review_test.rb +++ b/test/sandbox/test/experimentally_cacheable_review_test.rb @@ -77,6 +77,85 @@ def test_caching_still_applies_when_the_condition_is_met end end + def test_cache_conditions_accept_a_proc + component = Class.new(ConditionalCacheProbeComponent) do + def self.name + "ProcConditionCacheProbeComponent" + end + + cache_on :identity, if: -> { false } + end + + with_caching do + render_inline(component.new(cacheable: true)) + render_inline(component.new(cacheable: true)) + + assert_equal 2, component.render_count + end + end + + def test_cache_conditions_accept_unless + component = Class.new(ConditionalCacheProbeComponent) do + def self.name + "UnlessConditionCacheProbeComponent" + end + + cache_on :identity, unless: :cacheable? + end + + with_caching do + render_inline(component.new(cacheable: true)) + render_inline(component.new(cacheable: true)) + + assert_equal 2, component.render_count + end + end + + def test_cache_conditions_are_inherited + component = Class.new(ConditionalCacheProbeComponent) do + def self.name + "InheritedConditionCacheProbeComponent" + end + end + + with_caching do + render_inline(component.new(cacheable: false)) + render_inline(component.new(cacheable: false)) + + assert_equal 2, component.render_count + end + end + + def test_cache_on_rejects_unknown_options + error = assert_raises(ArgumentError) do + Class.new(ViewComponent::Base) do + include ViewComponent::ExperimentallyCacheable + + cache_on :identity, when: :something + end + end + + assert_match(/:when/, error.message) + end + + def test_undefined_cache_condition_method_raises + component = Class.new(ConditionalCacheProbeComponent) do + def self.name + "MissingConditionCacheProbeComponent" + end + + cache_on :identity, if: :nonexistent? + end + + with_caching do + error = assert_raises(ViewComponent::UndefinedCacheKeyMethodError) do + render_inline(component.new(cacheable: true)) + end + + assert_includes error.message, "nonexistent?" + end + end + # Issue 4: a block passed to `cache_on` is silently dropped, leaving the # component uncached with no indication why. def test_cache_on_rejects_a_block