Nested <form> elements error with "A React form was unexpectedly submitted."

[rickhanlonii]

Overview

Nested form elements are invalid HTML and cause a hydration error, falling back to client-rendering. When we client render, we add back the nested

element, but when you submit the form we error with a cryptic:

A React form was unexpectedly submitted. If you called form.submit() manually, consider using form.requestSubmit() instead. If you're trying to use event.stopPropagation() in a submit event handler, consider also calling event.preventDefault().

Instead, we should error saying that nested form elements are invalid HTML even after (on only with) client render. We could also match the browser behavior of ignoring the nested form element and submitting the parent form action.

There is an earlier warning about the invalid form elements:

Warning: validateDOMNesting(...):

cannot appear as a descendant of

.

React version: canary

Steps To Reproduce

https://codesandbox.io/p/sandbox/friendly-paper-8frfc9

[rickhanlonii]

Ideas for better errors:

If we block either form from submitting (current behavior):

Multiple forms attempted to submit, actions in nested <form> elements block all actions from submitting

If we call the top-most action, similar to the browser behavior:

Multiple forms attempted to submit, actions in nested <form> elements are ignored

[sebmarkbage]

The general issue is we don’t want to pay the cost of dom nesting checks in prod. It’s a penalty to everyone that actually reads their logs and fixes their bugs.

The important bit is that the log needs to come first.

[sebmarkbage]

Also, it's a common bug of error dialogs to treat window.onerror as a pop up, and console.error not, so you get confused by a later error. This is a bug in ~CodeSandbox~ or maybe it's CRA?

[rickhanlonii]

@sebmarkbage one idea I had is that during hydration, we could change the form action string to javascript:throw new Error('Multiple forms attempted to submit, actions in nested <form> elements block all actions from submitting.').

Would that also be expensive, and could that be done in a way that only de-opts if you use the nested pattern (so the correct pattern doesn't pay the cost)?

[sebmarkbage]

That would typically not be correct because the common reason to hit this code path is what it says in the original string, not nested forms.

[sebmarkbage]

We don't have too many ways to fix this because we have to be in the bubble phase 'submit' handler to play well with the rest of the event system. Additionally, it should ideally play with the Navigation API which would replace our current implementation.

So we can't just add any semantics we want.

We also have limited options to detect this is even what is happening.

[sebmarkbage]

I'm still confused why we don't end up in our event handler which calls preventDefault though.

[rickhanlonii]

@sebmarkbage what i was thinking is in createInstance if validateDOMNesting is false, we walk back up the tree and change the string for all parent forms (so it only changes if you are doing the nesting, all other cases show the original error).

But I guess what you mean is if you did submit incorrectly outside of the nested form, so it goes directly to the parent form, then the original error would be correct. Showing the nested error would be wrong when you did this:

function Component() {
  return (
    <form>
      <form>
        // this should show the nested error
        <Button />
      </form>
      
      // this should show the original error
      <ButtonWithBadSubmit />
    </form>
  )

}

so there's not a good way to fix this case which doesn't impact the other cases

[sebmarkbage]

well, if it's dev only we can also just disable all error logging after the first error in that case. But seems unnecessary since you should act on the first one and the other consequences can be important to dig into. So it's mainly a UI problem in the DX.

[sebmarkbage]

It guess the issue is that the browser stops propagation and doesn't let the submit event bubble up (which makes sense since it might fire the submit event on the parent which isn't right).

So it's as if the browser calls stopPropagation().

If we didn't use event delegation for these, we might have a way to intercept it but that has other consequences. So maybe we just wait to fix this until we fix the rest of the event system.

[HermanBide]

has this been solved ? if no, can i contribute?

[github-actions[bot]]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

[github-actions[bot]]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!