Ensure value and defaultValue do not assign functions and symbols (#1…

…1741)

* Ensure value and defaultValue do not assign functions and symbols

* Eliminate assignProperty method from ReactDOMInput

* Restore original placement of defaultValue reservedProp

* Reduce branching. Make assignment more consistent

* Control for warnings in symbol/function tests

* Add boolean to readOnly assignments

* Tweak the tests

* Invalid value attributes should convert to an empty string

* Revert ChangeEventPlugin update. See #11746

* Format

* Replace shouldSetAttribute call with value specific type check

DOMProperty.shouldSetAttribute runs a few other checks that aren't
appropriate for determining if a value or defaultValue should be
assigned on an input. This commit replaces that call with an input
specific check.

* Remove unused import

* Eliminate unnecessary numeric equality checks (#11751)

* Eliminate unnecessary numeric equality checks

This commit changes the way numeric equality for number inputs works
such that it compares against `input.valueAsNumber`. This eliminates
quite a bit of branching around numeric equality.

* There is no need to compare valueAsNumber

* Add test cases for empty string to 0.

* Avoid implicit boolean JSX props

* Split up numeric equality test to isolate eslint disable command

* Fix typo in ReactDOMInput test

* Add todos

* Update the attribute table

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)| `<empty string>` |
+| `defaultValue=(function)`| (initial, ssr mismatch)| `<empty string>` |
 | `defaultValue=(null)`| (initial, ssr warning)| `<empty string>` |
 | `defaultValue=(undefined)`| (initial)| `<empty string>` |
 
@@ -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)| `<empty string>` |
+| `value=(function)`| (initial, warning, ssr mismatch)| `<empty string>` |
 | `value=(null)`| (initial, warning, ssr warning)| `<empty string>` |
 | `value=(undefined)`| (initial)| `<empty string>` |
 
@@ -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)| `<empty string>` |
+| `value=(function)`| (initial, warning, ssr mismatch)| `<empty string>` |
 | `value=(null)`| (initial, warning, ssr warning)| `<empty string>` |
 | `value=(undefined)`| (initial)| `<empty string>` |
 
@@ -11818,7 +11818,7 @@
 | `value=(string 'false')`| (initial)| `<empty string>` |
 | `value=(string 'on')`| (initial)| `<empty string>` |
 | `value=(string 'off')`| (initial)| `<empty string>` |
-| `value=(symbol)`| (changed, error, warning, ssr error)| `` |
+| `value=(symbol)`| (initial, warning, ssr error, ssr mismatch)| `<empty string>` |
 | `value=(function)`| (initial, warning)| `<empty string>` |
 | `value=(null)`| (initial, warning, ssr warning)| `<empty string>` |
 | `value=(undefined)`| (initial)| `<empty string>` |

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 <input type="number" value={this.state.value} readOnly={true} />;
+      }
+    }
+
+    var stub = ReactTestUtils.renderIntoDocument(<Stub />);
+    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(<input type="text" value={0} />, container);
+    ReactDOM.render(<input type="text" value="0.0" />, 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(<input type="number" value="" />, container);
+    ReactDOM.render(<input type="number" value={0} />, 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(<input type="number" value="" />, container);
+    ReactDOM.render(<input type="number" value="0" />, 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(
+        <input value={Symbol('foobar')} 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 Symbol value as an empty string', function() {
+      spyOnDev(console, 'error');
+      var container = document.createElement('div');
+      ReactDOM.render(<input value="foo" onChange={() => {}} />, container);
+      ReactDOM.render(
+        <input value={Symbol('foobar')} 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 Symbol defaultValue as an empty string', function() {
+      var container = document.createElement('div');
+      ReactDOM.render(<input defaultValue={Symbol('foobar')} />, 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(<input defaultValue="foo" />, container);
+      ReactDOM.render(<input defaultValue={Symbol('foobar')} />, 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(
+        <input value={() => {}} 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(<input value="foo" onChange={() => {}} />, container);
+      ReactDOM.render(
+        <input value={() => {}} 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(<input defaultValue={() => {}} />, 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(<input defaultValue="foo" />, container);
+      ReactDOM.render(<input defaultValue={() => {}} />, container);
+      var node = container.firstChild;
+
+      expect(node.value).toBe('foo');
+      expect(node.getAttribute('value')).toBe('');
+      // TODO: we should warn here.
+    });
+  });
 });

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 '';
+  }
+}

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);
 }
 
 /**