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

Removing and adding same item in stream doesn't sometimes keeps the old state of inner live components #3664

Closed
sezaru opened this issue Feb 5, 2025 · 3 comments

Comments

@sezaru
Copy link
Contributor

sezaru commented Feb 5, 2025

Environment

  • Elixir version (elixir -v): 1.18.0
  • Phoenix version (mix deps): 1.7
  • Phoenix LiveView version (mix deps): main
  • Operating system: Fedora Silverblue 41
  • Browsers you attempted to reproduce this bug on (the more the merrier): Firefox and Chrome
  • Does the problem persist after removing "assets/node_modules" and trying again? Yes/no: no

Actual behavior

Consider that you have a stream in your LV and you have an item inside of it.

That item renders a LiveComponent. If I add some latency to the call liveSocket.enableLatencySim(1000) and I remove the item and then, around half a second after I re-add it, the live component update function will be called with the state of the removed component already inside socket.assigns.

Also, if that same live component has another live component inside of it, its update function will not be called at all.

Here is a code that reproduces the bug:

Application.put_env(:sample, Example.Endpoint,
  http: [ip: {127, 0, 0, 1}, port: 5001],
  server: true,
  live_view: [signing_salt: "aaaaaaaa"],
  secret_key_base: String.duplicate("a", 64)
)

Mix.install(
  [
    {:plug_cowboy, "~> 2.5"},
    {:jason, "~> 1.0"},
    {:phoenix, "~> 1.7"},
    # please test your issue using the latest version of LV from GitHub!
    {:phoenix_live_view,
     github: "phoenixframework/phoenix_live_view", branch: "main", override: true}
  ]
)

# if you're trying to test a specific LV commit, it may be necessary to manually build
# the JS assets. To do this, uncomment the following lines:
# this needs mix and npm available in your path!
#
# path = Phoenix.LiveView.__info__(:compile)[:source] |> Path.dirname() |> Path.join("../")
# System.cmd("mix", ["deps.get"], cd: path, into: IO.binstream())
# System.cmd("npm", ["install"], cd: Path.join(path, "./assets"), into: IO.binstream())
# System.cmd("mix", ["assets.build"], cd: path, into: IO.binstream())

defmodule Example.ErrorView do
  def render(template, _), do: Phoenix.Controller.status_message_from_template(template)
end

defmodule Example.HomeLive do
  use Phoenix.LiveView, layout: {__MODULE__, :live}

  def mount(_params, _session, socket) do
    socket =
      socket
      |> stream(:items, [], reset: true)

    {:ok, socket}
  end

  def render("live.html", assigns) do
    ~H"""
    <script src="/assets/phoenix/phoenix.js">
    </script>
    <script src="/assets/phoenix_live_view/phoenix_live_view.js">
    </script>
    <%!-- uncomment to use enable tailwind --%>
    <%!-- <script src="https://cdn.tailwindcss.com"></script> --%>
    <script>
      let liveSocket = new window.LiveView.LiveSocket("/live", window.Phoenix.Socket)
      liveSocket.connect()
    </script>
    <style>
      * { font-size: 1.1em; }
    </style>
    {@inner_content}
    """
  end

  def render(assigns) do
    ~H"""
    <button phx-click="add_item_1">+ 1</button>
    <button phx-click="remove_item_1">-</button>

    <div id="items_stream" phx-update="stream">
      <div :for={{dom_id, %{id: id}} <- @streams.items} id={dom_id}>
        <MyComponent.live_render id={id} />
      </div>
    </div>
    """
  end

  def handle_event("add_item_1", _params, socket) do
    socket = stream_insert(socket, :items, %{id: "a"})

    {:noreply, socket}
  end

  def handle_event("remove_item_1", _params, socket) do
    socket = stream_delete(socket, :items, %{id: "a"})

    {:noreply, socket}
  end
end

defmodule MyComponent do
  use Phoenix.LiveComponent

  def live_render(assigns), do: ~H"<.live_component module={__MODULE__} {assigns} />"

  def update(assigns, socket) do

    if Map.has_key?(socket.assigns, :blibs) do
      IO.puts("GOT THE BUG!")

      dbg(socket.assigns)
    end

    socket = socket |> assign(assigns) |> assign(blibs: 1)

    {:ok, socket}
  end

  def render(assigns) do
    ~H"""
    <div id={@id}>
      {@blibs}
      <MySecondComponent.live_render id={"#{@id}_second"} />
    </div>
    """
  end
end

defmodule MySecondComponent do
  use Phoenix.LiveComponent

  def live_render(assigns), do: ~H"<.live_component module={__MODULE__} {assigns} />"

  def update(assigns, socket) do
    dbg(">>> SECOND COMPONENT UPDATE CALLED")

    {:ok, assign(socket, assigns)}
  end

  def render(assigns) do
    ~H"""
    <div id={@id}>second component</div>
    """
  end
end

defmodule Example.Router do
  use Phoenix.Router
  import Phoenix.LiveView.Router

  pipeline :browser do
    plug(:accepts, ["html"])
  end

  scope "/", Example do
    pipe_through(:browser)

    live("/", HomeLive, :index)
  end
end

