phoenixframework / phoenix_live_view

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

After upgrade to `0.20.6` : `reset: true` not resetting stream in tests #3118

Closed nikhil-belchada closed 8 months ago

nikhil-belchada commented 9 months ago

Environment

Actual behavior

when assigning streams via reset: true in test the new stream is not applied causing test to fail.

# live_view.ex
  def handle_event("update_filters", params, socket) do
     ...
    {:noreply, socket |> stream(:surcharges, stream, reset: true)}
  end

Test were passing for version 0.20.4 and it started failing after upgrading to 0.20.6

Expected behavior

when assigning streams via reset: true in test we expect it to reassign new stream and update the live view

SteffenDE commented 9 months ago

Thank you for the report. We need a minimal example to reproduce this. If you can, please use this script as a template:

Application.put_env(:phoenix, 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.10"},
  {:phoenix_live_view, "~> 0.20.6"},
  {:floki, ">= 0.30.0"}
])

ExUnit.start()

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
    |> then(&{:ok, &1})
  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>
    <script>
      let liveSocket = new window.LiveView.LiveSocket("/live", window.Phoenix.Socket)
      liveSocket.connect()
    </script>
    <%= @inner_content %>
    """
  end

  def render(assigns) do
    ~H"""
    <p>The LiveView content goes here</p>
    """
  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: :phoenix
  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

defmodule Example.HomeLiveTest do
  use ExUnit.Case

  import Phoenix.ConnTest
  import Plug.Conn
  import Phoenix.LiveViewTest

  @endpoint Example.Endpoint

  test "works properly" do
    conn = Phoenix.ConnTest.build_conn()

    {:ok, _view, html} = live(conn, "/")

    assert html =~ "The LiveView content goes here"
  end
end

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

You can run this as elixir testfile.exs, assuming that the file is called testfile.exs.

tjarratt commented 9 months ago

Hey @SteffenDE thanks for your quick response. I've taken a shot at a minimal testcase to reproduce this -- let me know if you have any feedback on how to make this better and more useful for you.

On my machine using elixir 1.16.1 and erlang/OTP 26 this fails consistently looking for "4" in a table after the stream should have been reset, but instead it still sees the originaly "1,2,3" values in the table.

Changing the phoenix_live_view dependency to 0.20.5 is sufficient to make this fail. Changing it to 0.20.4 will result in the test passing.

I don't want to speak for @nikhil-belchada, but he and I might be interested in making a PR to fix this. Assuming this is an actual bug, if you can point us in the right direction for what regressed this behaviour and would like a contribution, let us know 😄 -- I'd be really keen to contribute back to Phoenix Live View

Application.put_env(:phoenix, 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.10"},
  {:phoenix_live_view, "~> 0.20.6"},
  {:floki, ">= 0.30.0"}
])

ExUnit.start()

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
    |> assign(:form, to_form(%{}))
    |> stream_configure(:example, dom_id: &"test-#{&1}")
    |> stream(:example, Stream.concat([1..3]))
    |> then(&{:ok, &1})
  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>
    <script>
      let liveSocket = new window.LiveView.LiveSocket("/live", window.Phoenix.Socket)
      liveSocket.connect()
    </script>
    <%= @inner_content %>
    """
  end

  def render(assigns) do
    ~H"""
    <.form for={@form} phx-change="reset_stream" id="test-form">
      <!-- <input field={@form[:trigger_reset]} /> -->
    </.form>

    <table phx-update="stream" id="table">
      <tbody id="unique">
        <tr :for={{dom_id, row} <- @streams.example} id={dom_id}>
          <td><%= row %></td>
        </tr>
      </tbody>
    </table>
    """
  end

  @impl Phoenix.LiveView
  def handle_event("reset_stream", _params, socket) do
    {:noreply, socket |> stream(:example, Stream.concat([4..4]), reset: true)}
  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: :phoenix
  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

defmodule Example.HomeLiveTest do
  use ExUnit.Case

  import Phoenix.ConnTest
  import Plug.Conn
  import Phoenix.LiveViewTest

  @endpoint Example.Endpoint

  test "works properly" do
    conn = Phoenix.ConnTest.build_conn()

    {:ok, view, _html} = live(conn, "/")

    table_html = view |> element("#table") |> render()

    assert table_html =~ "1"
    assert table_html =~ "2"
    assert table_html =~ "3"

    trigger_reset_stream(view, %{trigger_reset: true})

    table_html = view |> element("#table") |> render()

    assert table_html =~ "4"
    refute table_html =~ "1"
    refute table_html =~ "2"
    refute table_html =~ "3"
  end

  defp trigger_reset_stream(view, params) do
    view
    |> form("#test-form")
    |> render_change(params)
  end
end

{:ok, _} = Supervisor.start_link([Example.Endpoint], strategy: :one_for_one)
ExUnit.run()
Process.sleep(:infinity)
tjarratt commented 9 months ago

I've managed to narrow it down to this commit -- https://github.com/phoenixframework/phoenix_live_view/commit/553338498106910461ede5c64d78c7d783152a00

Specifying this version in the testcase is sufficient to make the test pass.

{
  :phoenix_live_view, 
  git: "https://github.com/phoenixframework/phoenix_live_view/", 
  ref: "553338498106910461ede5c64d78c7d783152a00"
}

When you specify the previous git sha, this test case passes.

SteffenDE commented 9 months ago

The problem is that you are rendering stream items not as a direct child of the phx-update stream container. This should work if you put phx-update on the tbody element.

SteffenDE commented 9 months ago

btw the documentation was always quite clear about this: https://hexdocs.pm/phoenix_live_view/Phoenix.LiveView.html#stream/4

Note: Failing to place phx-update="stream" on the immediate parent for each stream will result in broken behavior.

tjarratt commented 8 months ago

Ah you were absolutely right @SteffenDE -- we definitely saw that message when initially referring to the docs, but ended up with a solution that wasn't guaranteed to work.

Thanks for your help diagnosing that ❤️