Call maybe_escape_html directly in compiled call method (#2151)

* Call maybe_escape_html directly in compiled call method

* add changelog

* update allocation counts

* use better method name

* remove unused compiler accessor

* ignore DS_Store

Author: joelhawksley

.gitignore

@@ -18,6 +18,7 @@
 /test/sandbox/log/*
 /test/tmp/*
 /.yardoc
+.DS_Store
 
 ## Specific to RubyMotion:
 .dat*

docs/CHANGELOG.md

@@ -10,6 +10,10 @@ nav_order: 5
 
 ## main
 
+* Ensure HTML output safety wrapper is used for all inline templates.
+
+    *Joel Hawksley*
+
 * Expose `.identifier` method as part of public API.
 
     *Joel Hawksley*

lib/view_component/base.rb

@@ -108,14 +108,7 @@ def render_in(view_context, &block)
       before_render
 
       if render?
-        rendered_template =
-          if compiler.renders_template_for?(@__vc_variant, __vc_request&.format&.to_sym)
-            render_template_for(@__vc_variant, __vc_request&.format&.to_sym)
-          else
-            maybe_escape_html(render_template_for(@__vc_variant, __vc_request&.format&.to_sym)) do
-              Kernel.warn("WARNING: The #{self.class} component rendered HTML-unsafe output. The output will be automatically escaped, but you may want to investigate.")
-            end
-          end.to_s
+        rendered_template = render_template_for(@__vc_variant, __vc_request&.format&.to_sym).to_s
 
         # Avoid allocating new string when output_preamble and output_postamble are blank
         if output_preamble.blank? && output_postamble.blank?
@@ -358,10 +351,6 @@ def safe_output_postamble
       end
     end
 
-    def compiler
-      @compiler ||= self.class.compiler
-    end
-
     # Set the controller used for testing components:
     #
     # ```ruby

lib/view_component/compiler.rb

@@ -13,7 +13,6 @@ class Compiler
     def initialize(component)
       @component = component
       @lock = Mutex.new
-      @rendered_templates = Set.new
     end
 
     def compiled?
@@ -56,10 +55,6 @@ def compile(raise_errors: false, force: false)
       end
     end
 
-    def renders_template_for?(variant, format)
-      @rendered_templates.include?([variant, format])
-    end
-
     private
 
     attr_reader :templates
@@ -71,9 +66,9 @@ def define_render_template_for
 
       method_body =
         if @templates.one?
-          @templates.first.safe_method_name
+          @templates.first.safe_method_name_call
         elsif (template = @templates.find(&:inline?))
-          template.safe_method_name
+          template.safe_method_name_call
         else
           branches = []
 
@@ -88,13 +83,13 @@ def define_render_template_for
                 ].join(" && ")
               end
 
-            branches << [conditional, template.safe_method_name]
+            branches << [conditional, template.safe_method_name_call]
           end
 
           out = branches.each_with_object(+"") do |(conditional, branch_body), memo|
             memo << "#{(!memo.present?) ? "if" : "elsif"} #{conditional}\n  #{branch_body}\n"
           end
-          out << "else\n  #{templates.find { _1.variant.nil? && _1.default_format? }.safe_method_name}\nend"
+          out << "else\n  #{templates.find { _1.variant.nil? && _1.default_format? }.safe_method_name_call}\nend"
         end
 
       @component.silence_redefinition_of_method(:render_template_for)
@@ -196,10 +191,6 @@ def gather_templates
               variant: variant
             )
 
-            # TODO: We should consider inlining the HTML output safety logic into the compiled render_template_for
-            # instead of this indirect approach
-            @rendered_templates << [out.variant, out.this_format]
-
             out
           end
 

lib/view_component/template.rb

@@ -59,6 +59,14 @@ def #{@call_method_name}
       @component.define_method(safe_method_name, @component.instance_method(@call_method_name))
     end
 
+    def safe_method_name_call
+      return safe_method_name unless inline_call?
+
+      "maybe_escape_html(#{safe_method_name}) " \
+      "{ Kernel.warn('WARNING: The #{@component} component rendered HTML-unsafe output. " \
+      "The output will be automatically escaped, but you may want to investigate.') } "
+    end
+
     def requires_compiled_superclass?
       inline_call? && !defined_on_self?
     end

test/sandbox/test/rendering_test.rb

@@ -15,7 +15,7 @@ def test_render_inline_allocations
     ViewComponent::CompileCache.cache.delete(MyComponent)
     MyComponent.ensure_compiled
 
-    assert_allocations("3.4.0" => 110, "3.3.5" => 116, "3.3.0" => 129, "3.2.6" => 115, "3.1.6" => 115, "3.0.7" => 125) do
+    assert_allocations("3.4.0" => 109, "3.3.5" => 115, "3.3.0" => 127, "3.2.6" => 114, "3.1.6" => 114, "3.0.7" => 123) do
       render_inline(MyComponent.new)
     end