Refactor compiler and template to use requested details (#2158)

Author: sfnelson

docs/CHANGELOG.md

@@ -28,6 +28,17 @@ nav_order: 5
 
     *Stephen Nelson*
 
+* BREAKING: Use ActionView's `lookup_context` for picking templates instead of the request format.
+
+  3.15 added support for using templates that match the request format, i.e. if `/resource.csv` is requested then
+  ViewComponents would pick `_component.csv.erb` over `_component.html.erb`.
+
+  With this release, the request format is no longer considered and instead ViewComponent will use the Rails logic
+  for picking the most appropriate template type, i.e. the csv template will be used if it matches the `Accept` header
+  or because the controller uses a `respond_to` block to pick the response format.
+
+    *Stephen Nelson*
+
 * Ensure HTML output safety wrapper is used for all inline templates.
 
     *Joel Hawksley*

lib/view_component/base.rb

@@ -9,6 +9,7 @@
 require "view_component/errors"
 require "view_component/inline_template"
 require "view_component/preview"
+require "view_component/request_details"
 require "view_component/slotable"
 require "view_component/slotable_default"
 require "view_component/template"
@@ -63,6 +64,8 @@ def set_original_view_context(view_context)
       self.__vc_original_view_context = view_context
     end
 
+    using RequestDetails
+
     # Entrypoint for rendering components.
     #
     # - `view_context`: ActionView context from calling view
@@ -90,13 +93,12 @@ def render_in(view_context, &block)
       # For i18n
       @virtual_path ||= virtual_path
 
-      # For template variants (+phone, +desktop, etc.)
-      @__vc_variant ||= @lookup_context.variants.first
+      # Describes the inferred request constraints (locales, formats, variants)
+      @__vc_requested_details ||= @lookup_context.vc_requested_details
 
       # For caching, such as #cache_if
       @current_template = nil unless defined?(@current_template)
       old_current_template = @current_template
-      @current_template = self
 
       if block && defined?(@__vc_content_set_by_with_content)
         raise DuplicateContentError.new(self.class.name)
@@ -108,7 +110,7 @@ def render_in(view_context, &block)
       before_render
 
       if render?
-        rendered_template = render_template_for(@__vc_variant, __vc_request&.format&.to_sym).to_s
+        rendered_template = render_template_for(@__vc_requested_details).to_s
 
         # Avoid allocating new string when output_preamble and output_postamble are blank
         if output_preamble.blank? && output_postamble.blank?
@@ -156,7 +158,7 @@ def render_parent_to_string
         target_render = self.class.instance_variable_get(:@__vc_ancestor_calls)[@__vc_parent_render_level]
         @__vc_parent_render_level += 1
 
-        target_render.bind_call(self, @__vc_variant)
+        target_render.bind_call(self, @__vc_requested_details)
       ensure
         @__vc_parent_render_level -= 1
       end
@@ -267,11 +269,10 @@ def view_cache_dependencies
       []
     end
 
-    # For caching, such as #cache_if
-    #
-    # @private
+    # Rails expects us to define `format` on all renderables,
+    # but we do not know the `format` of a ViewComponent until runtime.
     def format
-      @__vc_variant if defined?(@__vc_variant)
+      nil
     end
 
     # The current request. Use sparingly as doing so introduces coupling that
@@ -328,7 +329,7 @@ def content_evaluated?
     end
 
     def maybe_escape_html(text)
-      return text if __vc_request && !__vc_request.format.html?
+      return text if @current_template && !@current_template.html?
       return text if text.blank?
 
       if text.html_safe?
@@ -517,12 +518,12 @@ def inherited(child)
         # meaning it will not be called for any children and thus not compile their templates.
         if !child.instance_methods(false).include?(:render_template_for) && !child.compiled?
           child.class_eval <<~RUBY, __FILE__, __LINE__ + 1
-            def render_template_for(variant = nil, format = nil)
+            def render_template_for(requested_details)
               # Force compilation here so the compiler always redefines render_template_for.
               # This is mostly a safeguard to prevent infinite recursion.
               self.class.compile(raise_errors: true, force: true)
               # .compile replaces this method; call the new one
-              render_template_for(variant, format)
+              render_template_for(requested_details)
             end
           RUBY
         end

lib/view_component/compiler.rb

@@ -55,6 +55,23 @@ def compile(raise_errors: false, force: false)
       end
     end
 
+    # @return all matching compiled templates, in priority order based on the requested details from LookupContext
+    #
+    # @param [ActionView::TemplateDetails::Requested] requested_details i.e. locales, formats, variants
+    def find_templates_for(requested_details)
+      filtered_templates = @templates.select do |template|
+        template.details.matches?(requested_details)
+      end
+
+      if filtered_templates.count > 1
+        filtered_templates.sort_by! do |template|
+          template.details.sort_key_for(requested_details)
+        end
+      end
+
+      filtered_templates
+    end
+
     private
 
     attr_reader :templates
@@ -64,40 +81,25 @@ def define_render_template_for
         template.compile_to_component
       end
 
-      method_body =
-        if @templates.one?
-          @templates.first.safe_method_name_call
-        elsif (template = @templates.find(&:inline?))
-          template.safe_method_name_call
-        else
-          branches = []
-
-          @templates.each do |template|
-            conditional =
-              if template.inline_call?
-                "variant&.to_sym == #{template.variant.inspect}"
-              else
-                [
-                  template.default_format? ? "(format == #{ViewComponent::Base::VC_INTERNAL_DEFAULT_FORMAT.inspect} || format.nil?)" : "format == #{template.format.inspect}",
-                  template.variant.nil? ? "variant.nil?" : "variant&.to_sym == #{template.variant.inspect}"
-                ].join(" && ")
-              end
-
-            branches << [conditional, template.safe_method_name_call]
-          end
+      @component.silence_redefinition_of_method(:render_template_for)
 
-          out = branches.each_with_object(+"") do |(conditional, branch_body), memo|
-            memo << "#{(!memo.present?) ? "if" : "elsif"} #{conditional}\n  #{branch_body}\n"
+      if @templates.one?
+        template = @templates.first
+        safe_call = template.safe_method_name_call
+        @component.define_method(:render_template_for) do |_|
+          @current_template = template
+          instance_exec(&safe_call)
+        end
+      else
+        compiler = self
+        @component.define_method(:render_template_for) do |details|
+          if (@current_template = compiler.find_templates_for(details).first)
+            instance_exec(&@current_template.safe_method_name_call)
+          else
+            raise MissingTemplateError.new(self.class.name, details)
           end
-          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)
-      @component.class_eval <<-RUBY, __FILE__, __LINE__ + 1
-      def render_template_for(variant = nil, format = nil)
-        #{method_body}
       end
