-
Notifications
You must be signed in to change notification settings - Fork 49.8k
Use FormData submitter parameter
#29028
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
base: main
Are you sure you want to change the base?
Conversation
|
Comparing: b9b510d...d5cd0ec 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
|
|
Looks like flow needs a fix: https://github.com/facebook/flow/blob/b75ba641223ff7fe597d43ca8d4ae76c52efe9f9/lib/bom.js#L628 |
|
The two browsers which we support that were the main culprits before was:
Once those are confirmed, we can move to the submitter parameter. |
|
@sebmarkbage just curious, is there a test matrix or something to automatically validate that? or is it more of a manual process/judgment call? |
bb3ec87 to
e33216e
Compare
|
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 |
e33216e to
05f1fc3
Compare
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
05f1fc3 to
bb0ab71
Compare
|
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 |
bb0ab71 to
b1117e6
Compare
|
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 🥲 |
|
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. |
|
bumpity bump |
|
Another bug caused by the current partial polyfill: #34944 cc @sebmarkbage |
Summary
Fixes #29018
Fixes #34944
Rather than continuing to maintain/fix an incomplete polyfill (e.g. #28056, #34990), just pass the
submitterto theFormDataconstructor, since it is now widely available. This ensures that the form data is populated consistently with a native form submission, and lets us remove a whole lot of code. This is a followup to this discussion over two years ago.Elaborating on that, if we're concerned about browsers that don't support it, it's important to note that:
SubmitEvent.submitter, which means the existing approach won't work for them eitherFormDatasubmittercan easily be polyfilled (as canSubmitEvent.submitter) for developers that care about named submit buttons working correctly in old browsersHow did you test this change?
Updated an existing test to more fully cover
submitterserialization, and ranyarn testandyarn test --prodsuccessfully.