Use an ordered property list for inputs, fix some Chrome number input issues

src/renderers/dom/client/wrappers/ReactDOMInput.js

@@ -29,7 +29,7 @@ var didWarnUncontrolledToControlled = false;
 function forceUpdateIfMounted() {
   if (this._rootNodeID) {
     // DOM component is still mounted; update
-    ReactDOMInput.updateWrapper(this);
+    ReactDOMInput.enforceControlledInputValue(this);
   }
 }
 
@@ -59,20 +59,7 @@ var ReactDOMInput = {
     var value = LinkedValueUtils.getValue(props);
     var checked = LinkedValueUtils.getChecked(props);
 
-    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,
+    var hostProps = Object.assign({}, props, {
       value: value != null ? value : inst._wrapperState.initialValue,
       checked: checked != null ? checked : inst._wrapperState.initialChecked,
       onChange: inst._wrapperState.onChange,
@@ -188,6 +175,14 @@ var ReactDOMInput = {
         didWarnControlledToUncontrolled = true;
       }
     }
+  },
+
+  // Ensure that there is no disconnect between an input's property
+  // value and component state. This should run during `onChange`.
+  enforceControlledInputValue: function(inst) {
+    var props = inst._currentElement.props;
+
+    ReactDOMInput.updateWrapper(inst);
 
     // TODO: Shouldn't this be getChecked(props)?
     var checked = props.checked;
@@ -199,75 +194,13 @@ var ReactDOMInput = {
       );
     }
 
-    var node = ReactDOMComponentTree.getNodeFromInstance(inst);
     var value = LinkedValueUtils.getValue(props);
     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.
-      var newValue = '' + value;
-
-      // To avoid side effects (such as losing text selection), only set value if changed
-      if (newValue !== node.value) {
-        node.value = newValue;
-      }
-    } else {
-      if (props.value == null && props.defaultValue != null) {
-        node.defaultValue = '' + props.defaultValue;
-      }
-      if (props.checked == null && props.defaultChecked != null) {
-        node.defaultChecked = !!props.defaultChecked;
-      }
-    }
-  },
-
-  postMountWrapper: function(inst) {
-    var props = inst._currentElement.props;
-
-    // This is in postMount because we need access to the DOM node, which is not
-    // available until after the component has mounted.
-    var node = ReactDOMComponentTree.getNodeFromInstance(inst);
-
-    // 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.
-
-    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;
-    }
-
-    // Normally, we'd just do `node.checked = node.checked` upon initial mount, less this bug
-    // this is needed to work around a chrome bug where setting defaultChecked
-    // will sometimes influence the value of checked (even after detachment).
-    // Reference: https://bugs.chromium.org/p/chromium/issues/detail?id=608416
-    // We need to temporarily unset name to avoid disrupting radio button groups.
-    var name = node.name;
-    if (name !== '') {
-      node.name = '';
-    }
-    node.defaultChecked = !node.defaultChecked;
-    node.defaultChecked = !node.defaultChecked;
-    if (name !== '') {
-      node.name = name;
+      DOMPropertyOperations.setValueForProperty(
+        ReactDOMComponentTree.getNodeFromInstance(inst),
+        'value',
+        value
+      );
     }
   },
 };

src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js

@@ -47,7 +47,6 @@ describe('ReactDOMInput', () => {
     stub = ReactTestUtils.renderIntoDocument(stub);
     var node = ReactDOM.findDOMNode(stub);
 
-    expect(node.getAttribute('value')).toBe('0');
     expect(node.value).toBe('0');
   });
 
