Skip to content

Commit

Permalink
Avoid validation warning when inputs change type (#7333)
Browse files Browse the repository at this point in the history
For controlled inputs, `updateWrapper` was getting called before the
`type` prop had a chance to update. This could lead to a case where
switching from the `text` to `number` type caused a validation error
that would prevent the proper input value from being assigned.

This commit moves the call to `ReactDOMInput.updateWrapper` below
`_updateProperties` to avoid this situation.
  • Loading branch information
nhunzaker authored and sophiebits committed Jul 23, 2016
1 parent 1fc5f28 commit 08a0895
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 6 deletions.
21 changes: 21 additions & 0 deletions src/renderers/dom/client/wrappers/__tests__/ReactDOMInput-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -769,4 +769,25 @@ describe('ReactDOMInput', function() {
);
expect(input.value).toBe('hi');
});

it('does not raise a validation warning when it switches types', function() {
var Input = React.createClass({
getInitialState() {
return { type: 'number', value: 1000 };
},
render() {
var { value, type } = this.state;
return (<input onChange={() => {}} type={type} value={value} />);
},
});

var input = ReactTestUtils.renderIntoDocument(<Input />);
var node = ReactDOM.findDOMNode(input);

// If the value is set before the type, a validation warning will raise and
// the value will not be assigned.
input.setState({ type: 'text', value: 'Test' });
expect(node.value).toEqual('Test');
});

});
21 changes: 15 additions & 6 deletions src/renderers/dom/shared/ReactDOMComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,6 @@ ReactDOMComponent.Mixin = {
nextProps = ReactDOMButton.getHostProps(this, nextProps);
break;
case 'input':
ReactDOMInput.updateWrapper(this);
lastProps = ReactDOMInput.getHostProps(this, lastProps);
nextProps = ReactDOMInput.getHostProps(this, nextProps);
break;
Expand All @@ -920,7 +919,6 @@ ReactDOMComponent.Mixin = {
nextProps = ReactDOMSelect.getHostProps(this, nextProps);
break;
case 'textarea':
ReactDOMTextarea.updateWrapper(this);
lastProps = ReactDOMTextarea.getHostProps(this, lastProps);
nextProps = ReactDOMTextarea.getHostProps(this, nextProps);
break;
Expand All @@ -935,10 +933,21 @@ ReactDOMComponent.Mixin = {
context
);

if (this._tag === 'select') {
// <select> value update needs to occur after <option> children
// reconciliation
transaction.getReactMountReady().enqueue(postUpdateSelectWrapper, this);
switch (this._tag) {
case 'input':
// Update the wrapper around inputs *after* updating props. This has to
// happen after `_updateDOMProperties`. Otherwise HTML5 input validations
// raise warnings and prevent the new value from being assigned.
ReactDOMInput.updateWrapper(this);
break;
case 'textarea':
ReactDOMTextarea.updateWrapper(this);
break;
case 'select':
// <select> value update needs to occur after <option> children
// reconciliation
transaction.getReactMountReady().enqueue(postUpdateSelectWrapper, this);
break;
}

if (__DEV__) {
Expand Down

0 comments on commit 08a0895

Please sign in to comment.