Feature/multiple templates arguments #1096

jsolas · 2021-10-13T17:50:40Z

Summary
Fixes #387

This PR builds on the branch by @joelhawksley, @fermion, @fsateler and adds the ability to specify arguments for the generated functions, based on what was discussed #387 (comment). In this PR:

1- Each template would define a method called render_<template_name>(locals = {})
2- We will not be defining the locals inside the generated method like rails does. They would need to be accessed with locals[:foo]

The logic being that one may want to use templates to extract sections, without wanting to build a full component. For example, I could have a Table component, and I would want to have the row definition in a sidecar template. This requires being able to pass in each item for every iteration:

class TableComponent < ViewComponent::Base
  def initialize(models)
    @models = models
  end
end

<%# table_component.html.erb %>
<table>
  <% @models.each do |model| %>
    <%= render_row model: model %>
  <% end %>
</table>

<%# row.html.erb %>
<tr>
  <%# I can use `model` here %>
  <td><%= locals[:model].name  %></td>
</tr>

Co-authored-by: Rob Sterner <fermion@github.com>

fsateler

Thanks for picking this up @jsolas! I've added a few comments.

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

fsateler · 2021-10-13T19:42:49Z

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

+
+The generated methods are only invokable via keyword arguments
+
+The generated methods cannot have arguments with default values.


No, but they can be simulated by the view.

fsateler · 2021-10-13T19:43:16Z

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

+
+The generated methods cannot have arguments with default values.
+
+The generated methods are public, and thus could be invoked by a third party.


This could be changed, but not sure if it's worth it?

I think it would be nice to introduce them as private to start.

lib/view_component/compiler.rb

test/view_component/view_component_test.rb

jsolas · 2021-10-16T19:04:36Z

Hello @BlakeWilliams, @camertron, @joelhawksley, i tried to do what was discussed in #387 (comment), I would like to receive your opinions and see if there is a possibility of moving forward with this. Thanks.

joelhawksley

Looking good! I think we're on the right track here so I've marked the ADR as accepted.

joelhawksley · 2021-10-21T16:28:12Z

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

@@ -0,0 +1,88 @@
+# 1. Allow multiple templates 


We need a name for this feature, I think. What about:

Suggested change

# 1. Allow multiple templates

# 3. Template parts

joelhawksley · 2021-10-21T16:28:22Z

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

+
+## Status
+
+Proposed.


Suggested change

Proposed.

Accepted

joelhawksley · 2021-10-21T16:30:20Z

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

+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. 


Suggested change

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.

As components become larger (for example, implementing a whole page), it can become useful to be able to extract sections of the view to a different file. ActionView has partials, and ViewComponent lacks a similar mechanism.

joelhawksley · 2021-10-21T16:30:48Z

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

+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.


Suggested change

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.

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.

joelhawksley · 2021-10-21T16:31:28Z

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

+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 = {})`


Suggested change

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 = {})`

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 = {})`.

joelhawksley · 2021-10-21T16:36:45Z

docs/CHANGELOG.md

@@ -36,6 +36,10 @@ title: Changelog

    *Hans Lemuet*

+* Add support for multiple templates.


Suggested change

* Add support for multiple templates.

* Add support for template parts.

joelhawksley · 2021-10-21T16:37:16Z

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


Suggested change

## Multiple templates with arguments

## Template parts

joelhawksley · 2021-10-21T16:37:25Z

docs/guide/templates.md

+
+## Multiple templates with arguments
+
+ViewComponents can render multiple templates defined in the sidecar directory and send arguments to it:


Suggested change

ViewComponents can render multiple templates defined in the sidecar directory and send arguments to it:

ViewComponents can render multiple templates defined in the sidecar directory:

joelhawksley · 2021-10-21T16:39:10Z

docs/guide/templates.md

+├── ...
+```
+
+Templates are compiled to methods in the format `render_#{template_basename}(locals = {})`, which can then be called in the component.


