diff --git a/adr/0003-multiple-templates-per-component.md b/adr/0003-multiple-templates-per-component.md new file mode 100644 index 000000000..6392ff2eb --- /dev/null +++ b/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_`. 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. \ No newline at end of file diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index d482a1c47..7105f662b 100644 --- a/docs/CHANGELOG.md +++ b/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. diff --git a/docs/guide/templates.md b/docs/guide/templates.md index f3144f5e0..abe693b81 100644 --- a/docs/guide/templates.md +++ b/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 +``` diff --git a/lib/view_component/base.rb b/lib/view_component/base.rb index 611bb7e0f..28ff812e0 100644 --- a/lib/view_component/base.rb +++ b/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 diff --git a/lib/view_component/compiler.rb b/lib/view_component/compiler.rb index 620075b95..4b369bb87 100644 --- a/lib/view_component/compiler.rb +++ b/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 } diff --git a/test/sandbox/app/components/multiple_templates_component.rb b/test/sandbox/app/components/multiple_templates_component.rb new file mode 100644 index 000000000..62aff04ea --- /dev/null +++ b/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 diff --git a/test/sandbox/app/components/multiple_templates_component/list.html.erb b/test/sandbox/app/components/multiple_templates_component/list.html.erb new file mode 100644 index 000000000..13683cae4 --- /dev/null +++ b/test/sandbox/app/components/multiple_templates_component/list.html.erb @@ -0,0 +1,5 @@ + diff --git a/test/sandbox/app/components/multiple_templates_component/multiple_templates_component.html.erb b/test/sandbox/app/components/multiple_templates_component/multiple_templates_component.html.erb new file mode 100644 index 000000000..e3d1c440f --- /dev/null +++ b/test/sandbox/app/components/multiple_templates_component/multiple_templates_component.html.erb @@ -0,0 +1,4 @@ +
+ <%= render_summary string: "foo" %> + <%= render_list number: 1 %> +
diff --git a/test/sandbox/app/components/multiple_templates_component/summary.html.erb b/test/sandbox/app/components/multiple_templates_component/summary.html.erb new file mode 100644 index 000000000..853913d47 --- /dev/null +++ b/test/sandbox/app/components/multiple_templates_component/summary.html.erb @@ -0,0 +1 @@ +
The items are: <%= @items.to_sentence %>, <%= locals[:string] %>
diff --git a/test/view_component/view_component_test.rb b/test/view_component/view_component_test.rb index 4e6adf427..842f6cba4 100644 --- a/test/view_component/view_component_test.rb +++ b/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)