From 2c560cb9952c05fb0a45aed87723c9a8bf6ff8d4 Mon Sep 17 00:00:00 2001
From: Dan Abramov <dan.abramov@gmail.com>
Date: Thu, 19 Jul 2018 02:16:10 +0100
Subject: [PATCH] Fix unwinding starting with a wrong Fiber on error in the
 complete phase (#13237)

* Add a repro case for profiler unwinding

This currently fails the tests due to an unexpected warning.

* Add a regression test for context stack

* Simplify the first test case

* Update nextUnitOfWork inside completeUnitOfWork()

The bug was caused by a structure like this:

    </Provider>
  </div>
</errorInCompletePhase>

We forgot to update nextUnitOfWork so it was still pointing at Provider when errorInCompletePhase threw. As a result, we would try to unwind from Provider (rather than from errorInCompletePhase), and thus pop the Provider twice.
---
 .../src/ReactFiberScheduler.js                |  3 ++-
 .../ReactNewContext-test.internal.js          | 21 +++++++++++++++++++
 .../__tests__/ReactProfiler-test.internal.js  | 11 ++++++++++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js
index 13a321f60d2e7..06b0041a76cf6 100644
--- a/packages/react-reconciler/src/ReactFiberScheduler.js
+++ b/packages/react-reconciler/src/ReactFiberScheduler.js
@@ -791,11 +791,12 @@ function completeUnitOfWork(workInProgress: Fiber): Fiber | null {
 
     if ((workInProgress.effectTag & Incomplete) === NoEffect) {
       // This fiber completed.
-      let next = completeWork(
+      nextUnitOfWork = completeWork(
         current,
         workInProgress,
         nextRenderExpirationTime,
       );
+      let next = nextUnitOfWork;
       stopWorkTimer(workInProgress);
       resetExpirationTime(workInProgress, nextRenderExpirationTime);
       if (__DEV__) {
diff --git a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js
index 5b56fbf28774d..09f9d6ec6f336 100644
--- a/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js
+++ b/packages/react-reconciler/src/__tests__/ReactNewContext-test.internal.js
@@ -1128,6 +1128,27 @@ describe('ReactNewContext', () => {
     ]);
   });
 
+  it('unwinds after errors in complete phase', () => {
+    const Context = React.createContext(0);
+
+    // This is a regression test for stack misalignment
+    // caused by unwinding the context from wrong point.
+    ReactNoop.render(
+      <errorInCompletePhase>
+        <Context.Provider />
+      </errorInCompletePhase>,
+    );
+    expect(ReactNoop.flush).toThrow('Error in host config.');
+
+    ReactNoop.render(
+      <Context.Provider value={10}>
+        <Context.Consumer>{value => <span prop={value} />}</Context.Consumer>
+      </Context.Provider>,
+    );
+    ReactNoop.flush();
+    expect(ReactNoop.getChildren()).toEqual([span(10)]);
+  });
+
   describe('fuzz test', () => {
     const Fragment = React.Fragment;
     const contextKeys = ['A', 'B', 'C', 'D', 'E', 'F', 'G'];
diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js
index 9c1bec7218e08..39a512a0c59ab 100644
--- a/packages/react/src/__tests__/ReactProfiler-test.internal.js
+++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js
@@ -999,6 +999,17 @@ describe('Profiler', () => {
             );
             expect(ReactNoop.flush).toThrow('Error in host config.');
 
+            // A similar case we've seen caused by an invariant in ReactDOM.
+            // It didn't reproduce without a host component inside.
+            ReactNoop.render(
+              <React.unstable_Profiler id="profiler" onRender={jest.fn()}>
+                <errorInCompletePhase>
+                  <span>hi</span>
+                </errorInCompletePhase>
+              </React.unstable_Profiler>,
+            );
+            expect(ReactNoop.flush).toThrow('Error in host config.');
+
             // So long as the profiler timer's fiber stack is reset correctly,
             // Subsequent renders should not error.
             ReactNoop.render(