Suggested change

Templates are compiled to methods in the format `render_#{template_basename}(locals = {})`, which can then be called in the component.

Template parts can then be rendered by calling `render_#{template_basename}`. Template parts accept a single positional argument that is accessible in the template as `locals`, defaulting to an empty hash if no argument is passed.

joelhawksley · 2021-10-21T16:40:00Z

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}}"]


What happens if there is a slot/subcomponent with a template in the Sidecar directory? How can we prevent/manage conflicts between the two?

BlakeWilliams · 2021-10-21T17:11:18Z

Thanks for opening this PR and taking this on!

Similar to how we handled Slots V2, is there a way we could mark this functionality as experimental and have components opt-in to this feature? I think there's a lot of value in trying this functionality out in real applications and making modifications as necessary before making this a stable, long-term API.

joelhawksley · 2021-10-22T22:29:36Z

@BlakeWilliams and I were chatting about this PR today and realized that there might be another option we could consider: private slots with templates but no sub-component, where the slot automatically accepts a single argument locals = {}.

For example, we might have:

# table_component.rb
class TableComponent < ViewComponent::Base
  renders_many :rows, private: true

  def initialize(items)
    items.each do |item|
      row({ title: item.title, author: item.author })
    end
  end
end

<!-- table_component.html.erb -->
<% rows.each do |row| %>
  <%= row %>
<% end %>

<!-- table_component/row.html.erb -->
<tr>
  <td><%= locals[:title] %></td>
  <td><%= locals[:author] %></td>
</tr>

I believe this would fulfill the desire for a private API and it would avoid allocating a new component for each row. What do y'all think?

fsateler · 2021-10-25T19:38:31Z

@joelhawksley What should the rows array contain? An array of (html-safe) strings? Or some other intermediate object? While possibly not allocating a component, it would still store a (possibly) large array of something.

joelhawksley · 2021-10-25T21:47:32Z

@fsateler yes, I think you'd have an array of HTML-safe strings.

Given the concerns about performance that are driving this work, I think it might be wise for us to implement a benchmark as part of this PR that compares the number of allocations with the new approach vs. an equivalent case with rendering a component for each row.

fsateler · 2021-11-08T17:42:48Z

@joelhawksley I feel this proposal is not very railsy. In particular, the loop in the initializer feels like boilerplate. I don't really like the API.

Moreover, I think this proposal would require to have something like what this PR is proposing and on top adding the private slot API

joelhawksley · 2021-11-18T19:13:48Z

@fsateler I think it would also be possible to use the plural slot setter if you didn't want to decompose items, like this:

# table_component.rb
class TableComponent < ViewComponent::Base
  renders_many :rows, private: true

  def initialize(items)
    rows(items)
  end
end

The difference between ☝🏻 and the proposal in this PR is that it remains inside of the existing Slots API, which I think would help reduce potential confusion around its introduction and use.

joelmoss · 2022-04-02T11:17:01Z

Has this moved along at all recently? I have a real need to be able to dynamically determine the template to render within a component, and this PR seems to enable that. Would be great to get something out, and I would be happy to help.

joelhawksley · 2022-04-08T15:29:23Z

@joelmoss I don't think it's moved since my last comment from what I can tell. @jsolas @fsateler would you be OK with @joelmoss taking this on?

fsateler · 2022-04-18T19:09:06Z

Not a problem on my side! If this helps things move forward, please go ahead!

fsateler · 2022-05-20T14:41:16Z

Hi! I've been thinking about @joelhawksley 's proposal for a new API. I don't think it's going to work, not without a lot of refactoring:

with_<slotname> creates a SlotV2 object for each item in the collection.
This creation is eager, because at render time we iterate through @_set_slots[slot_name], which must have been prepopulated by with_<slotname>. This means the SlotV2 object creation is eagerly done.

This means we will create an intermediate array of intermediate objects, which is what we wanted to avoid.

