mirror of
https://github.com/zebrajr/react.git
synced 2025-12-06 00:20:04 +01:00
Switch the default revealOrder to "forwards" and tail "hidden" on SuspenseList (#35018)
We have warned about this for a while now so we can make the switch. Often when you reach for SuspenseList, you mean forwards. It doesn't make sense to have the default to just be a noop. While "together" is another useful mode that's more like a Group so isn't so associated with the default as List. So we're switching it. However, tail=hidden isn't as obvious of a default it does allow for a convenient pattern for streaming in list of items by default. This doesn't yet switch the rendering order of "backwards". That's coming in a follow up.
This commit is contained in:
parent
c9ddee7e36
commit
26cf280480
|
|
@ -134,7 +134,7 @@ describe('ReactDOMFizzSuspenseList', () => {
|
|||
}
|
||||
|
||||
// @gate enableSuspenseList
|
||||
it('shows content independently by default', async () => {
|
||||
it('shows content forwards by default', async () => {
|
||||
const A = createAsyncText('A');
|
||||
const B = createAsyncText('B');
|
||||
const C = createAsyncText('C');
|
||||
|
|
@ -157,14 +157,32 @@ describe('ReactDOMFizzSuspenseList', () => {
|
|||
);
|
||||
}
|
||||
|
||||
await A.resolve();
|
||||
await C.resolve();
|
||||
|
||||
await serverAct(async () => {
|
||||
const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<Foo />);
|
||||
pipe(writable);
|
||||
});
|
||||
|
||||
assertLog(['A', 'Suspend! [B]', 'Suspend! [C]', 'Loading B', 'Loading C']);
|
||||
assertLog([
|
||||
'Suspend! [A]',
|
||||
'Suspend! [B]', // TODO: Defer rendering the content after fallback if previous suspended,
|
||||
'C',
|
||||
'Loading A',
|
||||
'Loading B',
|
||||
'Loading C',
|
||||
]);
|
||||
|
||||
expect(getVisibleChildren(container)).toEqual(
|
||||
<div>
|
||||
<span>Loading A</span>
|
||||
<span>Loading B</span>
|
||||
<span>Loading C</span>
|
||||
</div>,
|
||||
);
|
||||
|
||||
await serverAct(() => A.resolve());
|
||||
assertLog(['A']);
|
||||
|
||||
expect(getVisibleChildren(container)).toEqual(
|
||||
<div>
|
||||
|
|
@ -174,17 +192,6 @@ describe('ReactDOMFizzSuspenseList', () => {
|
|||
</div>,
|
||||
);
|
||||
|
||||
await serverAct(() => C.resolve());
|
||||
assertLog(['C']);
|
||||
|
||||
expect(getVisibleChildren(container)).toEqual(
|
||||
<div>
|
||||
<span>A</span>
|
||||
<span>Loading B</span>
|
||||
<span>C</span>
|
||||
</div>,
|
||||
);
|
||||
|
||||
await serverAct(() => B.resolve());
|
||||
assertLog(['B']);
|
||||
|
||||
|
|
|
|||
|
|
@ -2104,7 +2104,8 @@ export function validateSuspenseListChildren(
|
|||
) {
|
||||
if (__DEV__) {
|
||||
if (
|
||||
(revealOrder === 'forwards' ||
|
||||
(revealOrder == null ||
|
||||
revealOrder === 'forwards' ||
|
||||
revealOrder === 'backwards' ||
|
||||
revealOrder === 'unstable_legacy-backwards') &&
|
||||
children !== undefined &&
|
||||
|
|
|
|||
|
|
@ -3245,6 +3245,7 @@ function validateRevealOrder(revealOrder: SuspenseListRevealOrder) {
|
|||
if (__DEV__) {
|
||||
const cacheKey = revealOrder == null ? 'null' : revealOrder;
|
||||
if (
|
||||
revealOrder != null &&
|
||||
revealOrder !== 'forwards' &&
|
||||
revealOrder !== 'unstable_legacy-backwards' &&
|
||||
revealOrder !== 'together' &&
|
||||
|
|
@ -3252,13 +3253,7 @@ function validateRevealOrder(revealOrder: SuspenseListRevealOrder) {
|
|||
!didWarnAboutRevealOrder[cacheKey]
|
||||
) {
|
||||
didWarnAboutRevealOrder[cacheKey] = true;
|
||||
if (revealOrder == null) {
|
||||
console.error(
|
||||
'The default for the <SuspenseList revealOrder="..."> prop is changing. ' +
|
||||
'To be future compatible you must explictly specify either ' +
|
||||
'"independent" (the current default), "together", "forwards" or "legacy_unstable-backwards".',
|
||||
);
|
||||
} else if (revealOrder === 'backwards') {
|
||||
if (revealOrder === 'backwards') {
|
||||
console.error(
|
||||
'The rendering order of <SuspenseList revealOrder="backwards"> is changing. ' +
|
||||
'To be future compatible you must specify revealOrder="legacy_unstable-backwards" instead.',
|
||||
|
|
@ -3314,18 +3309,7 @@ function validateTailOptions(
|
|||
const cacheKey = tailMode == null ? 'null' : tailMode;
|
||||
if (!didWarnAboutTailOptions[cacheKey]) {
|
||||
if (tailMode == null) {
|
||||
if (
|
||||
revealOrder === 'forwards' ||
|
||||
revealOrder === 'backwards' ||
|
||||
revealOrder === 'unstable_legacy-backwards'
|
||||
) {
|
||||
didWarnAboutTailOptions[cacheKey] = true;
|
||||
console.error(
|
||||
'The default for the <SuspenseList tail="..."> prop is changing. ' +
|
||||
'To be future compatible you must explictly specify either ' +
|
||||
'"visible" (the current default), "collapsed" or "hidden".',
|
||||
);
|
||||
}
|
||||
// The default tail is now "hidden".
|
||||
} else if (
|
||||
tailMode !== 'visible' &&
|
||||
tailMode !== 'collapsed' &&
|
||||
|
|
@ -3338,6 +3322,7 @@ function validateTailOptions(
|
|||
tailMode,
|
||||
);
|
||||
} else if (
|
||||
revealOrder != null &&
|
||||
revealOrder !== 'forwards' &&
|
||||
revealOrder !== 'backwards' &&
|
||||
revealOrder !== 'unstable_legacy-backwards'
|
||||
|
|
@ -3345,7 +3330,7 @@ function validateTailOptions(
|
|||
didWarnAboutTailOptions[cacheKey] = true;
|
||||
console.error(
|
||||
'<SuspenseList tail="%s" /> is only valid if revealOrder is ' +
|
||||
'"forwards" or "backwards". ' +
|
||||
'"forwards" (default) or "backwards". ' +
|
||||
'Did you mean to specify revealOrder="forwards"?',
|
||||
tailMode,
|
||||
);
|
||||
|
|
@ -3449,30 +3434,6 @@ function updateSuspenseListComponent(
|
|||
workInProgress.memoizedState = null;
|
||||
} else {
|
||||
switch (revealOrder) {
|
||||
case 'forwards': {
|
||||
const lastContentRow = findLastContentRow(workInProgress.child);
|
||||
let tail;
|
||||
if (lastContentRow === null) {
|
||||
// The whole list is part of the tail.
|
||||
// TODO: We could fast path by just rendering the tail now.
|
||||
tail = workInProgress.child;
|
||||
workInProgress.child = null;
|
||||
} else {
|
||||
// Disconnect the tail rows after the content row.
|
||||
// We're going to render them separately later.
|
||||
tail = lastContentRow.sibling;
|
||||
lastContentRow.sibling = null;
|
||||
}
|
||||
initSuspenseListRenderState(
|
||||
workInProgress,
|
||||
false, // isBackwards
|
||||
tail,
|
||||
lastContentRow,
|
||||
tailMode,
|
||||
treeForkCount,
|
||||
);
|
||||
break;
|
||||
}
|
||||
case 'backwards':
|
||||
case 'unstable_legacy-backwards': {
|
||||
// We're going to find the first row that has existing content.
|
||||
|
|
@ -3517,10 +3478,37 @@ function updateSuspenseListComponent(
|
|||
);
|
||||
break;
|
||||
}
|
||||
default: {
|
||||
// The default reveal order is the same as not having
|
||||
case 'independent': {
|
||||
// The "independent" reveal order is the same as not having
|
||||
// a boundary.
|
||||
workInProgress.memoizedState = null;
|
||||
break;
|
||||
}
|
||||
// The default is now forwards.
|
||||
case 'forwards':
|
||||
default: {
|
||||
const lastContentRow = findLastContentRow(workInProgress.child);
|
||||
let tail;
|
||||
if (lastContentRow === null) {
|
||||
// The whole list is part of the tail.
|
||||
// TODO: We could fast path by just rendering the tail now.
|
||||
tail = workInProgress.child;
|
||||
workInProgress.child = null;
|
||||
} else {
|
||||
// Disconnect the tail rows after the content row.
|
||||
// We're going to render them separately later.
|
||||
tail = lastContentRow.sibling;
|
||||
lastContentRow.sibling = null;
|
||||
}
|
||||
initSuspenseListRenderState(
|
||||
workInProgress,
|
||||
false, // isBackwards
|
||||
tail,
|
||||
lastContentRow,
|
||||
tailMode,
|
||||
treeForkCount,
|
||||
);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -698,30 +698,8 @@ function cutOffTailIfNeeded(
|
|||
return;
|
||||
}
|
||||
switch (renderState.tailMode) {
|
||||
case 'hidden': {
|
||||
// Any insertions at the end of the tail list after this point
|
||||
// should be invisible. If there are already mounted boundaries
|
||||
// anything before them are not considered for collapsing.
|
||||
// Therefore we need to go through the whole tail to find if
|
||||
// there are any.
|
||||
let tailNode = renderState.tail;
|
||||
let lastTailNode = null;
|
||||
while (tailNode !== null) {
|
||||
if (tailNode.alternate !== null) {
|
||||
lastTailNode = tailNode;
|
||||
}
|
||||
tailNode = tailNode.sibling;
|
||||
}
|
||||
// Next we're simply going to delete all insertions after the
|
||||
// last rendered item.
|
||||
if (lastTailNode === null) {
|
||||
// All remaining items in the tail are insertions.
|
||||
renderState.tail = null;
|
||||
} else {
|
||||
// Detach the insertion after the last node that was already
|
||||
// inserted.
|
||||
lastTailNode.sibling = null;
|
||||
}
|
||||
case 'visible': {
|
||||
// Everything should remain as it was.
|
||||
break;
|
||||
}
|
||||
case 'collapsed': {
|
||||
|
|
@ -756,6 +734,34 @@ function cutOffTailIfNeeded(
|
|||
}
|
||||
break;
|
||||
}
|
||||
// Hidden is now the default.
|
||||
case 'hidden':
|
||||
default: {
|
||||
// Any insertions at the end of the tail list after this point
|
||||
// should be invisible. If there are already mounted boundaries
|
||||
// anything before them are not considered for collapsing.
|
||||
// Therefore we need to go through the whole tail to find if
|
||||
// there are any.
|
||||
let tailNode = renderState.tail;
|
||||
let lastTailNode = null;
|
||||
while (tailNode !== null) {
|
||||
if (tailNode.alternate !== null) {
|
||||
lastTailNode = tailNode;
|
||||
}
|
||||
tailNode = tailNode.sibling;
|
||||
}
|
||||
// Next we're simply going to delete all insertions after the
|
||||
// last rendered item.
|
||||
if (lastTailNode === null) {
|
||||
// All remaining items in the tail are insertions.
|
||||
renderState.tail = null;
|
||||
} else {
|
||||
// Detach the insertion after the last node that was already
|
||||
// inserted.
|
||||
lastTailNode.sibling = null;
|
||||
}
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -1795,7 +1801,8 @@ function completeWork(
|
|||
// This might have been modified.
|
||||
if (
|
||||
renderState.tail === null &&
|
||||
renderState.tailMode === 'hidden' &&
|
||||
renderState.tailMode !== 'collapsed' &&
|
||||
renderState.tailMode !== 'visible' &&
|
||||
!renderedTail.alternate &&
|
||||
!getIsHydrating() // We don't cut it if we're hydrating.
|
||||
) {
|
||||
|
|
|
|||
|
|
@ -79,10 +79,7 @@ export function findFirstSuspended(row: Fiber): null | Fiber {
|
|||
node.tag === SuspenseListComponent &&
|
||||
// Independent revealOrder can't be trusted because it doesn't
|
||||
// keep track of whether it suspended or not.
|
||||
(node.memoizedProps.revealOrder === 'forwards' ||
|
||||
node.memoizedProps.revealOrder === 'backwards' ||
|
||||
node.memoizedProps.revealOrder === 'unstable_legacy-backwards' ||
|
||||
node.memoizedProps.revealOrder === 'together')
|
||||
node.memoizedProps.revealOrder !== 'independent'
|
||||
) {
|
||||
const didSuspend = (node.flags & DidCapture) !== NoFlags;
|
||||
if (didSuspend) {
|
||||
|
|
|
|||
|
|
@ -218,7 +218,7 @@ describe('ReactSuspenseList', () => {
|
|||
});
|
||||
|
||||
// @gate enableSuspenseList
|
||||
it('warns if no revealOrder is specified', async () => {
|
||||
it('behaves as revealOrder=forwards by default', async () => {
|
||||
const A = createAsyncText('A');
|
||||
const B = createAsyncText('B');
|
||||
const C = createAsyncText('C');
|
||||
|
|
@ -239,54 +239,36 @@ describe('ReactSuspenseList', () => {
|
|||
);
|
||||
}
|
||||
|
||||
await A.resolve();
|
||||
|
||||
ReactNoop.render(<Foo />);
|
||||
|
||||
await waitForAll([
|
||||
'A',
|
||||
'Suspend! [B]',
|
||||
'Loading B',
|
||||
'Suspend! [C]',
|
||||
'Loading C',
|
||||
// pre-warming
|
||||
'Suspend! [B]',
|
||||
'Suspend! [C]',
|
||||
]);
|
||||
await waitForAll(['Suspend! [A]', 'Loading A']);
|
||||
|
||||
assertConsoleErrorDev([
|
||||
'The default for the <SuspenseList revealOrder="..."> prop is changing. ' +
|
||||
'To be future compatible you must explictly specify either ' +
|
||||
'"independent" (the current default), "together", "forwards" or "legacy_unstable-backwards".' +
|
||||
'\n in SuspenseList (at **)' +
|
||||
'\n in Foo (at **)',
|
||||
]);
|
||||
expect(ReactNoop).toMatchRenderedOutput(null);
|
||||
|
||||
await A.resolve();
|
||||
|
||||
await waitForAll(['A', 'Suspend! [B]', 'Loading B']);
|
||||
|
||||
// Incremental loading is suspended.
|
||||
jest.advanceTimersByTime(500);
|
||||
|
||||
expect(ReactNoop).toMatchRenderedOutput(<span>A</span>);
|
||||
|
||||
await act(() => B.resolve());
|
||||
assertLog(['B', 'Suspend! [C]', 'Loading C']);
|
||||
|
||||
// Incremental loading is suspended.
|
||||
jest.advanceTimersByTime(500);
|
||||
|
||||
expect(ReactNoop).toMatchRenderedOutput(
|
||||
<>
|
||||
<span>A</span>
|
||||
<span>Loading B</span>
|
||||
<span>Loading C</span>
|
||||
<span>B</span>
|
||||
</>,
|
||||
);
|
||||
|
||||
await act(() => C.resolve());
|
||||
assertLog(
|
||||
gate('alwaysThrottleRetries')
|
||||
? ['Suspend! [B]', 'C', 'Suspend! [B]']
|
||||
: ['C'],
|
||||
);
|
||||
|
||||
expect(ReactNoop).toMatchRenderedOutput(
|
||||
<>
|
||||
<span>A</span>
|
||||
<span>Loading B</span>
|
||||
<span>C</span>
|
||||
</>,
|
||||
);
|
||||
|
||||
await act(() => B.resolve());
|
||||
assertLog(['B']);
|
||||
assertLog(['C']);
|
||||
|
||||
expect(ReactNoop).toMatchRenderedOutput(
|
||||
<>
|
||||
|
|
@ -1699,26 +1681,65 @@ describe('ReactSuspenseList', () => {
|
|||
});
|
||||
|
||||
// @gate enableSuspenseList
|
||||
it('warns if no tail option is specified', async () => {
|
||||
it('behaves as tail=hidden if no tail option is specified', async () => {
|
||||
const A = createAsyncText('A');
|
||||
const B = createAsyncText('B');
|
||||
const C = createAsyncText('C');
|
||||
|
||||
function Foo() {
|
||||
return (
|
||||
<SuspenseList revealOrder="forwards">
|
||||
<Suspense fallback="Loading">A</Suspense>
|
||||
<Suspense fallback="Loading">B</Suspense>
|
||||
<Suspense fallback={<Text text="Loading A" />}>
|
||||
<A />
|
||||
</Suspense>
|
||||
<Suspense fallback={<Text text="Loading B" />}>
|
||||
<B />
|
||||
</Suspense>
|
||||
<Suspense fallback={<Text text="Loading C" />}>
|
||||
<C />
|
||||
</Suspense>
|
||||
</SuspenseList>
|
||||
);
|
||||
}
|
||||
|
||||
await act(() => {
|
||||
ReactNoop.render(<Foo />);
|
||||
});
|
||||
assertConsoleErrorDev([
|
||||
'The default for the <SuspenseList tail="..."> prop is changing. ' +
|
||||
'To be future compatible you must explictly specify either ' +
|
||||
'"visible" (the current default), "collapsed" or "hidden".' +
|
||||
'\n in SuspenseList (at **)' +
|
||||
'\n in Foo (at **)',
|
||||
]);
|
||||
ReactNoop.render(<Foo />);
|
||||
|
||||
await waitForAll(['Suspend! [A]', 'Loading A']);
|
||||
|
||||
expect(ReactNoop).toMatchRenderedOutput(null);
|
||||
|
||||
await A.resolve();
|
||||
|
||||
await waitForAll(['A', 'Suspend! [B]', 'Loading B']);
|
||||
|
||||
// Incremental loading is suspended.
|
||||
jest.advanceTimersByTime(500);
|
||||
|
||||
expect(ReactNoop).toMatchRenderedOutput(<span>A</span>);
|
||||
|
||||
await act(() => B.resolve());
|
||||
assertLog(['B', 'Suspend! [C]', 'Loading C']);
|
||||
|
||||
// Incremental loading is suspended.
|
||||
jest.advanceTimersByTime(500);
|
||||
|
||||
expect(ReactNoop).toMatchRenderedOutput(
|
||||
<>
|
||||
<span>A</span>
|
||||
<span>B</span>
|
||||
</>,
|
||||
);
|
||||
|
||||
await act(() => C.resolve());
|
||||
assertLog(['C']);
|
||||
|
||||
expect(ReactNoop).toMatchRenderedOutput(
|
||||
<>
|
||||
<span>A</span>
|
||||
<span>B</span>
|
||||
<span>C</span>
|
||||
</>,
|
||||
);
|
||||
});
|
||||
|
||||
// @gate enableSuspenseList
|
||||
|
|
@ -1758,7 +1779,7 @@ describe('ReactSuspenseList', () => {
|
|||
});
|
||||
assertConsoleErrorDev([
|
||||
'<SuspenseList tail="collapsed" /> is only valid if ' +
|
||||
'revealOrder is "forwards" or "backwards". ' +
|
||||
'revealOrder is "forwards" (default) or "backwards". ' +
|
||||
'Did you mean to specify revealOrder="forwards"?' +
|
||||
'\n in SuspenseList (at **)' +
|
||||
'\n in Foo (at **)',
|
||||
|
|
|
|||
8
packages/react-server/src/ReactFizzServer.js
vendored
8
packages/react-server/src/ReactFizzServer.js
vendored
|
|
@ -1913,7 +1913,7 @@ function renderSuspenseListRows(
|
|||
task: Task,
|
||||
keyPath: KeyNode,
|
||||
rows: Array<ReactNodeList>,
|
||||
revealOrder: 'forwards' | 'backwards' | 'unstable_legacy-backwards',
|
||||
revealOrder: void | 'forwards' | 'backwards' | 'unstable_legacy-backwards',
|
||||
): void {
|
||||
// This is a fork of renderChildrenArray that's aware of tracking rows.
|
||||
const prevKeyPath = task.keyPath;
|
||||
|
|
@ -2098,11 +2098,7 @@ function renderSuspenseList(
|
|||
const revealOrder: SuspenseListRevealOrder = props.revealOrder;
|
||||
// TODO: Support tail hidden/collapsed modes.
|
||||
// const tailMode: SuspenseListTailMode = props.tail;
|
||||
if (
|
||||
revealOrder === 'forwards' ||
|
||||
revealOrder === 'backwards' ||
|
||||
revealOrder === 'unstable_legacy-backwards'
|
||||
) {
|
||||
if (revealOrder !== 'independent' && revealOrder !== 'together') {
|
||||
// For ordered reveal, we need to produce rows from the children.
|
||||
if (isArray(children)) {
|
||||
renderSuspenseListRows(request, task, keyPath, children, revealOrder);
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user