defmodule Example.Endpoint do
  use Phoenix.Endpoint, otp_app: :sample
  socket("/live", Phoenix.LiveView.Socket)

  plug Plug.Static, from: {:phoenix, "priv/static"}, at: "/assets/phoenix"
  plug Plug.Static, from: {:phoenix_live_view, "priv/static"}, at: "/assets/phoenix_live_view"

  plug(Example.Router)
end

{:ok, _} = Supervisor.start_link([Example.Endpoint], strategy: :one_for_one)
Process.sleep(:infinity)

I recorded a small video showing the issue:

first_video.mp4

As you can see, when I remove the item, I wait around half a second after it disapers from the DOM and then I click to re-add it again. In the terminal, it says "GOT THE BUG!" from the first component, you can also see that blibs: 1 is inside the socket assigns even thought it should be empty, and I also don't get any message from the second component (because its update function is never called).

Expected behavior

Since I first removed the item from the stream and then I re-added it again, it should re-add the component with an empty state, not using the older one.

Here is a video showing the expected behavior, in this case I waited around 2 seconds to click in the re-add button again to not trigger the bug.

second_video.mp4
@sezaru
Copy link
Contributor Author

sezaru commented Feb 5, 2025

I tried to trigger the bug in older versions of LV to see when it was introduced and I went back until 0.20.0, and I still could trigger it. This made me think that maybe this is some kind of optimization instead of a bug.

Even if that is the case, this optimization seems to create a bunch of issues with hooks.

For example, in my case I have a hook that will draw a chart, I use a function in the component update function to send the data to the hook with an event. Since that update function is never called because of this bug, my chart shows as blank.

@SteffenDE
Copy link
Collaborator

@sezaru I think there's not much we can do to change this, as it's a consequence of how LiveComponents work. Basically, the LV protocol relies on the client to tell the server when a LiveComponent is removed from the page:

  1. Server pushes a diff that removes a LiveComponent
  2. Client applies the diff, afterwards looks for any removed LiveComponents. Client sends a cids_will_destroy message to the server, containing the IDs of removed LCs.
  3. Server receives the information about the removed LCs and marks them for deletion. Server replies with an ack.
  4. Client receives acknowledgement and looks into the DOM to see if the LC was actually added back. For any LC that is still missing, it sends a cids_destroyed.
  5. Server receives cids_destroyed and actually removes the affected LiveComponents.
  6. Client receives ack for cids_destroyed and removes the remaining component data

Now, let's assume a latency of 500ms (single trip) and look at how this would look like:

Time (ms)  0       500     1000    1500    2000    2500
           │         │       │       │       │       │
Timeline   ┼─────────┼───────┼───────┼───────┼───────┼───►
           │         │       │       │       │       │
Events     1a────►   2a      3a      4a      5a      6a
           │         │       │       │       │       │

Legend:
1a = Server sends remove LC diff
2a = Client receives diff, applies it, sends cids_will_destroy=LC
3a = Server receives cids_will_destroy=LC
4a = Client receives ack, checks DOM, sends cids_destroyed=LC
5a = Server receives cids_destroyed=LC, removes LC
6a = Client receives destroy ack, cleans up remaining component data

Total time: ~2500ms (2.5 seconds) for complete sequence

This is needed to fix race conditions. It also means that if you add the component back between step 1 and 5, which needs two round trips, the LiveComponent will not be destroyed on the server:

Time (ms)  0    250   500   750  1000  1250  1500  1750  2000  2250  2500
           │     │     │     │     │     │     │     │     │     │     │
Timeline   ┼─────┼─────┼─────┼─────┼─────┼─────┼─────┼─────┼─────┼─────┼───►
           │     │     │     │     │     │     │     │     │     │     │
Events     1a────────► 2a    1b    3a    2b    4a    3b    5a          6a
                              └──────────►│──────────►│

Legend:
Original flow:
1a (   0ms) = Server sends remove LC diff
2a ( 500ms) = Client receives diff, applies it, sends cids_will_destroy=LC
3a (1000ms) = Server receives cids_will_destroy=LC, marks LC for deletion
4a (1500ms) = Client receives ack, checks DOM, sends cids_destroyed=LC
5a (2000ms) = Server receives cids_destroyed (keeps component)
6a (2500ms) = Client receives ack (empty, not actually destroyed)

Re-add flow:
1b ( 750ms) = Client sends re-add event
2b (1250ms) = Server receives re-add, unmarks deletion, sends diff
3b (1750ms) = Client receives re-add diff

Total time: 2500ms (2.5 seconds) for complete sequence

If you need to ensure that you get a new LiveComponent, the only way is to roll the ID that you give the LiveComponent:

@@ -66,7 +66,7 @@
 
     <div id="items_stream" phx-update="stream">
       <div :for={{dom_id, %{id: id}} <- @streams.items} id={dom_id}>
-        <MyComponent.live_render id={id} />
+        <MyComponent.live_render id={"#{id}-#{System.unique_integer()}"} original_id={id} />
       </div>
     </div>
     """
@@ -105,9 +105,9 @@
 
   def render(assigns) do
     ~H"""
-    <div id={@id}>
+    <div id={@original_id}>
       {@blibs}
-      <MySecondComponent.live_render id={"#{@id}_second"} />
+      <MySecondComponent.live_render id={"#{@original_id}_second"} />
     </div>
     """
   end

@SteffenDE
Copy link
Collaborator

Oh and thank you very much for including the reproduction. I appreciate your effort very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants