Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PoC: AbstractController::Base#renderer_for #46202

Closed
wants to merge 1 commit into from

Conversation

casperisfine
Copy link
Contributor

Context

There has been some demands from view systems not based on paths, to offer some extension API that are better suited for them: https://discuss.rubyonrails.org/t/replace-to-partial-path-calls-with-a-more-abstract-to-renderable/81457

I'm in favor of the general idea, but not at all with the proposed solutions so far.

Existing API

Currently you can customize the behavior of render some_object in two ways:

  • The object can respond to to_partial_path and return a view path.
  • The object can respond to render_in and return a string representation.

What I don't like with both of these APIs is that they ask the object how to render itself. Applications often have several different ways to represent the same object based on the context, e.g. frontend vs admin panel, or HTML vs JSON.

Proposal

It would be way more powerful if the view or controller was in charge of selecting the proper renderer for a given object.

e.g. (pseudo code)

module Admin
  class BaseController < ApplicationController
    def renderer_for(object)
      { partial: "admin/#{object.class.underscore}" }
    end
  end

On paper, you'd probably want to define this on thew view, but in practive Rails users very rarely interact with the view, hence why I do this on the controller. But I'm maybe missing something.

Another benefit of such API would be to allow view component systems to better integrate with Rails by implementing their own conventions, e.g. (pseudo code)

module MyViewComponentExtension
  def renderer_for(object)
    component_name = "#{object.class.name}Component"
    "#{self.class.module_parent_name}#{component_name}".safe_constantize ||
      component_name.constantize
  end
end

It's somewhat similar to what Active Model Serializer used to do for its default serializer system:
https://github.com/rails-api/active_model_serializers/blob/0-9-stable/lib/action_controller/serialization.rb

It used to work quite well.

Note

his is purely a proof of concept to demonstrate the general idea, what capabilities it brings to the table.

I'm absolutely not happy with that implementation and I'm not suggesting to merge it. If we chose to go with the idea, we
should carefully study what the API would look like.

Currently you can customize the behavior of `render some_object` in
two ways:

- The object can respond to `to_partial_path` and return a view path.
- The object can respond to `render_in` and return a string representation.

What I don't like with both of these APIs is that they ask the object how
to render itself. Applications often have several different ways to represent
the same object based on the context, e.g. frontend vs admin panel, or
HTML vs JSON.

It would be way more powerful if the view or controller was in charge of
selecting the proper renderer for a given object.

e.g. (pseudo code)

```ruby
module Admin
  class BaseController < ApplicationController
    def renderer_for(object)
      { partial: "admin/#{object.class.underscore}" }
    end
  end
```

On paper, you'd probably want to define this on thew view, but
in practive Rails users very rarely interact with the view, hence
why I do this on the controller. But I'm maybe missing something.

Another benefit of such API would be to allow view component systems
to better integrate with Rails by implementing their own conventions,
e.g. (pseudo code)

```
module MyViewComponentExtension
  def renderer_for(object)
    component_name = "#{object.class.name}Component"
    "#{self.class.module_parent_name}#{component_name}".safe_constantize ||
      component_name.constantize
  end
end
```

It's somewhat similar to what Active Model Serializer used to do for
its default serializer system:
https://github.com/rails-api/active_model_serializers/blob/0-9-stable/lib/action_controller/serialization.rb

It used to work quite well.
@paracycle
Copy link
Contributor

paracycle commented Oct 17, 2022

I like this proposal but the only thing that bothers me is dropping the to_partial_path dependency. While the name of the method is bad, I think the "partial path" plays the role of a resource locator on the server side, and if we can decouple them from actual view paths on disk for a second, they would provide them most flexible way to decide how to render an object. This is similar to how URL paths can correspond to file locations on disk on the server, but don't have to.

In that sense, I would prefer if the API was similar to:

class ApplicationController
  def renderer_for(partial_path)
    if should_be_rendered_by_action_view?(partial_path)
      { partial: partial_path }
    else
      find_renderer_for(partial_path)
    end
  end
end

@casperisfine
Copy link
Contributor Author

@paracycle part of the reason is that not all rendering system are tied to file system paths, e.g. ViewComponent, Active Model Serializers, etc. I think we should try to support these better.

@byroot byroot closed this Sep 14, 2023
seanpdoyle added a commit to seanpdoyle/rails that referenced this pull request Jan 6, 2024
Follow-up to [rails#46202][]

Without overriding the new `#render_in` method, previous behavior will
be preserved: render the view partial determined by calling
`#to_partial_path`, then pass `object: self`.

With the following view partial:

```erb
<%# app/views/people/_person.html.erb %>
<% local_assigns.with_defaults(shout: false) => { shout: } %>

<%= shout ? person.name.upcase : person.name %>
```

Callers can render an instance of `Person` as a positional argument or a
`:renderable` option:

```ruby
person = Person.new(name: "Ralph")

render person                                       # => "Ralph"
render person, shout: true                          # => "RALPH"
render renderable: person                           # => "Ralph"
render renderable: person, locals: { shout: true }  # => "RALPH"
```

This preserves backward compatibility. At the same time, the
`#render_in` method provides applications with an more flexibility, and
an opportunity to manage how to transform models into Strings. For
example, users of ViewComponent can map a model directly to a
`ViewComponent::Base` instance.

[rails#46202]: rails#46202 (comment)
seanpdoyle added a commit to seanpdoyle/rails that referenced this pull request Jan 6, 2024
Follow-up to [rails#46202][]

Without overriding the new `#render_in` method, previous behavior will
be preserved: render the view partial determined by calling
`#to_partial_path`, then pass `object: self`.

With the following view partial:

```erb
<%# app/views/people/_person.html.erb %>
<% local_assigns.with_defaults(shout: false) => { shout: } %>

<%= shout ? person.name.upcase : person.name %>
```

Callers can render an instance of `Person` as a positional argument or a
`:renderable` option:

```ruby
person = Person.new(name: "Ralph")

render person                                       # => "Ralph"
render person, shout: true                          # => "RALPH"
render renderable: person                           # => "Ralph"
render renderable: person, locals: { shout: true }  # => "RALPH"
```

This preserves backward compatibility. At the same time, the
`#render_in` method provides applications with an more flexibility, and
an opportunity to manage how to transform models into Strings. For
example, users of ViewComponent can map a model directly to a
`ViewComponent::Base` instance.

[rails#46202]: rails#46202 (comment)
seanpdoyle added a commit to seanpdoyle/rails that referenced this pull request Jan 6, 2024
Follow-up to [rails#46202][]

Without overriding the new `#render_in` method, previous behavior will
be preserved: render the view partial determined by calling
`#to_partial_path`, then pass `object: self`.

With the following view partial:

```erb
<%# app/views/people/_person.html.erb %>
<% local_assigns.with_defaults(shout: false) => { shout: } %>

<%= shout ? person.name.upcase : person.name %>
```

Callers can render an instance of `Person` as a positional argument or a
`:renderable` option:

```ruby
person = Person.new(name: "Ralph")

render person                                       # => "Ralph"
render person, shout: true                          # => "RALPH"
render renderable: person                           # => "Ralph"
render renderable: person, locals: { shout: true }  # => "RALPH"
```

This preserves backward compatibility. At the same time, the
`#render_in` method provides applications with an more flexibility, and
an opportunity to manage how to transform models into Strings. For
example, users of ViewComponent can map a model directly to a
`ViewComponent::Base` instance.

[rails#46202]: rails#46202 (comment)
@seanpdoyle
Copy link
Contributor

@byroot I've opened #50623 to expand the capabilities of the render_in method to incorporate options like :locals passed at the render site.

On that heels of that change, I've also started to explore expanding ActiveModel::Conversion to define a baseline render_in implementation that preserves the existing behavior in a backwards compatible way: seanpdoyle#2

What I don't like with both of these APIs is that they ask the object how to render itself. Applications often have several different ways to represent the same object based on the context, e.g. frontend vs admin panel, or HTML vs JSON.

I relate to this concern. The motivation behind making options available to render_in aims to give the renderable more context. Through the view_context argument, the object can access the available variants, and the :formats (HTML, JSON, etc.) would be available through the :formats option or the lookup_context.

From an Action View perspective, that information could enable some flexibility. From a third-party integration perspective (like ViewComponent or Active Model Serializers), then information could be used to make similar encoding decisions.

These proposed changes are still at a different application layer than this PR (#46202), but they share similar goals.

seanpdoyle added a commit to seanpdoyle/rails that referenced this pull request Jan 6, 2024
Follow-up to [rails#46202][]

Without overriding the new `#render_in` method, previous behavior will
be preserved: render the view partial determined by calling
`#to_partial_path`, then pass `object: self`.

With the following view partial:

```erb
<%# app/views/people/_person.html.erb %>
<% local_assigns.with_defaults(shout: false) => { shout: } %>

<%= shout ? person.name.upcase : person.name %>
```

Callers can render an instance of `Person` as a positional argument or a
`:renderable` option:

```ruby
person = Person.new(name: "Ralph")

render person                                       # => "Ralph"
render person, shout: true                          # => "RALPH"
render renderable: person                           # => "Ralph"
render renderable: person, locals: { shout: true }  # => "RALPH"
```

This preserves backward compatibility. At the same time, the
`#render_in` method provides applications with an more flexibility, and
an opportunity to manage how to transform models into Strings. For
example, users of ViewComponent can map a model directly to a
`ViewComponent::Base` instance.

[rails#46202]: rails#46202 (comment)
seanpdoyle added a commit to seanpdoyle/rails that referenced this pull request Jan 9, 2024
Follow-up to [rails#46202][]

Without overriding the new `#render_in` method, previous behavior will
be preserved: render the view partial determined by calling
`#to_partial_path`, then pass `object: self`.

With the following view partial:

```erb
<%# app/views/people/_person.html.erb %>
<% local_assigns.with_defaults(shout: false) => { shout: } %>

<%= shout ? person.name.upcase : person.name %>
```

Callers can render an instance of `Person` as a positional argument or a
`:renderable` option:

```ruby
person = Person.new(name: "Ralph")

render person                                       # => "Ralph"
render person, shout: true                          # => "RALPH"
render renderable: person                           # => "Ralph"
render renderable: person, locals: { shout: true }  # => "RALPH"
```

This preserves backward compatibility. At the same time, the
`#render_in` method provides applications with an more flexibility, and
an opportunity to manage how to transform models into Strings. For
example, users of ViewComponent can map a model directly to a
`ViewComponent::Base` instance.

[rails#46202]: rails#46202 (comment)
seanpdoyle added a commit to seanpdoyle/rails that referenced this pull request Jan 9, 2024
Follow-up to [rails#46202][]

Without overriding the new `#render_in` method, previous behavior will
be preserved: render the view partial determined by calling
`#to_partial_path`, then pass `object: self`.

With the following view partial:

```erb
<%# app/views/people/_person.html.erb %>
<% local_assigns.with_defaults(shout: false) => { shout: } %>

<%= shout ? person.name.upcase : person.name %>
```

Callers can render an instance of `Person` as a positional argument or a
`:renderable` option:

```ruby
person = Person.new(name: "Ralph")

render person                                       # => "Ralph"
render person, shout: true                          # => "RALPH"
render renderable: person                           # => "Ralph"
render renderable: person, locals: { shout: true }  # => "RALPH"
```

This preserves backward compatibility. At the same time, the
`#render_in` method provides applications with an more flexibility, and
an opportunity to manage how to transform models into Strings. For
example, users of ViewComponent can map a model directly to a
`ViewComponent::Base` instance.

[rails#46202]: rails#46202 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants