Inputs should not mutate value on type conversion (#9806)

This is a follow-up on
https://github.com/facebook/react/pull/9584#discussion_r115642293. There
is no need to assign the value property of an input if the value
property of the React component changes types, but stringifies to the
same value. For example:

```javascript
DOM.render(<input value="true" />, el)
DOM.render(<input value={true} />, el)
```

In this case, the assignment to `input.value` will always be
cast to the string "true". There is no need to perform this
assignment. Particularly when we already cast the value to a string
later:

```javascript
// 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;
```
This commit is contained in:
Nathan Hunzaker
2017-05-30 15:25:44 -05:00
committed by Brandon Dail
parent 663ce58192
commit cabb38be9d
4 changed files with 44 additions and 2 deletions
+2
View File
@@ -1536,6 +1536,8 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
* should allow setting `value` to `false`
* should allow setting `value` to `objToString`
* should not incur unnecessary DOM mutations
* should not incur unnecessary DOM mutations for numeric type conversion
* should not incur unnecessary DOM mutations for the boolean type conversion
* should properly control a value of number `0`
* should properly control 0.0 for a text input
* should properly control 0.0 for a number input
@@ -209,7 +209,7 @@ var ReactDOMInput = {
// browsers typically do this as necessary, jsdom doesn't.
node.value = '' + value;
}
} else if (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;
@@ -471,6 +471,46 @@ describe('ReactDOMInput', () => {
expect(nodeValueSetter.mock.calls.length).toBe(1);
});
it('should not incur unnecessary DOM mutations for numeric type conversion', () => {
var container = document.createElement('div');
ReactDOM.render(<input value="0" />, container);
var node = container.firstChild;
var nodeValue = '0';
var nodeValueSetter = jest.genMockFn();
Object.defineProperty(node, 'value', {
get: function() {
return nodeValue;
},
set: nodeValueSetter.mockImplementation(function(newValue) {
nodeValue = newValue;
}),
});
ReactDOM.render(<input value={0} />, container);
expect(nodeValueSetter.mock.calls.length).toBe(0);
});
it('should not incur unnecessary DOM mutations for the boolean type conversion', () => {
var container = document.createElement('div');
ReactDOM.render(<input value="true" />, container);
var node = container.firstChild;
var nodeValue = 'true';
var nodeValueSetter = jest.genMockFn();
Object.defineProperty(node, 'value', {
get: function() {
return nodeValue;
},
set: nodeValueSetter.mockImplementation(function(newValue) {
nodeValue = newValue;
}),
});
ReactDOM.render(<input value={true} />, container);
expect(nodeValueSetter.mock.calls.length).toBe(0);
});
it('should properly control a value of number `0`', () => {
var stub = <input type="text" value={0} onChange={emptyFunction} />;
stub = ReactTestUtils.renderIntoDocument(stub);
@@ -201,7 +201,7 @@ var ReactDOMInput = {
// browsers typically do this as necessary, jsdom doesn't.
node.value = '' + value;
}
} else if (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;