Skip to content

[Fiber] Support AsyncIterable children in SuspenseList #33299

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 20, 2025

Conversation

sebmarkbage
Copy link
Collaborator

We support AsyncIterable (more so when it's a cached form like in coming from Flight) as children.

This fixes some warnings and bugs when passed to SuspenseList.

Ideally SuspenseList with tail="hidden" should support unblocking before the full result has resolved but that's an optimization on top. We also might want to change semantics for this for revealOrder="backwards" so it becomes possible to stream items in reverse order.

@sebmarkbage sebmarkbage requested a review from rickhanlonii May 18, 2025 02:42
@github-actions github-actions bot added the React Core Team Opened by a member of the React Core Team label May 18, 2025
) {
// 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.

@react-sizebot
Copy link

react-sizebot commented May 18, 2025

Comparing: 462d08f...504a75a

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.js = 6.68 kB 6.68 kB = 1.83 kB 1.83 kB
oss-stable/react-dom/cjs/react-dom-client.production.js +0.02% 529.74 kB 529.83 kB +0.03% 93.49 kB 93.52 kB
oss-experimental/react-dom/cjs/react-dom.production.js = 6.69 kB 6.69 kB = 1.83 kB 1.83 kB
oss-experimental/react-dom/cjs/react-dom-client.production.js +0.01% 651.48 kB 651.57 kB +0.02% 114.76 kB 114.78 kB
facebook-www/ReactDOM-prod.classic.js +0.01% 675.72 kB 675.81 kB +0.02% 118.85 kB 118.87 kB
facebook-www/ReactDOM-prod.modern.js +0.01% 666.00 kB 666.09 kB +0.02% 117.23 kB 117.26 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
test_utils/ReactAllWarnings.js +0.26% 65.70 kB 65.87 kB +0.24% 16.50 kB 16.54 kB

Generated by 🚫 dangerJS against 504a75a

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.

function Foo() {
return (
<SuspenseList revealOrder="forwards">
<Generator />
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This pattern would be very convenient but we can't support it since we need to resolve the rows first.

Interestingly this pattern is supported in Server Components because they get resolved into the iterable on the server and the component wrapper disappears. It's just not supported in Client Components.

@sebmarkbage sebmarkbage force-pushed the suspenselistasynciterator branch from 36c245f to 504a75a Compare May 18, 2025 20:53
@sebmarkbage sebmarkbage merged commit 4c6967b into facebook:main May 20, 2025
240 checks passed
github-actions bot pushed a commit that referenced this pull request May 20, 2025
We support AsyncIterable (more so when it's a cached form like in coming
from Flight) as children.

This fixes some warnings and bugs when passed to SuspenseList.

Ideally SuspenseList with `tail="hidden"` should support unblocking
before the full result has resolved but that's an optimization on top.
We also might want to change semantics for this for
`revealOrder="backwards"` so it becomes possible to stream items in
reverse order.

DiffTrain build for [4c6967b](4c6967b)
github-actions bot pushed a commit that referenced this pull request May 20, 2025
We support AsyncIterable (more so when it's a cached form like in coming
from Flight) as children.

This fixes some warnings and bugs when passed to SuspenseList.

Ideally SuspenseList with `tail="hidden"` should support unblocking
before the full result has resolved but that's an optimization on top.
We also might want to change semantics for this for
`revealOrder="backwards"` so it becomes possible to stream items in
reverse order.

DiffTrain build for [4c6967b](4c6967b)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants