[SuspenseList] Bug fix: Reset renderState when bailing out (#16278)
If there are adjacent updates we bail out of rendering the suspense list at all but we may still complete the node. We need to reset the render state in that case. I restructured so that this is in the same code path so we don't forget it in the future.
Sebastian Markbåge committed
Aug 1, 2019 at 22:02 UTC
a53f5cc22eab9617a1d5473e16ce872c7158edef
2 files changed
+72
-21
packages/react-reconciler/src/ReactFiberBeginWork.js
+26
-21
@@ -2782,29 +2782,26 @@ function beginWork(
2782
const didSuspendBefore =
2783
(current.effectTag & DidCapture) !== NoEffect;
2784
2785
- const childExpirationTime = workInProgress.childExpirationTime;
2786
- if (childExpirationTime < renderExpirationTime) {
2785
+ const hasChildWork =
2786
+ workInProgress.childExpirationTime >= renderExpirationTime;
2787
+
2788
+ if (didSuspendBefore) {
2789
+ if (hasChildWork) {
2790
+ // If something was in fallback state last time, and we have all the
2791
+ // same children then we're still in progressive loading state.
2792
+ // Something might get unblocked by state updates or retries in the
2793
+ // tree which will affect the tail. So we need to use the normal
2794
+ // path to compute the correct tail.
2795
+ return updateSuspenseListComponent(
2796
+ current,
2797
+ workInProgress,
2798
+ renderExpirationTime,
2799
+ );
2800
+ }
2801
// If none of the children had any work, that means that none of
2802
// them got retried so they'll still be blocked in the same way
2803
// as before. We can fast bail out.
2790
- pushSuspenseContext(workInProgress, suspenseStackCursor.current);
2791
- if (didSuspendBefore) {
2792
- workInProgress.effectTag |= DidCapture;
2793
- }
2794
- return null;
2795
- }
2796
-
2797
- if (didSuspendBefore) {
2798
- // If something was in fallback state last time, and we have all the
2799
- // same children then we're still in progressive loading state.
2800
- // Something might get unblocked by state updates or retries in the
2801
- // tree which will affect the tail. So we need to use the normal
2802
- // path to compute the correct tail.
2803
- return updateSuspenseListComponent(
2804
- current,
2805
- workInProgress,
2806
- renderExpirationTime,
2807
- );
2804
+ workInProgress.effectTag |= DidCapture;
2805
}
2806
2807
// If nothing suspended before and we're rendering the same children,
@@ -2818,7 +2815,15 @@ function beginWork(
2815
renderState.tail = null;
2816
}
2817
pushSuspenseContext(workInProgress, suspenseStackCursor.current);
2821
- break;
2818
+
2819
+ if (hasChildWork) {
2820
+ break;
2821
+ } else {
2822
+ // If none of the children had any work, that means that none of
2823
+ // them got retried so they'll still be blocked in the same way
2824
+ // as before. We can fast bail out.
2825
+ return null;
2826
+ }
2827
}
2828
}
2829
return bailoutOnAlreadyFinishedWork(
packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js
+46
@@ -1826,4 +1826,50 @@ describe('ReactSuspenseList', () => {
1826
</Fragment>,
1827
);
1828
});
1829
+
1830
+ it('can do unrelated adjacent updates', async () => {
1831
+ let updateAdjacent;
1832
+ function Adjacent() {
1833
+ let [text, setText] = React.useState('-');
1834
+ updateAdjacent = setText;
1835
+ return <Text text={text} />;
1836
+ }
1837
+
1838
+ function Foo() {
1839
+ return (
1840
+ <div>
1841
+ <SuspenseList revealOrder="forwards">
1842
+ <Text text="A" />
1843
+ <Text text="B" />
1844
+ </SuspenseList>
1845
+ <Adjacent />
1846
+ </div>
1847
+ );
1848
+ }
1849
+
1850
+ ReactNoop.render(<Foo />);
1851
+
1852
+ expect(Scheduler).toFlushAndYield(['A', 'B', '-']);
1853
+
1854
+ expect(ReactNoop).toMatchRenderedOutput(
1855
+ <div>
1856
+ <span>A</span>
1857
+ <span>B</span>
1858
+ <span>-</span>
1859
+ </div>,
1860
+ );
1861
+
1862
+ // Update the row adjacent to the list
1863
+ ReactNoop.act(() => updateAdjacent('C'));
1864
+
1865
+ expect(Scheduler).toHaveYielded(['C']);
1866
+
1867
+ expect(ReactNoop).toMatchRenderedOutput(
1868
+ <div>
1869
+ <span>A</span>
1870
+ <span>B</span>
1871
+ <span>C</span>
1872
+ </div>,
1873
+ );
1874
+ });
1875
});