Fix controlled radios, maybe for real this time (facebook#27443)

Fixes facebook#26876 for real?

In 18.2.0 (last stable), we set .checked unconditionally:


https://github.com/facebook/react/blob/v18.2.0/packages/react-dom/src/client/ReactDOMInput.js#L129-L135

This is important because if we are updating two radios' checkedness
from (false, true) to (true, false), we need to make sure that
input2.checked is explicitly set to false, even though setting
`input1.checked = true` already unchecks input2.

I think this fix is not complete because there is no guarantee that all
the inputs rerender at the same time? Hence the TODO. But in practice
they usually would and I _think_ this is comparable to what we had
before.

Also treating function and symbol as false like we used to and like we
do on initial mount.

packages/react-dom-bindings/src/client/ReactDOMInput.js

@@ -175,8 +175,13 @@ export function updateInput(
     }
   }
 
-  if (checked != null && node.checked !== !!checked) {
-    node.checked = checked;
+  if (checked != null) {
+    // Important to set this even if it's not a change in order to update input
+    // value tracking with radio buttons
+    // TODO: Should really update input value tracking for the whole radio
+    // button group in an effect or something (similar to #27024)
+    node.checked =
+      checked && typeof checked !== 'function' && typeof checked !== 'symbol';
   }
 
   if (

packages/react-dom/src/__tests__/ReactDOMComponent-test.js

@@ -1094,18 +1094,12 @@ describe('ReactDOMComponent', () => {
 
     it('should not incur unnecessary DOM mutations for boolean properties', () => {
       const container = document.createElement('div');
-      function onChange() {
-        // noop
-      }
-      ReactDOM.render(
-        <input type="checkbox" onChange={onChange} checked={true} />,
-        container,
-      );
+      ReactDOM.render(<audio muted={true} />, container);
 
       const node = container.firstChild;
       let nodeValue = true;
       const nodeValueSetter = jest.fn();
-      Object.defineProperty(node, 'checked', {
+      Object.defineProperty(node, 'muted', {
         get: function () {
           return nodeValue;
         },
@@ -1114,48 +1108,11 @@ describe('ReactDOMComponent', () => {
         }),
       });
 
-      ReactDOM.render(
-        <input
-          type="checkbox"
-          onChange={onChange}
-          checked={true}
-          data-unrelated={true}
-        />,
-        container,
-      );
-      expect(nodeValueSetter).toHaveBeenCalledTimes(0);
-
-      expect(() => {
-        ReactDOM.render(
-          <input type="checkbox" onChange={onChange} />,
-          container,
-        );
-      }).toErrorDev(
-        'A component is changing a controlled input to be uncontrolled. This is likely caused by ' +
-          'the value changing from a defined to undefined, which should not happen. Decide between ' +
-          'using a controlled or uncontrolled input element for the lifetime of the component.',
-      );
-      // This leaves the current checked value in place, just like text inputs.
+      ReactDOM.render(<audio muted={true} data-unrelated="yes" />, container);
       expect(nodeValueSetter).toHaveBeenCalledTimes(0);
 
-      expect(() => {
-        ReactDOM.render(
-          <input type="checkbox" onChange={onChange} checked={false} />,
-          container,
-        );
-      }).toErrorDev(
-        ' A component is changing an uncontrolled input to be controlled. This is likely caused by ' +
-          'the value changing from undefined to a defined value, which should not happen. Decide between ' +
-          'using a controlled or uncontrolled input element for the lifetime of the component.',
-      );
-
+      ReactDOM.render(<audio muted={false} data-unrelated="ok" />, container);
       expect(nodeValueSetter).toHaveBeenCalledTimes(1);
-
-      ReactDOM.render(
-        <input type="checkbox" onChange={onChange} checked={true} />,
-        container,
-      );
-      expect(nodeValueSetter).toHaveBeenCalledTimes(2);
     });
 
     it('should ignore attribute list for elements with the "is" attribute', () => {

packages/react-dom/src/__tests__/ReactDOMInput-test.js

@@ -1414,6 +1414,83 @@ describe('ReactDOMInput', () => {
     assertInputTrackingIsCurrent(container);
   });
 
+  it('should control radio buttons if the tree updates during render (case 2; #26876)', () => {
+    let thunk = null;
+    function App() {
+      const [disabled, setDisabled] = React.useState(false);
+      const [value, setValue] = React.useState('one');
+      function handleChange(e) {
+        setDisabled(true);
+        // Pretend this is in a setTimeout or something
+        thunk = () => {
+          setDisabled(false);
+          setValue(e.target.value);
+        };
+      }
+      return (
+        <>
+          <input
+            type="radio"
+            name="fruit"
+            value="one"
+            checked={value === 'one'}
+            onChange={handleChange}
+            disabled={disabled}
+          />
+          <input
+            type="radio"
+            name="fruit"
+            value="two"
+            checked={value === 'two'}
+            onChange={handleChange}
+            disabled={disabled}
+          />
+        </>
+      );
+    }
+    ReactDOM.render(<App />, container);
+    const [one, two] = container.querySelectorAll('input');
+    expect(one.checked).toBe(true);
+    expect(two.checked).toBe(false);
+    expect(isCheckedDirty(one)).toBe(true);
+    expect(isCheckedDirty(two)).toBe(true);
+    assertInputTrackingIsCurrent(container);
+
+    // Click two
+    setUntrackedChecked.call(two, true);
+    dispatchEventOnNode(two, 'click');
+    expect(one.checked).toBe(true);
+    expect(two.checked).toBe(false);
+    expect(isCheckedDirty(one)).toBe(true);
+    expect(isCheckedDirty(two)).toBe(true);
+    assertInputTrackingIsCurrent(container);
+
+    // After a delay...
+    ReactDOM.unstable_batchedUpdates(thunk);
+    expect(one.checked).toBe(false);
+    expect(two.checked).toBe(true);
+    expect(isCheckedDirty(one)).toBe(true);
+    expect(isCheckedDirty(two)).toBe(true);
+    assertInputTrackingIsCurrent(container);
+
+    // Click back to one
+    setUntrackedChecked.call(one, true);
+    dispatchEventOnNode(one, 'click');
+    expect(one.checked).toBe(false);
+    expect(two.checked).toBe(true);
+    expect(isCheckedDirty(one)).toBe(true);
+    expect(isCheckedDirty(two)).toBe(true);
+    assertInputTrackingIsCurrent(container);
+
+    // After a delay...
+    ReactDOM.unstable_batchedUpdates(thunk);
+    expect(one.checked).toBe(true);
+    expect(two.checked).toBe(false);
+    expect(isCheckedDirty(one)).toBe(true);
+    expect(isCheckedDirty(two)).toBe(true);
+    assertInputTrackingIsCurrent(container);
+  });
+
   it('should warn with value and no onChange handler and readOnly specified', () => {
     ReactDOM.render(
       <input type="text" value="zoink" readOnly={true} />,