Use FormDatasubmitter parameter

[jenseng]

Summary

Fixes#29018
Fixes#34944
Closes#34990
Closes#35354

Rather than continuing to maintain/fix an incomplete polyfill (e.g. #28056, #34990), just pass the submitter to the FormData constructor, 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:

• Most of them also don't support SubmitEvent.submitter, which means the existing approach won't work for them either
• The FormDatasubmittercan easily be polyfilled (as canSubmitEvent.submitter) for developers that care about named submit buttons working correctly in old browsers

Older browsers (Safari on old iOS, Samsung TVs), can polyfill FormData the way React did:

constOriginalFormData=globalThis.FormDataclassFormDataWithSubmitterPolyfillextendsOriginalFormData{constructor(form,submitter){console.log(form,submitter)if(submitter!=null){consttemp=submitter.ownerDocument.createElement('input');temp.name=submitter.name;temp.value=submitter.value;if(form.id){temp.setAttribute('form',form.id);}submitter.parentNode.insertBefore(temp,submitter);super(form)temp.parentNode.removeChild(temp)}else{super(form)}}}globalThis.FormData=FormDataWithSubmitterPolyfill

Note that the above doesn't apply to native submissions but neither did React's behavior. It only applied to React's Form Actions.

How did you test this change?

Updated an existing test to more fully cover submitter serialization, and ran yarn test and yarn test --prod successfully.

[react-sizebot]

Comparing: 454fc41...a497924

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.js	=	6.84 kB	6.84 kB	+0.05%	1.88 kB	1.88 kB
oss-stable/react-dom/cjs/react-dom-client.production.js	=	608.16 kB	607.60 kB	=	107.67 kB	107.54 kB
oss-experimental/react-dom/cjs/react-dom.production.js	=	6.84 kB	6.84 kB	=	1.88 kB	1.88 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js	=	667.39 kB	666.83 kB	=	117.55 kB	117.42 kB
facebook-www/ReactDOM-prod.classic.js	=	693.47 kB	692.91 kB	=	122.05 kB	121.92 kB
facebook-www/ReactDOM-prod.modern.js	=	683.89 kB	683.34 kB	=	120.43 kB	120.31 kB
oss-experimental/react-dom/unstable_server-external-runtime.js	=	18.59 kB	18.15 kB	=	4.19 kB	4.13 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-experimental/react-dom/unstable_server-external-runtime.js	=	18.59 kB	18.15 kB	=	4.19 kB	4.13 kB

Generated by 🚫 dangerJS against a497924

[jenseng]

Looks like flow needs a fix: https://github.com/facebook/flow/blob/b75ba641223ff7fe597d43ca8d4ae76c52efe9f9/lib/bom.js#L628

[sebmarkbage]

The two browsers which we support that were the main culprits before was:

1. Safari on older iOS versions that still wasn't updated to latest version. Safari takes a surprisingly long time to actually propagate. It's not enough that an iOS version is released.
2. Older Samsung TVs with web UIs. E.g. Facebook has TV apps on those.

Once those are confirmed, we can move to the submitter parameter.

[jenseng]

@sebmarkbage just curious, is there a test matrix or something to automatically validate that? or is it more of a manual process/judgment call?

[github-actions]

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.

[jenseng]

bump

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 3, 2024 6:27pm

[github-actions]

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.

[jenseng]

bump

[github-actions]

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.

[jenseng]

bump 🥲

[github-actions]

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.

[jenseng]

bumpity bump

[jenseng]

Another bug caused by the current partial polyfill: #34944

cc @sebmarkbage

[eps1lon]

We think it's fine to remove this. Browsers without support for the second arg can restore the original behavior by polyfilling FormData.

[jenseng]

Summary

Fixes#29018
Fixes#34944
Closes#34990
Closes#35354

Rather than continuing to maintain/fix an incomplete polyfill (e.g. #28056, #34990), just pass the submitter to the FormData constructor, 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:

• Most of them also don't support SubmitEvent.submitter, which means the existing approach won't work for them either
• The FormDatasubmittercan easily be polyfilled (as canSubmitEvent.submitter) for developers that care about named submit buttons working correctly in old browsers

Older browsers (Safari on old iOS, Samsung TVs), can polyfill FormData the way React did:

constOriginalFormData=globalThis.FormDataclassFormDataWithSubmitterPolyfillextendsOriginalFormData{constructor(form,submitter){console.log(form,submitter)if(submitter!=null){consttemp=submitter.ownerDocument.createElement('input');temp.name=submitter.name;temp.value=submitter.value;if(form.id){temp.setAttribute('form',form.id);}submitter.parentNode.insertBefore(temp,submitter);super(form)temp.parentNode.removeChild(temp)}else{super(form)}}}globalThis.FormData=FormDataWithSubmitterPolyfill

Note that the above doesn't apply to native submissions but neither did React's behavior. It only applied to React's Form Actions.

How did you test this change?

Updated an existing test to more fully cover submitter serialization, and ran yarn test and yarn test --prod successfully.

[react-sizebot]

Comparing: 454fc41...a497924

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.js	=	6.84 kB	6.84 kB	+0.05%	1.88 kB	1.88 kB
oss-stable/react-dom/cjs/react-dom-client.production.js	=	608.16 kB	607.60 kB	=	107.67 kB	107.54 kB
oss-experimental/react-dom/cjs/react-dom.production.js	=	6.84 kB	6.84 kB	=	1.88 kB	1.88 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js	=	667.39 kB	666.83 kB	=	117.55 kB	117.42 kB
facebook-www/ReactDOM-prod.classic.js	=	693.47 kB	692.91 kB	=	122.05 kB	121.92 kB
facebook-www/ReactDOM-prod.modern.js	=	683.89 kB	683.34 kB	=	120.43 kB	120.31 kB
oss-experimental/react-dom/unstable_server-external-runtime.js	=	18.59 kB	18.15 kB	=	4.19 kB	4.13 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-experimental/react-dom/unstable_server-external-runtime.js	=	18.59 kB	18.15 kB	=	4.19 kB	4.13 kB

Generated by 🚫 dangerJS against a497924

[jenseng]

Looks like flow needs a fix: https://github.com/facebook/flow/blob/b75ba641223ff7fe597d43ca8d4ae76c52efe9f9/lib/bom.js#L628

[sebmarkbage]

The two browsers which we support that were the main culprits before was:

1. Safari on older iOS versions that still wasn't updated to latest version. Safari takes a surprisingly long time to actually propagate. It's not enough that an iOS version is released.
2. Older Samsung TVs with web UIs. E.g. Facebook has TV apps on those.

Once those are confirmed, we can move to the submitter parameter.

[jenseng]

@sebmarkbage just curious, is there a test matrix or something to automatically validate that? or is it more of a manual process/judgment call?

[github-actions]

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.

[jenseng]

bump

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 3, 2024 6:27pm

[github-actions]

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.

[jenseng]

bump

[github-actions]

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.

[jenseng]

bump 🥲

[github-actions]

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.

[jenseng]

bumpity bump

[jenseng]

Another bug caused by the current partial polyfill: #34944

cc @sebmarkbage

[eps1lon]

We think it's fine to remove this. Browsers without support for the second arg can restore the original behavior by polyfilling FormData.

[jenseng]

Summary

Fixes#29018
Fixes#34944
Closes#34990
Closes#35354

Rather than continuing to maintain/fix an incomplete polyfill (e.g. #28056, #34990), just pass the submitter to the FormData constructor, 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:

• Most of them also don't support SubmitEvent.submitter, which means the existing approach won't work for them either
• The FormDatasubmittercan easily be polyfilled (as canSubmitEvent.submitter) for developers that care about named submit buttons working correctly in old browsers

Older browsers (Safari on old iOS, Samsung TVs), can polyfill FormData the way React did:

constOriginalFormData=globalThis.FormDataclassFormDataWithSubmitterPolyfillextendsOriginalFormData{constructor(form,submitter){console.log(form,submitter)if(submitter!=null){consttemp=submitter.ownerDocument.createElement('input');temp.name=submitter.name;temp.value=submitter.value;if(form.id){temp.setAttribute('form',form.id);}submitter.parentNode.insertBefore(temp,submitter);super(form)temp.parentNode.removeChild(temp)}else{super(form)}}}globalThis.FormData=FormDataWithSubmitterPolyfill

Note that the above doesn't apply to native submissions but neither did React's behavior. It only applied to React's Form Actions.

How did you test this change?

Updated an existing test to more fully cover submitter serialization, and ran yarn test and yarn test --prod successfully.

[react-sizebot]

Comparing: 454fc41...a497924

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.js	=	6.84 kB	6.84 kB	+0.05%	1.88 kB	1.88 kB
oss-stable/react-dom/cjs/react-dom-client.production.js	=	608.16 kB	607.60 kB	=	107.67 kB	107.54 kB
oss-experimental/react-dom/cjs/react-dom.production.js	=	6.84 kB	6.84 kB	=	1.88 kB	1.88 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js	=	667.39 kB	666.83 kB	=	117.55 kB	117.42 kB
facebook-www/ReactDOM-prod.classic.js	=	693.47 kB	692.91 kB	=	122.05 kB	121.92 kB
facebook-www/ReactDOM-prod.modern.js	=	683.89 kB	683.34 kB	=	120.43 kB	120.31 kB
oss-experimental/react-dom/unstable_server-external-runtime.js	=	18.59 kB	18.15 kB	=	4.19 kB	4.13 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-experimental/react-dom/unstable_server-external-runtime.js	=	18.59 kB	18.15 kB	=	4.19 kB	4.13 kB

Generated by 🚫 dangerJS against a497924

[jenseng]

Looks like flow needs a fix: https://github.com/facebook/flow/blob/b75ba641223ff7fe597d43ca8d4ae76c52efe9f9/lib/bom.js#L628

[sebmarkbage]

The two browsers which we support that were the main culprits before was:

1. Safari on older iOS versions that still wasn't updated to latest version. Safari takes a surprisingly long time to actually propagate. It's not enough that an iOS version is released.
2. Older Samsung TVs with web UIs. E.g. Facebook has TV apps on those.

Once those are confirmed, we can move to the submitter parameter.

[jenseng]

@sebmarkbage just curious, is there a test matrix or something to automatically validate that? or is it more of a manual process/judgment call?

[github-actions]

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.

[jenseng]

bump

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 3, 2024 6:27pm

[github-actions]

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.

[jenseng]

bump

[github-actions]

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.

[jenseng]

bump 🥲

[github-actions]

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.

[jenseng]

bumpity bump

[jenseng]

Another bug caused by the current partial polyfill: #34944

cc @sebmarkbage

[eps1lon]

We think it's fine to remove this. Browsers without support for the second arg can restore the original behavior by polyfilling FormData.

[jenseng]

Summary

Fixes#29018
Fixes#34944
Closes#34990
Closes#35354

Rather than continuing to maintain/fix an incomplete polyfill (e.g. #28056, #34990), just pass the submitter to the FormData constructor, 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:

• Most of them also don't support SubmitEvent.submitter, which means the existing approach won't work for them either
• The FormDatasubmittercan easily be polyfilled (as canSubmitEvent.submitter) for developers that care about named submit buttons working correctly in old browsers

Older browsers (Safari on old iOS, Samsung TVs), can polyfill FormData the way React did:

constOriginalFormData=globalThis.FormDataclassFormDataWithSubmitterPolyfillextendsOriginalFormData{constructor(form,submitter){console.log(form,submitter)if(submitter!=null){consttemp=submitter.ownerDocument.createElement('input');temp.name=submitter.name;temp.value=submitter.value;if(form.id){temp.setAttribute('form',form.id);}submitter.parentNode.insertBefore(temp,submitter);super(form)temp.parentNode.removeChild(temp)}else{super(form)}}}globalThis.FormData=FormDataWithSubmitterPolyfill

Note that the above doesn't apply to native submissions but neither did React's behavior. It only applied to React's Form Actions.

How did you test this change?

Updated an existing test to more fully cover submitter serialization, and ran yarn test and yarn test --prod successfully.

[react-sizebot]

Comparing: 454fc41...a497924

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.js	=	6.84 kB	6.84 kB	+0.05%	1.88 kB	1.88 kB
oss-stable/react-dom/cjs/react-dom-client.production.js	=	608.16 kB	607.60 kB	=	107.67 kB	107.54 kB
oss-experimental/react-dom/cjs/react-dom.production.js	=	6.84 kB	6.84 kB	=	1.88 kB	1.88 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js	=	667.39 kB	666.83 kB	=	117.55 kB	117.42 kB
facebook-www/ReactDOM-prod.classic.js	=	693.47 kB	692.91 kB	=	122.05 kB	121.92 kB
facebook-www/ReactDOM-prod.modern.js	=	683.89 kB	683.34 kB	=	120.43 kB	120.31 kB
oss-experimental/react-dom/unstable_server-external-runtime.js	=	18.59 kB	18.15 kB	=	4.19 kB	4.13 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-experimental/react-dom/unstable_server-external-runtime.js	=	18.59 kB	18.15 kB	=	4.19 kB	4.13 kB

Generated by 🚫 dangerJS against a497924

[jenseng]

Looks like flow needs a fix: https://github.com/facebook/flow/blob/b75ba641223ff7fe597d43ca8d4ae76c52efe9f9/lib/bom.js#L628

[sebmarkbage]

The two browsers which we support that were the main culprits before was:

1. Safari on older iOS versions that still wasn't updated to latest version. Safari takes a surprisingly long time to actually propagate. It's not enough that an iOS version is released.
2. Older Samsung TVs with web UIs. E.g. Facebook has TV apps on those.

Once those are confirmed, we can move to the submitter parameter.

[jenseng]

@sebmarkbage just curious, is there a test matrix or something to automatically validate that? or is it more of a manual process/judgment call?

[github-actions]

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.

[jenseng]

bump

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 3, 2024 6:27pm

[github-actions]

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.

[jenseng]

bump

[github-actions]

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.

[jenseng]

bump 🥲

[github-actions]

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.

[jenseng]

bumpity bump

[jenseng]

Another bug caused by the current partial polyfill: #34944

cc @sebmarkbage

[eps1lon]

We think it's fine to remove this. Browsers without support for the second arg can restore the original behavior by polyfilling FormData.

[jenseng]

Summary

Fixes#29018
Fixes#34944
Closes#34990
Closes#35354

Rather than continuing to maintain/fix an incomplete polyfill (e.g. #28056, #34990), just pass the submitter to the FormData constructor, 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:

• Most of them also don't support SubmitEvent.submitter, which means the existing approach won't work for them either
• The FormDatasubmittercan easily be polyfilled (as canSubmitEvent.submitter) for developers that care about named submit buttons working correctly in old browsers

Older browsers (Safari on old iOS, Samsung TVs), can polyfill FormData the way React did:

constOriginalFormData=globalThis.FormDataclassFormDataWithSubmitterPolyfillextendsOriginalFormData{constructor(form,submitter){console.log(form,submitter)if(submitter!=null){consttemp=submitter.ownerDocument.createElement('input');temp.name=submitter.name;temp.value=submitter.value;if(form.id){temp.setAttribute('form',form.id);}submitter.parentNode.insertBefore(temp,submitter);super(form)temp.parentNode.removeChild(temp)}else{super(form)}}}globalThis.FormData=FormDataWithSubmitterPolyfill

Note that the above doesn't apply to native submissions but neither did React's behavior. It only applied to React's Form Actions.

How did you test this change?

Updated an existing test to more fully cover submitter serialization, and ran yarn test and yarn test --prod successfully.

[react-sizebot]

Comparing: 454fc41...a497924

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.js	=	6.84 kB	6.84 kB	+0.05%	1.88 kB	1.88 kB
oss-stable/react-dom/cjs/react-dom-client.production.js	=	608.16 kB	607.60 kB	=	107.67 kB	107.54 kB
oss-experimental/react-dom/cjs/react-dom.production.js	=	6.84 kB	6.84 kB	=	1.88 kB	1.88 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js	=	667.39 kB	666.83 kB	=	117.55 kB	117.42 kB
facebook-www/ReactDOM-prod.classic.js	=	693.47 kB	692.91 kB	=	122.05 kB	121.92 kB
facebook-www/ReactDOM-prod.modern.js	=	683.89 kB	683.34 kB	=	120.43 kB	120.31 kB
oss-experimental/react-dom/unstable_server-external-runtime.js	=	18.59 kB	18.15 kB	=	4.19 kB	4.13 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-experimental/react-dom/unstable_server-external-runtime.js	=	18.59 kB	18.15 kB	=	4.19 kB	4.13 kB

Generated by 🚫 dangerJS against a497924

[jenseng]

Looks like flow needs a fix: https://github.com/facebook/flow/blob/b75ba641223ff7fe597d43ca8d4ae76c52efe9f9/lib/bom.js#L628

[sebmarkbage]

The two browsers which we support that were the main culprits before was:

1. Safari on older iOS versions that still wasn't updated to latest version. Safari takes a surprisingly long time to actually propagate. It's not enough that an iOS version is released.
2. Older Samsung TVs with web UIs. E.g. Facebook has TV apps on those.

Once those are confirmed, we can move to the submitter parameter.

[jenseng]

@sebmarkbage just curious, is there a test matrix or something to automatically validate that? or is it more of a manual process/judgment call?

[github-actions]

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.

[jenseng]

bump

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 3, 2024 6:27pm

[github-actions]

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.

[jenseng]

bump

[github-actions]

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.

[jenseng]

bump 🥲

[github-actions]

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.

[jenseng]

bumpity bump

[jenseng]

Another bug caused by the current partial polyfill: #34944

cc @sebmarkbage

[eps1lon]

We think it's fine to remove this. Browsers without support for the second arg can restore the original behavior by polyfilling FormData.

[jenseng]

Summary

Fixes#29018
Fixes#34944
Closes#34990
Closes#35354

Rather than continuing to maintain/fix an incomplete polyfill (e.g. #28056, #34990), just pass the submitter to the FormData constructor, 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:

• Most of them also don't support SubmitEvent.submitter, which means the existing approach won't work for them either
• The FormDatasubmittercan easily be polyfilled (as canSubmitEvent.submitter) for developers that care about named submit buttons working correctly in old browsers

Older browsers (Safari on old iOS, Samsung TVs), can polyfill FormData the way React did:

constOriginalFormData=globalThis.FormDataclassFormDataWithSubmitterPolyfillextendsOriginalFormData{constructor(form,submitter){console.log(form,submitter)if(submitter!=null){consttemp=submitter.ownerDocument.createElement('input');temp.name=submitter.name;temp.value=submitter.value;if(form.id){temp.setAttribute('form',form.id);}submitter.parentNode.insertBefore(temp,submitter);super(form)temp.parentNode.removeChild(temp)}else{super(form)}}}globalThis.FormData=FormDataWithSubmitterPolyfill

Note that the above doesn't apply to native submissions but neither did React's behavior. It only applied to React's Form Actions.

How did you test this change?

Updated an existing test to more fully cover submitter serialization, and ran yarn test and yarn test --prod successfully.

[react-sizebot]

Comparing: 454fc41...a497924

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.js	=	6.84 kB	6.84 kB	+0.05%	1.88 kB	1.88 kB
oss-stable/react-dom/cjs/react-dom-client.production.js	=	608.16 kB	607.60 kB	=	107.67 kB	107.54 kB
oss-experimental/react-dom/cjs/react-dom.production.js	=	6.84 kB	6.84 kB	=	1.88 kB	1.88 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js	=	667.39 kB	666.83 kB	=	117.55 kB	117.42 kB
facebook-www/ReactDOM-prod.classic.js	=	693.47 kB	692.91 kB	=	122.05 kB	121.92 kB
facebook-www/ReactDOM-prod.modern.js	=	683.89 kB	683.34 kB	=	120.43 kB	120.31 kB
oss-experimental/react-dom/unstable_server-external-runtime.js	=	18.59 kB	18.15 kB	=	4.19 kB	4.13 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-experimental/react-dom/unstable_server-external-runtime.js	=	18.59 kB	18.15 kB	=	4.19 kB	4.13 kB

Generated by 🚫 dangerJS against a497924

[jenseng]

Looks like flow needs a fix: https://github.com/facebook/flow/blob/b75ba641223ff7fe597d43ca8d4ae76c52efe9f9/lib/bom.js#L628

[sebmarkbage]

The two browsers which we support that were the main culprits before was:

1. Safari on older iOS versions that still wasn't updated to latest version. Safari takes a surprisingly long time to actually propagate. It's not enough that an iOS version is released.
2. Older Samsung TVs with web UIs. E.g. Facebook has TV apps on those.

Once those are confirmed, we can move to the submitter parameter.

[jenseng]

@sebmarkbage just curious, is there a test matrix or something to automatically validate that? or is it more of a manual process/judgment call?

[github-actions]

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.

[jenseng]

bump

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 3, 2024 6:27pm

[github-actions]

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.

[jenseng]

bump

[github-actions]

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.

[jenseng]

bump 🥲

[github-actions]

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.

[jenseng]

bumpity bump

[jenseng]

Another bug caused by the current partial polyfill: #34944

cc @sebmarkbage

[eps1lon]

We think it's fine to remove this. Browsers without support for the second arg can restore the original behavior by polyfilling FormData.

[jenseng]

Summary

Fixes#29018
Fixes#34944
Closes#34990
Closes#35354

Rather than continuing to maintain/fix an incomplete polyfill (e.g. #28056, #34990), just pass the submitter to the FormData constructor, 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:

• Most of them also don't support SubmitEvent.submitter, which means the existing approach won't work for them either
• The FormDatasubmittercan easily be polyfilled (as canSubmitEvent.submitter) for developers that care about named submit buttons working correctly in old browsers

Older browsers (Safari on old iOS, Samsung TVs), can polyfill FormData the way React did:

constOriginalFormData=globalThis.FormDataclassFormDataWithSubmitterPolyfillextendsOriginalFormData{constructor(form,submitter){console.log(form,submitter)if(submitter!=null){consttemp=submitter.ownerDocument.createElement('input');temp.name=submitter.name;temp.value=submitter.value;if(form.id){temp.setAttribute('form',form.id);}submitter.parentNode.insertBefore(temp,submitter);super(form)temp.parentNode.removeChild(temp)}else{super(form)}}}globalThis.FormData=FormDataWithSubmitterPolyfill

Note that the above doesn't apply to native submissions but neither did React's behavior. It only applied to React's Form Actions.

How did you test this change?

Updated an existing test to more fully cover submitter serialization, and ran yarn test and yarn test --prod successfully.

[react-sizebot]

Comparing: 454fc41...a497924

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.js	=	6.84 kB	6.84 kB	+0.05%	1.88 kB	1.88 kB
oss-stable/react-dom/cjs/react-dom-client.production.js	=	608.16 kB	607.60 kB	=	107.67 kB	107.54 kB
oss-experimental/react-dom/cjs/react-dom.production.js	=	6.84 kB	6.84 kB	=	1.88 kB	1.88 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js	=	667.39 kB	666.83 kB	=	117.55 kB	117.42 kB
facebook-www/ReactDOM-prod.classic.js	=	693.47 kB	692.91 kB	=	122.05 kB	121.92 kB
facebook-www/ReactDOM-prod.modern.js	=	683.89 kB	683.34 kB	=	120.43 kB	120.31 kB
oss-experimental/react-dom/unstable_server-external-runtime.js	=	18.59 kB	18.15 kB	=	4.19 kB	4.13 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-experimental/react-dom/unstable_server-external-runtime.js	=	18.59 kB	18.15 kB	=	4.19 kB	4.13 kB

Generated by 🚫 dangerJS against a497924

[jenseng]

Looks like flow needs a fix: https://github.com/facebook/flow/blob/b75ba641223ff7fe597d43ca8d4ae76c52efe9f9/lib/bom.js#L628

[sebmarkbage]

The two browsers which we support that were the main culprits before was:

1. Safari on older iOS versions that still wasn't updated to latest version. Safari takes a surprisingly long time to actually propagate. It's not enough that an iOS version is released.
2. Older Samsung TVs with web UIs. E.g. Facebook has TV apps on those.

Once those are confirmed, we can move to the submitter parameter.

[jenseng]

@sebmarkbage just curious, is there a test matrix or something to automatically validate that? or is it more of a manual process/judgment call?

[github-actions]

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.

[jenseng]

bump

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 3, 2024 6:27pm

[github-actions]

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.

[jenseng]

bump

[github-actions]

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.

[jenseng]

bump 🥲

[github-actions]

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.

[jenseng]

bumpity bump

[jenseng]

Another bug caused by the current partial polyfill: #34944

cc @sebmarkbage

[eps1lon]

We think it's fine to remove this. Browsers without support for the second arg can restore the original behavior by polyfilling FormData.

[jenseng]

Summary

Fixes#29018
Fixes#34944
Closes#34990
Closes#35354

Rather than continuing to maintain/fix an incomplete polyfill (e.g. #28056, #34990), just pass the submitter to the FormData constructor, 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:

• Most of them also don't support SubmitEvent.submitter, which means the existing approach won't work for them either
• The FormDatasubmittercan easily be polyfilled (as canSubmitEvent.submitter) for developers that care about named submit buttons working correctly in old browsers

Older browsers (Safari on old iOS, Samsung TVs), can polyfill FormData the way React did:

constOriginalFormData=globalThis.FormDataclassFormDataWithSubmitterPolyfillextendsOriginalFormData{constructor(form,submitter){console.log(form,submitter)if(submitter!=null){consttemp=submitter.ownerDocument.createElement('input');temp.name=submitter.name;temp.value=submitter.value;if(form.id){temp.setAttribute('form',form.id);}submitter.parentNode.insertBefore(temp,submitter);super(form)temp.parentNode.removeChild(temp)}else{super(form)}}}globalThis.FormData=FormDataWithSubmitterPolyfill

Note that the above doesn't apply to native submissions but neither did React's behavior. It only applied to React's Form Actions.

How did you test this change?

Updated an existing test to more fully cover submitter serialization, and ran yarn test and yarn test --prod successfully.

[react-sizebot]

Comparing: 454fc41...a497924

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-stable/react-dom/cjs/react-dom.production.js	=	6.84 kB	6.84 kB	+0.05%	1.88 kB	1.88 kB
oss-stable/react-dom/cjs/react-dom-client.production.js	=	608.16 kB	607.60 kB	=	107.67 kB	107.54 kB
oss-experimental/react-dom/cjs/react-dom.production.js	=	6.84 kB	6.84 kB	=	1.88 kB	1.88 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js	=	667.39 kB	666.83 kB	=	117.55 kB	117.42 kB
facebook-www/ReactDOM-prod.classic.js	=	693.47 kB	692.91 kB	=	122.05 kB	121.92 kB
facebook-www/ReactDOM-prod.modern.js	=	683.89 kB	683.34 kB	=	120.43 kB	120.31 kB
oss-experimental/react-dom/unstable_server-external-runtime.js	=	18.59 kB	18.15 kB	=	4.19 kB	4.13 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show

Name	+/-	Base	Current	+/- gzip	Base gzip	Current gzip
oss-experimental/react-dom/unstable_server-external-runtime.js	=	18.59 kB	18.15 kB	=	4.19 kB	4.13 kB

Generated by 🚫 dangerJS against a497924

[jenseng]

Looks like flow needs a fix: https://github.com/facebook/flow/blob/b75ba641223ff7fe597d43ca8d4ae76c52efe9f9/lib/bom.js#L628

[sebmarkbage]

The two browsers which we support that were the main culprits before was:

1. Safari on older iOS versions that still wasn't updated to latest version. Safari takes a surprisingly long time to actually propagate. It's not enough that an iOS version is released.
2. Older Samsung TVs with web UIs. E.g. Facebook has TV apps on those.

Once those are confirmed, we can move to the submitter parameter.

[jenseng]

@sebmarkbage just curious, is there a test matrix or something to automatically validate that? or is it more of a manual process/judgment call?

[github-actions]

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.

[jenseng]

bump

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 3, 2024 6:27pm

[github-actions]

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.

[jenseng]

bump

[github-actions]

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.

[jenseng]

bump 🥲

[github-actions]

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.

[jenseng]

bumpity bump

[jenseng]

Another bug caused by the current partial polyfill: #34944

cc @sebmarkbage

[eps1lon]

We think it's fine to remove this. Browsers without support for the second arg can restore the original behavior by polyfilling FormData.