Failing test for radio checked with changing names

[sophiebits]

 FAIL  packages/react-dom/src/__tests__/ReactDOMInput-test.js (8.01 s)
  ● ReactDOMInput › shouldn't get tricked by changing radio names, part 2

    expect(received).toBe(expected) // Object.is equality

    Expected: true
    Received: false

      1209 |       container
      1210 |     );
    > 1211 |     expect(container.querySelector('input[name="a"][value="1"]').checked).toBe(true);
           |                                                                               ^
      1212 |     expect(container.querySelector('input[name="b"][value="2"]').checked).toBe(true);
      1213 |   });
      1214 |

      at Object.<anonymous> (packages/react-dom/src/__tests__/ReactDOMInput-test.js:1211:79)

[react-sizebot]

Comparing: 451736b...89508f2

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.min.js	=	164.58 kB	164.58 kB	=	51.69 kB	51.69 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js	=	166.15 kB	166.15 kB	=	52.20 kB	52.20 kB
facebook-www/ReactDOM-prod.classic.js	=	554.05 kB	554.05 kB	=	98.17 kB	98.17 kB
facebook-www/ReactDOM-prod.modern.js	=	537.89 kB	537.89 kB	=	95.51 kB	95.51 kB

Significant size changes

Includes any change greater than 0.2%:

(No significant changes)

Generated by 🚫 dangerJS against 89508f2

[sophiebits]

(just confirmed this fails on 18.2.0)

[sophiebits]

I think this could work except maybe it's wasteful and I'm not sure if it mucks with dirty checkedness incorrectly:

diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js
index e9398cf07..a29aa4f21 100644
--- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js
+++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js
@@ -1243,11 +1243,14 @@ export function updateProperties(
       break;
     }
     case 'input': {
-      // Update checked *before* name.
-      // In the middle of an update, it is possible to have multiple checked.
-      // When a checked radio tries to change name, browser makes another radio's checked false.
-      if (nextProps.type === 'radio' && nextProps.name != null) {
-        updateInputChecked(domElement, nextProps);
+      // If a radio button's name or type change, then we need to be careful
+      // about the order of property assignment to ensure other radio buttons in
+      // the same (previous or next) group don't get incorrectly unchecked.
+      // Easiest way to do this is to unset checked and then restore it after.
+      let restoreChecked = false;
+      if ((lastProps.type === 'radio' || nextProps.type === 'radio') && domElement.checked) {
+        domElement.checked = false;
+        restoreChecked = true;
       }
       for (const propKey in lastProps) {
         const lastProp = lastProps[propKey];
@@ -1264,6 +1267,7 @@ export function updateProperties(
                 !!checked &&
                 typeof checked !== 'function' &&
                 checked !== 'symbol';
+              restoreChecked = false;
               break;
             }
             case 'value': {
@@ -1294,6 +1298,7 @@ export function updateProperties(
                 !!checked &&
                 typeof checked !== 'function' &&
                 checked !== 'symbol';
+              restoreChecked = false;
               break;
             }
             case 'value': {
@@ -1317,6 +1322,9 @@ export function updateProperties(
           }
         }
       }
+      if (restoreChecked) {
+        domElement.checked = true;
+      }
 
       if (__DEV__) {
         const wasControlled =
@@ -1654,11 +1662,14 @@ export function updatePropertiesWithDiff(
       break;
     }
     case 'input': {
-      // Update checked *before* name.
-      // In the middle of an update, it is possible to have multiple checked.
-      // When a checked radio tries to change name, browser makes another radio's checked false.
-      if (nextProps.type === 'radio' && nextProps.name != null) {
-        updateInputChecked(domElement, nextProps);
+      // If a radio button's name or type change, then we need to be careful
+      // about the order of property assignment to ensure other radio buttons in
+      // the same (previous or next) group don't get incorrectly unchecked.
+      // Easiest way to do this is to unset checked and then restore it after.
+      let restoreChecked = false;
+      if ((lastProps.type === 'radio' || nextProps.type === 'radio') && domElement.checked) {
+        domElement.checked = false;
+        restoreChecked = true;
       }
       for (let i = 0; i < updatePayload.length; i += 2) {
         const propKey = updatePayload[i];
@@ -1672,6 +1683,7 @@ export function updatePropertiesWithDiff(
               !!checked &&
               typeof checked !== 'function' &&
               checked !== 'symbol';
+            restoreChecked = false;
             break;
           }
           case 'value': {
@@ -1694,6 +1706,9 @@ export function updatePropertiesWithDiff(
           }
         }
       }
+      if (restoreChecked) {
+        domElement.checked = true;
+      }
 
       if (__DEV__) {
         const wasControlled =

[sophiebits]

merged as part of #26667