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

packages/react-reconciler/src/ReactFiberUnwindWork.js

@@ -205,9 +205,9 @@ function throwException(
     workInProgress = returnFiber;
     do {
       if (workInProgress.tag === SuspenseComponent) {
-        const state = workInProgress.memoizedState;
-        const didTimeout = state !== null && state.didTimeout;
-        if (!didTimeout) {
+        const fallback = workInProgress.memoizedProps.fallback;
+        const didTimeout = workInProgress.memoizedState;
+        if (!didTimeout && workInProgress.memoizedProps.fallback !== undefined) {
           // Found the nearest boundary.
 
           // If the boundary is not in concurrent mode, we should not suspend, and

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

@@ -57,11 +57,11 @@ describe('pure', () => {
         Counter = pure(Counter);
 
         ReactNoop.render(
-          <Suspense>
+          <Suspense fallback={<Text text="Loading..." />}>
             <Counter count={0} />
           </Suspense>,
         );
-        expect(ReactNoop.flush()).toEqual([]);
+        expect(ReactNoop.flush()).toEqual(['Loading...']);
         await Promise.resolve();
         expect(ReactNoop.flush()).toEqual([0]);
         expect(ReactNoop.getChildren()).toEqual([span(0)]);
@@ -107,7 +107,7 @@ describe('pure', () => {
           state = {count: 0};
           render() {
             return (
-              <Suspense>
+              <Suspense fallback={<Text text="Loading..." />}>
                 <CountContext.Provider value={this.state.count}>
                   <Counter label="Count" />
                 </CountContext.Provider>
@@ -118,7 +118,7 @@ describe('pure', () => {
 
         const parent = React.createRef(null);
         ReactNoop.render(<Parent ref={parent} />);
-        expect(ReactNoop.flush()).toEqual([]);
+        expect(ReactNoop.flush()).toEqual(['Loading...']);
         await Promise.resolve();
         expect(ReactNoop.flush()).toEqual(['Count: 0']);
         expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
@@ -148,11 +148,11 @@ describe('pure', () => {
         });
 
         ReactNoop.render(
-          <Suspense>
+          <Suspense fallback={<Text text="Loading..." />}>
             <Counter count={0} />
           </Suspense>,
         );
-        expect(ReactNoop.flush()).toEqual([]);
+        expect(ReactNoop.flush()).toEqual(['Loading...']);
         await Promise.resolve();
         expect(ReactNoop.flush()).toEqual([0]);
         expect(ReactNoop.getChildren()).toEqual([span(0)]);
@@ -204,7 +204,7 @@ describe('pure', () => {
         const divRef = React.createRef();
 
         ReactNoop.render(
-          <Suspense>
+          <Suspense fallback={<Text text="Loading..." />}>
             <Transparent ref={divRef} />
           </Suspense>,
         );
@@ -224,11 +224,11 @@ describe('pure', () => {
         const divRef2 = React.createRef();
 
         ReactNoop.render(
-          <Suspense>
+          <Suspense fallback={<Text text="Loading..." />}>
             <Transparent ref={divRef} />
           </Suspense>,
         );
-        expect(ReactNoop.flush()).toEqual([]);
+        expect(ReactNoop.flush()).toEqual(['Loading...']);
         await Promise.resolve();
         expect(ReactNoop.flush()).toEqual(['Text']);
         expect(divRef.current.type).toBe('div');

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

@@ -106,7 +106,7 @@ describe('ReactSuspense', () => {
     function Foo() {
       ReactTestRenderer.unstable_yield('Foo');
       return (
-        <Suspense>
+        <Suspense fallback={<Text text="Loading..." />}>
           <Bar>
             <AsyncText text="A" ms={100} />
             <Text text="B" />
@@ -126,6 +126,7 @@ describe('ReactSuspense', () => {
       'Suspend! [A]',
       // But we keep rendering the siblings
       'B',
+      'Loading...',
     ]);
     expect(root).toMatchRenderedOutput(null);
 
@@ -271,4 +272,49 @@ describe('ReactSuspense', () => {
     expect(ReactTestRenderer).toHaveYielded(['Hi', 'Did mount: Hi']);
     expect(root).toMatchRenderedOutput('Hi');
   });
+
+  it('only captures if `fallback` is defined', () => {
+    const root = ReactTestRenderer.create(
+      <Suspense fallback={<Text text="Loading..." />}>
+        <Suspense maxDuration={100}>
+          <AsyncText text="Hi" ms={5000} />
+        </Suspense>
+      </Suspense>,
+      {
+        unstable_isConcurrent: true,
+      },
+    );
+
+    expect(root).toFlushAndYield([
+      'Suspend! [Hi]',
+      // The outer fallback should be rendered, because the inner one does not
+      // have a `fallback` prop
+      'Loading...',
+    ]);
+    jest.advanceTimersByTime(1000);
+    expect(ReactTestRenderer).toHaveYielded([]);
+    expect(root).toFlushAndYield([]);
+    expect(root).toMatchRenderedOutput('Loading...');
+
+    jest.advanceTimersByTime(5000);
+    expect(ReactTestRenderer).toHaveYielded(['Promise resolved [Hi]']);
+    expect(root).toFlushAndYield(['Hi']);
+    expect(root).toMatchRenderedOutput('Hi');
+  });
+
+  it('throws if tree suspends and none of the Suspense ancestors have a fallback', () => {
+    const root = ReactTestRenderer.create(
+      <Suspense>
+        <AsyncText text="Hi" ms={1000} />
+      </Suspense>,
+      {
+        unstable_isConcurrent: true,
+      },
+    );
+
+    expect(root).toFlushAndThrow(
+      'An update was suspended, but no placeholder UI was provided.',
+    );
+    expect(ReactTestRenderer).toHaveYielded(['Suspend! [Hi]', 'Suspend! [Hi]']);
+  });
 });

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

@@ -104,7 +104,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     function Foo() {
       ReactNoop.yield('Foo');
       return (
-        <Suspense>
+        <Suspense fallback={<Text text="Loading..." />}>
           <Bar>
             <AsyncText text="A" ms={100} />
             <Text text="B" />
@@ -121,6 +121,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       'Suspend! [A]',
       // But we keep rendering the siblings
       'B',
+      'Loading...',
     ]);
     expect(ReactNoop.getChildren()).toEqual([]);
 
@@ -193,15 +194,21 @@ describe('ReactSuspenseWithNoopRenderer', () => {
 
   it('continues rendering siblings after suspending', async () => {
     ReactNoop.render(
-      <Suspense>
+      <Suspense fallback={<Text text="Loading..." />}>
         <Text text="A" />
         <AsyncText text="B" />
         <Text text="C" />
         <Text text="D" />
       </Suspense>,
     );
     // B suspends. Continue rendering the remaining siblings.
-    expect(ReactNoop.flush()).toEqual(['A', 'Suspend! [B]', 'C', 'D']);
+    expect(ReactNoop.flush()).toEqual([
+      'A',
+      'Suspend! [B]',
+      'C',
+      'D',
+      'Loading...',
+    ]);
     // Did not commit yet.
     expect(ReactNoop.getChildren()).toEqual([]);
 
@@ -243,7 +250,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     const errorBoundary = React.createRef();
     function App() {
       return (
-        <Suspense>
+        <Suspense fallback={<Text text="Loading..." />}>
           <ErrorBoundary ref={errorBoundary}>
             <AsyncText text="Result" ms={1000} />
           </ErrorBoundary>
@@ -252,7 +259,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     }
 
     ReactNoop.render(<App />);
-    expect(ReactNoop.flush()).toEqual(['Suspend! [Result]']);
+    expect(ReactNoop.flush()).toEqual(['Suspend! [Result]', 'Loading...']);
     expect(ReactNoop.getChildren()).toEqual([]);
 
     textResourceShouldFail = true;
@@ -278,7 +285,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     errorBoundary.current.reset();
     cache.invalidate();
 
-    expect(ReactNoop.flush()).toEqual(['Suspend! [Result]']);
+    expect(ReactNoop.flush()).toEqual(['Suspend! [Result]', 'Loading...']);
     ReactNoop.expire(1000);
     await advanceTimers(1000);
     expect(ReactNoop.flush()).toEqual(['Promise resolved [Result]', 'Result']);
@@ -356,7 +363,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
   it('can update at a higher priority while in a suspended state', async () => {
     function App(props) {
       return (
-        <Suspense>
+        <Suspense fallback={<Text text="Loading..." />}>
           <Text text={props.highPri} />
           <AsyncText text={props.lowPri} />
         </Suspense>
@@ -376,6 +383,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
       'A',
       // Suspends
       'Suspend! [2]',
+      'Loading...',
     ]);
 
     // While we're still waiting for the low-pri update to complete, update the
@@ -395,21 +403,21 @@ describe('ReactSuspenseWithNoopRenderer', () => {
   it('keeps working on lower priority work after being pinged', async () => {
     function App(props) {
       return (
-        <Suspense>
+        <Suspense fallback={<Text text="Loading..." />}>
           <AsyncText text="A" />
           {props.showB && <Text text="B" />}
         </Suspense>
       );
     }
 
     ReactNoop.render(<App showB={false} />);
-    expect(ReactNoop.flush()).toEqual(['Suspend! [A]']);
+    expect(ReactNoop.flush()).toEqual(['Suspend! [A]', 'Loading...']);
     expect(ReactNoop.getChildren()).toEqual([]);
 
     // Advance React's virtual time by enough to fall into a new async bucket.
     ReactNoop.expire(1200);
     ReactNoop.render(<App showB={true} />);
-    expect(ReactNoop.flush()).toEqual(['Suspend! [A]', 'B']);
+    expect(ReactNoop.flush()).toEqual(['Suspend! [A]', 'B', 'Loading...']);
     expect(ReactNoop.getChildren()).toEqual([]);
 
     await advanceTimers(0);
@@ -676,13 +684,17 @@ describe('ReactSuspenseWithNoopRenderer', () => {
 
   it('a Suspense component correctly handles more than one suspended child', async () => {
     ReactNoop.render(
-      <Suspense maxDuration={0}>
+      <Suspense maxDuration={0} fallback={<Text text="Loading..." />}>
         <AsyncText text="A" ms={100} />
         <AsyncText text="B" ms={100} />
       </Suspense>,
     );
-    expect(ReactNoop.expire(10000)).toEqual(['Suspend! [A]', 'Suspend! [B]']);
-    expect(ReactNoop.getChildren()).toEqual([]);
+    expect(ReactNoop.expire(10000)).toEqual([
+      'Suspend! [A]',
+      'Suspend! [B]',
+      'Loading...',
+    ]);
+    expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
 
     await advanceTimers(100);
 
@@ -722,7 +734,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
   it('starts working on an update even if its priority falls between two suspended levels', async () => {
     function App(props) {
       return (
-        <Suspense maxDuration={10000}>
+        <Suspense fallback={<Text text="Loading..." />} maxDuration={10000}>
           {props.text === 'C' ? (
             <Text text="C" />
           ) : (
@@ -735,7 +747,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     // Schedule an update
     ReactNoop.render(<App text="A" />);
     // The update should suspend.
-    expect(ReactNoop.flush()).toEqual(['Suspend! [A]']);
+    expect(ReactNoop.flush()).toEqual(['Suspend! [A]', 'Loading...']);
     expect(ReactNoop.getChildren()).toEqual([]);
 
     // Advance time until right before it expires. This number may need to
@@ -748,7 +760,7 @@ describe('ReactSuspenseWithNoopRenderer', () => {
     // Schedule another low priority update.
     ReactNoop.render(<App text="B" />);
     // This update should also suspend.
-    expect(ReactNoop.flush()).toEqual(['Suspend! [B]']);
+    expect(ReactNoop.flush()).toEqual(['Suspend! [B]', 'Loading...']);
     expect(ReactNoop.getChildren()).toEqual([]);
 
     // Schedule a high priority update. Its expiration time will fall between