diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index a78298878b373..9f954f4581d3c 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -714,7 +714,7 @@ describe('ReactDOMServerPartialHydration', () => { expect(container.textContent).toBe('Hi'); }); - it('shows the fallback of the outer if fallback is missing', async () => { + it('treats missing fallback the same as if it was defined', async () => { // This is the same exact test as above but with a nested Suspense without a fallback. // This should be a noop. let suspend = false; @@ -759,7 +759,8 @@ describe('ReactDOMServerPartialHydration', () => { Scheduler.unstable_flushAll(); jest.runAllTimers(); - expect(ref.current).toBe(null); + const span = container.getElementsByTagName('span')[0]; + expect(ref.current).toBe(span); // Render an update, but leave it still suspended. root.render(); @@ -768,9 +769,9 @@ describe('ReactDOMServerPartialHydration', () => { Scheduler.unstable_flushAll(); jest.runAllTimers(); - expect(container.getElementsByTagName('span').length).toBe(0); - expect(ref.current).toBe(null); - expect(container.textContent).toBe('Loading...'); + expect(container.getElementsByTagName('span').length).toBe(1); + expect(ref.current).toBe(span); + expect(container.textContent).toBe(''); // Unsuspending shows the content. suspend = false; @@ -780,7 +781,6 @@ describe('ReactDOMServerPartialHydration', () => { Scheduler.unstable_flushAll(); jest.runAllTimers(); - const span = container.getElementsByTagName('span')[0]; expect(span.textContent).toBe('Hi'); expect(span.className).toBe('hi'); expect(ref.current).toBe(span); diff --git a/packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js b/packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js index 12fc6987cf59a..6e2ec3150d7f7 100644 --- a/packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js +++ b/packages/react-dom/src/__tests__/ReactServerRenderingHydration-test.js @@ -471,10 +471,12 @@ describe('ReactDOMServerHydration', () => { element, ); - // Because this didn't have a fallback, it was hydrated as if it's - // not a Suspense boundary. - expect(ref.current).toBe(div); - expect(element.innerHTML).toBe('
Hello World
'); + // The content should've been client rendered. + expect(ref.current).not.toBe(div); + // Unfortunately, since we don't delete the tail at the root, a duplicate will remain. + expect(element.innerHTML).toBe( + '
Hello World
Hello World
', + ); }); // regression test for https://github.com/facebook/react/issues/17170 diff --git a/packages/react-dom/src/server/ReactPartialRenderer.js b/packages/react-dom/src/server/ReactPartialRenderer.js index 752af3b2c7377..455309db79c7d 100644 --- a/packages/react-dom/src/server/ReactPartialRenderer.js +++ b/packages/react-dom/src/server/ReactPartialRenderer.js @@ -1123,25 +1123,6 @@ class ReactDOMServerRenderer { case REACT_SUSPENSE_TYPE: { if (enableSuspenseServerRenderer) { const fallback = ((nextChild: any): ReactElement).props.fallback; - if (fallback === undefined) { - // If there is no fallback, then this just behaves as a fragment. - const nextChildren = toArray( - ((nextChild: any): ReactElement).props.children, - ); - const frame: Frame = { - type: null, - domNamespace: parentNamespace, - children: nextChildren, - childIndex: 0, - context: context, - footer: '', - }; - if (__DEV__) { - ((frame: any): FrameDev).debugElementStack = []; - } - this.stack.push(frame); - return ''; - } const fallbackChildren = toArray(fallback); const nextChildren = toArray( ((nextChild: any): ReactElement).props.children, diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 581bf6c76d45f..50a5a81a1f099 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -1867,12 +1867,8 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) { // This is a new mount or this boundary is already showing a fallback state. // Mark this subtree context as having at least one invisible parent that could // handle the fallback state. - // Boundaries without fallbacks or should be avoided are not considered since - // they cannot handle preferred fallback states. - if ( - nextProps.fallback !== undefined && - nextProps.unstable_avoidThisFallback !== true - ) { + // Avoided boundaries are not considered since they cannot handle preferred fallback states. + if (nextProps.unstable_avoidThisFallback !== true) { suspenseContext = addSubtreeSuspenseContext( suspenseContext, InvisibleParentSuspenseContext, @@ -1910,22 +1906,18 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) { if (current === null) { // Initial mount // If we're currently hydrating, try to hydrate this boundary. - // But only if this has a fallback. - if (nextProps.fallback !== undefined) { - tryToClaimNextHydratableInstance(workInProgress); - // This could've been a dehydrated suspense component. - if (enableSuspenseServerRenderer) { - const suspenseState: null | SuspenseState = - workInProgress.memoizedState; - if (suspenseState !== null) { - const dehydrated = suspenseState.dehydrated; - if (dehydrated !== null) { - return mountDehydratedSuspenseComponent( - workInProgress, - dehydrated, - renderLanes, - ); - } + tryToClaimNextHydratableInstance(workInProgress); + // This could've been a dehydrated suspense component. + if (enableSuspenseServerRenderer) { + const suspenseState: null | SuspenseState = workInProgress.memoizedState; + if (suspenseState !== null) { + const dehydrated = suspenseState.dehydrated; + if (dehydrated !== null) { + return mountDehydratedSuspenseComponent( + workInProgress, + dehydrated, + renderLanes, + ); } } } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index b4dde9e826d84..fa0a9840e758f 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -1867,12 +1867,8 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) { // This is a new mount or this boundary is already showing a fallback state. // Mark this subtree context as having at least one invisible parent that could // handle the fallback state. - // Boundaries without fallbacks or should be avoided are not considered since - // they cannot handle preferred fallback states. - if ( - nextProps.fallback !== undefined && - nextProps.unstable_avoidThisFallback !== true - ) { + // Avoided boundaries are not considered since they cannot handle preferred fallback states. + if (nextProps.unstable_avoidThisFallback !== true) { suspenseContext = addSubtreeSuspenseContext( suspenseContext, InvisibleParentSuspenseContext, @@ -1910,22 +1906,18 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) { if (current === null) { // Initial mount // If we're currently hydrating, try to hydrate this boundary. - // But only if this has a fallback. - if (nextProps.fallback !== undefined) { - tryToClaimNextHydratableInstance(workInProgress); - // This could've been a dehydrated suspense component. - if (enableSuspenseServerRenderer) { - const suspenseState: null | SuspenseState = - workInProgress.memoizedState; - if (suspenseState !== null) { - const dehydrated = suspenseState.dehydrated; - if (dehydrated !== null) { - return mountDehydratedSuspenseComponent( - workInProgress, - dehydrated, - renderLanes, - ); - } + tryToClaimNextHydratableInstance(workInProgress); + // This could've been a dehydrated suspense component. + if (enableSuspenseServerRenderer) { + const suspenseState: null | SuspenseState = workInProgress.memoizedState; + if (suspenseState !== null) { + const dehydrated = suspenseState.dehydrated; + if (dehydrated !== null) { + return mountDehydratedSuspenseComponent( + workInProgress, + dehydrated, + renderLanes, + ); } } } diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js index e11e15bac8a62..29c5fe48c4b2f 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js @@ -1045,9 +1045,7 @@ function completeWork( const nextDidTimeout = nextState !== null; let prevDidTimeout = false; if (current === null) { - if (workInProgress.memoizedProps.fallback !== undefined) { - popHydrationState(workInProgress); - } + popHydrationState(workInProgress); } else { const prevState: null | SuspenseState = current.memoizedState; prevDidTimeout = prevState !== null; diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js index ba6200d364a34..a119a6b975190 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js @@ -1045,9 +1045,7 @@ function completeWork( const nextDidTimeout = nextState !== null; let prevDidTimeout = false; if (current === null) { - if (workInProgress.memoizedProps.fallback !== undefined) { - popHydrationState(workInProgress); - } + popHydrationState(workInProgress); } else { const prevState: null | SuspenseState = current.memoizedState; prevDidTimeout = prevState !== null; diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js index b1077092d46a0..5ad7ae650249a 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js @@ -77,10 +77,6 @@ export function shouldCaptureSuspense( return false; } const props = workInProgress.memoizedProps; - // In order to capture, the Suspense component must have a fallback prop. - if (props.fallback === undefined) { - return false; - } // Regular boundaries always capture. if (props.unstable_avoidThisFallback !== true) { return true; diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js index b970646967608..51bef1df3a568 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js @@ -77,10 +77,6 @@ export function shouldCaptureSuspense( return false; } const props = workInProgress.memoizedProps; - // In order to capture, the Suspense component must have a fallback prop. - if (props.fallback === undefined) { - return false; - } // Regular boundaries always capture. if (props.unstable_avoidThisFallback !== true) { return true; diff --git a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js index c14588788b38b..53fb9c07589e4 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactHooks-test.internal.js @@ -19,8 +19,6 @@ let Scheduler; let ReactDOMServer; let act; -// Additional tests can be found in ReactHooksWithNoopRenderer. Plan is to -// gradually migrate those to this file. describe('ReactHooks', () => { beforeEach(() => { jest.resetModules(); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js index e00f5e119c5b1..c11dcfe692922 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -9,8 +9,6 @@ let act; let TextResource; let textResourceShouldFail; -// Additional tests can be found in ReactSuspenseWithNoopRenderer. Plan is -// to gradually migrate those to this file. describe('ReactSuspense', () => { beforeEach(() => { jest.resetModules(); @@ -391,44 +389,10 @@ describe('ReactSuspense', () => { expect(root).toMatchRenderedOutput('Hi'); }); - it('only captures if `fallback` is defined', () => { - const root = ReactTestRenderer.create( - }> - - - - , - { - unstable_isConcurrent: true, - }, - ); - - expect(Scheduler).toFlushAndYield([ - 'Suspend! [Hi]', - // The outer fallback should be rendered, because the inner one does not - // have a `fallback` prop - 'Loading...', - ]); - jest.advanceTimersByTime(1000); - expect(Scheduler).toHaveYielded([]); - expect(Scheduler).toFlushAndYield([]); - expect(root).toMatchRenderedOutput('Loading...'); - - jest.advanceTimersByTime(5000); - expect(Scheduler).toHaveYielded(['Promise resolved [Hi]']); - expect(Scheduler).toFlushAndYield(['Hi']); - expect(root).toMatchRenderedOutput('Hi'); - }); - - it('throws if tree suspends and none of the Suspense ancestors have a fallback', () => { - ReactTestRenderer.create( - - - , - { - unstable_isConcurrent: true, - }, - ); + it('throws if tree suspends and none of the Suspense ancestors have a boundary', () => { + ReactTestRenderer.create(, { + unstable_isConcurrent: true, + }); expect(Scheduler).toFlushAndThrow( 'AsyncText suspended while rendering, but no fallback UI was specified.', diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseFallback-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseFallback-test.js new file mode 100644 index 0000000000000..18e21a43c1347 --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseFallback-test.js @@ -0,0 +1,223 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ +let React; +let ReactNoop; +let Scheduler; +let Suspense; +let getCacheForType; +let caches; +let seededCache; + +describe('ReactSuspenseFallback', () => { + beforeEach(() => { + jest.resetModules(); + + React = require('react'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + Suspense = React.Suspense; + getCacheForType = React.unstable_getCacheForType; + caches = []; + seededCache = null; + }); + + function createTextCache() { + if (seededCache !== null) { + // Trick to seed a cache before it exists. + // TODO: Need a built-in API to seed data before the initial render (i.e. + // not a refresh because nothing has mounted yet). + const cache = seededCache; + seededCache = null; + return cache; + } + + const data = new Map(); + const version = caches.length + 1; + const cache = { + version, + data, + resolve(text) { + const record = data.get(text); + if (record === undefined) { + const newRecord = { + status: 'resolved', + value: text, + }; + data.set(text, newRecord); + } else if (record.status === 'pending') { + const thenable = record.value; + record.status = 'resolved'; + record.value = text; + thenable.pings.forEach(t => t()); + } + }, + reject(text, error) { + const record = data.get(text); + if (record === undefined) { + const newRecord = { + status: 'rejected', + value: error, + }; + data.set(text, newRecord); + } else if (record.status === 'pending') { + const thenable = record.value; + record.status = 'rejected'; + record.value = error; + thenable.pings.forEach(t => t()); + } + }, + }; + caches.push(cache); + return cache; + } + + function readText(text) { + const textCache = getCacheForType(createTextCache); + const record = textCache.data.get(text); + if (record !== undefined) { + switch (record.status) { + case 'pending': + Scheduler.unstable_yieldValue(`Suspend! [${text}]`); + throw record.value; + case 'rejected': + Scheduler.unstable_yieldValue(`Error! [${text}]`); + throw record.value; + case 'resolved': + return textCache.version; + } + } else { + Scheduler.unstable_yieldValue(`Suspend! [${text}]`); + + const thenable = { + pings: [], + then(resolve) { + if (newRecord.status === 'pending') { + thenable.pings.push(resolve); + } else { + Promise.resolve().then(() => resolve(newRecord.value)); + } + }, + }; + + const newRecord = { + status: 'pending', + value: thenable, + }; + textCache.data.set(text, newRecord); + + throw thenable; + } + } + + function Text({text}) { + Scheduler.unstable_yieldValue(text); + return ; + } + + function AsyncText({text, showVersion}) { + const version = readText(text); + const fullText = showVersion ? `${text} [v${version}]` : text; + Scheduler.unstable_yieldValue(fullText); + return ; + } + + function span(prop) { + return {type: 'span', children: [], prop, hidden: false}; + } + + // @gate enableCache + it('suspends and shows fallback', () => { + ReactNoop.render( + }> + + , + ); + + expect(Scheduler).toFlushAndYield(['Suspend! [A]', 'Loading...']); + expect(ReactNoop.getChildren()).toEqual([span('Loading...')]); + }); + + // @gate enableCache + it('suspends and shows null fallback', () => { + ReactNoop.render( + + + , + ); + + expect(Scheduler).toFlushAndYield([ + 'Suspend! [A]', + // null + ]); + expect(ReactNoop.getChildren()).toEqual([]); + }); + + // @gate enableCache + it('suspends and shows undefined fallback', () => { + ReactNoop.render( + + + , + ); + + expect(Scheduler).toFlushAndYield([ + 'Suspend! [A]', + // null + ]); + expect(ReactNoop.getChildren()).toEqual([]); + }); + + // @gate enableCache + it('suspends and shows inner fallback', () => { + ReactNoop.render( + }> + }> + + + , + ); + + expect(Scheduler).toFlushAndYield(['Suspend! [A]', 'Loading...']); + expect(ReactNoop.getChildren()).toEqual([span('Loading...')]); + }); + + // @gate enableCache + it('suspends and shows inner undefined fallback', () => { + ReactNoop.render( + }> + + + + , + ); + + expect(Scheduler).toFlushAndYield([ + 'Suspend! [A]', + // null + ]); + expect(ReactNoop.getChildren()).toEqual([]); + }); + + // @gate enableCache + it('suspends and shows inner null fallback', () => { + ReactNoop.render( + }> + + + + , + ); + + expect(Scheduler).toFlushAndYield([ + 'Suspend! [A]', + // null + ]); + expect(ReactNoop.getChildren()).toEqual([]); + }); +}); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js index ddd20f35edad7..e969a59c4f60f 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js @@ -771,6 +771,89 @@ describe('ReactSuspenseList', () => { ); }); + it('boundaries without fallbacks can be coordinate with SuspenseList', async () => { + const A = createAsyncText('A'); + const B = createAsyncText('B'); + const C = createAsyncText('C'); + + function Foo({showMore}) { + return ( + }> + + + + + {showMore ? ( + <> + + + + + + + + ) : null} + + + ); + } + + ReactNoop.render(); + + expect(Scheduler).toFlushAndYield([ + 'Suspend! [A]', + // null + ]); + + expect(ReactNoop).toMatchRenderedOutput(null); + + await A.resolve(); + + expect(Scheduler).toFlushAndYield(['A']); + + expect(ReactNoop).toMatchRenderedOutput(A); + + // Let's do an update that should consult the avoided boundaries. + ReactNoop.render(); + + expect(Scheduler).toFlushAndYield([ + 'A', + 'Suspend! [B]', + // null + 'Suspend! [C]', + // null + 'A', + // null + // null + ]); + + // This will suspend, since the boundaries are avoided. Give them + // time to display their loading states. + jest.advanceTimersByTime(500); + + // A is already showing content so it doesn't turn into a fallback. + expect(ReactNoop).toMatchRenderedOutput(A); + + await B.resolve(); + + expect(Scheduler).toFlushAndYield(['B', 'Suspend! [C]']); + + // Even though we could now show B, we're still waiting on C. + expect(ReactNoop).toMatchRenderedOutput(A); + + await C.resolve(); + + expect(Scheduler).toFlushAndYield(['B', 'C']); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + B + C + , + ); + }); + it('displays each items in "forwards" order', async () => { const A = createAsyncText('A'); const B = createAsyncText('B'); diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index fdd1a46dbb1e3..ac7d0665f3fb2 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -2303,6 +2303,55 @@ describe('ReactSuspenseWithNoopRenderer', () => { expect(ReactNoop.getChildren()).toEqual([span('A'), span('C'), span('B')]); }); + // @gate enableCache + it('does not show the parent fallback if the inner fallback is not defined', async () => { + function Foo({showC}) { + Scheduler.unstable_yieldValue('Foo'); + return ( + }> + + + {showC ? : null} + + + + ); + } + + ReactNoop.render(); + expect(Scheduler).toFlushAndYield([ + 'Foo', + 'Suspend! [A]', + 'B', + // null + ]); + expect(ReactNoop.getChildren()).toEqual([span('B')]); + + // Eventually we resolve and show the data. + await resolveText('A'); + expect(Scheduler).toFlushAndYield(['A']); + expect(ReactNoop.getChildren()).toEqual([span('A'), span('B')]); + + // Update to show C + ReactNoop.render(); + expect(Scheduler).toFlushAndYield([ + 'Foo', + 'A', + 'Suspend! [C]', + // null + 'B', + ]); + // Flush to skip suspended time. + Scheduler.unstable_advanceTime(600); + await advanceTimers(600); + expect(ReactNoop.getChildren()).toEqual([hiddenSpan('A'), span('B')]); + + // Later we load the data. + await resolveText('C'); + expect(Scheduler).toFlushAndYield(['A', 'C']); + expect(ReactNoop.getChildren()).toEqual([span('A'), span('C'), span('B')]); + }); + // @gate enableCache it('favors showing the inner fallback for nested top level avoided fallback', async () => { function Foo({showB}) { @@ -2393,6 +2442,57 @@ describe('ReactSuspenseWithNoopRenderer', () => { } }); + // @gate enableCache + it('keeps showing an undefined fallback if it is already showing', async () => { + function Foo({showB}) { + Scheduler.unstable_yieldValue('Foo'); + return ( + }> + + + {showB ? ( + + + + ) : null} + + + ); + } + + ReactNoop.render(); + expect(Scheduler).toFlushAndYield(['Foo', 'A']); + expect(ReactNoop.getChildren()).toEqual([span('A')]); + + if (gate(flags => flags.enableSyncDefaultUpdates)) { + React.startTransition(() => { + ReactNoop.render(); + }); + } else { + ReactNoop.render(); + } + + expect(Scheduler).toFlushAndYield([ + 'Foo', + 'A', + 'Suspend! [B]', + // Null + ]); + // Still suspended. + expect(ReactNoop.getChildren()).toEqual([span('A')]); + + // Flush to skip suspended time. + Scheduler.unstable_advanceTime(600); + await advanceTimers(600); + + if (gate(flags => flags.enableSyncDefaultUpdates)) { + // Transitions never fall back. + expect(ReactNoop.getChildren()).toEqual([span('A')]); + } else { + expect(ReactNoop.getChildren()).toEqual([span('A')]); + } + }); + // @gate enableCache it('commits a suspended idle pri render within a reasonable time', async () => { function Foo({renderContent}) {