diff --git a/packages/react-dom-bindings/src/client/ReactDOMInput.js b/packages/react-dom-bindings/src/client/ReactDOMInput.js index fcede97885728..0708b984f7c95 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMInput.js +++ b/packages/react-dom-bindings/src/client/ReactDOMInput.js @@ -84,7 +84,7 @@ export function validateInputProps(element: Element, props: Object) { export function updateInputChecked(element: Element, props: Object) { const node: HTMLInputElement = (element: any); const checked = props.checked; - if (checked != null) { + if (checked != null && node.checked !== !!checked) { node.checked = checked; } } diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js index b8c91fb86e18b..c2dd984823ed1 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js @@ -1043,6 +1043,63 @@ describe('ReactDOMComponent', () => { expect(nodeValueSetter).toHaveBeenCalledTimes(2); }); + it('should not incur unnecessary DOM mutations for controlled string properties', () => { + function onChange() {} + const container = document.createElement('div'); + ReactDOM.render(, container); + + const node = container.firstChild; + + let nodeValue = ''; + const nodeValueSetter = jest.fn(); + Object.defineProperty(node, 'value', { + get: function () { + return nodeValue; + }, + set: nodeValueSetter.mockImplementation(function (newValue) { + nodeValue = newValue; + }), + }); + + ReactDOM.render(, container); + expect(nodeValueSetter).toHaveBeenCalledTimes(1); + + ReactDOM.render( + , + container, + ); + expect(nodeValueSetter).toHaveBeenCalledTimes(1); + + expect(() => { + ReactDOM.render(, 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.', + ); + expect(nodeValueSetter).toHaveBeenCalledTimes(1); + + expect(() => { + ReactDOM.render(, container); + }).toErrorDev( + 'value` prop on `input` should not be null. Consider using an empty string to clear the ' + + 'component or `undefined` for uncontrolled components.', + ); + expect(nodeValueSetter).toHaveBeenCalledTimes(1); + + expect(() => { + ReactDOM.render(, 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.', + ); + expect(nodeValueSetter).toHaveBeenCalledTimes(2); + + ReactDOM.render(, container); + expect(nodeValueSetter).toHaveBeenCalledTimes(2); + }); + it('should not incur unnecessary DOM mutations for boolean properties', () => { const container = document.createElement('div'); function onChange() { @@ -1066,7 +1123,12 @@ describe('ReactDOMComponent', () => { }); ReactDOM.render( - , + , container, ); expect(nodeValueSetter).toHaveBeenCalledTimes(0); @@ -1094,15 +1156,13 @@ describe('ReactDOMComponent', () => { 'using a controlled or uncontrolled input element for the lifetime of the component.', ); - // TODO: Non-null values are updated twice on inputs. This is should ideally be fixed. - expect(nodeValueSetter).toHaveBeenCalledTimes(3); + expect(nodeValueSetter).toHaveBeenCalledTimes(2); ReactDOM.render( , container, ); - // TODO: Non-null values are updated twice on inputs. This is should ideally be fixed. - expect(nodeValueSetter).toHaveBeenCalledTimes(5); + expect(nodeValueSetter).toHaveBeenCalledTimes(3); }); it('should ignore attribute list for elements with the "is" attribute', () => {