Capture suspense boundaries with undefined fallbacks (#21854)

This commit is contained in:
Ricky 2021-07-12 14:50:33 -04:00 committed by GitHub
parent bfa50f8272
commit c2c6ea1fde
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
14 changed files with 452 additions and 129 deletions

View File

@ -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(<App text="Hi" className="hi" />);
@ -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);

View File

@ -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('<div>Hello World</div>');
// 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(
'<div>Hello World</div><div>Hello World</div>',
);
});
// regression test for https://github.com/facebook/react/issues/17170

View File

@ -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,

View File

@ -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,
);
}
}
}

View File

@ -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,
);
}
}
}

View File

@ -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;

View File

@ -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;

View File

@ -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;

View File

@ -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;

View File

@ -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();

View File

@ -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(
<Suspense fallback={<Text text="Loading..." />}>
<Suspense>
<AsyncText text="Hi" ms={5000} />
</Suspense>
</Suspense>,
{
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(
<Suspense>
<AsyncText text="Hi" ms={1000} />
</Suspense>,
{
unstable_isConcurrent: true,
},
);
it('throws if tree suspends and none of the Suspense ancestors have a boundary', () => {
ReactTestRenderer.create(<AsyncText text="Hi" ms={1000} />, {
unstable_isConcurrent: true,
});
expect(Scheduler).toFlushAndThrow(
'AsyncText suspended while rendering, but no fallback UI was specified.',

View File

@ -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 <span prop={text} />;
}
function AsyncText({text, showVersion}) {
const version = readText(text);
const fullText = showVersion ? `${text} [v${version}]` : text;
Scheduler.unstable_yieldValue(fullText);
return <span prop={fullText} />;
}
function span(prop) {
return {type: 'span', children: [], prop, hidden: false};
}
// @gate enableCache
it('suspends and shows fallback', () => {
ReactNoop.render(
<Suspense fallback={<Text text="Loading..." />}>
<AsyncText text="A" ms={100} />
</Suspense>,
);
expect(Scheduler).toFlushAndYield(['Suspend! [A]', 'Loading...']);
expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
});
// @gate enableCache
it('suspends and shows null fallback', () => {
ReactNoop.render(
<Suspense fallback={null}>
<AsyncText text="A" ms={100} />
</Suspense>,
);
expect(Scheduler).toFlushAndYield([
'Suspend! [A]',
// null
]);
expect(ReactNoop.getChildren()).toEqual([]);
});
// @gate enableCache
it('suspends and shows undefined fallback', () => {
ReactNoop.render(
<Suspense>
<AsyncText text="A" ms={100} />
</Suspense>,
);
expect(Scheduler).toFlushAndYield([
'Suspend! [A]',
// null
]);
expect(ReactNoop.getChildren()).toEqual([]);
});
// @gate enableCache
it('suspends and shows inner fallback', () => {
ReactNoop.render(
<Suspense fallback={<Text text="Should not show..." />}>
<Suspense fallback={<Text text="Loading..." />}>
<AsyncText text="A" ms={100} />
</Suspense>
</Suspense>,
);
expect(Scheduler).toFlushAndYield(['Suspend! [A]', 'Loading...']);
expect(ReactNoop.getChildren()).toEqual([span('Loading...')]);
});
// @gate enableCache
it('suspends and shows inner undefined fallback', () => {
ReactNoop.render(
<Suspense fallback={<Text text="Should not show..." />}>
<Suspense>
<AsyncText text="A" ms={100} />
</Suspense>
</Suspense>,
);
expect(Scheduler).toFlushAndYield([
'Suspend! [A]',
// null
]);
expect(ReactNoop.getChildren()).toEqual([]);
});
// @gate enableCache
it('suspends and shows inner null fallback', () => {
ReactNoop.render(
<Suspense fallback={<Text text="Should not show..." />}>
<Suspense fallback={null}>
<AsyncText text="A" ms={100} />
</Suspense>
</Suspense>,
);
expect(Scheduler).toFlushAndYield([
'Suspend! [A]',
// null
]);
expect(ReactNoop.getChildren()).toEqual([]);
});
});

View File

@ -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 (
<Suspense fallback={<Text text="Loading" />}>
<SuspenseList revealOrder="together">
<Suspense>
<A />
</Suspense>
{showMore ? (
<>
<Suspense>
<B />
</Suspense>
<Suspense>
<C />
</Suspense>
</>
) : null}
</SuspenseList>
</Suspense>
);
}
ReactNoop.render(<Foo />);
expect(Scheduler).toFlushAndYield([
'Suspend! [A]',
// null
]);
expect(ReactNoop).toMatchRenderedOutput(null);
await A.resolve();
expect(Scheduler).toFlushAndYield(['A']);
expect(ReactNoop).toMatchRenderedOutput(<span>A</span>);
// Let's do an update that should consult the avoided boundaries.
ReactNoop.render(<Foo showMore={true} />);
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(<span>A</span>);
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(<span>A</span>);
await C.resolve();
expect(Scheduler).toFlushAndYield(['B', 'C']);
expect(ReactNoop).toMatchRenderedOutput(
<>
<span>A</span>
<span>B</span>
<span>C</span>
</>,
);
});
it('displays each items in "forwards" order', async () => {
const A = createAsyncText('A');
const B = createAsyncText('B');

View File

@ -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 (
<Suspense fallback={<Text text="Initial load..." />}>
<Suspense>
<AsyncText text="A" />
{showC ? <AsyncText text="C" /> : null}
</Suspense>
<Text text="B" />
</Suspense>
);
}
ReactNoop.render(<Foo />);
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(<Foo showC={true} />);
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 (
<Suspense fallback={<Text text="Initial load..." />}>
<Suspense fallback={undefined}>
<Text text="A" />
{showB ? (
<Suspense fallback={undefined}>
<AsyncText text="B" />
</Suspense>
) : null}
</Suspense>
</Suspense>
);
}
ReactNoop.render(<Foo />);
expect(Scheduler).toFlushAndYield(['Foo', 'A']);
expect(ReactNoop.getChildren()).toEqual([span('A')]);
if (gate(flags => flags.enableSyncDefaultUpdates)) {
React.startTransition(() => {
ReactNoop.render(<Foo showB={true} />);
});
} else {
ReactNoop.render(<Foo showB={true} />);
}
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}) {