From a06de4bc4f5735aba767740d243b270e272ac6fa Mon Sep 17 00:00:00 2001 From: CommitSyncScript Date: Tue, 4 Jun 2013 12:52:30 -0700 Subject: [PATCH] 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). --- src/core/ReactCompositeComponent.js | 11 +++++++++-- .../__tests__/ReactCompositeComponent-test.js | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/core/ReactCompositeComponent.js b/src/core/ReactCompositeComponent.js index fdf3d8e7ae..468c200511 100644 --- a/src/core/ReactCompositeComponent.js +++ b/src/core/ReactCompositeComponent.js @@ -734,9 +734,16 @@ var ReactCompositeComponentMixin = { * @private */ _renderValidatedComponent: function() { + var renderedComponent; ReactCurrentOwner.current = this; - var renderedComponent = this.render(); - ReactCurrentOwner.current = null; + try { + renderedComponent = this.render(); + } catch (error) { + // IE8 requires `catch` in order to use `finally`. + throw error; + } finally { + ReactCurrentOwner.current = null; + } invariant( ReactComponent.isValidComponent(renderedComponent), '%s.render(): A valid ReactComponent must be returned.', diff --git a/src/core/__tests__/ReactCompositeComponent-test.js b/src/core/__tests__/ReactCompositeComponent-test.js index f2b666321c..178edaee1d 100644 --- a/src/core/__tests__/ReactCompositeComponent-test.js +++ b/src/core/__tests__/ReactCompositeComponent-test.js @@ -23,6 +23,7 @@ var MorphingComponent; var MorphingAutoBindComponent; var ChildUpdates; var React; +var ReactCurrentOwner; var ReactProps; var ReactTestUtils; @@ -35,6 +36,7 @@ describe('ReactCompositeComponent', function() { cx = require('cx'); reactComponentExpect = require('reactComponentExpect'); React = require('React'); + ReactCurrentOwner = require('ReactCurrentOwner'); ReactProps = require('ReactProps'); ReactTestUtils = require('ReactTestUtils'); @@ -302,4 +304,21 @@ describe('ReactCompositeComponent', function() { ); }); + it('should cleanup even if render() fatals', function() { + var BadComponent = React.createClass({ + render: function() { + throw new Error(); + } + }); + var instance = ; + + expect(ReactCurrentOwner.current).toBe(null); + + expect(function() { + ReactTestUtils.renderIntoDocument(instance); + }).toThrow(); + + expect(ReactCurrentOwner.current).toBe(null); + }); + });