Have defaultValue reach DOM node for html input box for facebook#4618

hayesgm · Dec 16, 2015 · 75ca295 · 75ca295
1 parent 55bd203
commit 75ca295
Show file tree

Hide file tree

Showing 6 changed files with 151 additions and 26 deletions.
diff --git a/src/renderers/dom/client/wrappers/ReactDOMInput.js b/src/renderers/dom/client/wrappers/ReactDOMInput.js
@@ -70,7 +70,7 @@ var ReactDOMInput = {
     var nativeProps = assign({}, props, {
       defaultChecked: undefined,
       defaultValue: undefined,
-      value: value != null ? value : inst._wrapperState.initialValue,
+      value: value != null ? value : props.defaultValue,
       checked: checked != null ? checked : inst._wrapperState.initialChecked,
       onChange: inst._wrapperState.onChange,
     });
@@ -103,10 +103,8 @@ var ReactDOMInput = {
       warnIfValueIsNull(props);
     }
 
-    var defaultValue = props.defaultValue;
     inst._wrapperState = {
       initialChecked: props.defaultChecked || false,
-      initialValue: defaultValue != null ? defaultValue : null,
       listeners: null,
       onChange: _handleChange.bind(inst),
     };
@@ -140,13 +138,16 @@ var ReactDOMInput = {
 
     var value = LinkedValueUtils.getValue(props);
     if (value != null) {
+      var node = ReactDOMComponentTree.getNodeFromInstance(inst);
+
       // Cast `value` to a string to ensure the value is set correctly. While
       // browsers typically do this as necessary, jsdom doesn't.
-      DOMPropertyOperations.setValueForProperty(
-        ReactDOMComponentTree.getNodeFromInstance(inst),
-        'value',
-        '' + value
-      );
+      var newValue = '' + value;
+
+      // To avoid side effects (such as losing text selection), only set value if changed
+      if (newValue !== node.value) {
+        node.value = newValue;
+      }
     }
   },
 };

diff --git a/src/renderers/dom/client/wrappers/ReactDOMTextarea.js b/src/renderers/dom/client/wrappers/ReactDOMTextarea.js
@@ -11,7 +11,6 @@
 
 'use strict';
 
-var DOMPropertyOperations = require('DOMPropertyOperations');
 var LinkedValueUtils = require('LinkedValueUtils');
 var ReactDOMComponentTree = require('ReactDOMComponentTree');
 var ReactUpdates = require('ReactUpdates');
@@ -143,13 +142,16 @@ var ReactDOMTextarea = {
 
     var value = LinkedValueUtils.getValue(props);
     if (value != null) {
+      var node = ReactDOMComponentTree.getNodeFromInstance(inst);
+
       // Cast `value` to a string to ensure the value is set correctly. While
       // browsers typically do this as necessary, jsdom doesn't.
-      DOMPropertyOperations.setValueForProperty(
-        ReactDOMComponentTree.getNodeFromInstance(inst),
-        'value',
-        '' + value
-      );
+      var newValue = '' + value;
+
+      // To avoid side effects (such as losing text selection), only set value if changed
+      if (newValue !== node.value) {
+        node.value = newValue;
+      }
     }
   },
 };

diff --git a/src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js b/src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js
@@ -36,6 +36,7 @@ describe('ReactDOMInput', function() {
     stub = ReactTestUtils.renderIntoDocument(stub);
     var node = ReactDOM.findDOMNode(stub);
 
+    expect(node.getAttribute('value')).toBe('0');
     expect(node.value).toBe('0');
   });
 
@@ -55,6 +56,68 @@ describe('ReactDOMInput', function() {
     expect(node.value).toBe('false');
   });
 
+  it('should update `defaultValue` for uncontrolled input', function() {
+    var container = document.createElement('div');
+
+    var el = ReactDOM.render(<input type="text" defaultValue="0" />, container);
+    var node = ReactDOM.findDOMNode(el);
+
+    expect(node.value).toBe('0');
+
+    ReactDOM.render(<input type="text" defaultValue="1" />, container);
+
+    expect(node.value).toBe('1');
+  });
+
+  it('should take `defaultValue` for a controlled input', function() {
+    var container = document.createElement('div');
+
+    var el = ReactDOM.render(<input type="text" value={null} defaultValue="0" />, container);
+    var node = ReactDOM.findDOMNode(el);
+
+    expect(node.value).toBe('0');
+
+    ReactDOM.render(<input type="text" value={null} defaultValue="1" />, container);
+
+    expect(node.value).toBe('1');
+
+    ReactDOM.render(<input type="text" value={3} defaultValue="2" />, container);
+
+    expect(node.value).toBe('3');
+
+    ReactDOM.render(<input type="text" value={5} defaultValue="4" />, container);
+
+    expect(node.value).toBe('5');
+    expect(node.getAttribute('value')).toBe('5');
+    expect(node.defaultValue).toBe('5');
+  });
+
+  it('should update `value` when changing to controlled input', function() {
+    var container = document.createElement('div');
+
+    var el = ReactDOM.render(<input type="text" defaultValue="0" />, container);
+    var node = ReactDOM.findDOMNode(el);
+
+    expect(node.value).toBe('0');
+
+    ReactDOM.render(<input type="text" value="1" defaultValue="0" />, container);
+
+    expect(node.value).toBe('1');
+  });
+
+  it('should take `defaultValue` when changing to uncontrolled input', function() {
+    var container = document.createElement('div');
+
+    var el = ReactDOM.render(<input type="text" value="0" readOnly="true" />, container);
+    var node = ReactDOM.findDOMNode(el);
+
+    expect(node.value).toBe('0');
+
+    ReactDOM.render(<input type="text" defaultValue="1" />, container);
+
+    expect(node.value).toBe('1');
+  });
+
   it('should display "foobar" for `defaultValue` of `objToString`', function() {
     var objToString = {
       toString: function() {
@@ -127,6 +190,29 @@ describe('ReactDOMInput', function() {
     expect(node.value).toEqual('foobar');
   });
 
+  it('should not incur unnecessary DOM mutations', function() {
+    var container = document.createElement('div');
+    ReactDOM.render(<input value="a" />, container);
+
+    var node = container.firstChild;
+    var nodeValue = 'a'; // node.value always returns undefined
+    var nodeValueSetter = jest.genMockFn();
+    Object.defineProperty(node, 'value', {
+      get: function() {
+        return nodeValue;
+      },
+      set: nodeValueSetter.mockImplementation(function(newValue) {
+        nodeValue = newValue;
+      }),
+    });
+
+    ReactDOM.render(<input value="a" />, container);
+    expect(nodeValueSetter.mock.calls.length).toBe(0);
+
+    ReactDOM.render(<input value="b"/>, container);
+    expect(nodeValueSetter.mock.calls.length).toBe(1);
+  });
+
   it('should properly control a value of number `0`', function() {
     var stub = <input type="text" value={0} onChange={emptyFunction} />;
     stub = ReactTestUtils.renderIntoDocument(stub);

diff --git a/src/renderers/dom/client/wrappers/__tests__/ReactDOMTextarea-test.js b/src/renderers/dom/client/wrappers/__tests__/ReactDOMTextarea-test.js
@@ -163,6 +163,42 @@ describe('ReactDOMTextarea', function() {
     expect(node.value).toEqual('foo');
   });
 
+  it('should not take updates to `defaultValue` for uncontrolled textarea', function() {
+    var container = document.createElement('div');
+
+    var el = ReactDOM.render(<textarea type="text" defaultValue="0" />, container);
+    var node = ReactDOM.findDOMNode(el);
+
+    expect(node.value).toBe('0');
+
+    ReactDOM.render(<textarea type="text" defaultValue="1" />, container);
+
+    expect(node.value).toBe('0');
+  });
+
+  it('should not incur unnecessary DOM mutations', function() {
+    var container = document.createElement('div');
+    ReactDOM.render(<textarea value="a" onChange={emptyFunction} />, container);
+
+    var node = container.firstChild;
+    var nodeValue = 'a'; // node.value always returns undefined
+    var nodeValueSetter = jest.genMockFn();
+    Object.defineProperty(node, 'value', {
+      get: function() {
+        return nodeValue;
+      },
+      set: nodeValueSetter.mockImplementation(function(newValue) {
+        nodeValue = newValue;
+      }),
+    });
+
+    ReactDOM.render(<textarea value="a" onChange={emptyFunction} />, container);
+    expect(nodeValueSetter.mock.calls.length).toBe(0);
+
+    ReactDOM.render(<textarea value="b" onChange={emptyFunction} />, container);
+    expect(nodeValueSetter.mock.calls.length).toBe(1);
+  });
+
   it('should properly control a value of number `0`', function() {
     var stub = <textarea value={0} onChange={emptyFunction} />;
     stub = renderTextarea(stub);
@@ -278,4 +314,5 @@ describe('ReactDOMTextarea', function() {
     ReactTestUtils.renderIntoDocument(<textarea value={null} />);
     expect(console.error.argsForCall.length).toBe(1);
   });
+
 });
diff --git a/src/renderers/dom/shared/HTMLDOMPropertyConfig.js b/src/renderers/dom/shared/HTMLDOMPropertyConfig.js
@@ -17,7 +17,6 @@ var ExecutionEnvironment = require('ExecutionEnvironment');
 var MUST_USE_ATTRIBUTE = DOMProperty.injection.MUST_USE_ATTRIBUTE;
 var MUST_USE_PROPERTY = DOMProperty.injection.MUST_USE_PROPERTY;
 var HAS_BOOLEAN_VALUE = DOMProperty.injection.HAS_BOOLEAN_VALUE;
-var HAS_SIDE_EFFECTS = DOMProperty.injection.HAS_SIDE_EFFECTS;
 var HAS_NUMERIC_VALUE = DOMProperty.injection.HAS_NUMERIC_VALUE;
 var HAS_POSITIVE_NUMERIC_VALUE =
   DOMProperty.injection.HAS_POSITIVE_NUMERIC_VALUE;
@@ -169,7 +168,7 @@ var HTMLDOMPropertyConfig = {
     // Setting .type throws on non-<input> tags
     type: MUST_USE_ATTRIBUTE,
     useMap: null,
-    value: MUST_USE_PROPERTY | HAS_SIDE_EFFECTS,
+    value: MUST_USE_ATTRIBUTE,
     width: MUST_USE_ATTRIBUTE,
     wmode: MUST_USE_ATTRIBUTE,
     wrap: MUST_USE_ATTRIBUTE,

diff --git a/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js b/src/renderers/dom/shared/__tests__/ReactDOMComponent-test.js
@@ -413,25 +413,25 @@ describe('ReactDOMComponent', function() {
 
     it('should not incur unnecessary DOM mutations', function() {
       var container = document.createElement('div');
-      ReactDOM.render(<div value="" />, container);
+      ReactDOM.render(<div id="" />, container);
 
       var node = container.firstChild;
-      var nodeValue = ''; // node.value always returns undefined
-      var nodeValueSetter = jest.genMockFn();
-      Object.defineProperty(node, 'value', {
+      var nodeId = ''; // node.value always returns undefined
+      var nodeIdSetter = jest.genMockFn();
+      Object.defineProperty(node, 'id', {
         get: function() {
-          return nodeValue;
+          return nodeId;
         },
-        set: nodeValueSetter.mockImplementation(function(newValue) {
-          nodeValue = newValue;
+        set: nodeIdSetter.mockImplementation(function(newValue) {
+          nodeId = newValue;
         }),
       });
 
-      ReactDOM.render(<div value="" />, container);
-      expect(nodeValueSetter.mock.calls.length).toBe(0);
+      ReactDOM.render(<div id="" />, container);
+      expect(nodeIdSetter.mock.calls.length).toBe(0);
 
       ReactDOM.render(<div />, container);
-      expect(nodeValueSetter.mock.calls.length).toBe(1);
+      expect(nodeIdSetter.mock.calls.length).toBe(1);
     });
 
     it('should ignore attribute whitelist for elements with the "is: attribute', function() {