Support slot rendering from controllers

[th-ad]

Feature request

Currently the render_component method in ViewComponent::RenderingComponentHelper does not accept a block. This prevents slots from being rendered in controllers.

Motivation

With the rise in popularity of HTML over the wire, this seems like a valuable feature to have in order to render more complex components.

class GalleryController < ApplicationController
  def index
    render_component(GalleryComponent.new) do |gallery|
      gallery.image
      ...
    end
  end
end

I saw the initial issue/PR for enabling component rendering in controllers (#235 ), but wasn't sure if there was a technical limitation with adding block support. Definitely happy to take on the issue if it's something people are interested in.

Edit: Now that I'm looking closer I also see this commit, which intentionally removed block support.

[lfalcao]

@th-ad there's any difference between using controller or view, besides less files? :thinking:

[boardfish]

@lfalcao I'd say there is. If you're rendering a component directly from the controller, then you get the benefit of encapsulation. So if you were rendering a component from a view file, you'd still have to pass the instance variables from the controller, e.g.:

class FooController < ApplicationController
  def foo
    @foo = 'Hello world'
  end

<%= render View::Foo.new(foo: @foo) %>

If you were rendering a component directly from the controller, you'd do so knowing that nothing leaks between the controller and component unless you pass it explicitly.

618 exists with the aim of making it possible to render components in place of traditional Action View templates. It'd be helpful to use slots here to make it easier to reuse a 'view' component between actions, or change components of the page based on controller logic, e.g.

def foo
  render(View::Foo.new) do |c|
    c.button(disabled: !logged_in?)
  end
end

Slots will be really key here in ensuring that logic stays at the controller level and doesn't get coupled to the component itself.

[boardfish]

I've put together a test case for this in #1010. However, my efforts to try and fix it have been in vain - I'm not sure of the timing on it, but I think that the feature was originally introduced back when the library needed to patch Rails to function. Now that render_in is supported directly in Rails itself, it's Rails' implementation that bars us from using blocks in the controller.

[joelhawksley]

Now that render_in is supported directly in Rails itself, it's Rails' implementation that bars us from using blocks in the controller.

That's correct. If we want to make this change, we need to make in Rails first. I want to avoid any further monkey patching if at all possible, at least for Rails main. That being said, I do think that we should look at adding this support to Rails. Are you up for having a look?

[boardfish]

I guess if anyone has strong feelings against allowing this, a possible argument would be that render_inline exists to test rendered output and doesn't do exactly as render does. That's to say that now that Rails directly supports render_in, we should stick to their implementation as we seem to be doing now and avoid returning to a monkey patch approach.

That having been said, I'm only speculating on the original decision to remove it here. @joelhawksley, do you remember why the original change was made? I'm sure there's a good reason behind it, but I'm also wondering whether it should be re-evaluated, particularly in light of #618.

[jonspalmer]

I'm curious if we really need blocks at all to support the slots pattern. I think of the block as mostly being for generating the content. If you want static content then with_content works great. If you want helpers like content_tag or link_tag then I think its entirely reasonable that you should use views

#this doesn't work
def foo
  render(View::Foo.new) do |c|
    "My Content"
  end
end

#but this does and is equivalent
def bar
  render(View::Foo.new..with_content("My Content")
end

# if you want this use a view
def qux
  render(View::Foo.new) do |c|
    contet_tag(:p, "My Content")
  end
end

With regards to slots. The desire for blocks is this:

def foo
  render(View::Foo.new) do |c|
    c.button(disabled: !logged_in?)
  end
end

But can't we just do this:

def foo
  foo_component = View::Foo.new
  foo_component.button(disabled: !logged_in?)
  render(foo_component)
end

Same caveats - if you need helpers for the slot content then use a view.

[jonspalmer]

BTW for what its worth the fact that the block is used both for calling slot methods AND for the return value to be the content of the component is super weird. I get that it makes the erb/haml/slim code simpler but it still very unexpected.

[boardfish]

That all sounds reasonable. Using tap on the component instance could be helpful to maintain the block style we already see in views, but I should verify that doing so works as intended.

I agree that if it comes down to tag helpers, that should be part of the template file for the component itself or the view, e.g.:

# In some controller...
render(View::Leaderboard.new.tap do |c|
  c.entries([...]) # Pass in the data at the controller layer...
end)
# Note that the brackets are necessary to ensure the block is associated with the call to `tap` as opposed to `render`.

# In the component...
renders_many :entries, ->(name:, score:) { tag.div ... }

I feel like @jonspalmer's comment has helped me to realize the original motivation behind the change: content blocks and passthrough slots can contain markup, so those features are more a responsibility of the view layer. The other slot types do take blocks, but can and generally do have a better-defined structure that lends well to just passing data through. I think that's worth a mention somewhere in the documentation, especially with regards to #618.

[joelhawksley]

@boardfish - I agree with @jonspalmer's responses here. I think this example is how I would use slots in controllers:

def foo
  foo_component = View::Foo.new
  foo_component.button(disabled: !logged_in?)
  render(foo_component)
end

[boardfish]

Mm. I think we're all in agreement, in that case - it's sufficient that components can manipulate slots and the content block via methods. Any discussion from here purely relates to style, then.

tap is an option right now if folks want to see a similar structure to how they'd declare a component in the view. That's how I've done it in #1010, and it's functionally identical to what you've got there. Maybe we can work support into upstream render_in so that it can be more naturally presented as:

  render(View::Foo.new) do |c|
    c.button(disabled: !logged_in?)
  end

This would make the developer experience more intuitive between rendering components in the view vs. in the controller. It's purely aesthetic, though.

[jonspalmer]

Honestly I think we should change the pattern of using the block to call slot methods altogether. It's just weird an unintuitive. For something as simple as this (in a controller or a view)

  render(View::Foo.new) do |c|
    c.button(disabled: !logged_in?)
  end

this code will put the result of the .button call (the slot instance) into the content variable for the Foo component!

Slots in a view looks like this:

<%# index.html.erb %>
<%= render BlogComponent.new do |c| %>
  <% c.header(classes: '') do %>
    <%= link_to "My blog", root_path %>
  <% end %>

  <% @posts.each do |post| %>
    <%= c.post(post: post) %>
  <% end %>
<% end %>

That could be written as:

<%# index.html.erb %>
<% c = BlogComponent.new %>
<% c.header(classes: '') do %>
  <%= link_to "My blog", root_path %>
<% end %>

<% @posts.each do |post| %>
  <%= c.post(post: post) %>
<% end %>
<%= render c do %>
  Blog Content
<% end %>

With this factoring it is crystal clear what the block is for - content of the component or slots. Rendering in the view would be identical to rendering in the control modulo the erb syntax.

Its also allows mixed mode rendering more obviously

# in the controller
@component = BlogComponent.new
@component.with_content("Blog Content")
@posts.each do |post| 
  @component.post(post: psot)
end

<%# index.html.erb   call the slots with erb content%>
<% c = BlogComponent.new %>
<% @component.header(classes: '') do %>
  <%= link_to "My blog", root_path %>
<% end %>
<%= render @component %>

[boardfish]

I'm inclined to disagree, for a couple of reasons. I think the block makes it clear that we're working in the context of the component instance, while also being concise and readable. Also, I feel that mixed-mode rendering isn't something we should aim to promote - components that the controller renders should receive only data, not anything close to markup.

The component's template should be responsible for the markup concerns, since the component as a whole is functionally the view model and view layer in one. So in the example you give there, I'd pass the link as attributes to the component rather than rendering them to content from within the view.

I can see where you're coming from with the lack of clear responsibility within the block, though. As it stands, we call and pass blocks to slots, and anything else becomes content. But content is functionally identical to a passthrough slot, so perhaps it should be treated as such?

[joelhawksley]

Closing as stale.