Add support for multiple formats (#2079)

* add test case * two tests remaining * first time all green * Fix final line endings * add docs, changelog, test helpers * Fix final line endings * streamline compiler method generation * simplification * refactor to remove index usage * clearer control flow * lint * md lint * remove remaining hardcoded formats * Update lib/view_component/base.rb Co-authored-by: Blake Williams <blakewilliams@github.com> * remove unnecessary `inspect` * remove unused `identifier` * inline single-use method * add safe navigation * consolidate template collision error messages to include format * add backticks around variant name in error * compiler should check for variant/format render combinations * standardrb --------- Co-authored-by: GitHub Actions Bot <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Blake Williams <blakewilliams@github.com>
ViewComponent · Sep 6, 2024 · 451543a · 451543a
1 parent 45ffb7a
commit 451543a
Show file tree

Hide file tree

Showing 16 changed files with 219 additions and 91 deletions.
diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md
@@ -10,6 +10,18 @@ nav_order: 5
 
 ## main
 
+* Add support for request formats.
+
+    *Joel Hawksley*
+
+* Add `rendered_json` test helper.
+
+    *Joel Hawksley*
+
+* Add `with_format` test helper.
+
+    *Joel Hawksley*
+
 * Warn if using Ruby < 3.1 or Rails < 7.0, which will not be supported by ViewComponent v4.
 
     *Joel Hawksley*

diff --git a/docs/guide/testing.md b/docs/guide/testing.md
@@ -135,6 +135,20 @@ def test_render_component_for_tablet
 end
 ```
 
+## Request formats
+
+Use the `with_format` helper to test specific request formats:
+
+```ruby
+def test_render_component_as_json
+  with_format :json do
+    render_inline(MultipleFormatsComponent.new)
+
+    assert_equal(rendered_json["hello"], "world")
+  end
+end
+```
+
 ## Configuring the controller used in tests
 
 Since 2.27.0

diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb
@@ -107,7 +107,14 @@ def render_in(view_context, &block)
 
       if render?
         # Avoid allocating new string when output_preamble and output_postamble are blank
-        rendered_template = safe_render_template_for(@__vc_variant).to_s
+        rendered_template =
+          if compiler.renders_template_for?(@__vc_variant, request&.format&.to_sym)
+            render_template_for(@__vc_variant, request&.format&.to_sym)
+          else
+            maybe_escape_html(render_template_for(@__vc_variant, 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
 
         if output_preamble.blank? && output_postamble.blank?
           rendered_template
@@ -330,16 +337,6 @@ def maybe_escape_html(text)
       end
     end
 
-    def safe_render_template_for(variant)
-      if compiler.renders_template_for_variant?(variant)
-        render_template_for(variant)
-      else
-        maybe_escape_html(render_template_for(variant)) 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
-    end
-
     def safe_output_preamble
       maybe_escape_html(output_preamble) do
         Kernel.warn("WARNING: The #{self.class} component was provided an HTML-unsafe preamble. The preamble will be automatically escaped, but you may want to investigate.")
@@ -500,13 +497,6 @@ def with_collection(collection, **args)
         Collection.new(self, collection, **args)
       end
 
-      # Provide identifier for ActionView template annotations
-      #
-      # @private
-      def short_identifier
-        @short_identifier ||= defined?(Rails.root) ? source_location.sub("#{Rails.root}/", "") : source_location
-      end
-
       # @private
       def inherited(child)
         # Compile so child will inherit compiled `call_*` template methods that
@@ -519,12 +509,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)
+            def render_template_for(variant = nil, format = nil)
               # 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)
+              render_template_for(variant, format)
             end
           RUBY
         end
@@ -586,22 +576,6 @@ def compiler
         @__vc_compiler ||= Compiler.new(self)
       end
 
-      # we'll eventually want to update this to support other types
-      # @private
-      def type
-        "text/html"
-      end
-
-      # @private
-      def format
-        :html
-      end
-
-      # @private
-      def identifier
-        source_location
-      end
-
       # Set the parameter name used when rendering elements of a collection ([documentation](/guide/collections)):
       #
       # ```ruby

diff --git a/lib/view_component/collection.rb b/lib/view_component/collection.rb
@@ -7,7 +7,6 @@ class Collection
     include Enumerable
     attr_reader :component
 
-    delegate :format, to: :component
     delegate :size, to: :@collection
 
     attr_accessor :__vc_original_view_context
@@ -41,6 +40,12 @@ def each(&block)
       components.each(&block)
     end
 
+    # Rails expects us to define `format` on all renderables,
+    # but we do not know the `format` of a ViewComponent until runtime.
+    def format
+      nil
+    end
+
     private
 
     def initialize(component, object, **options)

diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb
@@ -16,7 +16,7 @@ class Compiler
     def initialize(component_class)
       @component_class = component_class
       @redefinition_lock = Mutex.new
-      @variants_rendering_templates = Set.new
+      @rendered_templates = Set.new
     end
 
     def compiled?
@@ -61,22 +61,22 @@ def call
 
           component_class.silence_redefinition_of_method("render_template_for")
           component_class.class_eval <<-RUBY, __FILE__, __LINE__ + 1
-          def render_template_for(variant = nil)
+          def render_template_for(variant = nil, format = nil)
             _call_#{safe_class_name}
           end
           RUBY
         end
       else
         templates.each do |template|
-          method_name = call_method_name(template[:variant])
-          @variants_rendering_templates << template[:variant]
+          method_name = call_method_name(template[:variant], template[:format])
+          @rendered_templates << [template[:variant], template[:format]]
 
           redefinition_lock.synchronize do
             component_class.silence_redefinition_of_method(method_name)
             # rubocop:disable Style/EvalWithLocation
             component_class.class_eval <<-RUBY, template[:path], 0
             def #{method_name}
-              #{compiled_template(template[:path])}
+              #{compiled_template(template[:path], template[:format])}
             end
             RUBY
             # rubocop:enable Style/EvalWithLocation
@@ -97,37 +97,80 @@ def #{method_name}
       CompileCache.register(component_class)
     end
 
-    def renders_template_for_variant?(variant)
-      @variants_rendering_templates.include?(variant)
+    def renders_template_for?(variant, format)
+      @rendered_templates.include?([variant, format])
     end
 
     private
 
     attr_reader :component_class, :redefinition_lock
 
     def define_render_template_for
-      variant_elsifs = variants.compact.uniq.map do |variant|
-        safe_name = "_call_variant_#{normalized_variant_name(variant)}_#{safe_class_name}"
+      branches = []
+      default_method_name = "_call_#{safe_class_name}"
+
+      templates.each do |template|
+        safe_name = +"_call"
+        variant_name = normalized_variant_name(template[:variant])
+        safe_name << "_#{variant_name}" if variant_name.present?
+        safe_name << "_#{template[:format]}" if template[:format].present? && template[:format] != :html
+        safe_name << "_#{safe_class_name}"
+
+        if safe_name == default_method_name
+          next
+        else
+          component_class.define_method(
+            safe_name,
+            component_class.instance_method(
+              call_method_name(template[:variant], template[:format])
+            )
+          )
+        end
+
+        format_conditional =
+          if template[:format] == :html
+            "(format == :html || format.nil?)"
+          else
+            "format == #{template[:format].inspect}"
+          end
+
+        variant_conditional =
+          if template[:variant].nil?
+            "variant.nil?"
+          else
+            "variant&.to_sym == :'#{template[:variant]}'"
+          end
+
+        branches << ["#{variant_conditional} && #{format_conditional}", safe_name]
+      end
+
+      variants_from_inline_calls(inline_calls).compact.uniq.each do |variant|
+        safe_name = "_call_#{normalized_variant_name(variant)}_#{safe_class_name}"
         component_class.define_method(safe_name, component_class.instance_method(call_method_name(variant)))
 
-        "elsif variant.to_sym == :'#{variant}'\n    #{safe_name}"
-      end.join("\n")
+        branches << ["variant&.to_sym == :'#{variant}'", safe_name]
+      end
+
+      component_class.define_method(:"#{default_method_name}", component_class.instance_method(:call))
 
-      component_class.define_method(:"_call_#{safe_class_name}", component_class.instance_method(:call))
+      # Just use default method name if no conditional branches or if there is a single
+      # conditional branch that just calls the default method_name
+      if branches.empty? || (branches.length == 1 && branches[0].last == default_method_name)
+        body = default_method_name
+      else
+        body = +""
 
-      body = <<-RUBY
-        if variant.nil?
-          _call_#{safe_class_name}
-        #{variant_elsifs}
-        else
-          _call_#{safe_class_name}
+        branches.each do |conditional, method_body|
+          body << "#{(!body.present?) ? "if" : "elsif"} #{conditional}\n  #{method_body}\n"
         end
-      RUBY
+
+        body << "else\n  #{default_method_name}\nend"
+      end
 
       redefinition_lock.synchronize do
         component_class.silence_redefinition_of_method(:render_template_for)
         component_class.class_eval <<-RUBY, __FILE__, __LINE__ + 1
-        def render_template_for(variant = nil)
+        def render_template_for(variant = nil, format = nil)
           #{body}
         end
         RUBY
@@ -147,24 +190,16 @@ def template_errors
             errors << "Couldn't find a template file or inline render method for #{component_class}."
           end
 
-          if templates.count { |template| template[:variant].nil? } > 1
-            errors <<
-              "More than one template found for #{component_class}. " \
-              "There can only be one default template file per component."
-          end
+          templates
+            .map { |template| [template[:variant], template[:format]] }
+            .tally
+            .select { |_, count| count > 1 }
+            .each do |tally|
+            variant, this_format = tally[0]
 
-          invalid_variants =
-            templates
-              .group_by { |template| template[:variant] }
-              .map { |variant, grouped| variant if grouped.length > 1 }
-              .compact
-              .sort
+            variant_string = " for variant `#{variant}`" if variant.present?
 
-          unless invalid_variants.empty?
-            errors <<
-              "More than one template found for #{"variant".pluralize(invalid_variants.count)} " \
-              "#{invalid_variants.map { |v| "'#{v}'" }.to_sentence} in #{component_class}. " \
-              "There can only be one template file per variant."
+            errors << "More than one #{this_format.upcase} template found#{variant_string} for #{component_class}. "
           end
 
           if templates.find { |template| template[:variant].nil? } && inline_calls_defined_on_self.include?(:call)
@@ -213,6 +248,7 @@ def templates
             pieces = File.basename(path).split(".")
             memo << {
               path: path,
+              format: pieces[1..-2].join(".").split("+").first&.to_sym,
               variant: pieces[1..-2].join(".").split("+").second&.to_sym,
               handler: pieces.last
             }
@@ -239,6 +275,10 @@ def inline_calls_defined_on_self
       @inline_calls_defined_on_self ||= component_class.instance_methods(false).grep(/^call(_|$)/)
     end
 
+    def formats
+      @__vc_variants = (templates.map { |template| template[:format] }).compact.uniq
+    end
+
     def variants
       @__vc_variants = (
         templates.map { |template| template[:variant] } + variants_from_inline_calls(inline_calls)
@@ -258,37 +298,54 @@ def compiled_inline_template(template)
       compile_template(template, handler)
     end
 
-    def compiled_template(file_path)
+    def compiled_template(file_path, format)
       handler = ActionView::Template.handler_for_extension(File.extname(file_path).delete("."))
       template = File.read(file_path)
 
-      compile_template(template, handler)
+      compile_template(template, handler, file_path, format)
     end
 
-    def compile_template(template, handler)
+    def compile_template(template, handler, identifier = component_class.source_location, format = :html)
       template.rstrip! if component_class.strip_trailing_whitespace?
 
+      short_identifier = defined?(Rails.root) ? identifier.sub("#{Rails.root}/", "") : identifier
+      type = ActionView::Template::Types[format]
+
       if handler.method(:call).parameters.length > 1
-        handler.call(component_class, template)
+        handler.call(
+          OpenStruct.new(
+            format: format,
+            identifier: identifier,
+            short_identifier: short_identifier,
+            type: type
+          ),
+          template
+        )
       # :nocov:
       else
         handler.call(
           OpenStruct.new(
             source: template,
-            identifier: component_class.identifier,
-            type: component_class.type
+            identifier: identifier,
+            type: type
           )
         )
       end
       # :nocov:
     end
 
-    def call_method_name(variant)
-      if variant.present? && variants.include?(variant)
-        "call_#{normalized_variant_name(variant)}"
-      else
-        "call"
+    def call_method_name(variant, format = nil)
+      out = +"call"
+
+      if variant.present?
+        out << "_#{normalized_variant_name(variant)}"
+      end
+
+      if format.present? && format != :html && formats.length > 1
+        out << "_#{format}"
       end
+
+      out
     end
 
     def normalized_variant_name(variant)