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

Form does not display errors for nested embeds while using :let #448

Closed
rmoorman opened this issue Aug 17, 2024 · 9 comments
Closed

Form does not display errors for nested embeds while using :let #448

rmoorman opened this issue Aug 17, 2024 · 9 comments

Comments

@rmoorman
Copy link

When using nested ecto embeds in a live view through form and inputs_for, while simultaneously using the :let binding, errors for the nested embed no longer show up in the browser.

I don't know if that is to be expected or if this is a symptom of a problem with the let binding or the form component for example.

I came across this problem while fiddling around a bit with the example code for issue phoenixframework/phoenix#5901.

For demonstration purposes, I created another example repo which can be used to reproduce this specific issue.

The problem

The problem basically comes down to the following:
image
In the screenshot above, there are 3 embeds visible in the form, all of them have errors in the name fields, but only the first and the third embed show the error.
I did the same to all fields, clicked inside and typed another letter to try to trigger the error display.

Below the fields, I printed the form struct for reference where the errors can be seen.

The trigger

The problem is triggered when using the form, inputs_for, and input like this:

    <.form :let={f} for={@form} phx-change="change">
        <.inputs_for :let={nested} field={f[:nested_with_let]}>
          <.input field={nested[:name]} label="Name" />
        </.inputs_for>
    </.form>

The :let is used on the form to bind the form to f, which is then used to provide the field to inputs_for and somewhere there seems to be the culprit.

At first glance, there does not seem to be a problem with inputs_for's behavior. I added a more deeply nested example to the example repo (using the @form assign directly instead of f for the first level), and nesting inputs_for twice, while binding the form struct does seem to work there.

How to reproduce

As mentioned, there is an example repository that can be used to reproduce this problem. There, I bootstrapped a new phoenix application and added the example liveview in hello/lib/hello_web/live/form_live.ex which includes the schema and the template.

@josevalim
Copy link
Member

Errors are only shown after you attempt to insert/update the changeset. You can try calling Ecto.Changeset.apply_action or similar.

@rmoorman
Copy link
Author

rmoorman commented Aug 17, 2024

The nested changesets do have an action (:insert) and the main changeset has an action set as well. The screenshot in the issue description above was taken after the form fields of the nested embeds were changed and the matching handle_event callback (from phx-change) ran the changeset functions.

The example code does set an action using the action option of to_form but setting an action on the changeset does not seem to make any difference.

Regarding apply_action, this is happening in the (live) validation phase. It does happen on phx-submit as well though (where all errors should be shown because used/unused fields are not considered while rendering the form). I updated the example code with a submit handler. Furthermore I left out the form result handling part (persistence) to keep the example code more focused.

The problem is, that the validation errors show for the nested embeds, when <.inputs_for :let={nested} for={@form[:nested]}> is used, but are not rendered, when the :let binding from the <form :let={f} form={@form}> is used.

Looking at the documentation for :let usage with form it seems that in any case the "form" should be returned. The documentation refers explicitly to changesets that are converted to a form struct on the fly (presumably what to_form in your liveview would do). So I assumed that passing a form to the form component and then capturing it with :let and passing that value on instead of the @form assign would just work.
Maybe you are wondering why I would use the :let binding in the first place when I already have a form? I was copy and pasting around form code (re-organizing fields) and I thought I would like to try if it felt more consistent to also use let just as I do with the inputs_for for the embeds.

I am also aware that the documentation states that using the let binding is discouraged because "LiveView can better optimize your code if you access the form fields using @form[:field] rather than through the let-variable form". But from that I would still assume that it at least works, just like it does with <.inputs_for>.

Of course there is a "work-around" (as just using the assign also works), and it is by no means a huge problem, but I thought it would make more sense to report it nevertheless.

@josevalim
Copy link
Member

Everything works for me if I mark the action from the parent form as well. Since the actions cascade, if anything in the tree is not validated, then it disables the rest.

@SteffenDE
Copy link
Contributor

This might be the same as phoenixframework/phoenix_live_view#3493?
@rmoorman can you check with latest LV main?

@rmoorman
Copy link
Author

How do you "mark the action from the parent form"?

I am really wondering what I am doing wrong in the example I gave. It is puzzling that the field using the value from :let does not show the validation error while the field using the @form assign field does work correctly.

The only difference between these two fields is that field={@form[:nested]} vs field={f[:nested_with_let]}.

image

The data the fields receive seems to be quite identical as well

image

Also the generated HTML is the same, but of course for one field it does not include the validation errors.

I find it even stranger that my example does seem to work over here but doesn't on your side.

I will try to have a look into the code of inputs_for ...

And I just saw @SteffenDE reacting too. I will have a look!

@rmoorman
Copy link
Author

rmoorman commented Nov 17, 2024

Updating the example project's mix.ex to use main does work @SteffenDE and @josevalim.

So the fix that was implemented to resolve phoenixframework/phoenix_live_view#3493 resolves the issue I encountered a while back. Good to know.

When is this slated for release by the way?

@SteffenDE
Copy link
Contributor

Chris does LiveView releases, so I don’t know exactly. My educated guess would be in the next two weeks.

And I wanted to say sorry for letting this sit here so long - please just ping us if you feel like issues might have been forgotten. Sometimes they indeed have.

@josevalim
Copy link
Member

In my case, it worked if I assigned an action to the parent changeset by doing to_form(form, action: …). I think it makes sense, we would force things to always be changed instead of relying on the action changing, which was fixed in LV, and I assume you want the later (only show errors if it was changed?)

@rmoorman
Copy link
Author

rmoorman commented Nov 17, 2024

Thank you for fixing it and thank you for the sorry. No problem. Sometimes I feel a bit torn between trying to not be obnoxious end indeed trying to not be forgotten 😆
You all also have a busy schedule and I surely can imagine how things can get a bit lost sometimes.

In my case, it worked if I assigned an action to the parent changeset by doing to_form(form, action: …). I think it makes sense, we would force things to always be changed instead of relying on the action changing, which was fixed in LV, and I assume you want the later (only show errors if it was changed?)

@josevalim I at least would want both cases to work consistently no matter whether I use :let={f} or @form directly. Again, thank you for having a look 😃

Anyway. Keep up the good 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

3 participants