Merge origin/main into vc-digest-report-swallowed-errors

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: joelhawksley

.github/workflows/ci.yml

@@ -95,7 +95,7 @@ jobs:
         working-directory: 'view_component'
     - uses: actions/setup-node@v5
       with:
-        node-version: 20
+        node-version: 22
         cache: 'npm'
         cache-dependency-path: 'primer_view_components/package-lock.json'
     - name: Build and test with Rake

.github/workflows/lint.yml

@@ -126,10 +126,8 @@ jobs:
       uses: ruby/setup-ruby@v1
       with:
         ruby-version: 4.0
-    # audition needs Ruby 4.0+; installed standalone since rubydex is a
-    # precompiled native gem. Fails only on findings not in
-    # .audition-baseline.json. Static-only skips Ractor issues in dependencies.
-    - name: Install audition
-      run: gem install audition
+        bundler-cache: true
+    # Fails only on findings not in .audition-baseline.json. Static-only skips
+    # Ractor issues in dependencies.
     - name: Audition (Ractor readiness)
-      run: audition . --static-only --plain
+      run: bundle exec audition . --static-only --plain

docs/CHANGELOG.md

@@ -14,6 +14,24 @@ nav_order: 6
 
     *Erik Axel Nielsen*
 
+* Give `<% cache %>` blocks inside a component's own template a digest, for components that `include ViewComponent::ExperimentallyCacheable`.
+
+    Rails digests the virtual path of whichever template is rendering. Inside a component that path resolves to no template, because component templates aren't in the view paths, so the Digestor returned an empty digest and the fragment was never invalidated. The only signal was a `Couldn't find template for digesting` line in the log. 4.15.0 fixed the case where the `cache` block wraps the component in a view. This fixes the case where the block sits in the component's template.
+
+    *Erik Axel Nielsen*
+
+* Fix line numbers and, under coverage, missing output for ERB templates that Rails doesn't annotate.
+
+    On Rails 8.1+, ViewComponent compensated for the newline Rails adds to compiled ERB output ([rails/rails#53731](https://github.com/rails/rails/pull/53731)) for every ERB template, whether from a file or from `erb_template`. That newline lives inside the `<!-- BEGIN ... -->` annotation, which Rails only emits when `annotate_rendered_view_with_filenames` is enabled *and* the template's format is HTML. Compensating unconditionally shifted backtraces for non-HTML templates (`.text.erb`, `.css.erb`) and for every ERB template when annotations are disabled, such as in production, by one line. When coverage was running, the same mismatch made the annotation-stripping workaround remove real template source, so non-HTML templates rendered empty.
+
+    Inline templates now decide the compensation when they compile rather than when the component class is defined, matching file templates.
+
+    *Ryutaro Mizokami*
+
+* Reduce per-render allocations. Inline renders drop 2 to 3 allocations and collection renders drop 4 to 8 depending on Rails/Ruby version by caching the instrumentation-enabled flag at the module level, memoizing the empty-details `Requested` per `LookupContext`, hoisting per-item metadata lookups out of the collection render loop, and dropping a few gratuitous `**` splats on the `Collection` API boundary.
+
+    *Joel Hawksley*
+
 ## 4.15.0
 
 * Add experimental caching support, opt-in per component via `include ViewComponent::ExperimentallyCacheable`.

docs/best_practices.md

@@ -42,8 +42,6 @@ For example, `User::AvatarComponent` accepts a `User` ActiveRecord object and re
 
 ### Extract general-purpose ViewComponents
 
-"Good frameworks are extracted, not invented" - [DHH](https://dhh.dk/arc/000416.html)
-
 Just as ViewComponent itself was extracted from GitHub.com, general-purpose components are best extracted once they've proven helpful across more than one area:
 
 1. Single use-case component implemented.

docs/guide/caching.md

@@ -42,6 +42,21 @@ end
 
 That's all that's needed for the `<% cache %>` block above to work. The component is registered with Rails' digest tree, and the fragment is invalidated when the component's template, Ruby class, sidecar files, superclasses, child components, or rendered partials change, including components and partials rendered from an inline template or a `#call` method.
 
+## Caching inside a component template
+
+A `<% cache %>` block written inside a component's own template has the same problem, for the same reason: Rails digests the template that's rendering, and a component's template isn't in the view paths, so there's nothing to digest.
+
+```erb
+<%# app/components/post_component.html.erb %>
+<% cache @post do %>
+  <%= render CommentComponent.new(post: @post) %>
+<% end %>
+```
+
+Including the module fixes this too. The component's own digest is substituted for the empty one Rails computes, so the fragment is invalidated by the same set of changes listed above. A component that hasn't opted in gets no digest at all, and the fragment is never invalidated.
+
+`cache` blocks in partials the component renders are unaffected: those templates resolve through the view paths like any other, so Rails digests them itself.
+
 ## Self-caching
 
 To have a component cache its own output without needing a `cache` block, use `cache_on` to declare methods used for the component's cache key.

lib/view_component/base.rb

@@ -631,7 +631,7 @@ def sidecar_files(extensions)
       # @param spacer_component [ViewComponent::Base] Component instance to be rendered between items.
       # @param args [Arguments] Arguments to pass to the ViewComponent every time.
       def with_collection(collection, spacer_component: nil, **args)
