Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inputs should not mutate value on type conversion (when they stringify to the same thing) #9806

Merged
merged 1 commit into from May 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions scripts/fiber/tests-passing.txt
Expand Up @@ -1628,6 +1628,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
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/dom/fiber/wrappers/ReactDOMFiberInput.js
Expand Up @@ -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;
Expand Down
40 changes: 40 additions & 0 deletions src/renderers/dom/shared/wrappers/__tests__/ReactDOMInput-test.js
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/renderers/dom/stack/client/wrappers/ReactDOMInput.js
Expand Up @@ -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;
Expand Down