Reset instance vars before calling commit phase lifecycles

facebook · Aug 17, 2017 · dd70c0a · dd70c0a
1 parent 072b1d9
commit dd70c0a
Show file tree

Hide file tree

Showing 2 changed files with 78 additions and 4 deletions.
diff --git a/src/renderers/shared/fiber/ReactFiberCommitWork.js b/src/renderers/shared/fiber/ReactFiberCommitWork.js
@@ -63,10 +63,30 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
     getPublicInstance,
   } = config;
 
+  function callInstanceMethod<D, E, F>(
+    instance: any,
+    method: (d: D, e: E, f: F) => mixed,
+    props: mixed,
+    state: mixed,
+    d: D,
+    e: E,
+    f: F,
+  ): void {
+    instance.props = props;
+    instance.state = state;
+    const methodArgs = Array.prototype.slice.call(arguments, 4);
+    method.apply(instance, methodArgs);
+  }
+
   if (__DEV__) {
     var callComponentWillUnmountWithTimerInDev = function(current, instance) {
       startPhaseTimer(current, 'componentWillUnmount');
-      instance.componentWillUnmount();
+      callInstanceMethod(
+        instance,
+        instance.componentWillUnmount,
+        current.memoizedProps,
+        current.memoizedState,
+      );
       stopPhaseTimer();
     };
   }
@@ -87,7 +107,12 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
       }
     } else {
       try {
-        instance.componentWillUnmount();
+        callInstanceMethod(
+          instance,
+          instance.componentWillUnmount,
+          current.memoizedProps,
+          current.memoizedState,
+        );
       } catch (unmountError) {
         captureError(current, unmountError);
       }
@@ -495,7 +520,12 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
             if (__DEV__) {
               startPhaseTimer(finishedWork, 'componentDidMount');
             }
-            instance.componentDidMount();
+            callInstanceMethod(
+              instance,
+              instance.componentDidMount,
+              finishedWork.memoizedProps,
+              finishedWork.memoizedState,
+            );
             if (__DEV__) {
               stopPhaseTimer();
             }
@@ -505,7 +535,14 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
             if (__DEV__) {
               startPhaseTimer(finishedWork, 'componentDidUpdate');
             }
-            instance.componentDidUpdate(prevProps, prevState);
+            callInstanceMethod(
+              instance,
+              instance.componentDidUpdate,
+              finishedWork.memoizedProps,
+              finishedWork.memoizedState,
+              prevProps,
+              prevState,
+            );
             if (__DEV__) {
               stopPhaseTimer();
             }

diff --git a/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js b/src/renderers/shared/fiber/__tests__/ReactIncrementalErrorHandling-test.js
@@ -1123,4 +1123,41 @@ describe('ReactIncrementalErrorHandling', () => {
       );
     });
   });
+
+  it('resets instance variables before unmounting failed node', () => {
+    spyOn(console, 'error');
+
+    class ErrorBoundary extends React.Component {
+      state = {error: null};
+      componentDidCatch(error) {
+        this.setState({error});
+      }
+      render() {
+        return this.state.error ? null : this.props.children;
+      }
+    }
+    class Foo extends React.Component {
+      state = {step: 0};
+      componentDidMount() {
+        this.setState({step: 1});
+      }
+      componentWillUnmount() {
+        ReactNoop.yield('componentWillUnmount: ' + this.state.step);
+      }
+      render() {
+        ReactNoop.yield('render: ' + this.state.step);
+        if (this.state.step > 0) {
+          throw new Error('oops');
+        }
+        return null;
+      }
+    }
+
+    ReactNoop.render(<ErrorBoundary><Foo /></ErrorBoundary>);
+    expect(ReactNoop.flush()).toEqual([
+      'render: 0',
+      'render: 1',
+      'componentWillUnmount: 0',
+    ]);
+  });
 });