Guard top-level methods, not in every render call
Test Plan: grunt test
Ben Alpert committed
Apr 14, 2014 at 23:56 UTC
c9767c2822a7e365dd175fad59a1c6a04bc3603b
4 files changed
+41
-15
src/browser/ui/ReactMount.js
+23
@@ -19,6 +19,7 @@
19
"use strict";
20
21
var DOMProperty = require('DOMProperty');
22
+var ReactCurrentOwner = require('ReactCurrentOwner');
23
var ReactEventEmitter = require('ReactEventEmitter');
24
var ReactInstanceHandles = require('ReactInstanceHandles');
25
var ReactPerf = require('ReactPerf');
@@ -297,6 +298,16 @@ var ReactMount = {
298
nextComponent,
299
container,
300
shouldReuseMarkup) {
301
+ // Various parts of our code (such as ReactCompositeComponent's
302
+ // _renderValidatedComponent) assume that calls to render aren't nested;
303
+ // verify that that's the case.
304
+ invariant(
305
+ ReactCurrentOwner.current == null,
306
+ '_renderNewRootComponent(): Render methods should be a pure function ' +
307
+ 'of props and state; triggering nested component updates from ' +
308
+ 'render is not allowed. If necessary, trigger nested updates in ' +
309
+ 'componentDidUpdate.'
310
+ );
311
312
var componentInstance = instantiateReactComponent(nextComponent);
313
var reactRootID = ReactMount._registerComponent(
@@ -425,6 +436,18 @@ var ReactMount = {
436
* `container`
437
*/
438
unmountComponentAtNode: function(container) {
439
+ // Various parts of our code (such as ReactCompositeComponent's
440
+ // _renderValidatedComponent) assume that calls to render aren't nested;
441
+ // verify that that's the case. (Strictly speaking, unmounting won't cause a
442
+ // render but we still don't expect to be in a render call here.)
443
+ invariant(
444
+ ReactCurrentOwner.current == null,
445
+ 'unmountComponentAtNode(): Render methods should be a pure function of ' +
446
+ 'props and state; triggering nested component updates from render is ' +
447
+ 'not allowed. If necessary, trigger nested updates in ' +
448
+ 'componentDidUpdate.'
449
+ );
450
+
451
var reactRootID = getReactRootID(container);
452
var component = instancesByReactRootID[reactRootID];
453
if (!component) {
src/core/ReactCompositeComponent.js
-11
@@ -1164,17 +1164,6 @@ var ReactCompositeComponentMixin = {
1164
'ReactCompositeComponent',
1165
'_renderValidatedComponent',
1166
function() {
1167
- // We set ReactCurrentOwner.current back to null at the end of this
1168
- // function, so it should be null right now. If it's not, later code may
1169
- // give confusing errors.
1170
- invariant(
1171
- ReactCurrentOwner.current == null,
1172
- '%s.render(): Render methods should be a pure function of props and ' +
1173
- 'state; triggering nested component updates from render is not ' +
1174
- 'allowed. If necessary, trigger nested updates in ' +
1175
- 'componentDidUpdate.',
1176
- this.constructor.displayName || 'ReactCompositeComponent'
1177
- );
1167
var renderedComponent;
1168
var previousContext = ReactContext.current;
1169
ReactContext.current = this._processChildContext(
src/core/ReactUpdates.js
+14
@@ -18,6 +18,7 @@
18
19
"use strict";
20
21
+var ReactCurrentOwner = require('ReactCurrentOwner');
22
var ReactPerf = require('ReactPerf');
23
24
var invariant = require('invariant');
@@ -103,6 +104,19 @@ function enqueueUpdate(component, callback) {
104
);
105
ensureBatchingStrategy();
106
107
+ // Various parts of our code (such as ReactCompositeComponent's
108
+ // _renderValidatedComponent) assume that calls to render aren't nested;
109
+ // verify that that's the case. (This is called by each top-level update
110
+ // function, like setProps, setState, forceUpdate, etc.; creation and
111
+ // destruction of top-level components is guarded in ReactMount.)
112
+ invariant(
113
+ ReactCurrentOwner.current == null,
114
+ 'enqueueUpdate(): Render methods should be a pure function of props ' +
115
+ 'and state; triggering nested component updates from render is not ' +
116
+ 'allowed. If necessary, trigger nested updates in ' +
117
+ 'componentDidUpdate.'
118
+ );
119
+
120
if (!batchingStrategy.isBatchingUpdates) {
121
component.performUpdateIfNecessary();
122
callback && callback.call(component);
src/core/__tests__/ReactCompositeComponent-test.js
+4
-4
@@ -1361,10 +1361,10 @@ describe('ReactCompositeComponent', function() {
1361
expect(() => {
1362
ReactTestUtils.renderIntoDocument(<Outer />);
1363
}).toThrow(
1364
- 'Invariant Violation: Inner.render(): Render methods should be a pure ' +
1365
- 'function of props and state; triggering nested component updates ' +
1366
- 'from render is not allowed. If necessary, trigger nested updates in ' +
1367
- 'componentDidUpdate.'
1364
+ 'Invariant Violation: _renderNewRootComponent(): Render methods should ' +
1365
+ 'be a pure function of props and state; triggering nested component ' +
1366
+ 'updates from render is not allowed. If necessary, trigger nested ' +
1367
+ 'updates in componentDidUpdate.'
1368
);
1369
});
1370