Skip to content

Commit

Permalink
Merge pull request #1759 from spicyj/gh-1698
Browse files Browse the repository at this point in the history
Use setImmediate to defer value restoration
  • Loading branch information
zpao committed Jul 30, 2014
2 parents 0a1c5da + 354fb44 commit 590e505
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 38 deletions.
42 changes: 20 additions & 22 deletions src/browser/ui/dom/components/ReactDOMInput.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ var ReactBrowserComponentMixin = require('ReactBrowserComponentMixin');
var ReactCompositeComponent = require('ReactCompositeComponent');
var ReactDOM = require('ReactDOM');
var ReactMount = require('ReactMount');
var ReactUpdates = require('ReactUpdates');

var invariant = require('invariant');
var merge = require('merge');
Expand All @@ -34,6 +35,13 @@ var input = ReactDOM.input;

var instancesByReactID = {};

function forceUpdateIfMounted() {
/*jshint validthis:true */
if (this.isMounted()) {
this.forceUpdate();
}
}

/**
* Implements an <input> native component that allows setting these optional
* props: `checked`, `value`, `defaultChecked`, and `defaultValue`.
Expand All @@ -58,16 +66,11 @@ var ReactDOMInput = ReactCompositeComponent.createClass({
getInitialState: function() {
var defaultValue = this.props.defaultValue;
return {
checked: this.props.defaultChecked || false,
value: defaultValue != null ? defaultValue : null
initialChecked: this.props.defaultChecked || false,
initialValue: defaultValue != null ? defaultValue : null
};
},

shouldComponentUpdate: function() {
// Defer any updates to this component during the `onChange` handler.
return !this._isChanging;
},

render: function() {
// Clone `this.props` so we don't mutate the input.
var props = merge(this.props);
Expand All @@ -76,10 +79,10 @@ var ReactDOMInput = ReactCompositeComponent.createClass({
props.defaultValue = null;

var value = LinkedValueUtils.getValue(this);
props.value = value != null ? value : this.state.value;
props.value = value != null ? value : this.state.initialValue;

var checked = LinkedValueUtils.getChecked(this);
props.checked = checked != null ? checked : this.state.checked;
props.checked = checked != null ? checked : this.state.initialChecked;

props.onChange = this._handleChange;

Expand Down Expand Up @@ -119,14 +122,12 @@ var ReactDOMInput = ReactCompositeComponent.createClass({
var returnValue;
var onChange = LinkedValueUtils.getOnChange(this);
if (onChange) {
this._isChanging = true;
returnValue = onChange.call(this, event);
this._isChanging = false;
}
this.setState({
checked: event.target.checked,
value: event.target.value
});
// Here we use setImmediate to wait until all updates have propagated, which
// is important when using controlled components within layers:
// https:/facebook/react/issues/1698
ReactUpdates.setImmediate(forceUpdateIfMounted, this);

var name = this.props.name;
if (this.props.type === 'radio' && name != null) {
Expand Down Expand Up @@ -164,13 +165,10 @@ var ReactDOMInput = ReactCompositeComponent.createClass({
'ReactDOMInput: Unknown radio button ID %s.',
otherID
);
// In some cases, this will actually change the `checked` state value.
// In other cases, there's no change but this forces a reconcile upon
// which componentDidUpdate will reset the DOM property to whatever it
// should be.
otherInstance.setState({
checked: false
});
// If this is a controlled radio button group, forcing the input that
// was previously checked to update will cause it to be come re-checked
// as appropriate.
ReactUpdates.setImmediate(forceUpdateIfMounted, otherInstance);
}
}

Expand Down
23 changes: 15 additions & 8 deletions src/browser/ui/dom/components/ReactDOMSelect.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,21 @@ var LinkedValueUtils = require('LinkedValueUtils');
var ReactBrowserComponentMixin = require('ReactBrowserComponentMixin');
var ReactCompositeComponent = require('ReactCompositeComponent');
var ReactDOM = require('ReactDOM');
var ReactUpdates = require('ReactUpdates');

var merge = require('merge');

// Store a reference to the <select> `ReactDOMComponent`.
var select = ReactDOM.select;

function updateWithPendingValueIfMounted() {
/*jshint validthis:true */
if (this.isMounted()) {
this.setState({value: this._pendingValue});
this._pendingValue = 0;
}
}

/**
* Validation function for `value` and `defaultValue`.
* @private
Expand Down Expand Up @@ -114,6 +123,10 @@ var ReactDOMSelect = ReactCompositeComponent.createClass({
return {value: this.props.defaultValue || (this.props.multiple ? [] : '')};
},

componentWillMount: function() {
this._pendingValue = null;
},

componentWillReceiveProps: function(nextProps) {
if (!this.props.multiple && nextProps.multiple) {
this.setState({value: [this.state.value]});
Expand All @@ -122,11 +135,6 @@ var ReactDOMSelect = ReactCompositeComponent.createClass({
}
},

shouldComponentUpdate: function() {
// Defer any updates to this component during the `onChange` handler.
return !this._isChanging;
},

render: function() {
// Clone `this.props` so we don't mutate the input.
var props = merge(this.props);
Expand Down Expand Up @@ -154,9 +162,7 @@ var ReactDOMSelect = ReactCompositeComponent.createClass({
var returnValue;
var onChange = LinkedValueUtils.getOnChange(this);
if (onChange) {
this._isChanging = true;
returnValue = onChange.call(this, event);
this._isChanging = false;
}

var selectedValue;
Expand All @@ -172,7 +178,8 @@ var ReactDOMSelect = ReactCompositeComponent.createClass({
selectedValue = event.target.value;
}

this.setState({value: selectedValue});
this._pendingValue = selectedValue;
ReactUpdates.setImmediate(updateWithPendingValueIfMounted, this);
return returnValue;
}

Expand Down
17 changes: 9 additions & 8 deletions src/browser/ui/dom/components/ReactDOMTextarea.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ var LinkedValueUtils = require('LinkedValueUtils');
var ReactBrowserComponentMixin = require('ReactBrowserComponentMixin');
var ReactCompositeComponent = require('ReactCompositeComponent');
var ReactDOM = require('ReactDOM');
var ReactUpdates = require('ReactUpdates');

var invariant = require('invariant');
var merge = require('merge');
Expand All @@ -33,6 +34,13 @@ var warning = require('warning');
// Store a reference to the <textarea> `ReactDOMComponent`.
var textarea = ReactDOM.textarea;

function forceUpdateIfMounted() {
/*jshint validthis:true */
if (this.isMounted()) {
this.forceUpdate();
}
}

/**
* Implements a <textarea> native component that allows setting `value`, and
* `defaultValue`. This differs from the traditional DOM API because value is
Expand Down Expand Up @@ -92,11 +100,6 @@ var ReactDOMTextarea = ReactCompositeComponent.createClass({
};
},

shouldComponentUpdate: function() {
// Defer any updates to this component during the `onChange` handler.
return !this._isChanging;
},

render: function() {
// Clone `this.props` so we don't mutate the input.
var props = merge(this.props);
Expand Down Expand Up @@ -129,11 +132,9 @@ var ReactDOMTextarea = ReactCompositeComponent.createClass({
var returnValue;
var onChange = LinkedValueUtils.getOnChange(this);
if (onChange) {
this._isChanging = true;
returnValue = onChange.call(this, event);
this._isChanging = false;
}
this.setState({value: event.target.value});
ReactUpdates.setImmediate(forceUpdateIfMounted, this);
return returnValue;
}

Expand Down

0 comments on commit 590e505

Please sign in to comment.