Fix line numbers and empty output for ERB templates Rails doesn't annotate

docs/CHANGELOG.md

@@ -10,6 +10,14 @@ nav_order: 6
 
 ## main
 
+* 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*

lib/view_component/template.rb

@@ -60,13 +60,7 @@ def lineno
       # Strip the annotation line to maintain correct line numbers when coverage
       # is running (avoids segfault from negative lineno)
       def strip_annotation_line?
-        erb_newline_compensation_needed? &&
-          coverage_running? &&
-          ActionView::Base.annotate_rendered_view_with_filenames
-      end
-
-      def erb_newline_compensation_needed?
-        Rails::VERSION::MAJOR >= 8 && Rails::VERSION::MINOR > 0 && details.handler == :erb
+        erb_newline_compensation_needed? && coverage_running?
       end
 
       def compiled_source
@@ -82,22 +76,11 @@ class Inline < Template
       def initialize(component:, inline_template:)
         details = ActionView::TemplateDetails.new(nil, inline_template.language.to_sym, nil, nil)
 
-        # Rails 8.1 added a newline to compiled ERB output (rails/rails#53731).
-        # Subtract 1 to compensate for correct line numbers in stack traces.
-        # Inline templates start at line 2+ (defined inside a class), so this
-        # won't result in negative line numbers that cause segfaults with coverage.
-        lineno =
-          if Rails::VERSION::MAJOR >= 8 && Rails::VERSION::MINOR > 0 && details.handler == :erb
-            inline_template.lineno - 1
-          else
-            inline_template.lineno
-          end
-
         super(
           component: component,
           details: details,
           path: inline_template.path,
-          lineno: lineno,
+          lineno: inline_template.lineno,
         )
 
         @source = inline_template.source.dup
@@ -106,6 +89,19 @@ def initialize(component:, inline_template:)
       def type
         :inline
       end
+
+      private
+
+      # Rails 8.1 added a newline to compiled ERB output (rails/rails#53731).
+      # Subtract 1 to compensate for correct line numbers in stack traces.
+      # Inline templates start at line 2+ (defined inside a class), so this
+      # won't result in negative line numbers that cause segfaults with coverage.
+      #
+      # Computed at compile time rather than at initialization because the
+      # annotation setting can change between the two.
+      def lineno
+        erb_newline_compensation_needed? ? super - 1 : super
+      end
     end
 
     class InlineCall < Template
@@ -196,6 +192,20 @@ def normalized_variant_name
 
     attr_reader :lineno
 
+    def erb_newline_compensation_needed?
+      Rails::VERSION::MAJOR >= 8 && Rails::VERSION::MINOR > 0 &&
+        details.handler == :erb &&
+        annotated?
+    end
+
+    # The newline compensated for lives inside the `<!-- BEGIN ... -->`
+    # annotation, which Rails only prepends to HTML templates, and only when
+    # annotations are enabled. Compensating outside those conditions shifts
+    # backtraces by a line and, under coverage, strips real template source.
+    def annotated?
+      ActionView::Base.annotate_rendered_view_with_filenames && html?
+    end
+
     def compiled_source
       handler = details.handler_class
       this_source = source

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

@@ -0,0 +1,3 @@
+<div>
+  <%= raise ArgumentError, "oh no" %>
+</div>

test/sandbox/app/components/raising_formats_component.rb

@@ -0,0 +1,4 @@
+# frozen_string_literal: true
+
+class RaisingFormatsComponent < ViewComponent::Base
+end

test/sandbox/app/components/raising_formats_component.text.erb

@@ -0,0 +1,2 @@
+line one
+<%= raise ArgumentError, "oh no" %>

test/sandbox/test/inline_template_test.rb

@@ -119,6 +119,17 @@ class InlineComponentDerivedFromComponentSupportingVariants < Level2Component
     assert_match %r{test/sandbox/test/inline_template_test.rb:22}, error.backtrace[0]
   end
 
+  test "error backtrace locations work when template annotations are disabled" do
+    error = nil
+
+    without_template_annotations do
+      InlineRaiseErbComponent.__vc_compile(force: true)
+      error = assert_raises(ArgumentError) { render_inline(InlineRaiseErbComponent.new("Fox Mulder")) }
+    end
+
+    assert_match %r{test/sandbox/test/inline_template_test.rb:22}, error.backtrace[0]
+  end
+
   test "error backtrace locations work in slim" do
     error = assert_raises ArgumentError do
       render_inline(InlineRaiseSlimComponent.new("Fox Mulder"))

test/sandbox/test/template_annotation_test.rb

@@ -0,0 +1,74 @@
+# frozen_string_literal: true
+
+require "test_helper"
+
+class TemplateAnnotationTest < ViewComponent::TestCase
+  # Rails only prepends the `<!-- BEGIN ... -->` annotation, and the newline it
+  # contains, when annotations are enabled *and* the template's format is HTML.
+  # The line number compensation ViewComponent applies has to match those same
+  # conditions, otherwise backtraces point at the wrong line and, when coverage
+  # is running, template source is stripped away.
+  def setup
+    skip unless Rails::VERSION::MAJOR >= 8 && Rails::VERSION::MINOR > 0
+  end
+
+  def teardown
+    [RaisingFormatsComponent, MultipleFormatsComponent].each { |c| c.__vc_compile(force: true) }
+  end
+
+  def test_backtrace_line_number_for_html_template_with_annotations
+    RaisingFormatsComponent.__vc_compile(force: true)
+
+    error = assert_raises(ArgumentError) { render_inline(RaisingFormatsComponent.new) }
+
+    assert_equal 2, template_line_number(error, "raising_formats_component.html.erb")
+  end
+
+  def test_backtrace_line_number_for_html_template_without_annotations
+    error = nil
+
+    without_template_annotations do
+      RaisingFormatsComponent.__vc_compile(force: true)
+      error = assert_raises(ArgumentError) { render_inline(RaisingFormatsComponent.new) }
+    end
+
+    assert_equal 2, template_line_number(error, "raising_formats_component.html.erb")
+  end
+
+  def test_backtrace_line_number_for_non_html_template
+    RaisingFormatsComponent.__vc_compile(force: true)
+
+    error = assert_raises(ArgumentError) do
+      with_format(:text) { render_inline(RaisingFormatsComponent.new) }
+    end
+
+    assert_equal 2, template_line_number(error, "raising_formats_component.text.erb")
+  end
+
+  def test_non_html_template_renders_its_content_when_coverage_is_running
+    with_coverage_running { MultipleFormatsComponent.__vc_compile(force: true) }
+
+    with_format(:css) { render_inline(MultipleFormatsComponent.new) }
+
+    assert_includes @rendered_content, "Hello, CSS!"
+  end
+
+  private
+
+  def template_line_number(error, template_file)
+    entry = error.backtrace.find { |line| line.include?(template_file) }
+
+    refute_nil entry, "Expected #{template_file} in backtrace:\n#{error.backtrace.first(5).join("\n")}"
+
+    entry[/:(\d+):/, 1].to_i
+  end
+
+  def with_coverage_running
+    require "coverage"
+    already_running = Coverage.running?
+    Coverage.start unless already_running
+    yield
+  ensure
+    Coverage.result unless already_running
+  end
+end

test/test_helper.rb

@@ -155,7 +155,7 @@ def without_template_annotations(&block)
   app.reloader.reload! if defined?(app)
 
   with_new_cache(&block)
-
+ensure
   ActionView::Base.annotate_rendered_view_with_filenames = old_value
   app.reloader.reload! if defined?(app)
 end