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

Unused param not sent for hidden inputs in LV 1.0 #3620

Open
linusdm opened this issue Jan 8, 2025 · 7 comments
Open

Unused param not sent for hidden inputs in LV 1.0 #3620

linusdm opened this issue Jan 8, 2025 · 7 comments

Comments

@linusdm
Copy link
Contributor

linusdm commented Jan 8, 2025

Environment

  • Elixir version (elixir -v): 1.18.1
  • Phoenix version (mix deps): 1.7.18
  • Phoenix LiveView version (mix deps): 1.0.1

Actual behavior

After moving to LiveView 1.0 I encountered the following problem. We integrate flatpickr for date inputs using clients hooks (e.g. phx-hook).

This library dynamically generates an alternative form input element that allows interaction and hides the original input element (using the type="hidden" attribute).

After changing to the new way to track used inputs I noticed that inputs with this hook are never marked as unused. This makes the input immediately seem used after a handle_event cycle. In cases where the input is validated as required for example, this makes the input show up with its error markup and validation error, even though an unrelated input has triggered the validation while the date input is untouched.

I guess there are more JS libraries that use a similar pattern (adding an alternative input field, hide the original, and keep them in sync). All libraries using a similar pattern should see this change in behaviour with LV 1.0.

I see that hidden fields don’t get a companion parameter with the unused prefix by design: see here. There is also some reasoning about not sending the unused-parameters for hidden fields in this PR: #3244.

For an example integration see this gist of @mcrumm (which would also break with LV 1.0):
https://gist.github.com/mcrumm/88313d9f210ea17a640e673ff0d0232b

Expected behavior

Consider also adding the unused companion param for hidden inputs to allow this type of JS integration without breaking tracking of unused inputs.

@tmjoen
Copy link
Contributor

tmjoen commented Jan 8, 2025

Not sending _unused for hidden inputs were very helpful for us, since we have some huge forms with a lot of hidden inputs that generate a ton of extra noise on the wire, but I absolutely see the need in cases like this. Would be nice if you could opt-out of the _unused somehow?

@cw789
Copy link

cw789 commented Feb 13, 2025

Could I ask if a revert of #3244 would be accepted. This blocks us from upgrading to v1.0.

@SteffenDE
Copy link
Collaborator

@cw789 I don't think simply reverting is the way to go.

This library dynamically generates an alternative form input element that allows interaction and hides the original input element (using the type="hidden" attribute).

Even if we reverted that and sent _unused for hidden inputs, because it's a hidden field, it would never be marked as used, only on submit. I guess with phx-feedback-for, the correct feedback name was set on the library generated input?

If only showing errors on submit is fine, another solution here would to not rely on used_input? for those special inputs and instead only renders errors for submit events:

<.input type="my-special-date-input" errors={@submitted? && form[:date].errors || []}>

and then adjust the input component to use assign_new for errors.

If someone can provide an up to date example script that shows such an integration, that could also help move this forward.

This blocks us from upgrading to v1.0.

Note that we provide the phx-feedback-for shim exactly for such cases where a migration to used_input might be problematic. So until we find a good solution here, why not use the shim instead?

@cw789
Copy link

cw789 commented Feb 13, 2025

Okay, it seems not to be that simple. Thanks for your details.

I guess with phx-feedback-for, the correct feedback name was set on the library generated input?

My issues does not concern the mentioned library. It concerns the following code of the Petal Components library:
https://github.com/petalframework/petal_components/blob/main/lib/petal_components/field.ex#L221-L277

But also an custom hook we use to format numbers where the value is submitted on a hidden input.

In both cases there is no phx-feedback-for involved.
That's why the phx-feedback-for shim does not do anything for this cases.

I will try to provide a minimal example script.

@cw789
Copy link

cw789 commented Feb 13, 2025

Very minimal example using the Petal Components library.

# sample.exs
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"},
    {:phoenix_live_view,
     github: "phoenixframework/phoenix_live_view", branch: "main", override: true},
    {:petal_components, "~> 2.8"},
    {:heroicons, github: "tailwindlabs/heroicons", tag: "v2.2.0", app: false, compile: false, sparse: "optimized"}
  ]
)

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

  @impl true
  def mount(_params, _session, socket) do
    changeset = build_changeset()

    {:ok, assign_form(socket, changeset)}
  end

  @impl true
  def handle_event("validate", %{"object" => object_params}, socket) do
    changeset =
      object_params
      |> build_changeset()
      |> Map.put(:action, :validate)

    {:noreply, assign_form(socket, changeset)}
  end

  @impl true
  def handle_event("submit", %{"object" => object_params}, socket) do
    changeset = build_changeset(object_params)

    case validate_changeset(changeset) do
      {:ok, _object} ->
        socket =
          socket
          |> put_flash(:success, "Object successfully created")
          |> assign_form(build_changeset())

        {:noreply, socket}

      {:error, changeset} ->
        socket =
          socket
          |> put_flash(:error, inspect(changeset.errors))

        {:noreply, assign_form(socket, changeset)}
    end
  end

  defp assign_form(socket, changeset) do
    assign(socket, form: to_form(changeset, as: :object))
  end

  defp build_changeset(params \\ %{}) do
    data = %{}

    types = %{
      text: :string,
      checkbox_group: {:array, :string},
    }

    {data, types}
    |> Ecto.Changeset.cast(params, Map.keys(types))
    |> Ecto.Changeset.validate_required([:text, :checkbox_group])
    |> Ecto.Changeset.validate_length(:text, min: 3, max: 50)
  end

  defp validate_changeset(changeset) do
    Ecto.Changeset.apply_action(changeset, :validate)
  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 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"""
    <.container max_width="md" class="mx-auto my-20">
      <.h2>Simple form</.h2>

      <.form for={@form} phx-submit="submit" phx-change="validate">
        <.field
          required
          field={@form[:text]}
          placeholder="Text"
          phx-debounce="blur"
          label="Text"
          required
        />

        <.field
          type="checkbox-group"
          field={@form[:checkbox_group]}
          options={[{"Option 1", "1"}, {"Option 2", "2"}, {"Option 3", "3"}]}
          required
        />

        <.field
          type="radio-group"
          field={@form[:radio_group]}
          group_layout="row"
          options={[{"Option 1", "1"}, {"Option 2", "2"}, {"Option 3", "3"}]}
        />

        <div class="flex justify-end">
          <.button>Submit</.button>
        </div>
      </.form>
    </.container>
    """
  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)

@SteffenDE
Copy link
Collaborator

@cw789 this can be fixed in petal:

diff --git a/lib/petal_components/field.ex b/lib/petal_components/field.ex
index 1034f54..c9142dc 100644
--- a/lib/petal_components/field.ex
+++ b/lib/petal_components/field.ex
@@ -237,7 +237,7 @@ defmodule PetalComponents.Field do
       <.field_label required={@required} class={@label_class}>
         {@label}
       </.field_label>
-      <input type="hidden" name={@name} value="" />
+      <input type="hidden" name={@name <> "[]"} value="" />
       <div class={[
         "pc-checkbox-group",
         @group_layout == "row" && "pc-checkbox-group--row",

_unused works just fine if there is at least one non-hidden input with the same name. This is basically the same problem as #3577.

So I'd suggest to open up an issue in petal_components. Note that you need to filter the empty value on the server (which Ecto does by default iirc).

@cw789
Copy link

cw789 commented Feb 14, 2025

Thank you for putting me on the right track. Really appreciate your work.

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

4 participants