Enable (experimental) single file components #1556

BlakeWilliams · 2022-10-20T14:26:10Z

This change adds an experimental module,
ViewComponent::InlineTemplate that allows you to define your templates using a basic DSL directly within your component. This eliminates the need for sidecar templates.

This is what it looks like:

class MyComponent < ApplicationComponent
  include ViewComponent::InlineTemplate

  erb_template <<~ERB
    <div>
      <h1>Hello, <%= @name %>!</h1>
    </div>
  ERB

  def initialize(name:)
    @name = name
  end
end

This results in component being "self-contained", in that a single file can represent the entire component. This makes it easier to understand a component and requires less jumping around between files. This also has benefits like enabling the following, which feels closely related to our multi-template efforts:

(this is not implemented, just an example of what this functionality can enable)

class MyComponent < ApplicationComponent
  include ViewComponent::InlineTemplate

  erb_template <<~ERB
    <div>
      <h1>Hello, <%= @name %>!</h1>

      <% post.each do.%>
        <%= render_row(post) %>
      <% end %>
    </div>
  ERB

  erb_template :row, -> (post) {
    <<~ERB
    <tr>
      <td><%= post.title %></td>
      <td><%= post.body %></td>
    </tr>
    ERB
  }

  def initialize(name:)
    @name = name
  end

  def posts
    Post.all
  end
end

cheshire137 · 2022-10-20T14:46:25Z

eliminates the need for sidecar templates

What do you mean by a sidecar template? Just "the .html.erb file with the same name as the component .rb file in the same directory"?

BlakeWilliams · 2022-10-20T14:47:03Z

What do you mean by a sidecar template? Just "the .html.erb file with the same name as the component .rb file in the same directory"?

Yep, exactly.

joelhawksley · 2022-10-20T14:47:54Z

@fsateler would this satisfy your requirements for multiple templates?

fsateler · 2022-10-20T16:45:04Z

@fsateler would this satisfy your requirements for multiple templates?

If fragment is implemented, with arguments, yes. I will probably end up doing fragment :foo, File.read(_dir_ + '/foo.html.erb') anyway though.

Although it seems to me this is punting on the difficult part, which is the arguments. The (not implemented) example would have a lambda taking an argument that returns an ERB string, how would that get turned into def render_foo(post)? Invoke the lambda passing nil as argument? what if the user tries to use the arguments directly in the template (as in string interpolation: "#{post.name}"). ?

joelhawksley · 2022-10-20T19:17:05Z

@fsateler would this satisfy your requirements for multiple templates?

If fragment is implemented, with arguments, yes. I will probably end up doing fragment :foo, File.read(_dir_ + '/foo.html.erb') anyway though.

Although it seems to me this is punting on the difficult part, which is the arguments. The (not implemented) example would have a lambda taking an argument that returns an ERB string, how would that get turned into def render_foo(post)? Invoke the lambda passing nil as argument? what if the user tries to use the arguments directly in the template (as in string interpolation: "#{post.name}"). ?

@fsateler What if we used the arguments approach from your PR, including them in the heredoc?

BlakeWilliams · 2022-10-20T19:35:46Z

I don't think the initial PR should support fragment!, but I think we have a few ways that we could potentially support arguments. Ideally I'd like to keep it really simple and use Ruby primitives, but if we can't achieve that we can always make some compromises to keep it simple but functional. e.g.

fragment! :row, accepts: [:post], <<~ERB
  <tr>
    <td><%= post.title %></td>
    <td><%= post.body %></td>
  </tr>
ERB

We could take the accepts array and use that to build the build_row parameters definition e.g. def render_row(post:). I was hoping we could use Proc#parameters in the original example, but it (sadly) omits default argument values.

test/sandbox/test/inline_erb_test.rb

fsateler · 2022-10-20T21:02:44Z

@fsateler would this satisfy your requirements for multiple templates?

If fragment is implemented, with arguments, yes. I will probably end up doing fragment :foo, File.read(_dir_ + '/foo.html.erb') anyway though.
Although it seems to me this is punting on the difficult part, which is the arguments. The (not implemented) example would have a lambda taking an argument that returns an ERB string, how would that get turned into def render_foo(post)? Invoke the lambda passing nil as argument? what if the user tries to use the arguments directly in the template (as in string interpolation: "#{post.name}"). ?

@fsateler What if we used the arguments approach from your PR, including them in the heredoc?

