Feature/multiple templates arguments

adr/0003-multiple-templates-per-component.md

@@ -0,0 +1,88 @@
+# 1. Allow multiple templates 
+
+Date: 2021-10-13
+
+## Status
+
+Proposed.
+
+## Context
+
+As components become larger (for example, because you are implementing a whole page), it becomes
+useful to be able to extract sections of the view to a different file. ActionView has
+partials, and ViewComponent lacks a similar mechanism. 
+
+ActionView partials have the problem that their interface is not introspectable. Data
+may be passed into the partial via ivars or locals, and it is impossible to know
+which without actually opening up the file. Additionally, partials are globally
+invocable, thus making it difficult to detect if a given partial is in use or not,
+and who are its users.
+
+## Considered Options
+
+* Introduce component partials to components
+* Keep components as-is
+
+### Component partials
+
+Allow multiple ERB templates available within the component, and make it possible to
+invoke them from the main view.Templates are compiled to methods in the format `render_#{template_basename}(locals = {})`
+
+**Pros:**
+* Better performance due to lack of GC pressure and object creation
+* Reduces the number of components needed to express a more complex view.
+* Extracted sections are not exposed outside the component, thus reducing component library API surface.
+
+**Cons:**
+* Another concept for users of ViewComponent to learn and understand.
+* Components are no longer the only way to encapsulate behavior.
+
+### Partial components by [fstaler](https://github.com/fsateler)
+
+In this approach the template arguments are previously defined in the `template_arguments` method
+
+[See here](https://github.com/github/view_component/pull/451):
+
+**Pros:**
+* Better performance due to lack of GC pressure and object creation
+* Reduces the number of components needed to express a more complex view.
+* Extracted sections are not exposed outside the component, thus reducing component library API surface.
+
+**Cons:**
+* Another concept for users of ViewComponent to learn and understand.
+* Components are no longer the only way to encapsulate behavior.
+* `call_foo` api feels awkward and not very Rails like
+* Declare templates and their arguments explicitly before using them
+
+### Keeping components as-is
+
+**Pros:**
+* The API remains simple and components are the only way to encapsulate behavior.
+* Encourages creating reusable sub-components.
+
+**Cons:**
+* Extracting a component results in more GC and intermediate objects.
+* Extracting a component may result in tightly coupled but split components.
+* Creates new public components thus expanding component library API surface.
+
+## Decision
+
+We will allow having multiple templates in the sidecar asset. Each asset will be compiled to
+it's own method `render_<template_name>`. I think it is simple, similar to rails and meets what is expected
+
+## Consequences
+
+This implementation has better performance characteristics over both an extracted component
+and ActionView partials, because it avoids creating intermediate objects, and the overhead of
+creating bindings and `instance_exec`. 
+Having explicit arguments makes the interface explicit.
+
+TODO: The following are consequences of the current approach, but the approach might be extended
+to avoid them:
+
+The interface to render a sidecar partial would be a method call, and depart from the usual 
+`render(*)` interface used in ActionView.
+
+The generated methods cannot have arguments with default values.
+
+The generated methods are public, and thus could be invoked by a third party.

docs/CHANGELOG.md

@@ -36,6 +36,10 @@ title: Changelog
 
     *Hans Lemuet*
 
+* Add support for multiple templates.
+
+    *Rob Sterner*, *Joel Hawksley*
+
 ## 2.40.0
 
 * Replace antipatterns section in the documentation with best practices.

docs/guide/templates.md

@@ -92,3 +92,36 @@ Component subclasses inherit the parent component's template if they don't defin
 class MyLinkComponent < LinkComponent
 end
 ```
+
+## Multiple templates with arguments
+
+ViewComponents can render multiple templates defined in the sidecar directory and send arguments to it:
+
+```console
+app/components
+├── ...
+├── test_component.rb
+├── test_component
+|   ├── list.html.erb
+|   └── summary.html.erb
+├── ...
+```
+
+Templates are compiled to methods in the format `render_#{template_basename}(locals = {})`, which can then be called in the component.
+
+```ruby
+class TestComponent < ViewComponent::Base
+  def initialize(mode:)
+    @mode = mode
+  end
+
+  def call
+    case @mode
+    when :list
+      render_list number: 1
+    when :summary
+      render_summary string: "foo"
+    end
+  end
+end
+```

lib/view_component/base.rb

@@ -331,7 +331,7 @@ def _sidecar_files(extensions)
         # view files in the same directory as the component
         sidecar_files = Dir["#{directory}/#{component_name}.*{#{extensions}}"]
 
-        sidecar_directory_files = Dir["#{directory}/#{component_name}/#{filename}.*{#{extensions}}"]
+        sidecar_directory_files = Dir["#{directory}/#{component_name}/*.*{#{extensions}}"]
 
         (sidecar_files - [source_location] + sidecar_directory_files + nested_component_files).uniq
       end

lib/view_component/compiler.rb

@@ -43,16 +43,29 @@ def compile(raise_errors: false)
       templates.each do |template|
         # Remove existing compiled template methods,
         # as Ruby warns when redefining a method.
-        method_name = call_method_name(template[:variant])
+        pieces = File.basename(template[:path]).split(".")
+
+        method_name =
+          # If the template matches the name of the component,
+          # set the method name with call_method_name
+          if pieces.first == component_class.name.demodulize.underscore
+            call_method_name(template[:variant])
+          # Otherwise, append the name of the template to render_
+          else
+            "render_#{pieces.first.to_sym}"
+          end
 
         if component_class.instance_methods.include?(method_name.to_sym)
           component_class.send(:undef_method, method_name.to_sym)
         end
 
-        component_class.class_eval <<-RUBY, template[:path], -1
-          def #{method_name}
+        component_class.class_eval <<-RUBY, template[:path], -2
+          def #{method_name}(**locals)
+            old_buffer = @output_buffer if defined? @output_buffer
             @output_buffer = ActionView::OutputBuffer.new
             #{compiled_template(template[:path])}
+          ensure
+            @output_buffer = old_buffer
           end
         RUBY
       end
@@ -98,7 +111,14 @@ def template_errors
             errors << "Could not find a template file or inline render method for #{component_class}."
           end
 
-          if templates.count { |template| template[:variant].nil? } > 1
+          # Ensure that template base names are unique
+          # for each variant
+          unique_templates =
+            templates.map do |template|
+              template[:base_name] + template[:variant].to_s
+            end
+
+          if unique_templates.length != unique_templates.uniq.length
             errors <<
               "More than one template found for #{component_class}. " \
               "There can only be one default template file per component."
@@ -118,7 +138,14 @@ def template_errors
               "There can only be one template file per variant."
           end
 
-          if templates.find { |template| template[:variant].nil? } && inline_calls_defined_on_self.include?(:call)
+          default_template_exists =
+            templates.find do |template|
+              pieces = File.basename(template[:path]).split(".")
+
+              template[:variant].nil? && pieces.first == component_class.name.demodulize.underscore
+            end
+
+          if default_template_exists && inline_calls_defined_on_self.include?(:call)
             errors <<
               "Template file and inline render method found for #{component_class}. " \
               "There can only be a template file or inline render method per component."
@@ -151,6 +178,7 @@ def templates
             pieces = File.basename(path).split(".")
             memo << {
               path: path,
+              base_name: path.split(File::SEPARATOR).last.split(".").first,
               variant: pieces.second.split("+").second&.to_sym,
               handler: pieces.last
             }

test/sandbox/app/components/multiple_templates_component.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class MultipleTemplatesComponent < ViewComponent::Base
+  def initialize
+    @items = ["Apple", "Banana", "Pear"]
+  end
+end

test/sandbox/app/components/multiple_templates_component/list.html.erb

@@ -0,0 +1,5 @@
+<ul data-number="<%= locals[:number] %>">
+  <% @items.each do |item| %>
+    <li><%= item %></li>
+  <% end %>
+</ul>

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

@@ -0,0 +1,4 @@
+<div class="container">
+  <%= render_summary string: "foo" %>
+  <%= render_list number: 1 %>
+</div>

test/sandbox/app/components/multiple_templates_component/summary.html.erb

@@ -0,0 +1 @@
+<div>The items are: <%= @items.to_sentence %>, <%= locals[:string] %></div>

test/view_component/view_component_test.rb

@@ -776,6 +776,17 @@ def test_collection_component_with_trailing_comma_attr_reader
     assert_match(/ProductReaderOopsComponent initializer is empty or invalid/, exception.message)
   end
 
+  def test_render_multiple_templates
+    render_inline(MultipleTemplatesComponent.new)
+
+    assert_selector("div", text: "The items are: Apple, Banana, and Pear, foo")
+    assert_selector("li", text: "Apple")
+    assert_selector("li", text: "Banana")
+    assert_selector("li", text: "Pear")
+
+    assert_selector("div.container")
+  end
+
   def test_renders_component_using_rails_config
     render_inline(RailsConfigComponent.new)