-        Collection.new(self, collection, spacer_component, **args)
+        Collection.new(self, collection, spacer_component, args)
       end
 
       # @private

lib/view_component/collection.rb

@@ -12,6 +12,9 @@ class Collection
 
     delegate :size, to: :@collection
 
+    EMPTY_SPACER = "".html_safe.freeze
+    private_constant :EMPTY_SPACER
+
     def render_in(view_context, **_, &block)
       rendered = components.map! do |component|
         component.render_in(view_context, &block)
@@ -36,18 +39,27 @@ def format
     # Always rebuild child component instances per render to avoid leaking
     # request-scoped state from a previous render into a later one (GHSA).
     def components
-      iterator = ActionView::PartialIteration.new(@collection.size)
-
       component.__vc_validate_collection_parameter!(validate_default: true)
 
+      iterator = ActionView::PartialIteration.new(@collection.size)
+      collection_param = component.__vc_collection_parameter
+      counter_present = component.__vc_counter_argument_present?
+      counter_param = component.__vc_collection_counter_parameter if counter_present
+      iteration_present = component.__vc_iteration_argument_present?
+      iteration_param = component.__vc_collection_iteration_parameter if iteration_present
+      item_options = @options.dup
+
       @collection.map do |item|
-        component.new(**component_options(item, iterator)).tap do |_|
-          iterator.iterate!
-        end
+        item_options[collection_param] = item
+        item_options[counter_param] = iterator.index if counter_present
+        item_options[iteration_param] = iterator.dup if iteration_present
+        instance = component.new(**item_options)
+        iterator.iterate!
+        instance
       end
     end
 
-    def initialize(component, object, spacer_component, **options)
+    def initialize(component, object, spacer_component, options = {})
       @component = component
       @collection = collection_variable(object || [])
       @spacer_component = spacer_component
@@ -62,20 +74,11 @@ def collection_variable(object)
       end
     end
 
-    def component_options(item, iterator)
-      item_options = @options.dup
-      item_options[component.__vc_collection_parameter] = item
-      item_options[component.__vc_collection_counter_parameter] = iterator.index if component.__vc_counter_argument_present?
-      item_options[component.__vc_collection_iteration_parameter] = iterator.dup if component.__vc_iteration_argument_present?
-
-      item_options
-    end
-
     # Render the spacer through a fresh `dup` so a collection rendered multiple
     # times does not reuse (and trip the single-render guard on) the spacer
     # instance passed by the caller.
     def rendered_spacer(view_context)
-      return "" unless @spacer_component
+      return EMPTY_SPACER unless @spacer_component
 
       spacer = @spacer_component.dup
       if spacer.instance_variable_defined?(:@__vc_rendered)

lib/view_component/engine.rb

@@ -29,6 +29,7 @@ class Engine < Rails::Engine # :nodoc:
     initializer "view_component.enable_instrumentation" do |app|
       ActiveSupport.on_load(:view_component) do
         if app.config.view_component.instrumentation_enabled.present?
+          ViewComponent::Instrumentation.enabled = true
           ViewComponent::Base.prepend(ViewComponent::Instrumentation)
         end
       end

lib/view_component/experimentally_cacheable.rb

@@ -12,9 +12,10 @@ module ViewComponent
   # 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.
+  #    `<% cache %>` block is invalidated when the component's template, Ruby
+  #    class, sidecar files, or child components change. This covers blocks
+  #    wrapping the component in a view and blocks inside the component's own
+  #    template.
   # 2. Enables the `cache_on` macro, which caches the component's own rendered
   #    output.
   #
@@ -225,6 +226,33 @@ def cache_key(view_context = nil)
       )
     end
 
+    # The digest Rails mixes into the key of a `<% cache %>` block.
+    #
+    # `ActionView::Helpers::CacheHelper` digests the virtual path of whichever
+    # template is rendering. Inside a component that path is the component's
+    # own, which resolves to nothing: component templates aren't in the view
+    # paths. The Digestor returns an empty digest, `CacheHelper` falls back to
+    # the bare virtual path, and the fragment never invalidates.
+    #
+    # Substituting the digest the component already computes makes a `cache`
+    # block in a component template behave like one in a view. Everything else
+    # the component renders — a partial, say — keeps Rails' behavior.
+    #
+    # @private
+    def digest_path_from_template(template)
+      component_path = self.class.virtual_path
+      return super unless component_path && template.virtual_path == component_path
+
+      digest = self.class.cache_digest(
+        finder: lookup_context,
+        format: template.format || __vc_cache_format(lookup_context)
+      )
+
+      # An empty digest means the component couldn't be resolved. Falling back
+      # to the bare virtual path matches what `CacheHelper` does with one.
+      digest.present? ? "#{component_path}:#{digest}" : component_path
+    end
+
     private
 
     # Slots set by the caller via `with_*`. Checked before rendering, so slots

lib/view_component/instrumentation.rb

@@ -8,8 +8,12 @@ def self.included(mod)
       mod.prepend(self) unless self <= ViewComponent::Instrumentation
     end
 
+    class << self
+      attr_accessor :enabled
+    end
+
     def render_in(...)
-      return super if !Rails.application.config.view_component.instrumentation_enabled.present?
+      return super unless Instrumentation.enabled
 
       payload = {
         name: self.class.name,