@@ -825,7 +824,7 @@ describe('ReactDOMInput', () => {
   });
 
   it('sets type, step, min, max before value always', () => {
-    if (!ReactDOMFeatureFlags.useCreateElement) {
+    if (!ReactDOMFeatureFlags.us3eCreateElement) {
       return;
     }
     var log = [];
@@ -853,10 +852,8 @@ describe('ReactDOMInput', () => {
       'set step',
       'set min',
       'set max',
-      'set value',
-      'set value',
-      'set checked',
-      'set checked',
+      'set value', // attribute
+      'set value', // property
     ]);
   });
 
@@ -885,43 +882,4 @@ describe('ReactDOMInput', () => {
     input.setState({ type: 'text', value: 'Test' });
     expect(node.value).toEqual('Test');
   });
-
-  it('resets value of date/time input to fix bugs in iOS Safari', () => {
-    // https://github.com/facebook/react/issues/7233
-    if (!ReactDOMFeatureFlags.useCreateElement) {
-      return;
-    }
-
-    function strify(x) {
-      return JSON.stringify(x, null, 2);
-    }
-
-    var log = [];
-    var originalCreateElement = document.createElement;
-    spyOn(document, 'createElement').and.callFake(function(type) {
-      var el = originalCreateElement.apply(this, arguments);
-      if (type === 'input') {
-        Object.defineProperty(el, 'value', {
-          set: function(val) {
-            log.push(`node.value = ${strify(val)}`);
-          },
-        });
-        spyOn(el, 'setAttribute').and.callFake(function(name, val) {
-          log.push(`node.setAttribute(${strify(name)}, ${strify(val)})`);
-        });
-      }
-      return el;
-    });
-
-    ReactTestUtils.renderIntoDocument(<input type="date" defaultValue="1980-01-01" />);
-    expect(log).toEqual([
-      'node.setAttribute("data-reactroot", "")',
-      'node.setAttribute("type", "date")',
-      'node.setAttribute("value", "1980-01-01")',
-      'node.value = ""',
-      'node.value = ""',
-      'node.setAttribute("checked", "")',
-      'node.setAttribute("checked", "")',
-    ]);
-  });
 });

src/renderers/dom/shared/DOMProperty.js

@@ -23,6 +23,7 @@ var DOMPropertyInjection = {
    * specifies how the associated DOM property should be accessed or rendered.
    */
   MUST_USE_PROPERTY: 0x1,
+  NO_MARKUP: 0x2,
   HAS_BOOLEAN_VALUE: 0x4,
   HAS_NUMERIC_VALUE: 0x8,
   HAS_POSITIVE_NUMERIC_VALUE: 0x10 | 0x8,
@@ -70,6 +71,9 @@ var DOMPropertyInjection = {
       );
     }
 
