@samitouri / QOS-React / commits / e16cdd5b17

Always bail out timed out children even if they receive an update (#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

Andrew Clark committed Oct 19, 2018 at 20:31 UTC e16cdd5b178a82db09f51349fe6c636a5072516f
2 files changed +51
packages/react-reconciler/src/ReactFiberBeginWork.js
+3
@@ -1080,6 +1080,7 @@ function updateSuspenseComponent(
1080 ));
1081 fallbackChildFragment.effectTag |= Placement;
1082 child = primaryChildFragment;
1083 + primaryChildFragment.childExpirationTime = NoWork;
1084 // Skip the primary children, and continue working on the
1085 // fallback children.
1086 next = fallbackChildFragment;
@@ -1134,6 +1135,7 @@ function updateSuspenseComponent(
1135 ));
1136 fallbackChildFragment.effectTag |= Placement;
1137 child = primaryChildFragment;
1138 + primaryChildFragment.childExpirationTime = NoWork;
1139 // Skip the primary children, and continue working on the
1140 // fallback children.
1141 next = fallbackChildFragment;
@@ -1435,6 +1437,7 @@ function beginWork(
1437 const nextState = workInProgress.memoizedState;
1438 const nextDidTimeout = nextState !== null && nextState.didTimeout;
1439 if (nextDidTimeout) {
1440 + child.childExpirationTime = NoWork;
1441 return child.sibling;
1442 } else {
1443 return child;
packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js
+48
@@ -435,5 +435,53 @@ describe('ReactSuspense', () => {
435 ]);
436 expect(root).toMatchRenderedOutput('AB:2C');
437 });
438 +
439 + it('bails out on timed-out primary children even if they receive an update', () => {
440 + let instance;
441 + class Stateful extends React.Component {
442 + state = {step: 1};
443 + render() {
444 + instance = this;
445 + return <Text text="Stateful" />;
446 + }
447 + }
448 +
449 + function App(props) {
450 + return (
451 + <Suspense fallback={<Text text="Loading..." />}>
452 + <Stateful />
453 + <AsyncText ms={1000} text={props.text} />
454 + </Suspense>
455 + );
456 + }
457 +
458 + const root = ReactTestRenderer.create(<App text="A" />);
459 +
460 + expect(ReactTestRenderer).toHaveYielded([
461 + 'Stateful',
462 + 'Suspend! [A]',
463 + 'Loading...',
464 + ]);
465 +
466 + jest.advanceTimersByTime(1000);
467 + expect(ReactTestRenderer).toHaveYielded(['Promise resolved [A]', 'A']);
468 + expect(root).toMatchRenderedOutput('StatefulA');
469 +
470 + root.update(<App text="B" />);
471 + expect(ReactTestRenderer).toHaveYielded([
472 + 'Stateful',
473 + 'Suspend! [B]',
474 + 'Loading...',
475 + ]);
476 +
477 + instance.setState({step: 2});
478 +
479 + jest.advanceTimersByTime(1000);
480 + expect(ReactTestRenderer).toHaveYielded([
481 + 'Promise resolved [B]',
482 + 'Stateful',
483 + 'B',
484 + ]);
485 + });
486 });
487 });