ComponentWillUnmount should only ever be invoked once

facebook · Apr 25, 2016 · a8e64f1 · a8e64f1
1 parent 7dbc95f
commit a8e64f1
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 1 deletion.
diff --git a/src/renderers/shared/reconciler/ReactCompositeComponent.js b/src/renderers/shared/reconciler/ReactCompositeComponent.js
@@ -132,6 +132,9 @@ var ReactCompositeComponentMixin = {
 
     // See ReactUpdates and ReactUpdateQueue.
     this._pendingCallbacks = null;
+
+    // ComponentWillUnmount shall only be called once
+    this._calledComponentWillUnmount = false;
   },
 
   /**
@@ -405,7 +408,8 @@ var ReactCompositeComponentMixin = {
     }
     var inst = this._instance;
 
-    if (inst.componentWillUnmount) {
+    if (inst.componentWillUnmount && !inst._calledComponentWillUnmount) {
+      inst._calledComponentWillUnmount = true;
       if (safely) {
         var name = this.getName() + '.componentWillUnmount()';
         ReactErrorUtils.invokeGuardedCallback(name, inst.componentWillUnmount.bind(inst));

diff --git a/src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js b/src/renderers/shared/reconciler/__tests__/ReactCompositeComponent-test.js
@@ -1303,4 +1303,47 @@ describe('ReactCompositeComponent', function() {
 
   });
 
+  it('should only call componentWillUnmount once', function() {
+    var app;
+    var stage = 1;
+    var count = 0;
+
+    class App extends React.Component {
+      render() {
+        if (stage === 1) {
+          return <UnunmountableComponent />;
+        } else {
+          return null;
+        }
+      }
+    }
+
+    class UnunmountableComponent extends React.Component {
+      componentWillUnmount() {
+        app.setState({});
+        count++;
+        throw Error('always fails');
+      }
+
+      render() {
+        return <div>Hello {this.props.name}</div>;
+      }
+    }
+
+    var container = document.createElement('div');
+
+    var setRef = (ref) => {
+      if (ref) {
+        app = ref;
+      }
+    };
+
+    expect(function() {
+      ReactDOM.render(<App ref={setRef} />, container);
+      stage = 2;
+      ReactDOM.render(<App ref={setRef} />, container);
+    }).toThrow();
+    expect(count).toBe(1);
+  });
+
 });