Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions packages/react-reconciler/src/ReactChildFiber.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type {
Thenable,
ReactContext,
ReactDebugInfo,
SuspenseListRevealOrder,
} from 'shared/ReactTypes';
import type {Fiber} from './ReactInternalTypes';
import type {Lanes} from './ReactFiberLane';
Expand Down Expand Up @@ -2057,3 +2058,86 @@ export function resetChildFibers(workInProgress: Fiber, lanes: Lanes): void {
child = child.sibling;
}
}

function validateSuspenseListNestedChild(childSlot: mixed, index: number) {
if (__DEV__) {
const isAnArray = isArray(childSlot);
const isIterable =
!isAnArray && typeof getIteratorFn(childSlot) === 'function';
const isAsyncIterable =
enableAsyncIterableChildren &&
typeof childSlot === 'object' &&
childSlot !== null &&
typeof (childSlot: any)[ASYNC_ITERATOR] === 'function';
if (isAnArray || isIterable || isAsyncIterable) {
const type = isAnArray
? 'array'
: isAsyncIterable
? 'async iterable'
: 'iterable';
console.error(
'A nested %s was passed to row #%s in <SuspenseList />. Wrap it in ' +
'an additional SuspenseList to configure its revealOrder: ' +
'<SuspenseList revealOrder=...> ... ' +
'<SuspenseList revealOrder=...>{%s}</SuspenseList> ... ' +
'</SuspenseList>',
type,
index,
type,
);
return false;
}
}
return true;
}