Then that would be almost indistiguishable from my PR. A few extra steps if you want a separate file (which is likely if the component is big enough to want to split into multiple fragments).

I don't think the initial PR should support fragment!,

Honestly, that would be a bit frustrating, as that would probably delay the solving of #387.

but I think we have a few ways that we could potentially support arguments. Ideally I'd like to keep it really simple and use Ruby primitives, but if we can't achieve that we can always make some compromises to keep it simple but functional. e.g.
fragment! :row, accepts: [:post], <<~ERB
  <tr>
    <td><%= post.title %></td>
    <td><%= post.body %></td>
  </tr>
ERB
We could take the accepts array and use that to build the build_row parameters definition e.g. def render_row(post:). I was hoping we could use Proc#parameters in the original example, but it (sadly) omits default argument values.

That sounds a lot like one of the options we discussed (and discarded): #387 (comment)

joelhawksley · 2022-10-20T21:05:38Z

@fsateler @BlakeWilliams why not use the Strict Locals syntax here? It's a solved problem 😄

BlakeWilliams · 2022-10-20T21:07:57Z

@fsateler @BlakeWilliams why not use the Strict Locals syntax here? It's a solved problem 😄

Good point! I think fragment! would be a good place to experiment with that.

fsateler · 2022-10-20T23:08:12Z

It has just occurred to me, will this give proper line numbers in stack traces?

BlakeWilliams · 2022-10-20T23:12:36Z

It has just occurred to me, will this give proper line numbers in stack traces?

As-is no, it's something I need to write a test for, but it's definitely doable.

test/sandbox/test/inline_erb_test.rb

BlakeWilliams · 2022-10-24T21:04:41Z

I just marked this as ready for review. It's pretty minimal, but I think it covers the basics needed to start experimenting with inline templates.

We chatted a bit about fragments, but I want to keep that separate from this PR since that solve a separate problem and might require a lot more back-and-forth discussion. I'll tackle that in a follow-up PR assuming this PR gets a 👍.

Happy to hear thoughts and feedback on this approach and discuss any specific points folks make.

BlakeWilliams · 2022-10-24T21:09:21Z

lib/view_component/inline_template.rb

+
+ class_methods do
+ def template!(template)
+ caller = caller_locations(1..1)[0]


This makes a small assumption, which is that all template! calls will be formatted like:

template <<~ERB ERB

and not

template!( <<~ERB ERB )

The newline is what's problematic here, since we assume that the template will start on the same line as the method call (template!). I think that's fine, but if it becomes a problem we can provide more "fine grained" options like allowing an offset to be passed

It also cannot be:

template! "<div>Alert!" tmplt = "asdf" template! tmplt def self.generate_template "a_template" end template! generate_template template! File.read("some_file.html.erb")

I'm not a fan of imposing this constraint. We should accept all valid Ruby.

What if we were to wrap template errors with a message along the lines of "Error on line 3 of MyComponent inline template"

I have some ideas here on how we might be able to improve the error messages, but I'd prefer they weren't blocking. Any opposition to merging this as-is and exploring better error strategies later? This is marked as experimental, so a little bit of churn seems fine here imo.

Spone · 2022-10-24T21:19:30Z

Looks good! Happy to see an experiment around this.

I'm wondering:

if you plan on adding some docs already, or wait until it's no longer experimental
what's the rationale behind using bang methods for template! and template_engine!? preventing collision with existing user methods? just shouting at the component? 😆
if we should warn/raise when calling template! or template_engine! multiple times in a component

BlakeWilliams · 2022-10-25T13:04:22Z

if you plan on adding some docs already, or wait until it's no longer experimental

I was thinking about passing on documentation for the first round, then fill it in once it's been tried out for real in a few spots.

what's the rationale behind using bang methods for template! and template_engine!? preventing collision with existing user methods? just shouting at the component? 😆

Honestly, no real solid reason. I thought it would be nice to signal "this is happening only once!" but happy to change it if folks feel strongly.

if we should warn/raise when calling template! or template_engine! multiple times in a component

Good call. I think that will be easy to implement for template!, I can add that.

test/sandbox/test/inline_template_test.rb

@name