-      RUBY
     end
 
     def template_errors
@@ -168,15 +170,18 @@ def template_errors
 
     def gather_templates
       @templates ||=
-        begin
+        if @component.inline_template.present?
+          [Template::Inline.new(
+            component: @component,
+            inline_template: @component.inline_template
+          )]
+        else
           path_parser = ActionView::Resolver::PathParser.new
           templates = @component.sidecar_files(
             ActionView::Template.template_handler_extensions
           ).map do |path|
             details = path_parser.parse(path).details
-            out = Template::File.new(component: @component, path: path, details: details)
-
-            out
+            Template::File.new(component: @component, path: path, details: details)
           end
 
           component_instance_methods_on_self = @component.instance_methods(false)
@@ -186,17 +191,10 @@ def gather_templates
           ).flat_map { |ancestor| ancestor.instance_methods(false).grep(/^call(_|$)/) }
             .uniq
             .each do |method_name|
-              templates << Template::InlineCall.new(
-                component: @component,
-                method_name: method_name,
-                defined_on_self: component_instance_methods_on_self.include?(method_name)
-              )
-            end
-
-          if @component.inline_template.present?
-            templates << Template::Inline.new(
+            templates << Template::InlineCall.new(
               component: @component,
-              inline_template: @component.inline_template
+              method_name: method_name,
+              defined_on_self: component_instance_methods_on_self.include?(method_name)
             )
           end
 

lib/view_component/errors.rb

@@ -38,6 +38,22 @@ def initialize(example)
     end
   end
 
+  class MissingTemplateError < StandardError
+    MESSAGE =
+      "No templates for COMPONENT match the request DETAIL.\n\n" \
+      "To fix this issue, provide a suitable template."
+
+    def initialize(component, request_detail)
+      detail = {
+        locale: request_detail.locale,
+        formats: request_detail.formats,
+        variants: request_detail.variants,
+        handlers: request_detail.handlers
+      }
+      super(MESSAGE.gsub("COMPONENT", component).gsub("DETAIL", detail.inspect))
+    end
+  end
+
   class DuplicateContentError < StandardError
     MESSAGE =
       "It looks like a block was provided after calling `with_content` on COMPONENT, " \

lib/view_component/request_details.rb

@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+module ViewComponent
+  # LookupContext computes and encapsulates @details for each request
+  # so that it doesn't need to be recomputed on each partial render.
+  # This data is wrapped in ActionView::TemplateDetails::Requested and
+  # used by instances of ActionView::Resolver to choose which template
+  # best matches the request.
+  #
+  # ActionView considers this logic internal to template/partial resolution.
+  # We're exposing it to the compiler via `refine` so that ViewComponent
+  # can match Rails' template picking logic.
+  module RequestDetails
+    refine ActionView::LookupContext do
+      # Return an abstraction for matching and sorting available templates
+      # based on the current lookup context details.
+      #
+      # @return ActionView::TemplateDetails::Requested
+      # @see ActionView::LookupContext#detail_args_for
+      # @see ActionView::FileSystemResolver#_find_all
+      def vc_requested_details(user_details = {})
+        # The hash `user_details` would normally be the standard arguments that
+        # `render` accepts, but there's currently no mechanism for users to
+        # provide these when calling render on a ViewComponent.
+        details, cached = detail_args_for(user_details)
+        cached || ActionView::TemplateDetails::Requested.new(**details)
+      end
+    end
+  end
+end