From a17dc9ce08391fbc0ad491c990845da54f180b4a Mon Sep 17 00:00:00 2001 From: Nathan Hunzaker Date: Tue, 30 May 2017 16:25:44 -0400 Subject: [PATCH] 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(, el) DOM.render(, 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; ``` --- .../dom/client/wrappers/ReactDOMInput.js | 2 +- .../wrappers/__tests__/ReactDOMInput-test.js | 40 +++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/src/renderers/dom/client/wrappers/ReactDOMInput.js b/src/renderers/dom/client/wrappers/ReactDOMInput.js index 7263bca413f34..0eb9509a7337f 100644 --- a/src/renderers/dom/client/wrappers/ReactDOMInput.js +++ b/src/renderers/dom/client/wrappers/ReactDOMInput.js @@ -226,7 +226,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; diff --git a/src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js b/src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js index 03dbda5dcc40d..ababb9120c202 100644 --- a/src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js +++ b/src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js @@ -324,6 +324,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(, 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(, 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(, 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(, container); + expect(nodeValueSetter.mock.calls.length).toBe(0); + }); + it('should properly control a value of number `0`', () => { var stub = ; stub = ReactTestUtils.renderIntoDocument(stub);