phoenixframework / phoenix

Peace of mind from prototype to production
https://www.phoenixframework.org
MIT License
21.2k stars 2.86k forks source link

Using "Form" as a schema name in `phx.gen.live` causes components to misbehave #5871

Open osbre opened 1 month ago

osbre commented 1 month ago

Environment

Actual behavior

mix phx.gen.live Forms Form forms name:string

Visit /forms/new and you'll get attempting to reconnect and then this error:

protocol Phoenix.HTML.FormData not implemented for %Test.Forms.Form{__meta__: #Ecto.Schema.Metadata<:built, "forms">, id: nil, name: nil, inserted_at: nil, updated_at: nil} of type Test.Forms.Form (a struct). This protocol is implemented for the following type(s): Ecto.Changeset, Map

Expected behavior

Same as if it was generated with a different schema name (no crushing and error)

mix phx.gen.live Forms SubscriptionForm forms name:string
SteffenDE commented 1 month ago

While this is a problem with the generators and therefore better suited in the phoenix repo, I'd say that this is rather a special case and handling this would make the templates unnecessarily complex (@josevalim wdyt?). The issue is caused by the assign we use to store the Phoenix.HTML.Form struct inside the component. These are the necessary changes in the FormComponent to make it work:

diff --git a/lib/abcdefg_web/live/form_live/form_component.ex b/lib/abcdefg_web/live/form_live/form_component.ex
index ca1af8b..a148bfd 100644
--- a/lib/abcdefg_web/live/form_live/form_component.ex
+++ b/lib/abcdefg_web/live/form_live/form_component.ex
@@ -13,13 +13,13 @@ defmodule AbcdefgWeb.FormLive.FormComponent do
       </.header>

       <.simple_form
-        for={@form}
+        for={@the_form}
         id="form-form"
         phx-target={@myself}
         phx-change="validate"
         phx-submit="save"
       >
-        <.input field={@form[:name]} type="text" label="Name" />
+        <.input field={@the_form[:name]} type="text" label="Name" />
         <:actions>
           <.button phx-disable-with="Saving...">Save Form</.button>
         </:actions>
@@ -33,7 +33,7 @@ defmodule AbcdefgWeb.FormLive.FormComponent do
     {:ok,
      socket
      |> assign(assigns)
-     |> assign_new(:form, fn ->
+     |> assign_new(:the_form, fn ->
        to_form(Forms.change_form(form))
      end)}
   end
@@ -41,7 +41,7 @@ defmodule AbcdefgWeb.FormLive.FormComponent do
   @impl true
   def handle_event("validate", %{"form" => form_params}, socket) do
     changeset = Forms.change_form(socket.assigns.form, form_params)
-    {:noreply, assign(socket, form: to_form(changeset, action: :validate))}
+    {:noreply, assign(socket, the_form: to_form(changeset, action: :validate))}
   end

   def handle_event("save", %{"form" => form_params}, socket) do
@@ -59,7 +59,7 @@ defmodule AbcdefgWeb.FormLive.FormComponent do
          |> push_patch(to: socket.assigns.patch)}

       {:error, %Ecto.Changeset{} = changeset} ->
-        {:noreply, assign(socket, form: to_form(changeset))}
+        {:noreply, assign(socket, the_form: to_form(changeset))}
     end
   end

@@ -74,7 +74,7 @@ defmodule AbcdefgWeb.FormLive.FormComponent do
          |> push_patch(to: socket.assigns.patch)}

       {:error, %Ecto.Changeset{} = changeset} ->
-        {:noreply, assign(socket, form: to_form(changeset))}
+        {:noreply, assign(socket, the_form: to_form(changeset))}
     end
   end

I'd say that if you really have this special case it's probably fine to adjust this yourself afterwards, as the generators are always just the starting point. For a better user experience I see multiple ways forward:

PRs are always welcome, especially for doc improvements!

josevalim commented 1 month ago

I would raise in the generator if Form is used as schema name. We raise for other names already. It is expected that not all names can be used.