Don't bind callbacks to setState, setProps etc.
We added a bind to the public instance, but instead we can just pass the public instance as the context when we execute. This avoids a warning that fires when we call bind on auto-bound methods.
Sebastian Markbage committed
Jan 15, 2015 at 14:00 UTC
d7d3ea55608e53a19edf53172274d16014fdd021
4 files changed
+48
-6
src/classic/class/ReactClass.js
+3
-3
@@ -712,7 +712,7 @@ var ReactClassMixin = {
712
);
713
internalInstance.replaceState(
714
newState,
715
- callback && callback.bind(this)
715
+ callback
716
);
717
},
718
@@ -746,7 +746,7 @@ var ReactClassMixin = {
746
);
747
internalInstance.setProps(
748
partialProps,
749
- callback && callback.bind(this)
749
+ callback
750
);
751
},
752
@@ -762,7 +762,7 @@ var ReactClassMixin = {
762
replaceProps: function(newProps, callback) {
763
ReactInstanceMap.get(this).replaceProps(
764
newProps,
765
- callback && callback.bind(this)
765
+ callback
766
);
767
}
768
};
src/classic/class/__tests__/ReactBind-test.js
+42
@@ -124,4 +124,46 @@ describe('autobinding', function() {
124
expect(mouseDidClick.mock.instances[0]).toBe(mountedInstance1);
125
});
126
127
+ it('warns if you try to bind to this', function() {
128
+ spyOn(console, 'warn');
129
+
130
+ var TestBindComponent = React.createClass({
131
+ handleClick: function() { },
132
+ render: function() {
133
+ return <div onClick={this.handleClick.bind(this)} />;
134
+ }
135
+ });
136
+
137
+ ReactTestUtils.renderIntoDocument(<TestBindComponent />)
138
+
139
+ expect(console.warn.argsForCall.length).toBe(1);
140
+ expect(console.warn.argsForCall[0][0]).toContain(
141
+ 'You are binding a component method to the component. ' +
142
+ 'React does this for you automatically'
143
+ );
144
+ });
145
+
146
+ it('does not warn if you pass an auto-bound method to setState', function() {
147
+ spyOn(console, 'warn');
148
+
149
+ var TestBindComponent = React.createClass({
150
+ getInitialState: function() {
151
+ return { foo: 1 };
152
+ },
153
+ componentDidMount: function() {
154
+ this.setState({ foo: 2 }, this.handleUpdate);
155
+ },
156
+ handleUpdate: function() {
157
+
158
+ },
159
+ render: function() {
160
+ return <div onClick={this.handleClick} />;
161
+ }
162
+ });
163
+
164
+ ReactTestUtils.renderIntoDocument(<TestBindComponent />)
165
+
166
+ expect(console.warn.argsForCall.length).toBe(0);
167
+ });
168
+
169
});
src/core/ReactUpdates.js
+1
-1
@@ -152,7 +152,7 @@ function runBatchedUpdates(transaction) {
152
for (var j = 0; j < callbacks.length; j++) {
153
transaction.callbackQueue.enqueue(
154
callbacks[j],
155
- component
155
+ component.getPublicInstance()
156
);
157
}
158
}
src/modern/class/ReactComponentBase.js
+2
-2
@@ -61,7 +61,7 @@ ReactComponentBase.prototype.setState = function(partialState, callback) {
61
'component.'
62
);
63
internalInstance.setState(
64
- partialState, callback && callback.bind(this)
64
+ partialState, callback
65
);
66
};
67
@@ -87,7 +87,7 @@ ReactComponentBase.prototype.forceUpdate = function(callback) {
87
'components. This usually means you called forceUpdate() on an ' +
88
'unmounted component.'
89
);
90
- internalInstance.forceUpdate(callback && callback.bind(this));
90
+ internalInstance.forceUpdate(callback);
91
};
92
93
/**