It looks like with 0.5 the direction for handling events in components is to make sure they’re targeted explicitly using phx-target , otherwise the event gets sent to the parent LiveView. Having tested this, it makes sense in terms of ensuring that events aren’t sent to the wrong handler, but the phx-target approach is a little cumbersome right now as it’s necessary for the component to keep track of its own id then make sure to explicitly set the DOM ID of a rendered element and set the phx-target for element events. I feel like the use case wherein a given stateful component handles its own events by default is more typical.
With the current approach, the assumption also seems to be that LiveViews and LiveComponents are tightly coupled, and that a typical use case is for a stateful component to defer to a parent live view to handle events. Having played around with it, I think it would make more sense to pass events up to the nearest stateful component by default, with the ability to target a given component or the parent live view using phx-target if needed. If there’s no stateful component between the element that emitted the event and the parent live view, then the live view handles the event. I’d be interested to get your thoughts on this approach, if it’s something you’d be interested in using for the next release then I’d be happy to help out.
This is how it originally worked but it has the issue with composition like this:
<%= live_component SomeComponent do %>
<a phx-click="oops">Foo</a>
<% end %>
So even though “oops” is rendered inside the component, the event should be sent to the parent. We could have said this is the default and, only if you are using do/end you need to add phx-target but this would ultimately be confusing because you would most likely forget to do it and then wonder where phx-click is not working.
This is not actually the assumption. You are not really supposed to handle component events in the parent. It is just you should be explicit about components targets.
Would it make sense to add an attribute to the component’s container indicating whether or not it’s stateful? I see there’s already an attribute added to every component container, data-phx-component I believe. That way you could bypass stateless components when determining the target for an event.
My comment was mainly in response to your example of the stateless component wrapping the anchor tag with a phx-click attribute.
To break it down: the current 0.5 behaviour is to send events to the parent LiveView by default, unless a phx-target has been set explicitly. My proposal is to reverse this behaviour and have events sent to the nearest stateful component instead. My hypothesis is that this matches the expected behaviour for developers better; if I’m building a stateful component that renders HTML that include event generators then I’d expect my component to be the target for those events by default.
I did not say anywhere it was a stateless component. The issue will happen whenever a parent LiveView (or stateful component) pass a block to another stateful component. When you do so, the closest stateful component is not the one that should handle the event. I recommend you to re-read my first comment without thinking about stateless components, as it explains why your proposal did not work when we first implemented it. If it is still not clear, please let me know!
Ah, ok, I think I see where you’re coming from now. To be honest, I’m mostly trying to advocate for my own use case; I’ve not actually built do-block-style components. Instead I’m breaking down my application into components that are controllable through update parameters exclusively.
The difficulty I’m having with 0.5 is that it’s forcing me to do a bunch of extra work in order to get the same functionality I had for free in 0.4 (although I’m aware that nested LiveComponents didn’t exactly work correctly either!). Basically it’s now necessary to hold the component’s ID in its state, set the id field of a top-level DOM element that the component is rendering and then use phx-target every time I want to use events. I’ve ended up writing helper functions to simplify this, but it does make building components and testing them somewhat more complicated than it used to be. I hope this makes sense.
Btw, would your life be easier if you did not have to maintain a DOM ID and instead you just had to do something like phx-target="<%= @cid %>" ?
Definitely. I remember seeing this in the repo for a while before it was switched to the current approach and I think it’s definitely simpler to use. I appreciate the ability to send an event to more than one handler, as we can can now, and it would be good to be able to retain that while making the most common case of having a single handler simpler.
One difficulty I had, though, was in targeting components for events in tests. It’s necessary to pass a DOM ID into render_click etc. in order to target a specific component. I’m not sure how that would work in a world where the target value is a module attribute that you can’t access from within the test.
The issue will happen whenever a parent LiveView (or stateful component) pass a block to another stateful component. When you do so, the closest stateful component is not the one that should handle the event. I recommend you to re-read my first comment without thinking about stateless components, as it explains why your proposal did not work when we first implemented it. If it is still not clear, please let me know!
