phlex-rails
phlex-rails copied to clipboard
Change behavior of render "foo" to render a string instead of a Rails template
From Issue https://github.com/phlex-ruby/phlex-rails/issues/137, this PR makes it such that:
render "foo"
renders a String instead of the rails partial foo. Rendering a partial will be changed to:
render partial: "foo"
This will be a breaking change for Phlex 1.x and ultimately require a path to Phlex 2.x. As such, I'm going to have an Overrides::Version1 and Overrides::Verrsion2 module that get included depending on the setting specified. This will let people upgraded in 1.x to 2.x. When 2.x ships I'll remove 1.x and any updates will break.
Turns out render "foo" and render partial: "foo" behavior differently in Rails 🤦.
Given this Erb template:
Hi
<%= render "foo" do %>
The middle
<% end %>
<%= render partial: "foo" do %>
The middle
<% end %>
The first one will work, but the latter will throw this error:
'nil' is not an ActiveModel-compatible object. It must implement :to_partial_path.
New approach, which I think could eliminate a lot of the overrides.
render(rails("fizz") { "buzz" })
The rails method returns this object:
# frozen_string_literal: true
# @api private
module Phlex::Rails
class Rendition
attr_reader :args, :kwargs, :block
def initialize(*args, **kwargs, &block)
@args = args
@kwargs = kwargs
@block = block
end
end
end
Which is intercepted by the render override:
when Phlex::Rails::Rendition
captured_block = -> { capture(&renderable.block) } if renderable.block
@_context.target << @_view_context.render(*renderable.args, **renderable.kwargs, &captured_block)
I would propose moving this into an adapter, which would be included via the kitchen sink, but at least for all-Phlex apps the Rails renderer would never be invoked.
I opened a bug at https://github.com/rails/rails/issues/51015. If Rails decides this is a bug and fixes it, we should be set. If they don't then we'll have to figure out how to render from Phlex in a manner that doesn't obstruct the "fidelity" of rendering in Rails.
Had a conversation about the Rails issue here. https://discord.com/channels/849034466856665118/974005005768069211/1218911072522600481
@bradgessler any thoughts on what we can do about this for 2.0? 🤔
Yeah we could either take the approach above where we wrap Rails rendering calls in a class via render(rails("fizz") { "buzz" }). This would provide the most "faithful" rendition of Rails content since we'd let people continue calling into this undefined/broken behavior.
Another option is Phlex assumes render "string" does NOT call out to Rails and we instead require a keyword like partial: to call out into Rails renditions.
Another option is we could have a rails.render or rails_render method that makes it clear the call is into Rails and not into Phlex.
I think we should rule out patching Rails since (1) they don't seem interested and (2) getting patches through for stuff like this has historically been a 🥙 for me.
Agh, this is such a tricky issue. None of those options feel quite right, but we have to come up with something. 🤔