-
Notifications
You must be signed in to change notification settings - Fork 46.9k
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
ReactDOM: Fix missing form data when the submitter is outside the form #28056
ReactDOM: Fix missing form data when the submitter is outside the form #28056
Conversation
Comparing: 190cc99...0b4abb2 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
This pull request has been automatically marked as stale. If this pull request is still relevant, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize reviewing it yet. Your contribution is very much appreciated. |
bump |
@@ -92,6 +92,10 @@ function extractEvents( | |||
const temp = submitter.ownerDocument.createElement('input'); | |||
temp.name = submitter.name; | |||
temp.value = submitter.value; | |||
const isOutsideForm = !form.contains(submitter); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How expensive is this call? Could we just always set the form
attribute if form
has an ID? Checking if a form has an id seems cheaper to me.
Can you prepare a codesandbox using a build from this branch (https://react-builds.vercel.app/api/prs/28056/packages-react-dom) to verify this works in a browser? Our |
Fixes facebook#27391 `form-associated elements` can be associated with `<form>`s anywhere in the document (even if the element is outside the `<form>`), and as with any submitter, the `name` and `value` are expected to be reflected in the final `FormData`.
1933c2c
to
0b4abb2
Compare
@eps1lon you were right! it was simpler to check for the id of the form, I just updated the PR and created two sandboxes, both with the same code just different builds.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thank you! Form reset also works properly: https://codesandbox.io/p/sandbox/kind-butterfly-2knmgm
Completes #65845 which only updated Canary not Experimental. <details> <summary> React upstream changes</summary> - facebook/react#29026 - facebook/react#29025 - facebook/react#28743 - facebook/react#29022 - facebook/react#29023 - facebook/react#29015 - facebook/react#29016 - facebook/react#28988 - facebook/react#28987 - facebook/react#28986 - facebook/react#29014 - facebook/react#28982 - facebook/react#29006 - facebook/react#28973 - facebook/react#28841 - facebook/react#28964 - facebook/react#28990 - facebook/react#29003 - facebook/react#28989 - facebook/react#28893 - facebook/react#28887 - facebook/react#28807 - facebook/react#28978 - facebook/react#28963 - facebook/react#28972 - facebook/react#28970 - facebook/react#28816 - facebook/react#28977 - facebook/react#28974 - facebook/react#28976 - facebook/react#28975 - facebook/react#28969 - facebook/react#28966 - facebook/react#28056 </details>
Completes vercel#65845 which only updated Canary not Experimental. <details> <summary> React upstream changes</summary> - facebook/react#29026 - facebook/react#29025 - facebook/react#28743 - facebook/react#29022 - facebook/react#29023 - facebook/react#29015 - facebook/react#29016 - facebook/react#28988 - facebook/react#28987 - facebook/react#28986 - facebook/react#29014 - facebook/react#28982 - facebook/react#29006 - facebook/react#28973 - facebook/react#28841 - facebook/react#28964 - facebook/react#28990 - facebook/react#29003 - facebook/react#28989 - facebook/react#28893 - facebook/react#28887 - facebook/react#28807 - facebook/react#28978 - facebook/react#28963 - facebook/react#28972 - facebook/react#28970 - facebook/react#28816 - facebook/react#28977 - facebook/react#28974 - facebook/react#28976 - facebook/react#28975 - facebook/react#28969 - facebook/react#28966 - facebook/react#28056 </details>
Summary
Fixes #27391
Fix form actions to include the
name
andvalue
from theform-associated element
when it is located outside the form.How did you test this change?
react-dom
.