Closed wdiechmann closed 1 year ago
joeldrapper
commented
1 year ago What's the controller code that's rendering the turbo stream? The @_target being nil is strange and it sounds like an incompatibility between phlex and phlex-rails. There were a few times when phlex and phlex-rails were updated at the same time with breaking changes between them. It's best to have only phlex-rails in your Gemfile and let it pick the compatible version of phlex.
joeldrapper
commented
1 year ago If you’re upgrading, I recommend jumping to the latest version of phlex-rails and letting it find the latest version of phlex. Then fix each issue one-by-one. There's no need to go version-by-version, as you're likely to run into bugs that were already fixed.
All the breaking changes should have been documented in the release notes.
A lot of these can be fixed with simple find and replace. For example, the change from text to plain should be quite simple when you know it's coming. We had to rename this in order to support SVGs which have a text element. https://developer.mozilla.org/en-US/docs/Web/SVG/Element/text
wdiechmann
commented
1 year ago What's the controller code that's rendering the turbo stream?
Well, like they say in Hollywood: it's complicated mostly because I use a lot of OO-ing and DRY-ing to keep down the LOC 😆 (incidentally that is why I am head over heels into Phlex too!)
- but where Rails sort-a leaves it off to me, things start (leaving out routes.rb) here
# controllers/abstract_resources_controller.rb
def edit_resource
lockable? ?
render( turbo_stream: turbo_stream.replace( resource_form, partial: 'form', locals: { resource: resource } ) ) :
lock_warning
endThere's a little helper in there
# helpers/resources_helper.rb
def resource_form rs=nil
return ("form_%s" % (Current.user.id rescue '0')) if rs.nil?
"%s_%s_form" % [rs.class.to_s.underscore, (Current.user.id rescue 'new')]
endand a concern dealing with actually finding data
# controllers/concerns/resource_control.rb
#
# resource is divided into 2 main categories
#
# * 4 core resources from which a majority of resources are delegated_from
# * other resources like dashboards, roles, profiles, and other supporting resources
#
# the 4 core resources are Event, Message, Participant, Asset
# in which case calling for a resource will return the core resource
# provided the action is one of 'new' 'edit', 'create', 'update', 'destroy', more
#
def resource
@resource ||= fixate_resource
end
def fixate_resource
r = (_id.nil? ? new_resource : resource_class.find(_id) )
r = r.delegated_from if (r.respond_to? :delegated_from) && ( %w( new edit create update destroy clonez wizard release_lock ).include? params[:action] )
params[:action]=="clonez" ? r.clone_from : r
end
The meat so to speak is in the _form.html.erb (which I took the liberty of tersening* [leaving out multiple input fields performing likewise] to improving readability, and shortening the already way to long comment!!)
# views/equipment/_form.html.erb
<%= render_form( resource: resource, assoc: :assetable ) do |form| %>
<%= render_form_header( resource: resource, title: form_header_title, description: form_header_description ) %>
<%= render_form_errors(resource: resource) if resource.errors.size > 0 %>
<!-- Divider container -->
<div class="h-full py-6 space-y-6 sm:py-0 sm:space-y-0 sm:divide-y sm:divide-gray-200">
<!-- Name -->
<%= form.text_field( :name, required: true, focus: true, input_css: "block w-full shadow-sm sm:text-sm focus:ring-indigo-500 focus:border-indigo-500 border-gray-300 rounded-md") %>
<!-- access_token -->
<%= form.token_field( :access_token, assoc: :assetable) %>
---8<---
</div>
<%= render_form_action_buttons resource: resource, delete_url: resource_url(), deleteable: !resource.new_record? %>
<% end %>
Another helper sneaked in
# helpers/view_components_helper.rb
def render_form(**attribs, &block)
attribs[:action] ||= params[:action]
render Views::Components::Form::Form.new **attribs, &block
end
As for the rest of the journey – I've had a small blog post laying dormant and now seemed like a proper occasion to publish it and reveal my arteries 😢
It's best to have only
phlex-railsin your Gemfile and let it pick the compatible version ofphlex.
Sure, that's my general MO but when I stumbled over the to_str vs to_s issue in my own unelegant way I started looking for a ways out, obviously to no greater avale
If you’re upgrading, I recommend jumping to the latest version of
phlex-railsand letting it find the latest version ofphlex.
Yeah - like I said, that's when sh*t hit the fan, metaphorically speaking 😉
A lot of these can be fixed with simple find and replace. For example, the change from
texttoplainshould be quite simple when you know it's coming. We had to rename this in order to support SVGs which have atextelement. https://developer.mozilla.org/en-US/docs/Web/SVG/Element/text
(and) I understand and I was way out of order (but a moments brain fart had me ventilate all sorts of no-so-niceties which is a HUGE turn-off and something I cannot recall when I did last if ever, knowingly that is) - and I would like to take this opportunity to once again apologise! It was not cool!
* I know it's not a word, but if you can 'sweetening' something I cannot see why not tersening 😆
joeldrapper
commented
1 year ago I can’t quite follow all this code, but it doesn’t look right to me. To render a turbo stream, you should do something like this.
render turbo_stream.replace("some_id", SomeComponent.new(arguments, here))There's no need for partial: or locals: arguments. That's what you use if you're rendering an ERB partial.
wdiechmann
commented
1 year ago once the transition is complete I aim to do just that - but for now I need to allow for rendering both ways (the traditional 'partial' way - and using Phlex) hence the partial - that just calls the view_helper in turn calling the component
wdiechmann
commented
1 year ago i guess this is as far as we go on this issue - will close it with 🤞
maybe some future version will not throw errors like:
ActionView::Template::Error (undefined method `<<' for nil:NilClassand
ActionView::Template::Error (undefined method `to_str' for Location:ClassThe first error is because of incompatible versions of phlex and phlex-rails. What's happening is phlex-rails is trying to append something to the @_target buffer. But that buffer was moved to a new context object in phlex. phlex-rails was updated to support this, but you need to use the update if you're going to use the later version of phlex.
We simply can't make it so that you can use any version of phlex-rails with any version of phlex; we'd never be able to add features to either of them.
The second error is being addressed by #569
@joeldrapper you inquired in another issue whether I would elaborate on my observations regarding Turbo-streams. Here is the best I can do:
Etching my way towards latest versions of Phlex and Phlex-Rails
This is the output from 1.6.0 / 0.8.0
so that's as far as I go.
The 1.5.1 / 0.6.1 combo provides me with
Going back to 1.4.0 / 0.7.1 has me back on solid ground 😄
Tiny steps forward to 1.5.1 / 0.7.1 (1.5.0 did the same thing) produces a screenshot shared as 1.5.1-0.7.1.png
I'm not able to produce the combination - but somewhere in the mix I observed a reasonably well functioning dashboard and lists - only when producing the form overlays
the screenshot above hopefully gives you an idea as to what I'm aiming at) I had to open the developer console and look at what came "down the wire" (which really does not compute at all with it all happening localhost-wise - anyways):
The "gold standard" would look somewhat like this: