Always bail out timed out children even if they receive an update (fa…

…cebook#13901) * Always bail out timed out children even if they receive an update The fragment that wraps timed-out children should always have an expiration time of NoWork. * Don't need to set expirationTime, only childExpirationTime
jetoneza · Jan 23, 2019 · 37e3209 · 37e3209
1 parent 0445da5
commit 37e3209
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 0 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js
@@ -1080,6 +1080,7 @@ function updateSuspenseComponent(
         ));
         fallbackChildFragment.effectTag |= Placement;
         child = primaryChildFragment;
+        primaryChildFragment.childExpirationTime = NoWork;
         // Skip the primary children, and continue working on the
         // fallback children.
         next = fallbackChildFragment;
@@ -1134,6 +1135,7 @@ function updateSuspenseComponent(
         ));
         fallbackChildFragment.effectTag |= Placement;
         child = primaryChildFragment;
+        primaryChildFragment.childExpirationTime = NoWork;
         // Skip the primary children, and continue working on the
         // fallback children.
         next = fallbackChildFragment;
@@ -1435,6 +1437,7 @@ function beginWork(
             const nextState = workInProgress.memoizedState;
             const nextDidTimeout = nextState !== null && nextState.didTimeout;
             if (nextDidTimeout) {
+              child.childExpirationTime = NoWork;
               return child.sibling;
             } else {
               return child;

diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js
@@ -435,5 +435,53 @@ describe('ReactSuspense', () => {
       ]);
       expect(root).toMatchRenderedOutput('AB:2C');
     });
+
+    it('bails out on timed-out primary children even if they receive an update', () => {
+      let instance;
+      class Stateful extends React.Component {
+        state = {step: 1};
+        render() {
+          instance = this;
+          return <Text text="Stateful" />;
+        }
+      }
+
+      function App(props) {
+        return (
+          <Suspense fallback={<Text text="Loading..." />}>
+            <Stateful />
+            <AsyncText ms={1000} text={props.text} />
+          </Suspense>
+        );
+      }
+
+      const root = ReactTestRenderer.create(<App text="A" />);
+
+      expect(ReactTestRenderer).toHaveYielded([
+        'Stateful',
+        'Suspend! [A]',
+        'Loading...',
+      ]);
+
+      jest.advanceTimersByTime(1000);
+      expect(ReactTestRenderer).toHaveYielded(['Promise resolved [A]', 'A']);
+      expect(root).toMatchRenderedOutput('StatefulA');
+
+      root.update(<App text="B" />);
+      expect(ReactTestRenderer).toHaveYielded([
+        'Stateful',
+        'Suspend! [B]',
+        'Loading...',
+      ]);
+
+      instance.setState({step: 2});
+
+      jest.advanceTimersByTime(1000);
+      expect(ReactTestRenderer).toHaveYielded([
+        'Promise resolved [B]',
+        'Stateful',
+        'B',
+      ]);
+    });
   });
 });