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-bindings/src/client/ReactDOMTextarea.js b/packages/react-dom-bindings/src/client/ReactDOMTextarea.js index 544cee9952be9..c9862c9b00bef 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMTextarea.js +++ b/packages/react-dom-bindings/src/client/ReactDOMTextarea.js @@ -61,12 +61,6 @@ export function validateTextareaProps(element: Element, props: Object) { export function updateTextarea(element: Element, props: Object) { const node: HTMLTextAreaElement = (element: any); const value = getToStringValue(props.value); - const defaultValue = getToStringValue(props.defaultValue); - if (defaultValue != null) { - node.defaultValue = toString(defaultValue); - } else { - node.defaultValue = ''; - } if (value != null) { // Cast `value` to a string to ensure the value is set correctly. While // browsers typically do this as necessary, jsdom doesn't. @@ -76,10 +70,19 @@ export function updateTextarea(element: Element, props: Object) { node.value = newValue; } // TOOO: This should respect disableInputAttributeSyncing flag. - if (props.defaultValue == null && node.defaultValue !== newValue) { - node.defaultValue = newValue; + if (props.defaultValue == null) { + if (node.defaultValue !== newValue) { + node.defaultValue = newValue; + } + return; } } + const defaultValue = getToStringValue(props.defaultValue); + if (defaultValue != null) { + node.defaultValue = toString(defaultValue); + } else { + node.defaultValue = ''; + } } export function initTextarea(element: Element, props: Object) { 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', () => { diff --git a/packages/react-dom/src/__tests__/ReactDOMTextarea-test.js b/packages/react-dom/src/__tests__/ReactDOMTextarea-test.js index 396eef8d6d34e..347942234063d 100644 --- a/packages/react-dom/src/__tests__/ReactDOMTextarea-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMTextarea-test.js @@ -603,6 +603,7 @@ describe('ReactDOMTextarea', () => { ref={n => (node = n)} value="foo" onChange={emptyFunction} + data-count={this.state.count} /> );