export function validateSuspenseListChildren(
children: mixed,
revealOrder: SuspenseListRevealOrder,
) {
if (__DEV__) {
if (
(revealOrder === 'forwards' || revealOrder === 'backwards') &&
children !== undefined &&
children !== null &&
children !== false
) {
if (isArray(children)) {
for (let i = 0; i < children.length; i++) {
if (!validateSuspenseListNestedChild(children[i], i)) {
return;
}
}
} else {
const iteratorFn = getIteratorFn(children);
if (typeof iteratorFn === 'function') {
const childrenIterator = iteratorFn.call(children);
if (childrenIterator) {
let step = childrenIterator.next();
let i = 0;
for (; !step.done; step = childrenIterator.next()) {
if (!validateSuspenseListNestedChild(step.value, i)) {
return;
}
i++;
}
}
} else if (
enableAsyncIterableChildren &&
typeof (children: any)[ASYNC_ITERATOR] === 'function'
) {
// TODO: Technically we should warn for nested arrays inside the
// async iterable but it would require unwrapping the array.
// However, this mistake is not as easy to make so it's ok not to warn.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be costly to iterate this twice so we avoid it.

Ideally, we'd probably move the validateSuspenseListChildren warning to be integrated with the rest ChildFiber instead so that it can be done in a single pass. Even for sync iterators.

} else {
console.error(
'A single row was passed to a <SuspenseList revealOrder="%s" />. ' +
'This is not useful since it needs multiple rows. ' +
'Did you mean to pass multiple children or an array?',
revealOrder,
);
}
}
}
}
}
89 changes: 12 additions & 77 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ import {
enableViewTransition,
enableFragmentRefs,
} from 'shared/ReactFeatureFlags';
import isArray from 'shared/isArray';
import shallowEqual from 'shared/shallowEqual';
import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
import getComponentNameFromType from 'shared/getComponentNameFromType';
Expand All @@ -132,7 +131,6 @@ import {
REACT_LAZY_TYPE,
REACT_FORWARD_REF_TYPE,
REACT_MEMO_TYPE,
getIteratorFn,
} from 'shared/ReactSymbols';
import {setCurrentFiber} from './ReactCurrentFiber';
import {
Expand All @@ -145,6 +143,7 @@ import {
mountChildFibers,
reconcileChildFibers,
cloneChildFibers,
validateSuspenseListChildren,
} from './ReactChildFiber';
import {
processUpdateQueue,
Expand Down Expand Up @@ -3302,73 +3301,6 @@ function validateTailOptions(
}
}

function validateSuspenseListNestedChild(childSlot: mixed, index: number) {
if (__DEV__) {
const isAnArray = isArray(childSlot);
const isIterable =
!isAnArray && typeof getIteratorFn(childSlot) === 'function';
if (isAnArray || isIterable) {
const type = isAnArray ? 'array' : 'iterable';
console.error(
'A nested %s was passed to row #%s in <SuspenseList />. Wrap it in ' +
'an additional SuspenseList to configure its revealOrder: ' +
'<SuspenseList revealOrder=...> ... ' +
'<SuspenseList revealOrder=...>{%s}</SuspenseList> ... ' +
'</SuspenseList>',
type,
index,
type,
);
return false;
}
}
return true;
}

function validateSuspenseListChildren(
children: mixed,
revealOrder: SuspenseListRevealOrder,
) {
if (__DEV__) {
if (
(revealOrder === 'forwards' || revealOrder === 'backwards') &&
children !== undefined &&
children !== null &&
children !== false
) {
if (isArray(children)) {
for (let i = 0; i < children.length; i++) {
if (!validateSuspenseListNestedChild(children[i], i)) {
return;
}
}
} else {
const iteratorFn = getIteratorFn(children);
if (typeof iteratorFn === 'function') {
const childrenIterator = iteratorFn.call(children);
if (childrenIterator) {
let step = childrenIterator.next();
let i = 0;
for (; !step.done; step = childrenIterator.next()) {
if (!validateSuspenseListNestedChild(step.value, i)) {
return;
}
i++;
}
}
} else {
console.error(
'A single row was passed to a <SuspenseList revealOrder="%s" />. ' +
'This is not useful since it needs multiple rows. ' +
'Did you mean to pass multiple children or an array?',
revealOrder,
);
}
}
}
}
}

function initSuspenseListRenderState(
workInProgress: Fiber,
isBackwards: boolean,
Expand Down Expand Up @@ -3415,12 +3347,6 @@ function updateSuspenseListComponent(
const tailMode: SuspenseListTailMode = nextProps.tail;
const newChildren = nextProps.children;

validateRevealOrder(revealOrder);
validateTailOptions(tailMode, revealOrder);
validateSuspenseListChildren(newChildren, revealOrder);

reconcileChildren(current, workInProgress, newChildren, renderLanes);

let suspenseContext: SuspenseContext = suspenseStackCursor.current;

const shouldForceFallback = hasSuspenseListContext(
Expand All @@ -3434,6 +3360,17 @@ function updateSuspenseListComponent(
);
workInProgress.flags |= DidCapture;
} else {
suspenseContext = setDefaultShallowSuspenseListContext(suspenseContext);
}
pushSuspenseListContext(workInProgress, suspenseContext);

validateRevealOrder(revealOrder);
validateTailOptions(tailMode, revealOrder);
validateSuspenseListChildren(newChildren, revealOrder);

reconcileChildren(current, workInProgress, newChildren, renderLanes);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reconcileChildren can suspend which lead to the SuspenseListContext to be unbalanced so it needs to push first.


if (!shouldForceFallback) {
const didSuspendBefore =
current !== null && (current.flags & DidCapture) !== NoFlags;
if (didSuspendBefore) {
Expand All @@ -3446,9 +3383,7 @@ function updateSuspenseListComponent(
renderLanes,
);
}
suspenseContext = setDefaultShallowSuspenseListContext(suspenseContext);
}
pushSuspenseListContext(workInProgress, suspenseContext);

if (!disableLegacyMode && (workInProgress.mode & ConcurrentMode) === NoMode) {
// In legacy mode, SuspenseList doesn't work so we just
Expand Down
155 changes: 155 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3119,4 +3119,159 @@ describe('ReactSuspenseList', () => {
);
},
);

// @gate enableSuspenseList && enableAsyncIterableChildren
it('can display async iterable in "forwards" order', async () => {
const A = createAsyncText('A');
const B = createAsyncText('B');

// We use Cached elements to avoid rerender.
const ASlot = (
<Suspense key="A" fallback={<Text text="Loading A" />}>
<A />
</Suspense>
);

const BSlot = (
<Suspense key="B" fallback={<Text text="Loading B" />}>
<B />
</Suspense>
);

const iterable = {
async *[Symbol.asyncIterator]() {
yield ASlot;
yield BSlot;
},
};

function Foo() {
return <SuspenseList revealOrder="forwards">{iterable}</SuspenseList>;
}

await act(() => {
React.startTransition(() => {
ReactNoop.render(<Foo />);
});
});

assertLog([
'Suspend! [A]',
'Loading A',
'Loading B',
// pre-warming
'Suspend! [A]',
]);

assertConsoleErrorDev([
// We get this warning because the generator's promise themselves are not cached.
'A component was suspended by an uncached promise. ' +
'Creating promises inside a Client Component or hook is not yet supported, ' +
'except via a Suspense-compatible library or framework.\n' +
' in SuspenseList (at **)\n' +
' in Foo (at **)',
'A component was suspended by an uncached promise. ' +
'Creating promises inside a Client Component or hook is not yet supported, ' +
'except via a Suspense-compatible library or framework.\n' +
' in SuspenseList (at **)\n' +
' in Foo (at **)',
]);

expect(ReactNoop).toMatchRenderedOutput(
<>
<span>Loading A</span>
<span>Loading B</span>
</>,
);

await act(() => A.resolve());
assertLog(['A', 'Suspend! [B]', 'Suspend! [B]']);

assertConsoleErrorDev([
// We get this warning because the generator's promise themselves are not cached.
'A component was suspended by an uncached promise. ' +
'Creating promises inside a Client Component or hook is not yet supported, ' +
'except via a Suspense-compatible library or framework.\n' +
' in SuspenseList (at **)\n' +
' in Foo (at **)',
'A component was suspended by an uncached promise. ' +
'Creating promises inside a Client Component or hook is not yet supported, ' +
'except via a Suspense-compatible library or framework.\n' +
' in SuspenseList (at **)\n' +
' in Foo (at **)',
]);

expect(ReactNoop).toMatchRenderedOutput(
<>
<span>A</span>
<span>Loading B</span>
</>,
);

await act(() => B.resolve());
assertLog(['B']);

assertConsoleErrorDev([
// We get this warning because the generator's promise themselves are not cached.
'A component was suspended by an uncached promise. ' +
'Creating promises inside a Client Component or hook is not yet supported, ' +
'except via a Suspense-compatible library or framework.\n' +
' in SuspenseList (at **)\n' +
' in Foo (at **)',
]);

expect(ReactNoop).toMatchRenderedOutput(
<>
<span>A</span>
<span>B</span>
</>,
);
});

// @gate enableSuspenseList && enableAsyncIterableChildren
it('warns if a nested async iterable is passed to a "forwards" list', async () => {
function Foo({items}) {
return (
<SuspenseList revealOrder="forwards">
{items}
<div>Tail</div>
</SuspenseList>
);
}

const iterable = {
async *[Symbol.asyncIterator]() {
yield (
<Suspense key={'A'} fallback="Loading">
A
</Suspense>
);
yield (
<Suspense key={'B'} fallback="Loading">
B
</Suspense>
);
},
};

await act(() => {
React.startTransition(() => {
ReactNoop.render(<Foo items={iterable} />);
});
});
assertConsoleErrorDev([
'A nested async iterable was passed to row #0 in <SuspenseList />. ' +
'Wrap it in an additional SuspenseList to configure its revealOrder: ' +
'<SuspenseList revealOrder=...> ... ' +
'<SuspenseList revealOrder=...>{async iterable}</SuspenseList> ... ' +
'</SuspenseList>' +
'\n in SuspenseList (at **)' +
'\n in Foo (at **)',
// We get this warning because the generator's promise themselves are not cached.
'A component was suspended by an uncached promise. ' +
'Creating promises inside a Client Component or hook is not yet supported, ' +
'except via a Suspense-compatible library or framework.\n' +
' in Foo (at **)',
]);
});
});
Loading