diff --git a/fixtures/attribute-behavior/AttributeTableSnapshot.md b/fixtures/attribute-behavior/AttributeTableSnapshot.md index a54c608c6e23e..8a36ac9e87eb4 100644 --- a/fixtures/attribute-behavior/AttributeTableSnapshot.md +++ b/fixtures/attribute-behavior/AttributeTableSnapshot.md @@ -2593,8 +2593,8 @@ | `defaultValue=(string 'false')`| (changed)| `"false"` | | `defaultValue=(string 'on')`| (changed)| `"on"` | | `defaultValue=(string 'off')`| (changed)| `"off"` | -| `defaultValue=(symbol)`| (changed, error, warning, ssr error)| `` | -| `defaultValue=(function)`| (changed, ssr warning)| `"function f() {}"` | +| `defaultValue=(symbol)`| (initial, ssr error, ssr mismatch)| `` | +| `defaultValue=(function)`| (initial, ssr mismatch)| `` | | `defaultValue=(null)`| (initial, ssr warning)| `` | | `defaultValue=(undefined)`| (initial)| `` | @@ -11768,8 +11768,8 @@ | `value=(string 'false')`| (changed)| `"false"` | | `value=(string 'on')`| (changed)| `"on"` | | `value=(string 'off')`| (changed)| `"off"` | -| `value=(symbol)`| (changed, error, warning, ssr error)| `` | -| `value=(function)`| (changed, warning, ssr warning)| `"function f() {}"` | +| `value=(symbol)`| (initial, warning, ssr error, ssr mismatch)| `` | +| `value=(function)`| (initial, warning, ssr mismatch)| `` | | `value=(null)`| (initial, warning, ssr warning)| `` | | `value=(undefined)`| (initial)| `` | @@ -11793,8 +11793,8 @@ | `value=(string 'false')`| (changed)| `"false"` | | `value=(string 'on')`| (changed)| `"on"` | | `value=(string 'off')`| (changed)| `"off"` | -| `value=(symbol)`| (changed, error, warning, ssr error)| `` | -| `value=(function)`| (changed, warning)| `"function f() {}"` | +| `value=(symbol)`| (initial, warning, ssr error, ssr mismatch)| `` | +| `value=(function)`| (initial, warning, ssr mismatch)| `` | | `value=(null)`| (initial, warning, ssr warning)| `` | | `value=(undefined)`| (initial)| `` | @@ -11818,7 +11818,7 @@ | `value=(string 'false')`| (initial)| `` | | `value=(string 'on')`| (initial)| `` | | `value=(string 'off')`| (initial)| `` | -| `value=(symbol)`| (changed, error, warning, ssr error)| `` | +| `value=(symbol)`| (initial, warning, ssr error, ssr mismatch)| `` | | `value=(function)`| (initial, warning)| `` | | `value=(null)`| (initial, warning, ssr warning)| `` | | `value=(undefined)`| (initial)| `` | diff --git a/packages/react-dom/src/__tests__/ReactDOMInput-test.js b/packages/react-dom/src/__tests__/ReactDOMInput-test.js index 4e8948d49b5a2..c89802c202917 100644 --- a/packages/react-dom/src/__tests__/ReactDOMInput-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMInput-test.js @@ -248,6 +248,23 @@ describe('ReactDOMInput', () => { } }); + it('performs a state change from "" to 0', () => { + class Stub extends React.Component { + state = { + value: '', + }; + render() { + return ; + } + } + + var stub = ReactTestUtils.renderIntoDocument(); + var node = ReactDOM.findDOMNode(stub); + stub.setState({value: 0}); + + expect(node.value).toEqual('0'); + }); + it('distinguishes precision for extra zeroes in string number values', () => { spyOnDev(console, 'error'); class Stub extends React.Component { @@ -595,6 +612,7 @@ describe('ReactDOMInput', () => { var node = container.firstChild; expect(node.value).toBe('0'); + expect(node.defaultValue).toBe('0'); }); it('should properly transition from 0 to an empty value', function() { @@ -606,6 +624,43 @@ describe('ReactDOMInput', () => { var node = container.firstChild; expect(node.value).toBe(''); + expect(node.defaultValue).toBe(''); + }); + + it('should properly transition a text input from 0 to an empty 0.0', function() { + var container = document.createElement('div'); + + ReactDOM.render(, container); + ReactDOM.render(, container); + + var node = container.firstChild; + + expect(node.value).toBe('0.0'); + expect(node.defaultValue).toBe('0.0'); + }); + + it('should properly transition a number input from "" to 0', function() { + var container = document.createElement('div'); + + ReactDOM.render(, container); + ReactDOM.render(, container); + + var node = container.firstChild; + + expect(node.value).toBe('0'); + expect(node.defaultValue).toBe('0'); + }); + + it('should properly transition a number input from "" to "0"', function() { + var container = document.createElement('div'); + + ReactDOM.render(, container); + ReactDOM.render(, container); + + var node = container.firstChild; + + expect(node.value).toBe('0'); + expect(node.defaultValue).toBe('0'); }); it('should have the correct target value', () => { @@ -1585,4 +1640,132 @@ describe('ReactDOMInput', () => { } }); }); + + describe('When given a Symbol value', function() { + it('treats initial Symbol value as an empty string', function() { + spyOnDev(console, 'error'); + var container = document.createElement('div'); + ReactDOM.render( + {}} />, + container, + ); + var node = container.firstChild; + + expect(node.value).toBe(''); + expect(node.getAttribute('value')).toBe(''); + + if (__DEV__) { + expect(console.error.calls.count()).toBe(1); + expect(console.error.calls.argsFor(0)[0]).toContain( + 'Invalid value for prop `value`', + ); + } + }); + + it('treats updated Symbol value as an empty string', function() { + spyOnDev(console, 'error'); + var container = document.createElement('div'); + ReactDOM.render( {}} />, container); + ReactDOM.render( + {}} />, + container, + ); + var node = container.firstChild; + + expect(node.value).toBe(''); + expect(node.getAttribute('value')).toBe(''); + + if (__DEV__) { + expect(console.error.calls.count()).toBe(1); + expect(console.error.calls.argsFor(0)[0]).toContain( + 'Invalid value for prop `value`', + ); + } + }); + + it('treats initial Symbol defaultValue as an empty string', function() { + var container = document.createElement('div'); + ReactDOM.render(, container); + var node = container.firstChild; + + expect(node.value).toBe(''); + expect(node.getAttribute('value')).toBe(''); + // TODO: we should warn here. + }); + + it('treats updated Symbol defaultValue as an empty string', function() { + var container = document.createElement('div'); + ReactDOM.render(, container); + ReactDOM.render(, container); + var node = container.firstChild; + + expect(node.value).toBe('foo'); + expect(node.getAttribute('value')).toBe(''); + // TODO: we should warn here. + }); + }); + + describe('When given a function value', function() { + it('treats initial function value as an empty string', function() { + spyOnDev(console, 'error'); + var container = document.createElement('div'); + ReactDOM.render( + {}} onChange={() => {}} />, + container, + ); + var node = container.firstChild; + + expect(node.value).toBe(''); + expect(node.getAttribute('value')).toBe(''); + + if (__DEV__) { + expect(console.error.calls.count()).toBe(1); + expect(console.error.calls.argsFor(0)[0]).toContain( + 'Invalid value for prop `value`', + ); + } + }); + + it('treats updated function value as an empty string', function() { + spyOnDev(console, 'error'); + var container = document.createElement('div'); + ReactDOM.render( {}} />, container); + ReactDOM.render( + {}} onChange={() => {}} />, + container, + ); + var node = container.firstChild; + + expect(node.value).toBe(''); + expect(node.getAttribute('value')).toBe(''); + + if (__DEV__) { + expect(console.error.calls.count()).toBe(1); + expect(console.error.calls.argsFor(0)[0]).toContain( + 'Invalid value for prop `value`', + ); + } + }); + + it('treats initial function defaultValue as an empty string', function() { + var container = document.createElement('div'); + ReactDOM.render( {}} />, container); + var node = container.firstChild; + + expect(node.value).toBe(''); + expect(node.getAttribute('value')).toBe(''); + // TODO: we should warn here. + }); + + it('treats updated function defaultValue as an empty string', function() { + var container = document.createElement('div'); + ReactDOM.render(, container); + ReactDOM.render( {}} />, container); + var node = container.firstChild; + + expect(node.value).toBe('foo'); + expect(node.getAttribute('value')).toBe(''); + // TODO: we should warn here. + }); + }); }); diff --git a/packages/react-dom/src/client/ReactDOMFiberInput.js b/packages/react-dom/src/client/ReactDOMFiberInput.js index 22cf89c400b04..b82a4dcb0e7ac 100644 --- a/packages/react-dom/src/client/ReactDOMFiberInput.js +++ b/packages/react-dom/src/client/ReactDOMFiberInput.js @@ -116,12 +116,15 @@ export function initWrapperState(element: Element, props: Object) { } } - var defaultValue = props.defaultValue == null ? '' : props.defaultValue; var node = ((element: any): InputWithWrapperState); + var defaultValue = props.defaultValue == null ? '' : props.defaultValue; + node._wrapperState = { initialChecked: props.checked != null ? props.checked : props.defaultChecked, - initialValue: props.value != null ? props.value : defaultValue, + initialValue: getSafeValue( + props.value != null ? props.value : defaultValue, + ), controlled: isControlled(props), }; } @@ -175,36 +178,26 @@ export function updateWrapper(element: Element, props: Object) { updateChecked(element, props); - var value = props.value; - if (value != null) { - if (value === 0 && node.value === '') { - node.value = '0'; - // Note: IE9 reports a number inputs as 'text', so check props instead. - } else if (props.type === 'number') { - // Simulate `input.valueAsNumber`. IE9 does not support it - var valueAsNumber = parseFloat(node.value) || 0; + var value = getSafeValue(props.value); + if (value != null) { + if (props.type === 'number') { if ( + (value === 0 && node.value === '') || // eslint-disable-next-line - value != valueAsNumber || - // eslint-disable-next-line - (value == valueAsNumber && node.value != value) + node.value != value ) { - // Cast `value` to a string to ensure the value is set correctly. While - // browsers typically do this as necessary, jsdom doesn't. node.value = '' + value; } } else if (node.value !== '' + value) { - // Cast `value` to a string to ensure the value is set correctly. While - // browsers typically do this as necessary, jsdom doesn't. node.value = '' + value; } - synchronizeDefaultValue(node, props.type, value); - } else if ( - props.hasOwnProperty('value') || - props.hasOwnProperty('defaultValue') - ) { - synchronizeDefaultValue(node, props.type, props.defaultValue); + } + + if (props.hasOwnProperty('value')) { + setDefaultValue(node, props.type, value); + } else if (props.hasOwnProperty('defaultValue')) { + setDefaultValue(node, props.type, getSafeValue(props.defaultValue)); } if (props.checked == null && props.defaultChecked != null) { @@ -214,19 +207,18 @@ export function updateWrapper(element: Element, props: Object) { export function postMountWrapper(element: Element, props: Object) { var node = ((element: any): InputWithWrapperState); - var initialValue = node._wrapperState.initialValue; - if (props.value != null || props.defaultValue != null) { + if (props.hasOwnProperty('value') || props.hasOwnProperty('defaultValue')) { // Do not assign value if it is already set. This prevents user text input // from being lost during SSR hydration. if (node.value === '') { - node.value = initialValue; + node.value = '' + node._wrapperState.initialValue; } // value must be assigned before defaultValue. This fixes an issue where the // visually displayed value of date inputs disappears on mobile Safari and Chrome: // https://github.com/facebook/react/issues/7233 - node.defaultValue = initialValue; + node.defaultValue = '' + node._wrapperState.initialValue; } // Normally, we'd just do `node.checked = node.checked` upon initial mount, less this bug @@ -307,20 +299,34 @@ function updateNamedCousins(rootNode, props) { // when the user is inputting text // // https://github.com/facebook/react/issues/7253 -export function synchronizeDefaultValue( +export function setDefaultValue( node: InputWithWrapperState, type: ?string, value: *, ) { if ( // Focused number inputs synchronize on blur. See ChangeEventPlugin.js - (type !== 'number' || node.ownerDocument.activeElement !== node) && - node.defaultValue !== '' + value + type !== 'number' || + node.ownerDocument.activeElement !== node ) { - if (value != null) { + if (value == null) { + node.defaultValue = '' + node._wrapperState.initialValue; + } else if (node.defaultValue !== '' + value) { node.defaultValue = '' + value; - } else { - node.defaultValue = node._wrapperState.initialValue; } } } + +function getSafeValue(value: *): * { + switch (typeof value) { + case 'boolean': + case 'number': + case 'object': + case 'string': + case 'undefined': + return value; + default: + // function, symbol are assigned as empty strings + return ''; + } +} diff --git a/packages/react-dom/src/events/ChangeEventPlugin.js b/packages/react-dom/src/events/ChangeEventPlugin.js index cb98d19a9b9fa..9d759907b5ef8 100644 --- a/packages/react-dom/src/events/ChangeEventPlugin.js +++ b/packages/react-dom/src/events/ChangeEventPlugin.js @@ -17,7 +17,7 @@ import getEventTarget from './getEventTarget'; import isEventSupported from './isEventSupported'; import {getNodeFromInstance} from '../client/ReactDOMComponentTree'; import * as inputValueTracking from '../client/inputValueTracking'; -import {synchronizeDefaultValue} from '../client/ReactDOMFiberInput'; +import {setDefaultValue} from '../client/ReactDOMFiberInput'; var eventTypes = { change: { @@ -236,7 +236,7 @@ function handleControlledInputBlur(inst, node) { } // If controlled, assign the value attribute to the current value on blur - synchronizeDefaultValue(node, 'number', node.value); + setDefaultValue(node, 'number', node.value); } /** diff --git a/packages/react-dom/src/shared/DOMProperty.js b/packages/react-dom/src/shared/DOMProperty.js index 4a11eaf34c548..a6e7a09778ab9 100644 --- a/packages/react-dom/src/shared/DOMProperty.js +++ b/packages/react-dom/src/shared/DOMProperty.js @@ -12,6 +12,9 @@ import warning from 'fbjs/lib/warning'; var RESERVED_PROPS = { children: true, dangerouslySetInnerHTML: true, + // TODO: This prevents the assignment of defaultValue to regular + // elements (not just inputs). Now that ReactDOMInput assigns to the + // defaultValue property -- do we need this? defaultValue: true, defaultChecked: true, innerHTML: true,