Use setImmediate to defer value restoration
Depends on #1758. Fixes #1698. Previously, controlled components would update too soon when using something like ReactLayeredComponentMixin (i.e., before the layer's updates could propagate from the parent), causing the cursor to jump even when always updating the new model value to match the DOM state. With this change, we defer the update until after all nested updates have had a chance to finish, which prevents the cursor from misbehaving. Also cleaned up the logic around updating a bit -- the .value and .checked updates in ReactDOMInput weren't being relied on at all so I removed them and opted for a simple forceUpdate instead. I also got rid of _isChanging which hasn't been necessary since the introduction of update batching. Test Plan: Tested the example in http://jsfiddle.net/Bobris/ZZtXn/2/ and verified that the cursor didn't jump. Changed the code to filter out numbers and verified that the field prevents typing numbers (attempting to do so still causes the cursor to jump to the end). Also verified that controlled and uncontrolled radio buttons, textareas, and select boxes work.
Ben Alpert committed
Jun 28, 2014 at 16:45 UTC
354fb4429978a8d77df72781ce1754c2ffcce9b3
3 files changed
+44
-38
src/browser/ui/dom/components/ReactDOMInput.js
+20
-22
@@ -25,6 +25,7 @@ var ReactBrowserComponentMixin = require('ReactBrowserComponentMixin');
25
var ReactCompositeComponent = require('ReactCompositeComponent');
26
var ReactDOM = require('ReactDOM');
27
var ReactMount = require('ReactMount');
28
+var ReactUpdates = require('ReactUpdates');
29
30
var invariant = require('invariant');
31
var merge = require('merge');
@@ -34,6 +35,13 @@ var input = ReactDOM.input;
35
36
var instancesByReactID = {};
37
38
+function forceUpdateIfMounted() {
39
+ /*jshint validthis:true */
40
+ if (this.isMounted()) {
41
+ this.forceUpdate();
42
+ }
43
+}
44
+
45
/**
46
* Implements an <input> native component that allows setting these optional
47
* props: `checked`, `value`, `defaultChecked`, and `defaultValue`.
@@ -58,16 +66,11 @@ var ReactDOMInput = ReactCompositeComponent.createClass({
66
getInitialState: function() {
67
var defaultValue = this.props.defaultValue;
68
return {
61
- checked: this.props.defaultChecked || false,
62
- value: defaultValue != null ? defaultValue : null
69
+ initialChecked: this.props.defaultChecked || false,
70
+ initialValue: defaultValue != null ? defaultValue : null
71
};
72
},
73
66
- shouldComponentUpdate: function() {
67
- // Defer any updates to this component during the `onChange` handler.
68
- return !this._isChanging;
69
- },
70
-
74
render: function() {
75
// Clone `this.props` so we don't mutate the input.
76
var props = merge(this.props);
@@ -76,10 +79,10 @@ var ReactDOMInput = ReactCompositeComponent.createClass({
79
props.defaultValue = null;
80
81
var value = LinkedValueUtils.getValue(this);
79
- props.value = value != null ? value : this.state.value;
82
+ props.value = value != null ? value : this.state.initialValue;
83
84
var checked = LinkedValueUtils.getChecked(this);
82
- props.checked = checked != null ? checked : this.state.checked;
85
+ props.checked = checked != null ? checked : this.state.initialChecked;
86
87
props.onChange = this._handleChange;
88
@@ -119,14 +122,12 @@ var ReactDOMInput = ReactCompositeComponent.createClass({
122
var returnValue;
123
var onChange = LinkedValueUtils.getOnChange(this);
124
if (onChange) {
122
- this._isChanging = true;
125
returnValue = onChange.call(this, event);
124
- this._isChanging = false;
126
}
126
- this.setState({
127
- checked: event.target.checked,
128
- value: event.target.value
129
- });
127
+ // Here we use setImmediate to wait until all updates have propagated, which
128
+ // is important when using controlled components within layers:
129
+ // https://github.com/facebook/react/issues/1698
130
+ ReactUpdates.setImmediate(forceUpdateIfMounted, this);
131
132
var name = this.props.name;
133
if (this.props.type === 'radio' && name != null) {
@@ -164,13 +165,10 @@ var ReactDOMInput = ReactCompositeComponent.createClass({
165
'ReactDOMInput: Unknown radio button ID %s.',
166
otherID
167
);
167
- // In some cases, this will actually change the `checked` state value.
168
- // In other cases, there's no change but this forces a reconcile upon
169
- // which componentDidUpdate will reset the DOM property to whatever it
170
- // should be.
171
- otherInstance.setState({
172
- checked: false
173
- });
168
+ // If this is a controlled radio button group, forcing the input that
169
+ // was previously checked to update will cause it to be come re-checked
170
+ // as appropriate.
171
+ ReactUpdates.setImmediate(forceUpdateIfMounted, otherInstance);
172
}
173
}
174
src/browser/ui/dom/components/ReactDOMSelect.js
+15
-8
@@ -23,12 +23,21 @@ var LinkedValueUtils = require('LinkedValueUtils');
23
var ReactBrowserComponentMixin = require('ReactBrowserComponentMixin');
24
var ReactCompositeComponent = require('ReactCompositeComponent');
25
var ReactDOM = require('ReactDOM');
26
+var ReactUpdates = require('ReactUpdates');
27
28
var merge = require('merge');
29
30
// Store a reference to the <select> `ReactDOMComponent`.
31
var select = ReactDOM.select;
32
33
+function updateWithPendingValueIfMounted() {
34
+ /*jshint validthis:true */
35
+ if (this.isMounted()) {
36
+ this.setState({value: this._pendingValue});
37
+ this._pendingValue = 0;
38
+ }
39
+}
40
+
41
/**
42
* Validation function for `value` and `defaultValue`.
43
* @private
@@ -114,6 +123,10 @@ var ReactDOMSelect = ReactCompositeComponent.createClass({
123
return {value: this.props.defaultValue || (this.props.multiple ? [] : '')};
124
},
125
126
+ componentWillMount: function() {
127
+ this._pendingValue = null;
128
+ },
129
+
130
componentWillReceiveProps: function(nextProps) {
131
if (!this.props.multiple && nextProps.multiple) {
132
this.setState({value: [this.state.value]});
@@ -122,11 +135,6 @@ var ReactDOMSelect = ReactCompositeComponent.createClass({
135
}
136
},
137
125
- shouldComponentUpdate: function() {
126
- // Defer any updates to this component during the `onChange` handler.
127
- return !this._isChanging;
128
- },
129
-
138
render: function() {
139
// Clone `this.props` so we don't mutate the input.
140
var props = merge(this.props);
@@ -154,9 +162,7 @@ var ReactDOMSelect = ReactCompositeComponent.createClass({
162
var returnValue;
163
var onChange = LinkedValueUtils.getOnChange(this);
164
if (onChange) {
157
- this._isChanging = true;
165
returnValue = onChange.call(this, event);
159
- this._isChanging = false;
166
}
167
168
var selectedValue;
@@ -172,7 +178,8 @@ var ReactDOMSelect = ReactCompositeComponent.createClass({
178
selectedValue = event.target.value;
179
}
180
175
- this.setState({value: selectedValue});
181
+ this._pendingValue = selectedValue;
182
+ ReactUpdates.setImmediate(updateWithPendingValueIfMounted, this);
183
return returnValue;
184
}
185
src/browser/ui/dom/components/ReactDOMTextarea.js
+9
-8
@@ -24,6 +24,7 @@ var LinkedValueUtils = require('LinkedValueUtils');
24
var ReactBrowserComponentMixin = require('ReactBrowserComponentMixin');
25
var ReactCompositeComponent = require('ReactCompositeComponent');
26
var ReactDOM = require('ReactDOM');
27
+var ReactUpdates = require('ReactUpdates');
28
29
var invariant = require('invariant');
30
var merge = require('merge');
@@ -33,6 +34,13 @@ var warning = require('warning');
34
// Store a reference to the <textarea> `ReactDOMComponent`.
35
var textarea = ReactDOM.textarea;
36
37
+function forceUpdateIfMounted() {
38
+ /*jshint validthis:true */
39
+ if (this.isMounted()) {
40
+ this.forceUpdate();
41
+ }
42
+}
43
+
44
/**
45
* Implements a <textarea> native component that allows setting `value`, and
46
* `defaultValue`. This differs from the traditional DOM API because value is
@@ -92,11 +100,6 @@ var ReactDOMTextarea = ReactCompositeComponent.createClass({
100
};
101
},
102
95
- shouldComponentUpdate: function() {
96
- // Defer any updates to this component during the `onChange` handler.
97
- return !this._isChanging;
98
- },
99
-
103
render: function() {
104
// Clone `this.props` so we don't mutate the input.
105
var props = merge(this.props);
@@ -129,11 +132,9 @@ var ReactDOMTextarea = ReactCompositeComponent.createClass({
132
var returnValue;
133
var onChange = LinkedValueUtils.getOnChange(this);
134
if (onChange) {
132
- this._isChanging = true;
135
returnValue = onChange.call(this, event);
134
- this._isChanging = false;
136
}
136
- this.setState({value: event.target.value});
137
+ ReactUpdates.setImmediate(forceUpdateIfMounted, this);
138
return returnValue;
139
}
140