This change adds an experimental module, `ViewComponent::InlineTemplate` that allows you to define your templates using a basic DSL directly within your component. This eliminates the need for sidecar templates. This is what it looks like: ```erb class MyComponent < ApplicationComponent include ViewComponent::InlineTemplate template <<~ERB <div> <h1>Hello, <%= @name %>!</h1> </div> ERB def initialize(name:) @name = name end end ``` This results in component being "self-contained", in that a single file can represent the entire component. This makes it easier to understand a component and requires less jumping around between files. This also has benefits like enabling the following, which feels closely related to our multi-template efforts: (this is not implemented, just an example of what this functionality can enable) ```erb class MyComponent < ApplicationComponent include ViewComponent::InlineTemplate template <<~ERB <div> <h1>Hello, <%= @name %>!</h1> <% post.each do.%> <%= render_row(post) %> <% end %> </div> ERB fragment :row, -> (post) { <<~ERB <tr> <td><%= post.title %></td> <td><%= post.body %></td> </tr> ERB } def initialize(name:) @name = name end def posts Post.all end end ```

test/sandbox/test/inline_template_test.rb

joelhawksley

LGTM, mind adding docs? Happy to edit heavily if you can get something rough together.

joelhawksley · 2023-02-17T16:56:41Z

lib/view_component/compiler.rb

- # Remove existing compiled template methods,
- # as Ruby warns when redefining a method.
- method_name = call_method_name(template[:variant])
+ if component_class.respond_to?(:inline_template) && component_class.inline_template.present?


Could we use has_inline_template? here?

cheshire137 · 2023-02-22T19:20:46Z

Thanks for this! I think this will be really nice for tiny little components.

Bestra · 2023-02-22T20:06:16Z

Oooh, I really like this! Thanks for keeping DX improvements like this coming!

@name

* Enable (experimental) single file components This change adds an experimental module, `ViewComponent::InlineTemplate` that allows you to define your templates using a basic DSL directly within your component. This eliminates the need for sidecar templates. This is what it looks like: ```erb class MyComponent < ApplicationComponent include ViewComponent::InlineTemplate template <<~ERB <div> <h1>Hello, <%= @name %>!</h1> </div> ERB def initialize(name:) @name = name end end ``` This results in component being "self-contained", in that a single file can represent the entire component. This makes it easier to understand a component and requires less jumping around between files. This also has benefits like enabling the following, which feels closely related to our multi-template efforts: (this is not implemented, just an example of what this functionality can enable) ```erb class MyComponent < ApplicationComponent include ViewComponent::InlineTemplate template <<~ERB <div> <h1>Hello, <%= @name %>!</h1> <% post.each do.%> <%= render_row(post) %> <% end %> </div> ERB fragment :row, -> (post) { <<~ERB <tr> <td><%= post.title %></td> <td><%= post.body %></td> </tr> ERB } def initialize(name:) @name = name end def posts Post.all end end ``` * Use consistent method names * Add test using slots * add docs * use has_inline_template? method --------- Co-authored-by: Joel Hawksley <[email protected]>

@name

* Enable (experimental) single file components This change adds an experimental module, `ViewComponent::InlineTemplate` that allows you to define your templates using a basic DSL directly within your component. This eliminates the need for sidecar templates. This is what it looks like: ```erb class MyComponent < ApplicationComponent include ViewComponent::InlineTemplate template <<~ERB <div> <h1>Hello, <%= @name %>!</h1> </div> ERB def initialize(name:) @name = name end end ``` This results in component being "self-contained", in that a single file can represent the entire component. This makes it easier to understand a component and requires less jumping around between files. This also has benefits like enabling the following, which feels closely related to our multi-template efforts: (this is not implemented, just an example of what this functionality can enable) ```erb class MyComponent < ApplicationComponent include ViewComponent::InlineTemplate template <<~ERB <div> <h1>Hello, <%= @name %>!</h1> <% post.each do.%> <%= render_row(post) %> <% end %> </div> ERB fragment :row, -> (post) { <<~ERB <tr> <td><%= post.title %></td> <td><%= post.body %></td> </tr> ERB } def initialize(name:) @name = name end def posts Post.all end end ``` * Use consistent method names * Add test using slots * add docs * use has_inline_template? method --------- Co-authored-by: Joel Hawksley <[email protected]>

BlakeWilliams marked this pull request as draft October 20, 2022 14:26

BlakeWilliams commented Oct 20, 2022

View reviewed changes

test/sandbox/test/inline_erb_test.rb Outdated Show resolved Hide resolved

joelhawksley reviewed Oct 24, 2022

View reviewed changes

test/sandbox/test/inline_erb_test.rb Outdated Show resolved Hide resolved

BlakeWilliams marked this pull request as ready for review October 24, 2022 20:59

