Skip to content

Commit f933244

Browse files
nhunzakerflarnie
authored andcommitted
Inputs should not mutate value on type conversion (#9806)
This is a follow-up on #9584 (comment). 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; ```
1 parent 98fde61 commit f933244

File tree

2 files changed

+41
-1
lines changed

2 files changed

+41
-1
lines changed

src/renderers/dom/client/wrappers/ReactDOMInput.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,7 @@ var ReactDOMInput = {
226226
// browsers typically do this as necessary, jsdom doesn't.
227227
node.value = '' + value;
228228
}
229-
} else if (node.value !== value) {
229+
} else if (node.value !== '' + value) {
230230
// Cast `value` to a string to ensure the value is set correctly. While
231231
// browsers typically do this as necessary, jsdom doesn't.
232232
node.value = '' + value;

src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js

+40
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,46 @@ describe('ReactDOMInput', () => {
324324
expect(nodeValueSetter.mock.calls.length).toBe(1);
325325
});
326326

327+
it('should not incur unnecessary DOM mutations for numeric type conversion', () => {
328+
var container = document.createElement('div');
329+
ReactDOM.render(<input value="0" />, container);
330+
331+
var node = container.firstChild;
332+
var nodeValue = '0';
333+
var nodeValueSetter = jest.genMockFn();
334+
Object.defineProperty(node, 'value', {
335+
get: function() {
336+
return nodeValue;
337+
},
338+
set: nodeValueSetter.mockImplementation(function(newValue) {
339+
nodeValue = newValue;
340+
}),
341+
});
342+
343+
ReactDOM.render(<input value={0} />, container);
344+
expect(nodeValueSetter.mock.calls.length).toBe(0);
345+
});
346+
347+
it('should not incur unnecessary DOM mutations for the boolean type conversion', () => {
348+
var container = document.createElement('div');
349+
ReactDOM.render(<input value="true" />, container);
350+
351+
var node = container.firstChild;
352+
var nodeValue = 'true';
353+
var nodeValueSetter = jest.genMockFn();
354+
Object.defineProperty(node, 'value', {
355+
get: function() {
356+
return nodeValue;
357+
},
358+
set: nodeValueSetter.mockImplementation(function(newValue) {
359+
nodeValue = newValue;
360+
}),
361+
});
362+
363+
ReactDOM.render(<input value={true} />, container);
364+
expect(nodeValueSetter.mock.calls.length).toBe(0);
365+
});
366+
327367
it('should properly control a value of number `0`', () => {
328368
var stub = <input type="text" value={0} onChange={emptyFunction} />;
329369
stub = ReactTestUtils.renderIntoDocument(stub);

0 commit comments

Comments
 (0)