diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 693aeffd7c4fe..098285224c171 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -1297,32 +1297,36 @@ export function updateProperties( let type = null; let value = null; let defaultValue = null; + let lastDefaultValue = null; let checked = null; let defaultChecked = null; for (const propKey in lastProps) { const lastProp = lastProps[propKey]; - if ( - lastProps.hasOwnProperty(propKey) && - lastProp != null && - !nextProps.hasOwnProperty(propKey) - ) { + if (lastProps.hasOwnProperty(propKey) && lastProp != null) { switch (propKey) { case 'checked': { - const checkedValue = nextProps.defaultChecked; - const inputElement: HTMLInputElement = (domElement: any); - inputElement.checked = - !!checkedValue && - typeof checkedValue !== 'function' && - checkedValue !== 'symbol'; + if (!nextProps.hasOwnProperty(propKey)) { + const checkedValue = nextProps.defaultChecked; + const inputElement: HTMLInputElement = (domElement: any); + inputElement.checked = + !!checkedValue && + typeof checkedValue !== 'function' && + checkedValue !== 'symbol'; + } break; } case 'value': { // This is handled by updateWrapper below. break; } + case 'defaultValue': { + lastDefaultValue = lastProp; + } // defaultChecked and defaultValue are ignored by setProp + // Fallthrough default: { - setProp(domElement, tag, propKey, null, nextProps, lastProp); + if (!nextProps.hasOwnProperty(propKey)) + setProp(domElement, tag, propKey, null, nextProps, lastProp); } } } @@ -1473,6 +1477,7 @@ export function updateProperties( domElement, value, defaultValue, + lastDefaultValue, checked, defaultChecked, type, @@ -1809,6 +1814,7 @@ export function updatePropertiesWithDiff( const type = nextProps.type; const value = nextProps.value; const defaultValue = nextProps.defaultValue; + const lastDefaultValue = lastProps.defaultValue; const checked = nextProps.checked; const defaultChecked = nextProps.defaultChecked; for (let i = 0; i < updatePayload.length; i += 2) { @@ -1934,6 +1940,7 @@ export function updatePropertiesWithDiff( domElement, value, defaultValue, + lastDefaultValue, checked, defaultChecked, type, diff --git a/packages/react-dom-bindings/src/client/ReactDOMInput.js b/packages/react-dom-bindings/src/client/ReactDOMInput.js index 9bacf8e046314..4e8f9b32aa7e3 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMInput.js +++ b/packages/react-dom-bindings/src/client/ReactDOMInput.js @@ -85,19 +85,41 @@ export function updateInput( element: Element, value: ?string, defaultValue: ?string, + lastDefaultValue: ?string, checked: ?boolean, defaultChecked: ?boolean, type: ?string, ) { const node: HTMLInputElement = (element: any); + if (value != null) { + if (type === 'number') { + if ( + // $FlowFixMe[incompatible-type] + (value === 0 && node.value === '') || + // We explicitly want to coerce to number here if possible. + // eslint-disable-next-line + node.value != (value: any) + ) { + node.value = toString(getToStringValue(value)); + } + } else if (node.value !== toString(getToStringValue(value))) { + node.value = toString(getToStringValue(value)); + } + } else if (type === 'submit' || type === 'reset') { + // Submit/reset inputs need the attribute removed completely to avoid + // blank-text buttons. + node.removeAttribute('value'); + return; + } + if (disableInputAttributeSyncing) { // When not syncing the value attribute, React only assigns a new value // whenever the defaultValue React prop has changed. When not present, // React does nothing if (defaultValue != null) { setDefaultValue(node, type, getToStringValue(defaultValue)); - } else { + } else if (lastDefaultValue != null) { node.removeAttribute('value'); } } else { @@ -110,7 +132,7 @@ export function updateInput( setDefaultValue(node, type, getToStringValue(value)); } else if (defaultValue != null) { setDefaultValue(node, type, getToStringValue(defaultValue)); - } else { + } else if (lastDefaultValue != null) { node.removeAttribute('value'); } } @@ -135,27 +157,6 @@ export function updateInput( if (checked != null && node.checked !== !!checked) { node.checked = checked; } - - if (value != null) { - if (type === 'number') { - if ( - // $FlowFixMe[incompatible-type] - (value === 0 && node.value === '') || - // We explicitly want to coerce to number here if possible. - // eslint-disable-next-line - node.value != (value: any) - ) { - node.value = toString(getToStringValue(value)); - } - } else if (node.value !== toString(getToStringValue(value))) { - node.value = toString(getToStringValue(value)); - } - } else if (type === 'submit' || type === 'reset') { - // Submit/reset inputs need the attribute removed completely to avoid - // blank-text buttons. - node.removeAttribute('value'); - return; - } } export function initInput( @@ -286,6 +287,7 @@ export function restoreControlledInputState(element: Element, props: Object) { rootNode, props.value, props.defaultValue, + props.defaultValue, props.checked, props.defaultChecked, props.type, @@ -341,6 +343,7 @@ export function restoreControlledInputState(element: Element, props: Object) { otherNode, otherProps.value, otherProps.defaultValue, + otherProps.defaultValue, otherProps.checked, otherProps.defaultChecked, otherProps.type, diff --git a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js index 67d6f7bd465ab..bf523c4494e81 100644 --- a/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js +++ b/packages/react-dom/src/__tests__/DOMPropertyOperations-test.js @@ -1166,7 +1166,11 @@ describe('DOMPropertyOperations', () => { ).toErrorDev( 'A component is changing a controlled input to be uncontrolled', ); - expect(container.firstChild.hasAttribute('value')).toBe(false); + if (disableInputAttributeSyncing) { + expect(container.firstChild.hasAttribute('value')).toBe(false); + } else { + expect(container.firstChild.getAttribute('value')).toBe('foo'); + } expect(container.firstChild.value).toBe('foo'); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMInput-test.js b/packages/react-dom/src/__tests__/ReactDOMInput-test.js index 47fca9522fac8..39fabad88b501 100644 --- a/packages/react-dom/src/__tests__/ReactDOMInput-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMInput-test.js @@ -1952,7 +1952,11 @@ describe('ReactDOMInput', () => { expect(renderInputWithStringThenWithUndefined).toErrorDev( 'A component is changing a controlled input to be uncontrolled.', ); - expect(input.getAttribute('value')).toBe(null); + if (disableInputAttributeSyncing) { + expect(input.getAttribute('value')).toBe(null); + } else { + expect(input.getAttribute('value')).toBe('latest'); + } }); it('preserves the value property', () => { @@ -1998,7 +2002,11 @@ describe('ReactDOMInput', () => { 'or `undefined` for uncontrolled components.', 'A component is changing a controlled input to be uncontrolled.', ]); - expect(input.hasAttribute('value')).toBe(false); + if (disableInputAttributeSyncing) { + expect(input.getAttribute('value')).toBe(null); + } else { + expect(input.getAttribute('value')).toBe('latest'); + } }); it('preserves the value property', () => { @@ -2183,4 +2191,26 @@ describe('ReactDOMInput', () => { ReactDOM.render(, container); expect(node.defaultValue).toBe(''); }); + + it('should notice input changes when reverting back to original value', () => { + const log = []; + function onChange(e) { + log.push(e.target.value); + } + ReactDOM.render( + , + container, + ); + ReactDOM.render( + , + container, + ); + + const node = container.firstChild; + setUntrackedValue.call(node, ''); + dispatchEventOnNode(node, 'input'); + + expect(log).toEqual(['']); + expect(node.value).toBe('a'); + }); });