From cfefc81ab2f5103bedc9e45701e8c00c0689f499 Mon Sep 17 00:00:00 2001
From: Andrew Clark <git@andrewclark.io>
Date: Fri, 17 Apr 2020 16:32:55 -0700
Subject: [PATCH] Fix for #18657 (#18663)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Failing test for #18657

* Remove incorrect priority check

I think this was just poor factoring on my part in #18411. Honestly it
doesn't make much sense to me, but my best guess is that I must have
thought that when `baseTime > currentChildExpirationTime`, the function
would fall through to the
`currentChildExpirationTime < renderExpirationTime` branch below.

Really I think just made an oopsie.

Regardless, this logic is galaxy brainéd. A goal of the Lanes refactor
I'm working on is to make these types of checks -- is there remaining
work in this tree? -- a lot easier to think about. Hopefully.
---
 .../src/ReactFiberBeginWork.new.js            |  6 +--
 .../src/ReactFiberBeginWork.old.js            |  6 +--
 .../ReactSuspenseWithNoopRenderer-test.js     | 49 +++++++++++++++++++
 3 files changed, 51 insertions(+), 10 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
index 338191f23666..b88f17c1cb16 100644
--- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
@@ -1681,11 +1681,7 @@ function getRemainingWorkInPrimaryTree(
     // This boundary already timed out. Check if this render includes the level
     // that previously suspended.
     const baseTime = currentSuspenseState.baseTime;
-    if (
-      baseTime !== NoWork &&
-      baseTime < renderExpirationTime &&
-      baseTime > currentChildExpirationTime
-    ) {
+    if (baseTime !== NoWork && baseTime < renderExpirationTime) {
       // There's pending work at a lower level that might now be unblocked.
       return baseTime;
     }
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js
index 4fde99d80c71..6fbfd5cd9707 100644
--- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js
+++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js
@@ -1681,11 +1681,7 @@ function getRemainingWorkInPrimaryTree(
     // This boundary already timed out. Check if this render includes the level
     // that previously suspended.
     const baseTime = currentSuspenseState.baseTime;
-    if (
-      baseTime !== NoWork &&
-      baseTime < renderExpirationTime &&
-      baseTime > currentChildExpirationTime
-    ) {
+    if (baseTime !== NoWork && baseTime < renderExpirationTime) {
       // There's pending work at a lower level that might now be unblocked.
       return baseTime;
     }
diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js
index cfe3d244b510..5f5469bcd904 100644
--- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js
@@ -3869,4 +3869,53 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       expect(root).toMatchRenderedOutput(<span prop="b" />);
     });
   });
+
+  it('regression: #18657', async () => {
+    const {useState} = React;
+
+    let setText;
+    function App() {
+      const [text, _setText] = useState('A');
+      setText = _setText;
+      return <AsyncText text={text} />;
+    }
+
+    const root = ReactNoop.createRoot();
+    await ReactNoop.act(async () => {
+      await resolveText('A');
+      root.render(
+        <Suspense fallback={<Text text="Loading..." />}>
+          <App />
+        </Suspense>,
+      );
+    });
+    expect(Scheduler).toHaveYielded(['A']);
+    expect(root).toMatchRenderedOutput(<span prop="A" />);
+
+    await ReactNoop.act(async () => {
+      setText('B');
+      Scheduler.unstable_runWithPriority(
+        Scheduler.unstable_IdlePriority,
+        () => {
+          setText('B');
+        },
+      );
+      // Suspend the first update. The second update doesn't run because it has
+      // Idle priority.
+      expect(Scheduler).toFlushAndYield(['Suspend! [B]', 'Loading...']);
+
+      // Commit the fallback. Now we'll try working on Idle.
+      jest.runAllTimers();
+
+      // It also suspends.
+      expect(Scheduler).toFlushAndYield(['Suspend! [B]']);
+    });
+
+    await ReactNoop.act(async () => {
+      setText('B');
+      await resolveText('B');
+    });
+    expect(Scheduler).toHaveYielded(['Promise resolved [B]', 'B']);
+    expect(root).toMatchRenderedOutput(<span prop="B" />);
+  });
 });