Skip to content

Commit 8ced545

Browse files
author
Andrew Clark
authored
Suspense component does not capture if fallback is not defined (#13879)
* 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 * Change order of checks
1 parent b738ced commit 8ced545

File tree

4 files changed

+87
-29
lines changed

4 files changed

+87
-29
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -205,9 +205,9 @@ function throwException(
205205
workInProgress = returnFiber;
206206
do {
207207
if (workInProgress.tag === SuspenseComponent) {
208-
const state = workInProgress.memoizedState;
209-
const didTimeout = state !== null && state.didTimeout;
210-
if (!didTimeout) {
208+
const fallback = workInProgress.memoizedProps.fallback;
209+
const didTimeout = workInProgress.memoizedState;
210+
if (!didTimeout && workInProgress.memoizedProps.fallback !== undefined) {
211211
// Found the nearest boundary.
212212

213213
// 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

@@ -271,4 +272,49 @@ describe('ReactSuspense', () => {
271272
expect(ReactTestRenderer).toHaveYielded(['Hi', 'Did mount: Hi']);
272273
expect(root).toMatchRenderedOutput('Hi');
273274
});
275+
276+
it('only captures if `fallback` is defined', () => {
277+
const root = ReactTestRenderer.create(
278+
<Suspense fallback={<Text text="Loading..." />}>
279+
<Suspense maxDuration={100}>
280+
<AsyncText text="Hi" ms={5000} />
281+
</Suspense>
282+
</Suspense>,
283+
{
284+
unstable_isConcurrent: true,
285+
},
286+
);
287+
288+
expect(root).toFlushAndYield([
289+
'Suspend! [Hi]',
290+
// The outer fallback should be rendered, because the inner one does not
291+
// have a `fallback` prop
292+
'Loading...',
293+
]);
294+
jest.advanceTimersByTime(1000);
295+
expect(ReactTestRenderer).toHaveYielded([]);
296+
expect(root).toFlushAndYield([]);
297+
expect(root).toMatchRenderedOutput('Loading...');
298+
299+
jest.advanceTimersByTime(5000);
300+
expect(ReactTestRenderer).toHaveYielded(['Promise resolved [Hi]']);
301+
expect(root).toFlushAndYield(['Hi']);
302+
expect(root).toMatchRenderedOutput('Hi');
303+
});
304+
305+
it('throws if tree suspends and none of the Suspense ancestors have a fallback', () => {
306+
const root = ReactTestRenderer.create(
307+
<Suspense>
308+
<AsyncText text="Hi" ms={1000} />
309+
</Suspense>,
310+
{
311+
unstable_isConcurrent: true,
312+
},
313+
);
314+
315+
expect(root).toFlushAndThrow(
316+
'An update was suspended, but no placeholder UI was provided.',
317+
);
318+
expect(ReactTestRenderer).toHaveYielded(['Suspend! [Hi]', 'Suspend! [Hi]']);
319+
});
274320
});

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

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
104104
function Foo() {
105105
ReactNoop.yield('Foo');
106106
return (
107-
<Suspense>
107+
<Suspense fallback={<Text text="Loading..." />}>
108108
<Bar>
109109
<AsyncText text="A" ms={100} />
110110
<Text text="B" />
@@ -121,6 +121,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
121121
'Suspend! [A]',
122122
// But we keep rendering the siblings
123123
'B',
124+
'Loading...',
124125
]);
125126
expect(ReactNoop.getChildren()).toEqual([]);
126127

@@ -193,15 +194,21 @@ describe('ReactSuspenseWithNoopRenderer', () => {
193194

194195
it('continues rendering siblings after suspending', async () => {
195196
ReactNoop.render(
196-
<Suspense>
197+
<Suspense fallback={<Text text="Loading..." />}>
197198
<Text text="A" />
198199
<AsyncText text="B" />
199200
<Text text="C" />
200201
<Text text="D" />
201202
</Suspense>,
202203
);
203204
// B suspends. Continue rendering the remaining siblings.
204-
expect(ReactNoop.flush()).toEqual(['A', 'Suspend! [B]', 'C', 'D']);
205+
expect(ReactNoop.flush()).toEqual([
206+
'A',
207+
'Suspend! [B]',
208+
'C',
209+
'D',
210+
'Loading...',
211+
]);
205212
// Did not commit yet.
206213
expect(ReactNoop.getChildren()).toEqual([]);
207214

@@ -243,7 +250,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
243250
const errorBoundary = React.createRef();
244251
function App() {
245252
return (
246-
<Suspense>
253+
<Suspense fallback={<Text text="Loading..." />}>
247254
<ErrorBoundary ref={errorBoundary}>
248255
<AsyncText text="Result" ms={1000} />
249256
</ErrorBoundary>
@@ -252,7 +259,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
252259
}
253260

254261
ReactNoop.render(<App />);
255-
expect(ReactNoop.flush()).toEqual(['Suspend! [Result]']);
262+
expect(ReactNoop.flush()).toEqual(['Suspend! [Result]', 'Loading...']);
256263
expect(ReactNoop.getChildren()).toEqual([]);
257264

258265
textResourceShouldFail = true;
@@ -278,7 +285,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
278285
errorBoundary.current.reset();
279286
cache.invalidate();
280287

281-
expect(ReactNoop.flush()).toEqual(['Suspend! [Result]']);
288+
expect(ReactNoop.flush()).toEqual(['Suspend! [Result]', 'Loading...']);
282289
ReactNoop.expire(1000);
283290
await advanceTimers(1000);
284291
expect(ReactNoop.flush()).toEqual(['Promise resolved [Result]', 'Result']);
@@ -356,7 +363,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
356363
it('can update at a higher priority while in a suspended state', async () => {
357364
function App(props) {
358365
return (
359-
<Suspense>
366+
<Suspense fallback={<Text text="Loading..." />}>
360367
<Text text={props.highPri} />
361368
<AsyncText text={props.lowPri} />
362369
</Suspense>
@@ -376,6 +383,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
376383
'A',
377384
// Suspends
378385
'Suspend! [2]',
386+
'Loading...',
379387
]);
380388

381389
// While we're still waiting for the low-pri update to complete, update the
@@ -395,21 +403,21 @@ describe('ReactSuspenseWithNoopRenderer', () => {
395403
it('keeps working on lower priority work after being pinged', async () => {
396404
function App(props) {
397405
return (
398-
<Suspense>
406+
<Suspense fallback={<Text text="Loading..." />}>
399407
<AsyncText text="A" />
400408
{props.showB && <Text text="B" />}
401409
</Suspense>
402410
);
403411
}
404412

405413
ReactNoop.render(<App showB={false} />);
406-
expect(ReactNoop.flush()).toEqual(['Suspend! [A]']);
414+
expect(ReactNoop.flush()).toEqual(['Suspend! [A]', 'Loading...']);
407415
expect(ReactNoop.getChildren()).toEqual([]);
408416

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

415423
await advanceTimers(0);
@@ -676,13 +684,17 @@ describe('ReactSuspenseWithNoopRenderer', () => {
676684

677685
it('a Suspense component correctly handles more than one suspended child', async () => {
678686
ReactNoop.render(
679-
<Suspense maxDuration={0}>
687+
<Suspense maxDuration={0} fallback={<Text text="Loading..." />}>
680688
<AsyncText text="A" ms={100} />
681689
<AsyncText text="B" ms={100} />
682690
</Suspense>,
683691
);
684-
expect(ReactNoop.expire(10000)).toEqual(['Suspend! [A]', 'Suspend! [B]']);
685-
expect(ReactNoop.getChildren()).toEqual([]);
692+
expect(ReactNoop.expire(10000)).toEqual([
693+
'Suspend! [A]',
694+
'Suspend! [B]',
695+
'Loading...',
696+
]);
697+
expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
686698

687699
await advanceTimers(100);
688700

@@ -722,7 +734,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
722734
it('starts working on an update even if its priority falls between two suspended levels', async () => {
723735
function App(props) {
724736
return (
725-
<Suspense maxDuration={10000}>
737+
<Suspense fallback={<Text text="Loading..." />} maxDuration={10000}>
726738
{props.text === 'C' ? (
727739
<Text text="C" />
728740
) : (
@@ -735,7 +747,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
735747
// Schedule an update
736748
ReactNoop.render(<App text="A" />);
737749
// The update should suspend.
738-
expect(ReactNoop.flush()).toEqual(['Suspend! [A]']);
750+
expect(ReactNoop.flush()).toEqual(['Suspend! [A]', 'Loading...']);
739751
expect(ReactNoop.getChildren()).toEqual([]);
740752

741753
// Advance time until right before it expires. This number may need to
@@ -748,7 +760,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
748760
// Schedule another low priority update.
749761
ReactNoop.render(<App text="B" />);
750762
// This update should also suspend.
751-
expect(ReactNoop.flush()).toEqual(['Suspend! [B]']);
763+
expect(ReactNoop.flush()).toEqual(['Suspend! [B]', 'Loading...']);
752764
expect(ReactNoop.getChildren()).toEqual([]);
753765

754766
// Schedule a high priority update. Its expiration time will fall between

0 commit comments

Comments
 (0)