The library for web and native user interfaces.
Cleanup `ReactCurrentOwner` on Fatal
If a React component's render() fatals, it may contaminate ReactCurrentOwner. This will cause the owner to be set improperly for the next React.renderComponent() invocation (which causes an owner to be set when there shouldn't be one).
CommitSyncScript committed
Jun 4, 2013 at 12:52 UTC
a06de4bc4f5735aba767740d243b270e272ac6fa
2 files changed
+28
-2
src/core/ReactCompositeComponent.js
+9
-2
@@ -734,9 +734,16 @@ var ReactCompositeComponentMixin = {
734
* @private
735
*/
736
_renderValidatedComponent: function() {
737
+ var renderedComponent;
738
ReactCurrentOwner.current = this;
738
- var renderedComponent = this.render();
739
- ReactCurrentOwner.current = null;
739
+ try {
740
+ renderedComponent = this.render();
741
+ } catch (error) {
742
+ // IE8 requires `catch` in order to use `finally`.
743
+ throw error;
744
+ } finally {
745
+ ReactCurrentOwner.current = null;
746
+ }
747
invariant(
748
ReactComponent.isValidComponent(renderedComponent),
749
'%s.render(): A valid ReactComponent must be returned.',
src/core/__tests__/ReactCompositeComponent-test.js
+19
@@ -23,6 +23,7 @@ var MorphingComponent;
23
var MorphingAutoBindComponent;
24
var ChildUpdates;
25
var React;
26
+var ReactCurrentOwner;
27
var ReactProps;
28
var ReactTestUtils;
29
@@ -35,6 +36,7 @@ describe('ReactCompositeComponent', function() {
36
cx = require('cx');
37
reactComponentExpect = require('reactComponentExpect');
38
React = require('React');
39
+ ReactCurrentOwner = require('ReactCurrentOwner');
40
ReactProps = require('ReactProps');
41
ReactTestUtils = require('ReactTestUtils');
42
@@ -302,4 +304,21 @@ describe('ReactCompositeComponent', function() {
304
);
305
});
306
307
+ it('should cleanup even if render() fatals', function() {
308
+ var BadComponent = React.createClass({
309
+ render: function() {
310
+ throw new Error();
311
+ }
312
+ });
313
+ var instance = <BadComponent />;
314
+
315
+ expect(ReactCurrentOwner.current).toBe(null);
316
+
317
+ expect(function() {
318
+ ReactTestUtils.renderIntoDocument(instance);
319
+ }).toThrow();
320
+
321
+ expect(ReactCurrentOwner.current).toBe(null);
322
+ });
323
+
324
});