The support for blocks was somewhat incidental here. My main goal was to support passing :locals provided at the call sites.

@joelhawksley (and @BlakeWilliams, since you've authored #45432) does this change support behavior that's desirable to ViewComponents?

Thanks for adding block support! I lost that one in my TODO's a while back since I've been pretty busy lately.

I'm not positive the locals changes as-is would help us much since we expect all data to be passed in to components via the constructor, but also with the new API it's no longer clear to me when to pass something as a local vs constructor argument. e.g.

render(Greeting.new(name: "Fox Mulder"), name: "Dana Scully"))

I think the API changes make that valid code, but it's not obvious to me which argument would take precedence or how Greeting should respond to locals from a ViewComponent perspective (we use render_in as a hook into ActionView, but otherwise components are largely just Ruby including the constructor). I don't think this would be limited to ViewComponents, but all classes that implement render_in would have to reconcile constructor args with locals and provide guidance on when to use each.

Coming at this with a relatively heavy ViewComponent bias, I could see an API that delegates to the constructor making sense, maybe like:

render(Greeting.new(name: "local"))
render(Greeting, name: "local") # Equivalent to the above since Greeting has a `render_in` instance_method. `name:` is passed to the constructor

All of that has a heavy ViewComponent bias of course, but I'm curious if you had any particular use-cases for the locals changes in mind, ViewComponent or otherwise?

All of that has a heavy ViewComponent bias of course

I can relate to that! Since ViewComponent was the pilot tool for driving out render_in, that makes a lot of sense. In my understanding of where ViewComponent fits in, its constructor state is its render context. That's also reasonable, since it's aiming to serve as a View Model. In my mind, ViewComponent has an opportunity to take a stance on requiring constructor-time state and rejecting render-time locals.

I could see an API that delegates to the constructor making sense

With access to the options, that'd be possible as a class-method:

class ViewComponent::Base
  def self.render_in(view_context, locals: {}, **options, &block)
    view_context.render renderable: new(locals, &block), **options 
  end

  def render_in(view_context, **, &)
    # render without :locals or &block
  end
end

class Greeting < ViewComponent::Base
  # ...
end

render Greeting, name: "local" do 
  "from a block"
end

# the abbreviated form above is equivalent to the long-form below
render renderable: Greeting, locals: { name: "local" } do 
  "from a block"
end

Since render_in is meant to be flexible enough to hand-off all rendering responsibilities to the object, it has an opportunity to make as much information available at render-time. ViewComponent made the design decision to exclude the local_assigns in favor of attr_{accessor,reader}, and instance variables. I don't think imposing that design decision on other integrators is necessary. A separate integrator might envision a View Model as a different balance between View and Model. Instead of accepting :name as a :locals value, it might expect that to be available from the object whereas a :size or :variant might be expected as a :locals value.

I'm curious if you had any particular use-cases for the locals changes in mind, ViewComponent or otherwise?

At the moment, the main motivating use case is internal to Rails. I'm in the process of exploring an Action Text branch that adds support for editors other than Trix.

Incidentally, one part of the code I'm exploring re-structuring involves the ActionText::ContentHelper helper module, namely the render_action_text_attachment method:

It's a :nodoc: method, and is only called within the module itself:

The implementation is fairly concerned with checking whether or not the attachment responds to various methods, all for the sake of building up the correct Hash or options to pass to render. One extra layer on top of that is that between the two call sites, the only difference is the :in_gallery local variable (true in one place, false in the other).

I'm imagining a world where each editor adapter (ckeditor5, tinymce, etc) have an opportunity to subclass ActionText::Attachment and want to override the default behavior. They wouldn't be able (or advised to) reach into the module-private :nodoc: method. However, if render_in accepted options, they'd be available to the subclasses at call time.

class ActionText::Attachment
  def to_attachable_partial_path
    # to be overridden
    to_partial_path
  end

  def render_in(view_context, **options)
    view_context.render options.with_defaults(
      partial: to_attachable_partial_path,
      object: self,
      as: model_name.element
    )
  end
end

That's a circumstance where the constructor arguments to the object and the values passed into the :locals wouldn't be similar enough to cause confusion. In this use case, it's more about the context with which they're rendered. The ActionText::Attachment objects are constructed in far-flung parts of Action Text very separate from the HTML rendering render_in calls. It would also provide the framework with an opportunity to maintain the consistency in the construction of the ActionText::Attachment instances while providing integrators with flexibility at render-time.

Outside of that use case, I'm imagining other scenarios where an application might want to build renderable objects (maybe even ViewComponents) prior to their render. I don't have a concrete example handy, but I'm imagining a component that yields itself into another component without knowledge of how the receiving component would want to treat it.

@BlakeWilliams if this change is accepted, I'm also exploring adding a default #render_in implementation to the ActiveModel::Conversion module:

seanpdoyle/rails@action-view-render-in-options...seanpdoyle:rails:active-model-conversion-render-in#diff-8ec888bfb07b8d4a67ffa913e3286d4ffcb052d1bf3b232d2874c7177cd96fbc

The idea there would be that any Active Model or Active Record instance would provide a seam for applications to take over control of rendering (for example, mapping a Person to a PersonComponent class).

Thank you for the final review @rafaelfranca !

@joelhawksley @BlakeWilliams I've opened #57349 as a follow-up to this change to provide an opportunity to bridge from Active Model to other view layer integrations.

@seanpdoyle Just a note that libraries like Lexxy that call super in their render_in method like this:

  class Editor::LexxyEditor::Tag < Editor::Tag
    def render_in(view_context, ...)
      options[:value] = options[:value].to_str if options[:value].respond_to?(:to_str)
      super
    end
  end

will get an exception when super invokes Editor::Tag#render_in:

ArgumentError: wrong number of arguments (given 2, expected 1)

I've opened #57356 to fix it.