Skip to content

Commit 45df778

Browse files
acdliteacdlite
committed
Suspense component does not capture if fallback is not defined
A missing fallback prop means the exception should propagate to the next parent (like a rethrow). That way a Suspense component can specify other props like maxDuration without needing to provide a fallback, too. Closes #13864
1 parent 4f0bd45 commit 45df778

File tree

4 files changed

+88
-29
lines changed

4 files changed

+88
-29
lines changed

‎packages/react-reconciler/src/ReactFiberUnwindWork.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,8 +211,9 @@ function throwException(
211211
workInProgress = returnFiber;
212212
do {
213213
if (workInProgress.tag === SuspenseComponent) {
214+
const fallback = workInProgress.memoizedProps.fallback;
214215
const didTimeout = workInProgress.memoizedState;
215-
if (!didTimeout) {
216+
if (fallback !== undefined && !didTimeout) {
216217
// Found the nearest boundary.
217218

218219
// If the boundary is not in concurrent mode, we should not suspend, and

‎packages/react-reconciler/src/__tests__/ReactPure-test.internal.js‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,11 +57,11 @@ describe('pure', () => {
5757
Counter = pure(Counter);
5858

5959
ReactNoop.render(
60-
<Suspense>
60+
<Suspense fallback={<Text text="Loading..." />}>
6161
<Counter count={0} />
6262
</Suspense>,
6363
);
64-
expect(ReactNoop.flush()).toEqual([]);
64+
expect(ReactNoop.flush()).toEqual(['Loading...']);
6565
await Promise.resolve();
6666
expect(ReactNoop.flush()).toEqual([0]);
6767
expect(ReactNoop.getChildren()).toEqual([span(0)]);
@@ -107,7 +107,7 @@ describe('pure', () => {
107107
state = {count: 0};
108108
render() {
109109
return (
110-
<Suspense>
110+
<Suspense fallback={<Text text="Loading..." />}>
111111
<CountContext.Provider value={this.state.count}>
112112
<Counter label="Count" />
113113
</CountContext.Provider>
@@ -118,7 +118,7 @@ describe('pure', () => {
118118

119119
const parent = React.createRef(null);
120120
ReactNoop.render(<Parent ref={parent} />);
121-
expect(ReactNoop.flush()).toEqual([]);
121+
expect(ReactNoop.flush()).toEqual(['Loading...']);
122122
await Promise.resolve();
123123
expect(ReactNoop.flush()).toEqual(['Count: 0']);
124124
expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
@@ -148,11 +148,11 @@ describe('pure', () => {
148148
});
149149

150150
ReactNoop.render(
151-
<Suspense>
151+
<Suspense fallback={<Text text="Loading..." />}>
152152
<Counter count={0} />
153153
</Suspense>,
154154
);
155-
expect(ReactNoop.flush()).toEqual([]);
155+
expect(ReactNoop.flush()).toEqual(['Loading...']);
156156
await Promise.resolve();
157157
expect(ReactNoop.flush()).toEqual([0]);
158158
expect(ReactNoop.getChildren()).toEqual([span(0)]);
@@ -204,7 +204,7 @@ describe('pure', () => {
204204
const divRef = React.createRef();
205205

206206
ReactNoop.render(
207-
<Suspense>
207+
<Suspense fallback={<Text text="Loading..." />}>
208208
<Transparent ref={divRef} />
209209
</Suspense>,
210210
);
@@ -224,11 +224,11 @@ describe('pure', () => {
224224
const divRef2 = React.createRef();
225225

226226
ReactNoop.render(
227-
<Suspense>
227+
<Suspense fallback={<Text text="Loading..." />}>
228228
<Transparent ref={divRef} />
229229
</Suspense>,
230230
);
231-
expect(ReactNoop.flush()).toEqual([]);
231+
expect(ReactNoop.flush()).toEqual(['Loading...']);
232232
await Promise.resolve();
233233
expect(ReactNoop.flush()).toEqual(['Text']);
234234
expect(divRef.current.type).toBe('div');

‎packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js‎

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ describe('ReactSuspense', () => {
106106
function Foo() {
107107
ReactTestRenderer.unstable_yield('Foo');
108108
return (
109-
<Suspense>
109+
<Suspense fallback={<Text text="Loading..." />}>
110110
<Bar>
111111
<AsyncText text="A" ms={100} />
112112
<Text text="B" />
@@ -126,6 +126,7 @@ describe('ReactSuspense', () => {
126126
'Suspend! [A]',
127127
// But we keep rendering the siblings
128128
'B',
129+
'Loading...',
129130
]);
130131
expect(root).toMatchRenderedOutput(null);
131132

@@ -241,4 +242,49 @@ describe('ReactSuspense', () => {
241242
]);
242243
expect(root).toMatchRenderedOutput('AsyncSibling');
243244
});
245+
246+
it('only captures if `fallback` is defined', () => {
247+
const root = ReactTestRenderer.create(
248+
<Suspense fallback={<Text text="Loading..." />}>
249+
<Suspense maxDuration={100}>
250+
<AsyncText text="Hi" ms={5000} />
251+
</Suspense>
252+
</Suspense>,
253+
{
254+
unstable_isConcurrent: true,
255+
},
256+
);
257+
258+
expect(root).toFlushAndYield([
259+
'Suspend! [Hi]',
260+
// The outer fallback should be rendered, because the inner one does not
261+
// have a `fallback` prop
262+
'Loading...',
263+
]);
264+
jest.advanceTimersByTime(1000);
265+
expect(ReactTestRenderer).toHaveYielded([]);
266+
expect(root).toFlushAndYield([]);
267+
expect(root).toMatchRenderedOutput('Loading...');
268+
269+
jest.advanceTimersByTime(5000);
270+
expect(ReactTestRenderer).toHaveYielded(['Promise resolved [Hi]']);
271+
expect(root).toFlushAndYield(['Hi']);
272+
expect(root).toMatchRenderedOutput('Hi');
273+
});
274+
275+
it('throws if tree suspends and none of the Suspense ancestors have a fallback', () => {
276+
const root = ReactTestRenderer.create(
277+
<Suspense>
278+
<AsyncText text="Hi" ms={1000} />
279+
</Suspense>,
280+
{
281+
unstable_isConcurrent: true,
282+
},
283+
);
284+
285+
expect(root).toFlushAndThrow(
286+
'An update was suspended, but no placeholder UI was provided.',
287+
);
288+
expect(ReactTestRenderer).toHaveYielded(['Suspend! [Hi]', 'Suspend! [Hi]']);
289+
});
244290
});

‎packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js‎

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
102102
function Foo() {
103103
ReactNoop.yield('Foo');
104104
return (
105-
<Suspense>
105+
<Suspense fallback={<Text text="Loading..." />}>
106106
<Bar>
107107
<AsyncText text="A" ms={100} />
108108
<Text text="B" />
@@ -119,6 +119,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
119119
'Suspend! [A]',
120120
// But we keep rendering the siblings
121121
'B',
122+
'Loading...',
122123
]);
123124
expect(ReactNoop.getChildren()).toEqual([]);
124125

@@ -191,15 +192,21 @@ describe('ReactSuspenseWithNoopRenderer', () => {
191192

192193
it('continues rendering siblings after suspending', async () => {
193194
ReactNoop.render(
194-
<Suspense>
195+
<Suspense fallback={<Text text="Loading..." />}>
195196
<Text text="A" />
196197
<AsyncText text="B" />
197198
<Text text="C" />
198199
<Text text="D" />
199200
</Suspense>,
200201
);
201202
// B suspends. Continue rendering the remaining siblings.
202-
expect(ReactNoop.flush()).toEqual(['A', 'Suspend! [B]', 'C', 'D']);
203+
expect(ReactNoop.flush()).toEqual([
204+
'A',
205+
'Suspend! [B]',
206+
'C',
207+
'D',
208+
'Loading...',
209+
]);
203210
// Did not commit yet.
204211
expect(ReactNoop.getChildren()).toEqual([]);
205212

@@ -241,7 +248,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
241248
const errorBoundary = React.createRef();
242249
function App() {
243250
return (
244-
<Suspense>
251+
<Suspense fallback={<Text text="Loading..." />}>
245252
<ErrorBoundary ref={errorBoundary}>
246253
<AsyncText text="Result" ms={1000} />
247254
</ErrorBoundary>
@@ -250,7 +257,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
250257
}
251258

252259
ReactNoop.render(<App />);
253-
expect(ReactNoop.flush()).toEqual(['Suspend! [Result]']);
260+
expect(ReactNoop.flush()).toEqual(['Suspend! [Result]', 'Loading...']);
254261
expect(ReactNoop.getChildren()).toEqual([]);
255262

256263
textResourceShouldFail = true;
@@ -276,7 +283,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
276283
errorBoundary.current.reset();
277284
cache.invalidate();
278285

279-
expect(ReactNoop.flush()).toEqual(['Suspend! [Result]']);
286+
expect(ReactNoop.flush()).toEqual(['Suspend! [Result]', 'Loading...']);
280287
ReactNoop.expire(1000);
281288
await advanceTimers(1000);
282289
expect(ReactNoop.flush()).toEqual(['Promise resolved [Result]', 'Result']);
@@ -354,7 +361,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
354361
it('can update at a higher priority while in a suspended state', async () => {
355362
function App(props) {
356363
return (
357-
<Suspense>
364+
<Suspense fallback={<Text text="Loading..." />}>
358365
<Text text={props.highPri} />
359366
<AsyncText text={props.lowPri} />
360367
</Suspense>
@@ -374,6 +381,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
374381
'A',
375382
// Suspends
376383
'Suspend! [2]',
384+
'Loading...',
377385
]);
378386

379387
// While we're still waiting for the low-pri update to complete, update the
@@ -393,21 +401,21 @@ describe('ReactSuspenseWithNoopRenderer', () => {
393401
it('keeps working on lower priority work after being pinged', async () => {
394402
function App(props) {
395403
return (
396-
<Suspense>
404+
<Suspense fallback={<Text text="Loading..." />}>
397405
<AsyncText text="A" />
398406
{props.showB && <Text text="B" />}
399407
</Suspense>
400408
);
401409
}
402410

403411
ReactNoop.render(<App showB={false} />);
404-
expect(ReactNoop.flush()).toEqual(['Suspend! [A]']);
412+
expect(ReactNoop.flush()).toEqual(['Suspend! [A]', 'Loading...']);
405413
expect(ReactNoop.getChildren()).toEqual([]);
406414

407415
// Advance React's virtual time by enough to fall into a new async bucket.
408416
ReactNoop.expire(1200);
409417
ReactNoop.render(<App showB={true} />);
410-
expect(ReactNoop.flush()).toEqual(['Suspend! [A]', 'B']);
418+
expect(ReactNoop.flush()).toEqual(['Suspend! [A]', 'B', 'Loading...']);
411419
expect(ReactNoop.getChildren()).toEqual([]);
412420

413421
await advanceTimers(0);
@@ -678,13 +686,17 @@ describe('ReactSuspenseWithNoopRenderer', () => {
678686

679687
it('a Suspense component correctly handles more than one suspended child', async () => {
680688
ReactNoop.render(
681-
<Suspense maxDuration={0}>
689+
<Suspense maxDuration={0} fallback={<Text text="Loading..." />}>
682690
<AsyncText text="A" ms={100} />
683691
<AsyncText text="B" ms={100} />
684692
</Suspense>,
685693
);
686-
expect(ReactNoop.expire(10000)).toEqual(['Suspend! [A]', 'Suspend! [B]']);
687-
expect(ReactNoop.getChildren()).toEqual([]);
694+
expect(ReactNoop.expire(10000)).toEqual([
695+
'Suspend! [A]',
696+
'Suspend! [B]',
697+
'Loading...',
698+
]);
699+
expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
688700

689701
await advanceTimers(100);
690702

@@ -724,7 +736,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
724736
it('starts working on an update even if its priority falls between two suspended levels', async () => {
725737
function App(props) {
726738
return (
727-
<Suspense maxDuration={10000}>
739+
<Suspense fallback={<Text text="Loading..." />} maxDuration={10000}>
728740
{props.text === 'C' ? (
729741
<Text text="C" />
730742
) : (
@@ -737,7 +749,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
737749
// Schedule an update
738750
ReactNoop.render(<App text="A" />);
739751
// The update should suspend.
740-
expect(ReactNoop.flush()).toEqual(['Suspend! [A]']);
752+
expect(ReactNoop.flush()).toEqual(['Suspend! [A]', 'Loading...']);
741753
expect(ReactNoop.getChildren()).toEqual([]);
742754

743755
// Advance time until right before it expires. This number may need to
@@ -750,7 +762,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
750762
// Schedule another low priority update.
751763
ReactNoop.render(<App text="B" />);
752764
// This update should also suspend.
753-
expect(ReactNoop.flush()).toEqual(['Suspend! [B]']);
765+
expect(ReactNoop.flush()).toEqual(['Suspend! [B]', 'Loading...']);
754766
expect(ReactNoop.getChildren()).toEqual([]);
755767

756768
// Schedule a high priority update. Its expiration time will fall between
@@ -773,7 +785,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
773785
it('can hide a tree to unblock its surroundings', async () => {
774786
function App() {
775787
return (
776-
<Suspense maxDuration={1000}>
788+
<Suspense fallback={<Text text="Fallback" />} maxDuration={1000}>
777789
{didTimeout => (
778790
<Fragment>
779791
<div hidden={didTimeout}>
@@ -859,7 +871,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
859871

860872
function Delay({ms}) {
861873
return (
862-
<Suspense maxDuration={ms}>
874+
<Suspense fallback={<Text text="Loading..." />} maxDuration={ms}>
863875
{didTimeout => {
864876
if (didTimeout) {
865877
// Once ms has elapsed, render null. This allows the rest of the

0 commit comments

Comments
 (0)