From 563fe764f4b8beb5d19e1abd24cd0afe9e69c900 Mon Sep 17 00:00:00 2001
From: Andrew Clark <git@andrewclark.io>
Date: Fri, 26 Jul 2019 14:06:40 -0700
Subject: [PATCH 1/2] Add failing test for #16215

Next commit fixes it.
---
 .../ReactSuspenseCallback-test.internal.js    | 67 +++++++++++++++++++
 1 file changed, 67 insertions(+)

diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.internal.js
index 8a6997107b47b..77daba9d97fcf 100644
--- a/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.internal.js
+++ b/packages/react-reconciler/src/__tests__/ReactSuspenseCallback-test.internal.js
@@ -238,4 +238,71 @@ describe('ReactSuspense', () => {
     expect(ops1).toEqual([]);
     expect(ops2).toEqual([]);
   });
+
+  if (__DEV__) {
+    it('regression test for #16215 that relies on implementation details', async () => {
+      // Regression test for https://github.com/facebook/react/pull/16215.
+      // The bug only happens if there's an error earlier in the commit phase.
+      // The first error is the one that gets thrown, so to observe the later
+      // error, I've mocked the ReactErrorUtils module.
+      //
+      // If this test starts failing because the implementation details change,
+      // you can probably just delete it. It's not worth the hassle.
+      jest.resetModules();
+
+      let errors = [];
+      let hasCaughtError = false;
+      jest.mock('shared/ReactErrorUtils', () => ({
+        invokeGuardedCallback(name, fn, context, ...args) {
+          try {
+            return fn.call(context, ...args);
+          } catch (error) {
+            hasCaughtError = true;
+            errors.push(error);
+          }
+        },
+        hasCaughtError() {
+          return hasCaughtError;
+        },
+        clearCaughtError() {
+          hasCaughtError = false;
+          return errors[errors.length - 1];
+        },
+      }));
+
+      ReactFeatureFlags = require('shared/ReactFeatureFlags');
+      ReactFeatureFlags.enableSuspenseCallback = true;
+
+      React = require('react');
+      ReactNoop = require('react-noop-renderer');
+      Scheduler = require('scheduler');
+
+      const {useEffect} = React;
+      const {PromiseComp} = createThenable();
+      function App() {
+        useEffect(() => {
+          Scheduler.unstable_yieldValue('Passive Effect');
+        });
+        return (
+          <React.Suspense
+            suspenseCallback={() => {
+              throw Error('Oops!');
+            }}
+            fallback="Loading...">
+            <PromiseComp />
+          </React.Suspense>
+        );
+      }
+      const root = ReactNoop.createRoot();
+      await ReactNoop.act(async () => {
+        root.render(<App />);
+        expect(Scheduler).toFlushAndThrow('Oops!');
+      });
+
+      // Should have only received a single error. Before the bug fix, there was
+      // also a second error related to the Suspense update queue.
+      expect(errors.length).toBe(1);
+      expect(errors[0].message).toEqual('Oops!');
+    });
+  }
 });

From a10010d45ed4d74b8756e71e83741c98d076b9ed Mon Sep 17 00:00:00 2001
From: Andrew Clark <git@andrewclark.io>
Date: Thu, 25 Jul 2019 17:49:57 -0700
Subject: [PATCH 2/2] [Bugfix] Check tag before calling hook effects

TODO: Test that triggers this
---
 .../src/ReactFiberCommitWork.js                 | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js
index d840173c1195f..8f3156041c0fa 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.js
@@ -55,10 +55,12 @@ import {
   clearCaughtError,
 } from 'shared/ReactErrorUtils';
 import {
+  NoEffect,
   ContentReset,
   Placement,
   Snapshot,
   Update,
+  Passive,
 } from 'shared/ReactSideEffectTags';
 import getComponentName from 'shared/getComponentName';
 import invariant from 'shared/invariant';
@@ -383,8 +385,19 @@ function commitHookEffectList(
 }
 
 export function commitPassiveHookEffects(finishedWork: Fiber): void {
-  commitHookEffectList(UnmountPassive, NoHookEffect, finishedWork);
-  commitHookEffectList(NoHookEffect, MountPassive, finishedWork);
+  if ((finishedWork.effectTag & Passive) !== NoEffect) {
+    switch (finishedWork.tag) {
+      case FunctionComponent:
+      case ForwardRef:
+      case SimpleMemoComponent: {
+        commitHookEffectList(UnmountPassive, NoHookEffect, finishedWork);
+        commitHookEffectList(NoHookEffect, MountPassive, finishedWork);
+        break;
+      }
+      default:
+        break;
+    }
+  }
 }
 
 function commitLifeCycles(