diff --git a/packages/react-dom-bindings/src/client/ReactDOMInput.js b/packages/react-dom-bindings/src/client/ReactDOMInput.js index 4411887e3cbf0..9570f25c84fee 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMInput.js +++ b/packages/react-dom-bindings/src/client/ReactDOMInput.js @@ -292,12 +292,10 @@ export function initInput( typeof checkedOrDefault !== 'symbol' && !!checkedOrDefault; - // The checked property never gets assigned. It must be manually set. - // We don't want to do this when hydrating so that existing user input isn't - // modified - // TODO: I'm pretty sure this is a bug because initialValueTracking won't be - // correct for the hydration case then. - if (!isHydrating) { + if (isHydrating) { + // Detach .checked from .defaultChecked but leave user input alone + node.checked = node.checked; + } else { node.checked = !!initialChecked; } diff --git a/packages/react-dom/src/__tests__/ReactDOMInput-test.js b/packages/react-dom/src/__tests__/ReactDOMInput-test.js index 3e984b9efc18c..6c4e55fb14407 100644 --- a/packages/react-dom/src/__tests__/ReactDOMInput-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMInput-test.js @@ -17,7 +17,11 @@ function emptyFunction() {} describe('ReactDOMInput', () => { let React; let ReactDOM; + let ReactDOMClient; let ReactDOMServer; + let Scheduler; + let act; + let assertLog; let setUntrackedValue; let setUntrackedChecked; let container; @@ -87,7 +91,11 @@ describe('ReactDOMInput', () => { React = require('react'); ReactDOM = require('react-dom'); + ReactDOMClient = require('react-dom/client'); ReactDOMServer = require('react-dom/server'); + Scheduler = require('scheduler'); + act = require('internal-test-utils').act; + assertLog = require('internal-test-utils').assertLog; container = document.createElement('div'); document.body.appendChild(container); @@ -1235,6 +1243,175 @@ describe('ReactDOMInput', () => { assertInputTrackingIsCurrent(container); }); + it('should hydrate controlled radio buttons', async () => { + function App() { + const [current, setCurrent] = React.useState('a'); + return ( + <> + { + Scheduler.log('click a'); + setCurrent('a'); + }} + /> + { + Scheduler.log('click b'); + setCurrent('b'); + }} + /> + { + Scheduler.log('click c'); + // Let's say the user can't pick C + }} + /> + + ); + } + const html = ReactDOMServer.renderToString(); + container.innerHTML = html; + const [a, b, c] = container.querySelectorAll('input'); + expect(a.checked).toBe(true); + expect(b.checked).toBe(false); + expect(c.checked).toBe(false); + expect(isCheckedDirty(a)).toBe(false); + expect(isCheckedDirty(b)).toBe(false); + expect(isCheckedDirty(c)).toBe(false); + + // Click on B before hydrating + b.checked = true; + expect(isCheckedDirty(a)).toBe(true); + expect(isCheckedDirty(b)).toBe(true); + expect(isCheckedDirty(c)).toBe(false); + + await act(async () => { + ReactDOMClient.hydrateRoot(container, ); + }); + + // Currently, we don't fire onChange when hydrating + assertLog([]); + // Strangely, we leave `b` checked even though we rendered A with + // checked={true} and B with checked={false}. Arguably this is a bug. + expect(a.checked).toBe(false); + expect(b.checked).toBe(true); + expect(c.checked).toBe(false); + expect(isCheckedDirty(a)).toBe(true); + expect(isCheckedDirty(b)).toBe(true); + expect(isCheckedDirty(c)).toBe(true); + assertInputTrackingIsCurrent(container); + + // If we click on C now though... + await act(async () => { + setUntrackedChecked.call(c, true); + dispatchEventOnNode(c, 'click'); + }); + + // then since C's onClick doesn't set state, A becomes rechecked. + assertLog(['click c']); + expect(a.checked).toBe(true); + expect(b.checked).toBe(false); + expect(c.checked).toBe(false); + expect(isCheckedDirty(a)).toBe(true); + expect(isCheckedDirty(b)).toBe(true); + expect(isCheckedDirty(c)).toBe(true); + assertInputTrackingIsCurrent(container); + + // And we can also change to B properly after hydration. + await act(async () => { + setUntrackedChecked.call(b, true); + dispatchEventOnNode(b, 'click'); + }); + assertLog(['click b']); + expect(a.checked).toBe(false); + expect(b.checked).toBe(true); + expect(c.checked).toBe(false); + expect(isCheckedDirty(a)).toBe(true); + expect(isCheckedDirty(b)).toBe(true); + expect(isCheckedDirty(c)).toBe(true); + assertInputTrackingIsCurrent(container); + }); + + it('should hydrate uncontrolled radio buttons', async () => { + function App() { + return ( + <> + Scheduler.log('click a')} + /> + Scheduler.log('click b')} + /> + Scheduler.log('click c')} + /> + + ); + } + const html = ReactDOMServer.renderToString(); + container.innerHTML = html; + const [a, b, c] = container.querySelectorAll('input'); + expect(a.checked).toBe(true); + expect(b.checked).toBe(false); + expect(c.checked).toBe(false); + expect(isCheckedDirty(a)).toBe(false); + expect(isCheckedDirty(b)).toBe(false); + expect(isCheckedDirty(c)).toBe(false); + + // Click on B before hydrating + b.checked = true; + expect(isCheckedDirty(a)).toBe(true); + expect(isCheckedDirty(b)).toBe(true); + expect(isCheckedDirty(c)).toBe(false); + + await act(async () => { + ReactDOMClient.hydrateRoot(container, ); + }); + + // Currently, we don't fire onChange when hydrating + assertLog([]); + expect(a.checked).toBe(false); + expect(b.checked).toBe(true); + expect(c.checked).toBe(false); + expect(isCheckedDirty(a)).toBe(true); + expect(isCheckedDirty(b)).toBe(true); + expect(isCheckedDirty(c)).toBe(true); + assertInputTrackingIsCurrent(container); + + // Click back to A + await act(async () => { + setUntrackedChecked.call(a, true); + dispatchEventOnNode(a, 'click'); + }); + + assertLog(['click a']); + expect(a.checked).toBe(true); + expect(b.checked).toBe(false); + expect(c.checked).toBe(false); + expect(isCheckedDirty(a)).toBe(true); + expect(isCheckedDirty(b)).toBe(true); + expect(isCheckedDirty(c)).toBe(true); + assertInputTrackingIsCurrent(container); + }); + it('should check the correct radio when the selected name moves', () => { class App extends React.Component { state = {