Merge pull request #786 from spicyj/gh-781

Fix potential memory leak when unmounting

src/core/ReactCompositeComponent.js

@@ -736,10 +736,11 @@ var ReactCompositeComponentMixin = {
 
     this._defaultProps = null;
 
-    ReactComponent.Mixin.unmountComponent.call(this);
     this._renderedComponent.unmountComponent();
     this._renderedComponent = null;
 
+    ReactComponent.Mixin.unmountComponent.call(this);
+
     if (this.refs) {
       this.refs = null;
     }

src/core/ReactDOMComponent.js

@@ -380,9 +380,9 @@ ReactDOMComponent.Mixin = {
    * @internal
    */
   unmountComponent: function() {
+    this.unmountChildren();
     ReactEventEmitter.deleteAllListeners(this._rootNodeID);
     ReactComponent.Mixin.unmountComponent.call(this);
-    this.unmountChildren();
   }
 
 };

src/core/__tests__/ReactCompositeComponent-test.js

@@ -22,6 +22,7 @@
 var MorphingComponent;
 var ChildUpdates;
 var React;
+var ReactComponent;
 var ReactCurrentOwner;
 var ReactPropTypes;
 var ReactTestUtils;
@@ -40,6 +41,7 @@ describe('ReactCompositeComponent', function() {
 
     reactComponentExpect = require('reactComponentExpect');
     React = require('React');
+    ReactComponent = require('ReactComponent');
     ReactCurrentOwner = require('ReactCurrentOwner');
     ReactDoNotBindDeprecated = require('ReactDoNotBindDeprecated');
     ReactPropTypes = require('ReactPropTypes');
@@ -583,6 +585,42 @@ describe('ReactCompositeComponent', function() {
     });
   });
 
+  it('should call componentWillUnmount before unmounting', function() {
+    var container = document.createElement('div');
+    var innerUnmounted = false;
+
+    spyOn(ReactMount, 'purgeID').andCallThrough();
+
+    var Component = React.createClass({
+      render: function() {
+        return <div>
+          <Inner />
+        </div>;
+      }
+    });
+    var Inner = React.createClass({
+      componentWillUnmount: function() {
+        // It's important that ReactMount.purgeID be called after any component
+        // lifecycle methods, because a componentWillMount implementation is
+        // likely call this.getDOMNode(), which will repopulate the node cache
+        // after it's been cleared, causing a memory leak.
+        expect(ReactMount.purgeID.callCount).toBe(0);
+        innerUnmounted = true;
+      },
+      render: function() {
+        return <div />;
+      }
+    });
+
+    React.renderComponent(<Component />, container);
+    React.unmountComponentAtNode(container);
+    expect(innerUnmounted).toBe(true);
+
+    // <Component />, <Inner />, and both <div /> elements each call
+    // unmountIDFromEnvironment which calls purgeID, for a total of 4.
+    expect(ReactMount.purgeID.callCount).toBe(4);
+  });
+
   it('should detect valid CompositeComponent classes', function() {
     var Component = React.createClass({
       render: function() {