BlakeWilliams commented Oct 24, 2022

View reviewed changes

joelhawksley reviewed Jan 25, 2023

View reviewed changes

test/sandbox/test/inline_template_test.rb Outdated Show resolved Hide resolved

joelhawksley mentioned this pull request Jan 25, 2023

Multiple templates per component, allowing arguments #451

Open

BlakeWilliams force-pushed the bmw/inline-templates branch from 504d92c to dfef286 Compare January 27, 2023 19:25

Use consistent method names

2e8ed8a

joelhawksley reviewed Jan 27, 2023

View reviewed changes

test/sandbox/test/inline_template_test.rb Show resolved Hide resolved

BlakeWilliams and others added 2 commits January 28, 2023 15:43

Add test using slots

ab0c15b

Merge branch 'main' into bmw/inline-templates

7edf4c0

joelhawksley reviewed Feb 17, 2023

View reviewed changes

joelhawksley added 2 commits February 22, 2023 11:33

add docs

9709667

use has_inline_template? method

823ed4c

joelhawksley approved these changes Feb 22, 2023

View reviewed changes

Merge branch 'main' into bmw/inline-templates

8b41eee

joelhawksley enabled auto-merge (squash) February 22, 2023 19:01

joelhawksley disabled auto-merge February 22, 2023 19:01

joelhawksley merged commit 2113f25 into main Feb 22, 2023

joelhawksley deleted the bmw/inline-templates branch February 22, 2023 19:01

This was referenced Feb 22, 2023

Render behavior in component classes #569

Closed

Look into using single file ViewComponents primer/view_components#1842

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable (experimental) single file components #1556

Enable (experimental) single file components #1556

BlakeWilliams commented Oct 20, 2022 •

edited

Loading

cheshire137 commented Oct 20, 2022

BlakeWilliams commented Oct 20, 2022

joelhawksley commented Oct 20, 2022

fsateler commented Oct 20, 2022 •

edited

Loading

joelhawksley commented Oct 20, 2022

BlakeWilliams commented Oct 20, 2022 •

edited

Loading

fsateler commented Oct 20, 2022

joelhawksley commented Oct 20, 2022

BlakeWilliams commented Oct 20, 2022

fsateler commented Oct 20, 2022

BlakeWilliams commented Oct 20, 2022

BlakeWilliams commented Oct 24, 2022

BlakeWilliams Oct 24, 2022

fsateler Oct 25, 2022

joelhawksley Nov 2, 2022

joelhawksley Nov 2, 2022

BlakeWilliams Jan 30, 2023

Spone commented Oct 24, 2022

BlakeWilliams commented Oct 25, 2022

joelhawksley left a comment

joelhawksley Feb 17, 2023

cheshire137 commented Feb 22, 2023

Bestra commented Feb 22, 2023

Enable (experimental) single file components #1556

Enable (experimental) single file components #1556

Conversation

BlakeWilliams commented Oct 20, 2022 • edited Loading

cheshire137 commented Oct 20, 2022

BlakeWilliams commented Oct 20, 2022

joelhawksley commented Oct 20, 2022

fsateler commented Oct 20, 2022 • edited Loading

joelhawksley commented Oct 20, 2022

BlakeWilliams commented Oct 20, 2022 • edited Loading

fsateler commented Oct 20, 2022

joelhawksley commented Oct 20, 2022

BlakeWilliams commented Oct 20, 2022

fsateler commented Oct 20, 2022

BlakeWilliams commented Oct 20, 2022

BlakeWilliams commented Oct 24, 2022

BlakeWilliams Oct 24, 2022

Choose a reason for hiding this comment

fsateler Oct 25, 2022

Choose a reason for hiding this comment

joelhawksley Nov 2, 2022

Choose a reason for hiding this comment

joelhawksley Nov 2, 2022

Choose a reason for hiding this comment

BlakeWilliams Jan 30, 2023

Choose a reason for hiding this comment

Spone commented Oct 24, 2022

BlakeWilliams commented Oct 25, 2022

joelhawksley left a comment

Choose a reason for hiding this comment

joelhawksley Feb 17, 2023

Choose a reason for hiding this comment

cheshire137 commented Feb 22, 2023

Bestra commented Feb 22, 2023

BlakeWilliams commented Oct 20, 2022 •

edited

Loading

fsateler commented Oct 20, 2022 •

edited

Loading

BlakeWilliams commented Oct 20, 2022 •

edited

Loading