Moreover, we still would need a lower-level api for actually rendering the template (which would be used by the slot interface). Please lets move forward with this as the lower-level API 🙏

Add support for multiple templates.

4e864d5

Co-authored-by: Rob Sterner <fermion@github.com>

jsolas requested a review from a team as a code owner October 13, 2021 17:50

jsolas force-pushed the feature/multiple-templates-arguments branch 2 times, most recently from 86b4392 to 666913c Compare October 13, 2021 19:06

fsateler suggested changes Oct 13, 2021

View reviewed changes

jsolas force-pushed the feature/multiple-templates-arguments branch 2 times, most recently from fcdace0 to 37dd2fb Compare October 15, 2021 13:19

Add ability to define templates that take parameters

dff7c0d

jsolas force-pushed the feature/multiple-templates-arguments branch from 37dd2fb to dff7c0d Compare October 16, 2021 18:43

joelhawksley reviewed Oct 21, 2021

View reviewed changes

joelhawksley mentioned this pull request Jun 17, 2022

Deprecate Base#with_variant #1041

Merged

fsateler mentioned this pull request Aug 16, 2022

Support multiple views #387

Open

joelhawksley added the active label Apr 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/multiple templates arguments #1096

Feature/multiple templates arguments #1096

jsolas commented Oct 13, 2021

fsateler left a comment

fsateler Oct 13, 2021

fsateler Oct 13, 2021

joelhawksley Oct 21, 2021

jsolas commented Oct 16, 2021

joelhawksley left a comment

joelhawksley Oct 21, 2021

joelhawksley Oct 21, 2021

joelhawksley Oct 21, 2021

joelhawksley Oct 21, 2021

joelhawksley Oct 21, 2021

joelhawksley Oct 21, 2021

joelhawksley Oct 21, 2021

joelhawksley Oct 21, 2021

joelhawksley Oct 21, 2021

joelhawksley Oct 21, 2021

BlakeWilliams commented Oct 21, 2021 •

edited

joelhawksley commented Oct 22, 2021

fsateler commented Oct 25, 2021

joelhawksley commented Oct 25, 2021

fsateler commented Nov 8, 2021 •

edited

joelhawksley commented Nov 18, 2021

joelmoss commented Apr 2, 2022

joelhawksley commented Apr 8, 2022

fsateler commented Apr 18, 2022

fsateler commented May 20, 2022


		The generated methods are only invokable via keyword arguments

		The generated methods cannot have arguments with default values.


		The generated methods cannot have arguments with default values.

		The generated methods are public, and thus could be invoked by a third party.

		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 = {})`

		@@ -36,6 +36,10 @@ title: Changelog

		Hans Lemuet

		* Add support for multiple templates.

	* Add support for multiple templates.
	* Add support for template parts.


		## Multiple templates with arguments

		ViewComponents can render multiple templates defined in the sidecar directory and send arguments to it:

	Templates are compiled to methods in the format `render_#{template_basename}(locals = {})`, which can then be called in the component.
	Template parts can then be rendered by calling `render_#{template_basename}`. Template parts accept a single positional argument that is accessible in the template as `locals`, defaulting to an empty hash if no argument is passed.

Feature/multiple templates arguments #1096

Are you sure you want to change the base?

Feature/multiple templates arguments #1096

Conversation

jsolas commented Oct 13, 2021

fsateler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsolas commented Oct 16, 2021

joelhawksley left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BlakeWilliams commented Oct 21, 2021 • edited

joelhawksley commented Oct 22, 2021

fsateler commented Oct 25, 2021

joelhawksley commented Oct 25, 2021

fsateler commented Nov 8, 2021 • edited

joelhawksley commented Nov 18, 2021

joelmoss commented Apr 2, 2022

joelhawksley commented Apr 8, 2022

fsateler commented Apr 18, 2022

fsateler commented May 20, 2022

BlakeWilliams commented Oct 21, 2021 •

edited

fsateler commented Nov 8, 2021 •

edited