Use defaultValue instead of setAttribute('value')

fixtures/dom/src/components/fixtures/number-inputs/index.js

@@ -27,9 +27,9 @@ function NumberInputs() {
         <NumberTestCase />
 
         <p className="footnote">
-          <b>Notes:</b> Chrome and Safari clear trailing decimals on blur. React
-          makes this concession so that the value attribute remains in sync with
-          the value property.
+          <b>Notes:</b> Modern Chrome and Safari {'<='} 6 clear trailing
+          decimals on blur. React makes this concession so that the value
+          attribute remains in sync with the value property.
         </p>
       </TestCase>
 

packages/react-dom/src/__tests__/ReactDOMInput-test.js

@@ -1249,15 +1249,20 @@ describe('ReactDOMInput', () => {
     var originalCreateElement = document.createElement;
     spyOnDevAndProd(document, 'createElement').and.callFake(function(type) {
       var el = originalCreateElement.apply(this, arguments);
+      var value = '';
+
       if (type === 'input') {
         Object.defineProperty(el, 'value', {
-          get: function() {},
-          set: function() {
-            log.push('set value');
+          get: function() {
+            return value;
+          },
+          set: function(val) {
+            value = '' + val;
+            log.push('set property value');
           },
         });
-        spyOnDevAndProd(el, 'setAttribute').and.callFake(function(name, value) {
-          log.push('set ' + name);
+        spyOnDevAndProd(el, 'setAttribute').and.callFake(function(name) {
+          log.push('set attribute ' + name);
         });
       }
       return el;
@@ -1267,14 +1272,14 @@ describe('ReactDOMInput', () => {
       <input value="0" type="range" min="0" max="100" step="1" />,
     );
     expect(log).toEqual([
-      'set type',
-      'set step',
-      'set min',
-      'set max',
-      'set value',
-      'set value',
-      'set checked',
-      'set checked',
+      'set attribute type',
+      'set attribute min',
+      'set attribute max',
+      'set attribute step',
+      'set property value',
+      'set attribute value',
+      'set attribute checked',
+      'set attribute checked',
     ]);
   });
 
@@ -1313,9 +1318,14 @@ describe('ReactDOMInput', () => {
     var originalCreateElement = document.createElement;
     spyOnDevAndProd(document, 'createElement').and.callFake(function(type) {
       var el = originalCreateElement.apply(this, arguments);
+      var value = '';
       if (type === 'input') {
         Object.defineProperty(el, 'value', {
+          get: function() {
+            return value;
+          },
           set: function(val) {
+            value = '' + val;
             log.push(`node.value = ${strify(val)}`);
           },
         });
@@ -1331,9 +1341,8 @@ describe('ReactDOMInput', () => {
     );
     expect(log).toEqual([
       'node.setAttribute("type", "date")',
+      'node.value = "1980-01-01"',
       'node.setAttribute("value", "1980-01-01")',
-      'node.value = ""',
-      'node.value = ""',
       'node.setAttribute("checked", "")',
       'node.setAttribute("checked", "")',
     ]);
@@ -1420,4 +1429,66 @@ describe('ReactDOMInput', () => {
       expect(node.getAttribute('value')).toBe('1');
     });
   });
+
+  describe('setting a controlled input to undefined', () => {
+    var input;
+
+    beforeEach(() => {
+      class Input extends React.Component {
+        state = {value: 'first'};
+        render() {
+          return (
+            <input
+              onChange={e => this.setState({value: e.target.value})}
+              value={this.state.value}
+            />
+          );
+        }
+      }
+
+      var stub = ReactTestUtils.renderIntoDocument(<Input />);
+      input = ReactDOM.findDOMNode(stub);
+      ReactTestUtils.Simulate.change(input, {target: {value: 'latest'}});
+      ReactTestUtils.Simulate.change(input, {target: {value: undefined}});
+    });
+
+    it('reverts the value attribute to the initial value', () => {
+      expect(input.getAttribute('value')).toBe('first');
+    });
+
+    it('preserves the value property', () => {
+      expect(input.value).toBe('latest');
+    });
+  });
+
+  describe('setting a controlled input to null', () => {
+    var input;
+
+    beforeEach(() => {
+      class Input extends React.Component {
+        state = {value: 'first'};
+        render() {
+          return (
+            <input
+              onChange={e => this.setState({value: e.target.value})}
+              value={this.state.value}
+            />
+          );
+        }
+      }
+
+      var stub = ReactTestUtils.renderIntoDocument(<Input />);
+      input = ReactDOM.findDOMNode(stub);
+      ReactTestUtils.Simulate.change(input, {target: {value: 'latest'}});
+      ReactTestUtils.Simulate.change(input, {target: {value: null}});
+    });
+
+    it('reverts the value attribute to the initial value', () => {
+      expect(input.getAttribute('value')).toBe('first');
+    });
+
+    it('preserves the value property', () => {
+      expect(input.value).toBe('latest');
+    });
+  });
 });

packages/react-dom/src/client/DOMPropertyOperations.js

@@ -73,8 +73,7 @@ export function getValueForProperty(node, name, expected) {
   if (__DEV__) {
     var propertyInfo = getPropertyInfo(name);
     if (propertyInfo) {
-      var mutationMethod = propertyInfo.mutationMethod;
-      if (mutationMethod || propertyInfo.mustUseProperty) {
+      if (propertyInfo.mustUseProperty) {
         return node[propertyInfo.propertyName];
       } else {
         var attributeName = propertyInfo.attributeName;
@@ -157,10 +156,7 @@ export function setValueForProperty(node, name, value) {
   var propertyInfo = getPropertyInfo(name);
 
   if (propertyInfo && shouldSetAttribute(name, value)) {
-    var mutationMethod = propertyInfo.mutationMethod;
-    if (mutationMethod) {
-      mutationMethod(node, value);
-    } else if (shouldIgnoreValue(propertyInfo, value)) {
+    if (shouldIgnoreValue(propertyInfo, value)) {
       deleteValueForProperty(node, name);
       return;
     } else if (propertyInfo.mustUseProperty) {
@@ -233,10 +229,7 @@ export function deleteValueForAttribute(node, name) {
 export function deleteValueForProperty(node, name) {
   var propertyInfo = getPropertyInfo(name);
   if (propertyInfo) {
-    var mutationMethod = propertyInfo.mutationMethod;
-    if (mutationMethod) {
-      mutationMethod(node, undefined);
-    } else if (propertyInfo.mustUseProperty) {
+    if (propertyInfo.mustUseProperty) {
       var propName = propertyInfo.propertyName;
       if (propertyInfo.hasBooleanValue) {
         node[propName] = false;

packages/react-dom/src/client/ReactDOMFiberInput.js

@@ -19,7 +19,7 @@ import * as inputValueTracking from './inputValueTracking';
 
 type InputWithWrapperState = HTMLInputElement & {
   _wrapperState: {
-    initialValue: ?string,
+    initialValue: string,
     initialChecked: ?boolean,
     controlled?: boolean,
   },
@@ -58,30 +58,14 @@ function isControlled(props) {
 
 export function getHostProps(element: Element, props: Object) {
   var node = ((element: any): InputWithWrapperState);
-  var value = props.value;
   var checked = props.checked;
 
-  var hostProps = Object.assign(
-    {
-      // Make sure we set .type before any other properties (setting .value
-      // before .type means .value is lost in IE11 and below)
-      type: undefined,
-      // Make sure we set .step before .value (setting .value before .step
-      // means .value is rounded on mount, based upon step precision)
-      step: undefined,
-      // Make sure we set .min & .max before .value (to ensure proper order
-      // in corner cases such as min or max deriving from value, e.g. Issue #7170)
-      min: undefined,
-      max: undefined,
-    },
-    props,
-    {
-      defaultChecked: undefined,
-      defaultValue: undefined,
-      value: value != null ? value : node._wrapperState.initialValue,
-      checked: checked != null ? checked : node._wrapperState.initialChecked,
-    },
-  );
+  var hostProps = Object.assign({}, props, {
+    defaultChecked: undefined,
+    defaultValue: undefined,
+    value: undefined,
+    checked: checked != null ? checked : node._wrapperState.initialChecked,
+  });
 
   return hostProps;
 }
@@ -132,7 +116,7 @@ export function initWrapperState(element: Element, props: Object) {
     }
   }
 
-  var defaultValue = props.defaultValue;
+  var defaultValue = props.defaultValue == null ? '' : props.defaultValue;
   var node = ((element: any): InputWithWrapperState);
   node._wrapperState = {
     initialChecked:
@@ -215,54 +199,34 @@ export function updateWrapper(element: Element, props: Object) {
       // browsers typically do this as necessary, jsdom doesn't.
       node.value = '' + value;
     }
-  } else {
-    if (props.value == null && props.defaultValue != null) {
-      // In Chrome, assigning defaultValue to certain input types triggers input validation.
-      // For number inputs, the display value loses trailing decimal points. For email inputs,
-      // Chrome raises "The specified value <x> is not a valid email address".
-      //
-      // Here we check to see if the defaultValue has actually changed, avoiding these problems
-      // when the user is inputting text
-      //
-      // https://github.com/facebook/react/issues/7253
-      if (node.defaultValue !== '' + props.defaultValue) {
-        node.defaultValue = '' + props.defaultValue;
-      }
-    }
-    if (props.checked == null && props.defaultChecked != null) {
-      node.defaultChecked = !!props.defaultChecked;
-    }
+    synchronizeDefaultValue(node, props.type, value);
+  } else if (
+    props.hasOwnProperty('value') ||
+    props.hasOwnProperty('defaultValue')
+  ) {
+    synchronizeDefaultValue(node, props.type, props.defaultValue);
+  }
+
+  if (props.checked == null && props.defaultChecked != null) {
+    node.defaultChecked = !!props.defaultChecked;
   }
 }
 
 export function postMountWrapper(element: Element, props: Object) {
   var node = ((element: any): InputWithWrapperState);
+  var initialValue = node._wrapperState.initialValue;
 
-  // Detach value from defaultValue. We won't do anything if we're working on
-  // submit or reset inputs as those values & defaultValues are linked. They
-  // are not resetable nodes so this operation doesn't matter and actually
-  // removes browser-default values (eg "Submit Query") when no value is
-  // provided.
+  if (props.value != null || props.defaultValue != null) {
+    // 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;
+    }
 
-  switch (props.type) {
-    case 'submit':
-    case 'reset':
-      break;
-    case 'color':
-    case 'date':
-    case 'datetime':
-    case 'datetime-local':
-    case 'month':
-    case 'time':
-    case 'week':
-      // This fixes the no-show issue on iOS Safari and Android Chrome:
-      // https://github.com/facebook/react/issues/7233
-      node.value = '';
-      node.value = node.defaultValue;
-      break;
-    default:
-      node.value = node.value;
-      break;
+    // 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;
   }
 
   // Normally, we'd just do `node.checked = node.checked` upon initial mount, less this bug
@@ -334,3 +298,29 @@ function updateNamedCousins(rootNode, props) {
     }
   }
 }
+
+// In Chrome, assigning defaultValue to certain input types triggers input validation.
+// For number inputs, the display value loses trailing decimal points. For email inputs,
+// Chrome raises "The specified value <x> is not a valid email address".
+//
+// Here we check to see if the defaultValue has actually changed, avoiding these problems
+// when the user is inputting text
+//
+// https://github.com/facebook/react/issues/7253
+function synchronizeDefaultValue(
+  node: InputWithWrapperState,
+  type: ?string,
+  value: *,
+) {
+  if (
+    // Focused number inputs synchronize on blur. See ChangeEventPlugin.js
+    (type !== 'number' || node.ownerDocument.activeElement !== node) &&
+    node.defaultValue !== '' + value
+  ) {
+    if (value != null) {
+      node.defaultValue = '' + value;
+    } else {
+      node.defaultValue = node._wrapperState.initialValue;
+    }
+  }
+}

packages/react-dom/src/shared/DOMProperty.js

@@ -54,17 +54,13 @@ var DOMPropertyInjection = {
    * DOMPropertyNames: similar to DOMAttributeNames but for DOM properties.
    * Property names not specified use the normalized name.
    *
-   * DOMMutationMethods: Properties that require special mutation methods. If
-   * `value` is undefined, the mutation method should unset the property.
-   *
    * @param {object} domPropertyConfig the config as described above.
    */
   injectDOMPropertyConfig: function(domPropertyConfig) {
     var Injection = DOMPropertyInjection;
     var Properties = domPropertyConfig.Properties || {};
     var DOMAttributeNamespaces = domPropertyConfig.DOMAttributeNamespaces || {};
     var DOMAttributeNames = domPropertyConfig.DOMAttributeNames || {};
-    var DOMMutationMethods = domPropertyConfig.DOMMutationMethods || {};
 
     for (var propName in Properties) {
       invariant(
@@ -83,7 +79,6 @@ var DOMPropertyInjection = {
         attributeName: lowerCased,
         attributeNamespace: null,
         propertyName: propName,
-        mutationMethod: null,
 
         mustUseProperty: checkMask(propConfig, Injection.MUST_USE_PROPERTY),
         hasBooleanValue: checkMask(propConfig, Injection.HAS_BOOLEAN_VALUE),
@@ -121,10 +116,6 @@ var DOMPropertyInjection = {
         propertyInfo.attributeNamespace = DOMAttributeNamespaces[propName];
       }
 
-      if (DOMMutationMethods.hasOwnProperty(propName)) {
-        propertyInfo.mutationMethod = DOMMutationMethods[propName];
-      }
-
       // Downcase references to whitelist properties to check for membership
       // without case-sensitivity. This allows the whitelist to pick up
       // `allowfullscreen`, which should be written using the property configuration
@@ -154,9 +145,6 @@ export const ROOT_ATTRIBUTE_NAME = 'data-reactroot';
  * propertyName:
  *   Used on DOM node instances. (This includes properties that mutate due to
  *   external factors.)
- * mutationMethod:
- *   If non-null, used instead of the property or `setAttribute()` after
- *   initial render.
  * mustUseProperty:
  *   Whether the property must be accessed and mutated as an object property.
  * hasBooleanValue: