Controlled inputs do not synchronize value or checked attribute

This commit is a follow up to prior work to resolve issues with number
inputs in React. Inputs keep their value/checked attribute in sync with the
value/checked property. This is a React behavior. Traditionally
browser DOM manipulation does not rely on keeping the value/checked
attribute in sync.

It's also very problematic for number inputs. After discussion, it was
decided to make a breaking change to no longer sync up the
value/checked attribute with it's assoicated property.

For this to work, I made the following changes:

- The value, defaultValue, checked and defaultChecked properties are
  now maintained within the HTML property config.
- This required adding a filter to strip out the value property on
  selects and textareas
- The logic to defer assignment of the value attribute has been
  removed form ChangeEventPlugin
- defaultValue and defaultChecked are aliased to `value` and `checked`
  so that uncontrolled input attribute assignment works as intended.

Author: nhunzaker

scripts/fiber/tests-passing.txt

@@ -717,8 +717,6 @@ src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js
 * should set className to empty string instead of null
 * should remove property properly for boolean properties
 * should remove property properly even with different name
-* should update an empty attribute to zero
-* should always assign the value attribute for non-inputs
 * should remove attributes for normal properties
 * should not remove attributes for special properties
 * should not leave all options selected when deleting multiple
@@ -1805,7 +1803,7 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
 * should control values in reentrant events with different targets
 * does change the number 2 to "2.0" with no change handler
 * does change the string "2" to "2.0" with no change handler
-* changes the number 2 to "2.0" using a change handler
+* changes the value property from number 2 to "2.0" using a change handler
 * does change the string ".98" to "0.98" with no change handler
 * distinguishes precision for extra zeroes in string number values
 * should display `defaultValue` of number 0
@@ -1867,11 +1865,7 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
 * sets value properly with type coming later in props
 * does not raise a validation warning when it switches types
 * resets value of date/time input to fix bugs in iOS Safari
-* always sets the attribute when values change on text inputs
-* does not set the value attribute on number inputs if focused
-* sets the value attribute on number inputs on blur
-* an uncontrolled number input will not update the value attribute on blur
-* an uncontrolled text input will not update the value attribute on blur
+* does not set the attribute when values change on text inputs
 
 src/renderers/dom/shared/wrappers/__tests__/ReactDOMOption-test.js
 * should flatten children to a string

scripts/rollup/results.json

@@ -25,36 +25,36 @@
       "gzip": 7214
     },
     "react-dom.development.js (UMD_DEV)": {
-      "size": 613141,
-      "gzip": 140395
+      "size": 621770,
+      "gzip": 142030
     },
     "react-dom.production.min.js (UMD_PROD)": {
-      "size": 126584,
-      "gzip": 39853
+      "size": 125762,
+      "gzip": 39791
     },
     "react-dom.development.js (NODE_DEV)": {
-      "size": 570841,
-      "gzip": 130520
+      "size": 580431,
+      "gzip": 132387
     },
     "react-dom.production.min.js (NODE_PROD)": {
-      "size": 122880,
-      "gzip": 38546
+      "size": 122127,
+      "gzip": 38480
     },
     "ReactDOMFiber-dev.js (FB_DEV)": {
-      "size": 570125,
-      "gzip": 130563
+      "size": 579715,
+      "gzip": 132422
     },
     "ReactDOMFiber-prod.js (FB_PROD)": {
-      "size": 428502,
-      "gzip": 96996
+      "size": 425463,
+      "gzip": 96331
     },
     "react-dom-test-utils.development.js (NODE_DEV)": {
-      "size": 53025,
-      "gzip": 13685
+      "size": 53430,
+      "gzip": 13782
     },
     "ReactTestUtils-dev.js (FB_DEV)": {
-      "size": 52904,
-      "gzip": 13646
+      "size": 53309,
+      "gzip": 13741
     },
     "ReactDOMServerStack-dev.js (FB_DEV)": {
       "size": 460810,
@@ -65,20 +65,20 @@
       "gzip": 81957
     },
     "react-dom-server.development.js (UMD_DEV)": {
-      "size": 308329,
-      "gzip": 77617
+      "size": 307012,
+      "gzip": 76850
     },
     "react-dom-server.production.min.js (UMD_PROD)": {
-      "size": 66111,
-      "gzip": 22613
+      "size": 65184,
+      "gzip": 22155
     },
     "react-dom-server.development.js (NODE_DEV)": {
-      "size": 266194,
-      "gzip": 67866
+      "size": 265858,
+      "gzip": 67373
     },
     "react-dom-server.production.min.js (NODE_PROD)": {
-      "size": 62380,
-      "gzip": 21260
+      "size": 61522,
+      "gzip": 20857
     },
     "ReactDOMServerStream-dev.js (FB_DEV)": {
       "size": 264750,
@@ -89,64 +89,64 @@
       "gzip": 51047
     },
     "react-art.development.js (UMD_DEV)": {
-      "size": 362062,
-      "gzip": 80236
+      "size": 359303,
+      "gzip": 79940
     },
     "react-art.production.min.js (UMD_PROD)": {
-      "size": 99126,
-      "gzip": 30132
+      "size": 97521,
+      "gzip": 29904
     },
     "react-art.development.js (NODE_DEV)": {
-      "size": 283458,
-      "gzip": 60201
+      "size": 280721,
+      "gzip": 59867
     },
     "react-art.production.min.js (NODE_PROD)": {
-      "size": 60504,
-      "gzip": 18189
+      "size": 58905,
+      "gzip": 17961
     },
     "ReactARTFiber-dev.js (FB_DEV)": {
-      "size": 282891,
-      "gzip": 60125
+      "size": 280154,
+      "gzip": 59786
     },
     "ReactARTFiber-prod.js (FB_PROD)": {
-      "size": 220185,
-      "gzip": 45704
+      "size": 215532,
+      "gzip": 44949
     },
     "ReactNativeStack-dev.js (RN_DEV)": {
       "size": 197039,
-      "gzip": 36193
+      "gzip": 36189
     },
     "ReactNativeStack-prod.js (RN_PROD)": {
       "size": 136606,
       "gzip": 25990
     },
     "ReactNativeFiber-dev.js (RN_DEV)": {
-      "size": 301278,
-      "gzip": 51431
+      "size": 298654,
+      "gzip": 51338
     },
     "ReactNativeFiber-prod.js (RN_PROD)": {
-      "size": 221863,
-      "gzip": 38015
+      "size": 218380,
+      "gzip": 37833
     },
     "react-test-renderer.development.js (NODE_DEV)": {
-      "size": 280651,
-      "gzip": 59110
+      "size": 277864,
+      "gzip": 58787
     },
     "ReactTestRendererFiber-dev.js (FB_DEV)": {
-      "size": 280075,
-      "gzip": 59030
+      "size": 277288,
+      "gzip": 58707
     },
     "react-test-renderer-shallow.development.js (NODE_DEV)": {
-      "size": 8179,
-      "gzip": 2288
+      "size": 8437,
+      "gzip": 2308
     },
     "ReactShallowRenderer-dev.js (FB_DEV)": {
-      "size": 8080,
-      "gzip": 2237
+      "size": 8338,
+      "gzip": 2255
     },
     "react-noop-renderer.development.js (NODE_DEV)": {
-      "size": 274713,
-      "gzip": 57491
+      "size": 272012,
+      "gzip": 57213
     },
     "ReactHTMLString-dev.js (FB_DEV)": {
       "size": 265654,
@@ -181,20 +181,20 @@
       "gzip": 50920
     },
     "ReactDOMServer-dev.js (FB_DEV)": {
-      "size": 265645,
-      "gzip": 67788
+      "size": 265309,
+      "gzip": 67297
     },
     "ReactDOMServer-prod.js (FB_PROD)": {
-      "size": 197859,
-      "gzip": 51191
+      "size": 195492,
+      "gzip": 50325
     },
     "react-dom-node-stream.development.js (NODE_DEV)": {
-      "size": 265427,
-      "gzip": 67670
+      "size": 267552,
+      "gzip": 67871
     },
     "react-dom-node-stream.production.min.js (NODE_PROD)": {
-      "size": 62695,
-      "gzip": 21279
+      "size": 62459,
+      "gzip": 21187
     },
     "ReactDOMNodeStream-dev.js (FB_DEV)": {
       "size": 264918,

src/renderers/dom/fiber/ReactDOMFiberComponent.js

@@ -811,6 +811,7 @@ var ReactDOMFiberComponent = {
         break;
       case 'select':
         ReactDOMFiberSelect.initWrapperState(domElement, rawProps);
+        rawProps = ReactDOMFiberSelect.getHostProps(domElement, rawProps);
         trapBubbledEventsLocal(domElement, tag);
         // For controlled components we always need to ensure we're listening
         // to onChange. Even if there is no listener.
@@ -846,8 +847,10 @@ var ReactDOMFiberComponent = {
           // Controlled attributes are not validated
           // TODO: Only ignore them on controlled tags.
           case 'value':
+          case 'defaultValue':
             break;
           case 'checked':
+          case 'defaultChecked':
             break;
           case 'selected':
             break;
@@ -901,7 +904,9 @@ var ReactDOMFiberComponent = {
           // Controlled attributes are not validated
           // TODO: Only ignore them on controlled tags.
           propKey === 'value' ||
+          propKey === 'defaultValue' ||
           propKey === 'checked' ||
+          propKey === 'defaultChecked' ||
           propKey === 'selected'
         ) {
           // Noop

src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js

@@ -79,10 +79,14 @@ var ReactDOMInput = {
       },
       props,
       {
-        defaultChecked: undefined,
-        defaultValue: undefined,
-        value: value != null ? value : node._wrapperState.initialValue,
-        checked: checked != null ? checked : node._wrapperState.initialChecked,
+        defaultValue: value == null
+          ? props.defaultValue
+          : node._wrapperState.initialValue,
+        defaultChecked: checked == null
+          ? props.defaultChecked
+          : node._wrapperState.initialChecked,
+        value: undefined,
+        checked: undefined,
       },
     );
 

src/renderers/dom/fiber/wrappers/ReactDOMFiberSelect.js

@@ -133,6 +133,7 @@ var ReactDOMSelect = {
   getHostProps: function(element: Element, props: Object) {
     return Object.assign({}, props, {
       value: undefined,
+      defaultValue: undefined,
     });
   },
 

src/renderers/dom/shared/HTMLDOMPropertyConfig.js

@@ -49,6 +49,7 @@ var HTMLDOMPropertyConfig = {
     charSet: 0,
     challenge: 0,
     checked: MUST_USE_PROPERTY | HAS_BOOLEAN_VALUE,
+    defaultChecked: HAS_BOOLEAN_VALUE,
     cite: 0,
     classID: 0,
     className: 0,
@@ -159,6 +160,7 @@ var HTMLDOMPropertyConfig = {
     type: 0,
     useMap: 0,
     value: 0,
+    defaultValue: 0,
     width: 0,
     wmode: 0,
     wrap: 0,
@@ -211,36 +213,11 @@ var HTMLDOMPropertyConfig = {
     className: 'class',
     htmlFor: 'for',
     httpEquiv: 'http-equiv',
+    defaultValue: 'value',
+    defaultChecked: 'checked',
   },
   DOMPropertyNames: {},
-  DOMMutationMethods: {
-    value: function(node, value) {
-      if (value == null) {
-        return node.removeAttribute('value');
-      }
-
-      // Number inputs get special treatment due to some edge cases in
-      // Chrome. Let everything else assign the value attribute as normal.
-      // https://github.com/facebook/react/issues/7253#issuecomment-236074326
-      if (node.type !== 'number' || node.hasAttribute('value') === false) {
-        node.setAttribute('value', '' + value);
-      } else if (
-        node.validity &&
-        !node.validity.badInput &&
-        node.ownerDocument.activeElement !== node
-      ) {
-        // Don't assign an attribute if validation reports bad
-        // input. Chrome will clear the value. Additionally, don't
-        // operate on inputs that have focus, otherwise Chrome might
-        // strip off trailing decimal places and cause the user's
-        // cursor position to jump to the beginning of the input.
-        //
-        // In ReactDOMInput, we have an onBlur event that will trigger
-        // this function again when focus is lost.
-        node.setAttribute('value', '' + value);
-      }
-    },
-  },
+  DOMMutationMethods: {},
 };
 
 module.exports = HTMLDOMPropertyConfig;

src/renderers/dom/shared/__tests__/DOMPropertyOperations-test.js

@@ -297,35 +297,6 @@ describe('DOMPropertyOperations', () => {
     });
   });
 
-  describe('value mutation method', function() {
-    it('should update an empty attribute to zero', function() {
-      var stubNode = document.createElement('input');
-      var stubInstance = {_debugID: 1};
-      ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
-
-      stubNode.setAttribute('type', 'radio');
-
-      DOMPropertyOperations.setValueForProperty(stubNode, 'value', '');
-      spyOn(stubNode, 'setAttribute');
-      DOMPropertyOperations.setValueForProperty(stubNode, 'value', 0);
-
-      expect(stubNode.setAttribute.calls.count()).toBe(1);
-    });
-
-    it('should always assign the value attribute for non-inputs', function() {
-      var stubNode = document.createElement('progress');
-      var stubInstance = {_debugID: 1};
-      ReactDOMComponentTree.precacheNode(stubInstance, stubNode);
-
-      spyOn(stubNode, 'setAttribute');
-
-      DOMPropertyOperations.setValueForProperty(stubNode, 'value', 30);
-      DOMPropertyOperations.setValueForProperty(stubNode, 'value', '30');
-
-      expect(stubNode.setAttribute.calls.count()).toBe(2);
-    });
-  });
-
   describe('deleteValueForProperty', () => {
     var stubNode;
     var stubInstance;