phoenixframework / phoenix_live_view

Rich, real-time user experiences with server-rendered HTML
https://hex.pm/packages/phoenix_live_view
MIT License
6.14k stars 921 forks source link

Item component's child component lives on even after the container stream is reset and updated as such - a bug? #3277

Closed DaTrader closed 4 months ago

DaTrader commented 4 months ago

The case is as follows:

- LiveView instance
  - ItemList LiveComponent with a stream container element in it
    - Item LiveComponent
      - Item's child LiveComponent

So, there's a control to load the list anew and the result may be a list sharing some of the items of the old list. Now, regardless of whether the stream is reset first and even updated and rendered as such (empty), once the the items that were present in the stream prior to its reset (and display and render) and that have their state unchanged from the last time will not get remounted if inserted again in the next update cycle. In other words, those item LiveComponents keep their managed state intact, although an entire update and render cycle went on when they were not logically referenced by anything else (given the stream was reset and updated and rendered as empty).

I imagine this is some sort of optimization thing by design, although I wonder why if the stream was first reset and rendered without the items.

Also, since the stream is the only thing logically referencing its children Items and if this is not a bug but a by-design behavior, at what point can we expect the item LiveComponents (and their children) to be remounted when inserted into the streamed again, regardless of if their state results as not having changed since? Do they live "forever" within the LiveView instance and never get collected?

Thanks

SteffenDE commented 4 months ago

given the stream was reset and updated and rendered as empty

So, I think that multiple things are possibly happening here:

First of all, live components are kept when a stream is reset if they are inserted in the same patch:

https://github.com/phoenixframework/phoenix_live_view/blob/8683020484a12fe6c552e27820469e49c87157c9/assets/js/phoenix_live_view/dom_patch.js#L332-L337

This means that if you do something like

def handle_event("reset-stream", _params, socket) do
  socket
  |> stream(:items, [], reset: true)
  |> stream_insert(...)
end

components that are inserted directly after are kept. There should be no cids_will_destroy message sent over the socket for those component ids. It doesn't sound like this is the case from your message, but I still wanted to mention it.

Furthermore, the client is responsible for telling the server to destroy a live component:

When a live component is removed from the DOM (e.g. because the stream where it was rendered in was reset), the client sends a cids_will_destroy message with the component's id. The LV receives this messages and marks those live components for deletion. But, if in the meantime those components were added back, after the server acknowledges the cids_will_destroy message, the client checks if those components are still missing from the DOM. Only if they are indeed still gone, the client sends a cids_destroyed message. And only when the LV receives this message, the component is removed on the server.

So it could be that:

  1. You reset your stream.
  2. The client receives the diff and clears the stream.
  3. The client sends a cids_will_destroy message to the LV.
  4. In the meantime you added the component back into the stream server-side.
  5. The server sends a new diff to the client with the LC included again.
  6. The client patches the DOM based on the new diff. The LC is now back in the DOM.
  7. The server acknowledges the cids_will_destroy message and marks the component for deletion.
  8. The client checks if the cids_will_destroy components should actually be destroyed and sees that is is added back. It does not send the cids_destroyed message.

Therefore, the behavior you are describing sounds like correct behavior to me. If you want to force a live component to be recreated, the only guaranteed way is to render it with a new id.

Maybe @chrismccord has something to add, but for now I'll close this as everything seems to work as expected :)

DaTrader commented 4 months ago

@SteffenDE Thanks for the explanation.

If you want to force a live component to be recreated, the only guaranteed way is to render it with a new id.

You can't do this if you're purposely deriving the component ID from the ID of the underlying item because you need the updates reflected accordingly - i.e. you could, but then you would need to introduce something akin to versioning them.

Anyway, I've had this issue causing me trouble in one specific case where the items are managing they're own state so that state stays that way despite the stream getting reset and all. And you're right, I did try postponing the actual update to the first cycle after having the stream reset and displayed empty and it's obviously fitting your description.

My current workaround is simply having the list component supply the items an attribute instructing them to reset the state when the stream is reset and so they do.

Btw, the behavior you detailed above - is it documented anywhere? For if not, I think it should be.