+    // Some tags must assign properties in a specific order. Assign that here:
+    Object.assign(DOMProperty.order, domPropertyConfig.Order);
+
     for (var propName in Properties) {
       invariant(
         !DOMProperty.properties.hasOwnProperty(propName),
@@ -90,6 +94,7 @@ var DOMPropertyInjection = {
         mutationMethod: null,
 
         mustUseProperty: checkMask(propConfig, Injection.MUST_USE_PROPERTY),
+        noMarkup: checkMask(propConfig, Injection.NO_MARKUP),
         hasBooleanValue: checkMask(propConfig, Injection.HAS_BOOLEAN_VALUE),
         hasNumericValue: checkMask(propConfig, Injection.HAS_NUMERIC_VALUE),
         hasPositiveNumericValue:
@@ -175,6 +180,8 @@ var DOMProperty = {
    *   initial render.
    * mustUseProperty:
    *   Whether the property must be accessed and mutated as an object property.
+   * noMarkup:
+   *   Whether the property will generate HTML when rendered to a string.
    * hasBooleanValue:
    *   Whether the property should be removed when set to a falsey value.
    * hasNumericValue:
@@ -190,6 +197,12 @@ var DOMProperty = {
    */
   properties: {},
 
+  /**
+   * Some elements need specific attribute insertion order. This property
+   * stores that configuration.
+   */
+  order: {},
+
   /**
    * Mapping from lowercase property names to the properly cased version, used
    * to warn in the case of missing properties. Available only in __DEV__.

src/renderers/dom/shared/DOMPropertyOperations.js

@@ -91,7 +91,7 @@ var DOMPropertyOperations = {
     var propertyInfo = DOMProperty.properties.hasOwnProperty(name) ?
         DOMProperty.properties[name] : null;
     if (propertyInfo) {
-      if (shouldIgnoreValue(propertyInfo, value)) {
+      if (propertyInfo.noMarkup || shouldIgnoreValue(propertyInfo, value)) {
         return '';
       }
       var attributeName = propertyInfo.attributeName;

src/renderers/dom/shared/HTMLDOMPropertyConfig.js

@@ -14,6 +14,7 @@
 var DOMProperty = require('DOMProperty');
 
 var MUST_USE_PROPERTY = DOMProperty.injection.MUST_USE_PROPERTY;
+var NO_MARKUP = DOMProperty.injection.NO_MARKUP;
 var HAS_BOOLEAN_VALUE = DOMProperty.injection.HAS_BOOLEAN_VALUE;
 var HAS_NUMERIC_VALUE = DOMProperty.injection.HAS_NUMERIC_VALUE;
 var HAS_POSITIVE_NUMERIC_VALUE =
@@ -25,6 +26,24 @@ var HTMLDOMPropertyConfig = {
   isCustomAttribute: RegExp.prototype.test.bind(
     new RegExp('^(data|aria)-[' + DOMProperty.ATTRIBUTE_NAME_CHAR + ']*$')
   ),
+
+  Order: {
+    input: [
+      // Make sure we set .type before any other form properties (setting .value
+      // before .type means .value is lost in IE11 and below)
+      'type',
+      // Make sure we set .step before .value (setting .value before .step
+      // means .value is rounded on mount: based upon step precision)
+      'step',
+      // Fix bug in range inputs initial render
+      // https://github.com/facebook/react/issues/7170
+      'min',
+      'max',
+      'value',
+      'checked',
+    ],
+  },
+
   Properties: {
     /**
      * Standard Properties
@@ -36,8 +55,6 @@ var HTMLDOMPropertyConfig = {
     allowFullScreen: HAS_BOOLEAN_VALUE,
     allowTransparency: 0,
     alt: 0,
-    // specifies target context for links with `preload` type
-    as: 0,
     async: HAS_BOOLEAN_VALUE,
     autoComplete: 0,
     // autoFocus is polyfilled/normalized by AutoFocusUtils
@@ -49,6 +66,7 @@ var HTMLDOMPropertyConfig = {
     charSet: 0,
     challenge: 0,
     checked: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE,
+    defaultChecked: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE | NO_MARKUP,
     cite: 0,
     classID: 0,
     className: 0,
@@ -117,7 +135,6 @@ var HTMLDOMPropertyConfig = {
     optimum: 0,
     pattern: 0,
     placeholder: 0,
-    playsInline: HAS_BOOLEAN_VALUE,
     poster: 0,
     preload: 0,
     profile: 0,
@@ -155,7 +172,8 @@ var HTMLDOMPropertyConfig = {
     // Setting .type throws on non-<input> tags
     type: 0,
     useMap: 0,
-    value: 0,
+    value: MUST_USE_PROPERTY,
+    defaultValue: MUST_USE_PROPERTY | NO_MARKUP,
     width: 0,
     wmode: 0,
     wrap: 0,
@@ -211,6 +229,60 @@ var HTMLDOMPropertyConfig = {
   },
   DOMPropertyNames: {
   },
+  DOMMutationMethods: {
+    value: function(node, next) {
+      if (next == null) {
+        node.removeAttribute('value');
+      } else {
+        node.setAttribute('value', '' + next);
+      }
+
+      if (next == null) {
+        next = '';
+      } else if (next === 0) {
+        // Since we use loose type checking below, zero is
+        // falsy, so we need to manually cover it
+        next = '0';
+      }
+
+      if (node.value != next) { // eslint-disable-line eqeqeq
+        // Set value directly cause it has already been modified
+        node.value = next;
+      }
+    },
+
+    defaultValue: function(node, next) {
+      // If a value is present, intentially re-assign it to detatch it
+      // from defaultValue. Values derived from server-rendered markup
+      // will not had a prior changes to assign value as a property.
+      //
+      // Make an exception for multi-selects
+      if (!node.multiple && node.value !== '') {
+        node.value = node.value;
+      }
+
+      node.defaultValue = next;
+    },
+
+    // Chrome ~50 does not properly detatch defaultChecked, this mutation method
+    // is a work around to mitigate a bug where setting defaultChecked changes
+    // the value of checked, even after detachment:
+    // Reference: https://bugs.chromium.org/p/chromium/issues/detail?id=608416
+    defaultChecked: function(node, next) {
+      // The property priority list mandates that `checked` be assigned
+      // before `defaultChecked`. Additionally, ReactDOMInput ensures that
+      // `checked` assigns `defaultChecked` if it is not otherwise specified.
+      // This means that we can always force checked to be the original without
+      // any influence from `defaultChecked`.
+      var checked = node.checked;
+      node.defaultChecked = next;
+
+      // No need to touch the DOM again if the property is the same
+      if (checked !== next) {
+        node.checked = next;
+      }
+    },
+  },
 };
 
 module.exports = HTMLDOMPropertyConfig;