Fix potential memory leak when unmounting
Fixes #781.
Ben Alpert committed
Jan 2, 2014 at 16:36 UTC
e2f094614f8e8dd5111f19690ad32cea00d407f2
3 files changed
+42
-2
src/core/ReactCompositeComponent.js
+2
-1
@@ -736,10 +736,11 @@ var ReactCompositeComponentMixin = {
736
737
this._defaultProps = null;
738
739
- ReactComponent.Mixin.unmountComponent.call(this);
739
this._renderedComponent.unmountComponent();
740
this._renderedComponent = null;
741
742
+ ReactComponent.Mixin.unmountComponent.call(this);
743
+
744
if (this.refs) {
745
this.refs = null;
746
}
src/core/ReactDOMComponent.js
+1
-1
@@ -364,9 +364,9 @@ ReactDOMComponent.Mixin = {
364
* @internal
365
*/
366
unmountComponent: function() {
367
+ this.unmountChildren();
368
ReactEventEmitter.deleteAllListeners(this._rootNodeID);
369
ReactComponent.Mixin.unmountComponent.call(this);
369
- this.unmountChildren();
370
}
371
372
};
src/core/__tests__/ReactCompositeComponent-test.js
+39
@@ -22,6 +22,7 @@
22
var MorphingComponent;
23
var ChildUpdates;
24
var React;
25
+var ReactComponent;
26
var ReactCurrentOwner;
27
var ReactPropTypes;
28
var ReactTestUtils;
@@ -40,6 +41,7 @@ describe('ReactCompositeComponent', function() {
41
42
reactComponentExpect = require('reactComponentExpect');
43
React = require('React');
44
+ ReactComponent = require('ReactComponent');
45
ReactCurrentOwner = require('ReactCurrentOwner');
46
ReactDoNotBindDeprecated = require('ReactDoNotBindDeprecated');
47
ReactPropTypes = require('ReactPropTypes');
@@ -506,6 +508,43 @@ describe('ReactCompositeComponent', function() {
508
});
509
});
510
511
+ it('should call componentWillUnmount before unmounting', function() {
512
+ var container = document.createElement('div');
513
+ var innerUnmounted = false;
514
+
515
+ spyOn(ReactComponent, 'unmountIDFromEnvironment').andCallThrough();
516
+
517
+ var Component = React.createClass({
518
+ render: function() {
519
+ return <div>
520
+ <Inner />
521
+ </div>;
522
+ }
523
+ });
524
+ var Inner = React.createClass({
525
+ componentWillUnmount: function() {
526
+ // It's important that unmountIDFromEnvironment (which clears
527
+ // ReactMount's node cache) be called after any component lifecycle
528
+ // methods, because a componentWillMount implementation is likely call
529
+ // this.getDOMNode(), which will repopulate the node cache after it's
530
+ // been cleared, causing a memory leak.
531
+ expect(ReactComponent.unmountIDFromEnvironment.callCount).toBe(0);
532
+ innerUnmounted = true;
533
+ },
534
+ render: function() {
535
+ return <div />;
536
+ }
537
+ });
538
+
539
+ React.renderComponent(<Component />, container);
540
+ React.unmountComponentAtNode(container);
541
+ expect(innerUnmounted).toBe(true);
542
+
543
+ // <Component />, <Inner />, and both <div /> elements each call
544
+ // unmountIDFromEnvironment, for a total of 4.
545
+ expect(ReactComponent.unmountIDFromEnvironment.callCount).toBe(4);
546
+ });
547
+
548
it('should detect valid CompositeComponent classes', function() {
549
var Component = React.createClass({
550
render: function() {