phoenixframework / phoenix_live_view

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

<form> with hidden input inside stream gets removed #3262

Closed teamon closed 1 month ago

teamon commented 1 month ago

Environment

Actual behavior

When updating one item inside stream, the <form> for another item disappears when there is a hidden input with name="id".

https://github.com/phoenixframework/phoenix_live_view/assets/8083/c6b1baca-6935-4c89-8b59-321822888c56

Expected behavior

The <form> is not removed.

Single-file reproduction ```elixir 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}, {:ecto, "~> 3.0"}, {:phoenix_ecto, "~> 4.4"} ]) # build the LiveView JavaScript assets (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.Item do use Ecto.Schema embedded_schema do field(:name, :string) end end defmodule Example.HomeLive do use Phoenix.LiveView, layout: {__MODULE__, :live} alias Example.Item @items [ %Item{id: 1, name: "Hello"}, %Item{id: 2, name: "World"} ] def mount(_params, _session, socket) do {:ok, stream_configure(socket, :items, dom_id: fn {%{id: id}, _} -> "items-#{id}" end)} end def handle_params(_params, _url, socket) do items = Enum.map(@items, &build/1) {:noreply, socket |> stream(:items, items)} end def handle_event("toggle-editing", %{"id" => id}, socket) do item = Enum.find(@items, &(to_string(&1.id) == id)) {:noreply, stream_insert(socket, :items, build(item, %{editing: true}))} end def handle_event("submit", %{"id" => id}, socket) do item = Enum.find(@items, &(to_string(&1.id) == id)) {:noreply, stream_insert(socket, :items, build(item))} end defp build(item, ui \\ %{}) do changeset = Ecto.Changeset.change(item) {to_form(changeset, id: "form-#{changeset.data.id}"), ui} end def render("live.html", assigns) do ~H""" <%= @inner_content %> """ end def render(assigns) do ~H"""
<%= for {id, {%{data: item} = form, ui}} <- @streams.items do %>
editing? = <%= ui[:editing] %>
<%= if ui[:editing] do %> <.form :let={f} for={form} phx-value-id={item.id} phx-submit="submit" > <%!-- The problematic hidden input --%> <% else %>
<%= item.name %>
<% end %>
<% end %>
""" 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, _} = Example.Endpoint.start_link() Process.sleep(:infinity) ```

Workaround

Use phx-value-id attribute on the <form> tag instead. Note that it currently does not work in tests - see https://github.com/phoenixframework/phoenix_live_view/issues/3261

SteffenDE commented 1 month ago

Having name="id" will probably cause other issues as well, see https://github.com/phoenixframework/phoenix_live_view/issues/2315#issuecomment-1335734334.

josevalim commented 1 month ago

@SteffenDE should we emit a warning on HEEx engine/tokenizer?

SteffenDE commented 1 month ago

@josevalim If we can do that, I think that would be nice

josevalim commented 1 month ago

Yes, totally doable and we can do it for input tags only to avoid false positives.

SteffenDE commented 1 month ago

But even for something like

defmodule MyMod do
  use Phoenix.Component

  def render(assigns) do
    assigns =
      assigns
      |> assign(:name, "id")

    ~H"""
    <input name={@name}>
    """
  end
end

we would only know at runtime that name="id", right? If it doesn't warn for the most likely case (CoreComponents.input together with Phoenix.HTML.FormField) I don't think it would be that helpful.

I'm happy to take a shot if you can point me in the right direction :)

josevalim commented 1 month ago

Here is an example of a place where we emit a warning: https://github.com/phoenixframework/phoenix_live_view/blob/main/lib/phoenix_live_view/tag_engine.ex#L1240 - however that's for all tags. So somewhere around that you would need to pass a tag or add tag specific warnings. Glad to help if you have more questions!

SteffenDE commented 1 month ago

Closing in favor of #3269.