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; ```
Nathan Hunzaker committed
May 30, 2017 at 16:25 UTC
74044e0ebacd9510e78e9a3879aa65e724dce4a3
4 files changed
+44
-2
scripts/fiber/tests-passing.txt
+2
@@ -1536,6 +1536,8 @@ src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
1536
* should allow setting `value` to `false`
1537
* should allow setting `value` to `objToString`
1538
* should not incur unnecessary DOM mutations
1539
+* should not incur unnecessary DOM mutations for numeric type conversion
1540
+* should not incur unnecessary DOM mutations for the boolean type conversion
1541
* should properly control a value of number `0`
1542
* should properly control 0.0 for a text input
1543
* should properly control 0.0 for a number input
src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js
+1
-1
@@ -209,7 +209,7 @@ var ReactDOMInput = {
209
// browsers typically do this as necessary, jsdom doesn't.
210
node.value = '' + value;
211
}
212
- } else if (node.value !== value) {
212
+ } else if (node.value !== '' + value) {
213
// Cast `value` to a string to ensure the value is set correctly. While
214
// browsers typically do this as necessary, jsdom doesn't.
215
node.value = '' + value;
src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
+40
@@ -471,6 +471,46 @@ describe('ReactDOMInput', () => {
471
expect(nodeValueSetter.mock.calls.length).toBe(1);
472
});
473
474
+ it('should not incur unnecessary DOM mutations for numeric type conversion', () => {
475
+ var container = document.createElement('div');
476
+ ReactDOM.render(<input value="0" />, container);
477
+
478
+ var node = container.firstChild;
479
+ var nodeValue = '0';
480
+ var nodeValueSetter = jest.genMockFn();
481
+ Object.defineProperty(node, 'value', {
482
+ get: function() {
483
+ return nodeValue;
484
+ },
485
+ set: nodeValueSetter.mockImplementation(function(newValue) {
486
+ nodeValue = newValue;
487
+ }),
488
+ });
489
+
490
+ ReactDOM.render(<input value={0} />, container);
491
+ expect(nodeValueSetter.mock.calls.length).toBe(0);
492
+ });
493
+
494
+ it('should not incur unnecessary DOM mutations for the boolean type conversion', () => {
495
+ var container = document.createElement('div');
496
+ ReactDOM.render(<input value="true" />, container);
497
+
498
+ var node = container.firstChild;
499
+ var nodeValue = 'true';
500
+ var nodeValueSetter = jest.genMockFn();
501
+ Object.defineProperty(node, 'value', {
502
+ get: function() {
503
+ return nodeValue;
504
+ },
505
+ set: nodeValueSetter.mockImplementation(function(newValue) {
506
+ nodeValue = newValue;
507
+ }),
508
+ });
509
+
510
+ ReactDOM.render(<input value={true} />, container);
511
+ expect(nodeValueSetter.mock.calls.length).toBe(0);
512
+ });
513
+
514
it('should properly control a value of number `0`', () => {
515
var stub = <input type="text" value={0} onChange={emptyFunction} />;
516
stub = ReactTestUtils.renderIntoDocument(stub);
src/renderers/dom/stack/client/wrappers/ReactDOMInput.js
+1
-1
@@ -201,7 +201,7 @@ var ReactDOMInput = {
201
// browsers typically do this as necessary, jsdom doesn't.
202
node.value = '' + value;
203
}
204
- } else if (node.value !== value) {
204
+ } else if (node.value !== '' + value) {
205
// Cast `value` to a string to ensure the value is set correctly. While
206
// browsers typically do this as necessary, jsdom doesn't.
207
node.value = '' + value;