Skip to content
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

[Fiber/Fizz] Support AsyncIterable as Children and AsyncGenerator Client Components #28868

Merged
merged 4 commits into from
Apr 22, 2024

Conversation

sebmarkbage
Copy link
Collaborator

Stacked on #28849, #28854, #28853. Behind a flag.

If you're following along from the side-lines. This is probably not what you think it is.

It's NOT a way to get updates to a component over time. The AsyncIterable works like an Iterable already works in React which is how an Array works. I.e. it's a list of children - not the value of a child over time.

It also doesn't actually render one component at a time. The way it works is more like awaiting the entire list to become an array and then it shows up. Before that it suspends the parent.

To actually get these to display one at a time, you have to opt-in with <SuspenseList> to describe how they should appear. That's really the interesting part and that not implemented yet.

Additionally, since these are effectively Async Functions and uncached promises, they're not actually fully "supported" on the client yet for the same reason rendering plain Promises and Async Functions aren't. They warn. It's only really useful when paired with RSC that produces instrumented versions of these. Ideally we'd published instrumented helpers to help with map/filter style operations that yield new instrumented AsyncIterables.

The way the implementation works basically just relies on unwrapThenable and otherwise works like a plain Iterator.

There is one quirk with these that are different than just promises. We ask for a new iterator each time we rerender. This means that upon retry we kick off another iteration which itself might kick off new requests that block iterating further. To solve this and make it actually efficient enough to use on the client we'd need to stash something like a buffer of the previous iteration and maybe iterator on the iterable so that we can continue where we left off or synchronously iterate if we've seen it before. Similar to our .value convention on Promises.

In Fizz, I had to do a special case because when we render an iterator child we don't actually rerender the parent again like we do in Fiber. However, it's more efficient to just continue on where we left off by reusing the entries from the thenable state from before in that case.

@sebmarkbage sebmarkbage requested a review from acdlite April 18, 2024 17:26
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Apr 18, 2024
@react-sizebot
Copy link

react-sizebot commented Apr 18, 2024

Comparing: 5b903cd...453d657

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 = 497.54 kB 497.54 kB = 88.93 kB 88.89 kB
oss-experimental/react-dom/cjs/react-dom.production.js +0.19% 502.86 kB 503.83 kB +0.08% 89.84 kB 89.91 kB
facebook-www/ReactDOM-prod.classic.js = 591.00 kB 591.01 kB = 103.95 kB 103.89 kB
facebook-www/ReactDOM-prod.modern.js = 566.82 kB 566.82 kB = 100.15 kB 100.10 kB
oss-experimental/react-server/cjs/react-server.development.js +2.16% 209.21 kB 213.73 kB +1.23% 48.66 kB 49.26 kB
test_utils/ReactAllWarnings.js Deleted 64.68 kB 0.00 kB Deleted 16.15 kB 0.00 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/react-server/cjs/react-server.development.js +2.16% 209.21 kB 213.73 kB +1.23% 48.66 kB 49.26 kB
oss-experimental/react-server/cjs/react-server.production.js +1.57% 114.16 kB 115.95 kB +1.44% 20.65 kB 20.95 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.development.js +1.05% 430.60 kB 435.12 kB +0.62% 96.17 kB 96.77 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.development.js +1.04% 436.57 kB 441.09 kB +0.61% 98.01 kB 98.61 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.development.js +1.04% 436.58 kB 441.10 kB +0.61% 98.01 kB 98.61 kB
oss-experimental/react-dom/cjs/react-dom-server.node.development.js +1.02% 443.86 kB 448.38 kB +0.61% 98.31 kB 98.91 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.development.js +1.01% 445.49 kB 450.01 kB +0.60% 99.16 kB 99.76 kB
oss-experimental/react-dom/cjs/react-dom-server.edge.development.js +1.01% 446.07 kB 450.59 kB +0.60% 99.29 kB 99.89 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.browser.production.js +0.81% 209.43 kB 211.12 kB +0.67% 38.08 kB 38.33 kB
oss-experimental/react-dom/cjs/react-dom-server.bun.production.js +0.79% 214.28 kB 215.97 kB +0.70% 39.04 kB 39.32 kB
oss-experimental/react-dom/cjs/react-dom-server-legacy.node.production.js +0.79% 214.47 kB 216.16 kB +0.68% 39.93 kB 40.21 kB
oss-experimental/react-dom/cjs/react-dom-server.browser.production.js +0.72% 233.79 kB 235.48 kB +0.67% 40.94 kB 41.21 kB
oss-experimental/react-dom/cjs/react-dom-server.node.production.js +0.72% 235.88 kB 237.57 kB +0.66% 42.31 kB 42.59 kB
oss-experimental/react-dom/cjs/react-dom-server.edge.production.js +0.71% 239.51 kB 241.20 kB +0.64% 42.96 kB 43.23 kB
oss-experimental/react-art/cjs/react-art.production.js +0.34% 294.00 kB 294.99 kB +0.15% 50.79 kB 50.87 kB
oss-experimental/react-art/cjs/react-art.development.js +0.30% 821.65 kB 824.08 kB +0.16% 177.98 kB 178.26 kB
oss-experimental/react-reconciler/cjs/react-reconciler.production.js +0.28% 379.57 kB 380.61 kB +0.14% 62.26 kB 62.34 kB
oss-experimental/react-reconciler/cjs/react-reconciler.development.js +0.26% 924.58 kB 927.00 kB +0.13% 198.72 kB 198.98 kB
oss-experimental/react-reconciler/cjs/react-reconciler.profiling.js +0.26% 407.77 kB 408.81 kB +0.13% 66.25 kB 66.34 kB
oss-stable-semver/react-server/cjs/react-server.production.js +0.23% 104.96 kB 105.20 kB +0.10% 19.26 kB 19.28 kB
oss-stable/react-server/cjs/react-server.production.js +0.23% 104.96 kB 105.20 kB +0.10% 19.26 kB 19.28 kB
test_utils/ReactAllWarnings.js Deleted 64.68 kB 0.00 kB Deleted 16.15 kB 0.00 kB

Generated by 🚫 dangerJS against 453d657

@unstubbable
Copy link
Collaborator

If you're following along from the side-lines.

Thanks for letting us follow along by writing these detailed PR descriptions! 👍
Also, the unit tests help a lot. 🙂

@sebmarkbage
Copy link
Collaborator Author

Without SuspenseList these are mainly useful for parity between SuspenseList children and other children. It's also a performance optimization when used with RSC because you can start streaming in Server Components over the wire earlier instead of blocking for all items of a list to be known.

Copy link
Collaborator

@acdlite acdlite left a comment

Choose a reason for hiding this comment

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

We should add this as a Usable type, too, yes?

@sebmarkbage
Copy link
Collaborator Author

sebmarkbage commented Apr 19, 2024

Well, currently it's not actually possible to pass to use(). It's more like a variant of Iterable / Array than a variant of Promise. So it's a type of ReactNode. We can separately add it to use() though if we decide it's useful (pun intended).

@acdlite
Copy link
Collaborator

acdlite commented Apr 19, 2024

yeah by "add" I meant "implement". Might be nice for parity, if nothing else.

@sebmarkbage sebmarkbage force-pushed the asynciterablechildren branch from 1f93797 to d720122 Compare April 21, 2024 16:55
This is just a variant of an async function.
This just unwraps each thenable using the use() logic and then iterates
over the it as an iterator.
@sebmarkbage sebmarkbage force-pushed the asynciterablechildren branch from d720122 to 453d657 Compare April 21, 2024 17:10
@sebmarkbage sebmarkbage merged commit 9f2eebd into facebook:main Apr 22, 2024
38 checks passed
github-actions bot pushed a commit that referenced this pull request Apr 22, 2024
…ent Components (#28868)

Stacked on #28849, #28854, #28853. Behind a flag.

If you're following along from the side-lines. This is probably not what
you think it is.

It's NOT a way to get updates to a component over time. The
AsyncIterable works like an Iterable already works in React which is how
an Array works. I.e. it's a list of children - not the value of a child
over time.

It also doesn't actually render one component at a time. The way it
works is more like awaiting the entire list to become an array and then
it shows up. Before that it suspends the parent.

To actually get these to display one at a time, you have to opt-in with
`<SuspenseList>` to describe how they should appear. That's really the
interesting part and that not implemented yet.

Additionally, since these are effectively Async Functions and uncached
promises, they're not actually fully "supported" on the client yet for
the same reason rendering plain Promises and Async Functions aren't.
They warn. It's only really useful when paired with RSC that produces
instrumented versions of these. Ideally we'd published instrumented
helpers to help with map/filter style operations that yield new
instrumented AsyncIterables.

The way the implementation works basically just relies on unwrapThenable
and otherwise works like a plain Iterator.

There is one quirk with these that are different than just promises. We
ask for a new iterator each time we rerender. This means that upon retry
we kick off another iteration which itself might kick off new requests
that block iterating further. To solve this and make it actually
efficient enough to use on the client we'd need to stash something like
a buffer of the previous iteration and maybe iterator on the iterable so
that we can continue where we left off or synchronously iterate if we've
seen it before. Similar to our `.value` convention on Promises.

In Fizz, I had to do a special case because when we render an iterator
child we don't actually rerender the parent again like we do in Fiber.
However, it's more efficient to just continue on where we left off by
reusing the entries from the thenable state from before in that case.

DiffTrain build for commit 9f2eebd.
github-actions bot pushed a commit that referenced this pull request Apr 22, 2024
…ent Components (#28868)

Stacked on #28849, #28854, #28853. Behind a flag.

If you're following along from the side-lines. This is probably not what
you think it is.

It's NOT a way to get updates to a component over time. The
AsyncIterable works like an Iterable already works in React which is how
an Array works. I.e. it's a list of children - not the value of a child
over time.

It also doesn't actually render one component at a time. The way it
works is more like awaiting the entire list to become an array and then
it shows up. Before that it suspends the parent.

To actually get these to display one at a time, you have to opt-in with
`<SuspenseList>` to describe how they should appear. That's really the
interesting part and that not implemented yet.

Additionally, since these are effectively Async Functions and uncached
promises, they're not actually fully "supported" on the client yet for
the same reason rendering plain Promises and Async Functions aren't.
They warn. It's only really useful when paired with RSC that produces
instrumented versions of these. Ideally we'd published instrumented
helpers to help with map/filter style operations that yield new
instrumented AsyncIterables.

The way the implementation works basically just relies on unwrapThenable
and otherwise works like a plain Iterator.

There is one quirk with these that are different than just promises. We
ask for a new iterator each time we rerender. This means that upon retry
we kick off another iteration which itself might kick off new requests
that block iterating further. To solve this and make it actually
efficient enough to use on the client we'd need to stash something like
a buffer of the previous iteration and maybe iterator on the iterable so
that we can continue where we left off or synchronously iterate if we've
seen it before. Similar to our `.value` convention on Promises.

In Fizz, I had to do a special case because when we render an iterator
child we don't actually rerender the parent again like we do in Fiber.
However, it's more efficient to just continue on where we left off by
reusing the entries from the thenable state from before in that case.

DiffTrain build for [9f2eebd](9f2eebd)
bigfootjon pushed a commit that referenced this pull request Apr 25, 2024
…ent Components (#28868)

Stacked on #28849, #28854, #28853. Behind a flag.

If you're following along from the side-lines. This is probably not what
you think it is.

It's NOT a way to get updates to a component over time. The
AsyncIterable works like an Iterable already works in React which is how
an Array works. I.e. it's a list of children - not the value of a child
over time.

It also doesn't actually render one component at a time. The way it
works is more like awaiting the entire list to become an array and then
it shows up. Before that it suspends the parent.

To actually get these to display one at a time, you have to opt-in with
`<SuspenseList>` to describe how they should appear. That's really the
interesting part and that not implemented yet.

Additionally, since these are effectively Async Functions and uncached
promises, they're not actually fully "supported" on the client yet for
the same reason rendering plain Promises and Async Functions aren't.
They warn. It's only really useful when paired with RSC that produces
instrumented versions of these. Ideally we'd published instrumented
helpers to help with map/filter style operations that yield new
instrumented AsyncIterables.

The way the implementation works basically just relies on unwrapThenable
and otherwise works like a plain Iterator.

There is one quirk with these that are different than just promises. We
ask for a new iterator each time we rerender. This means that upon retry
we kick off another iteration which itself might kick off new requests
that block iterating further. To solve this and make it actually
efficient enough to use on the client we'd need to stash something like
a buffer of the previous iteration and maybe iterator on the iterable so
that we can continue where we left off or synchronously iterate if we've
seen it before. Similar to our `.value` convention on Promises.

In Fizz, I had to do a special case because when we render an iterator
child we don't actually rerender the parent again like we do in Fiber.
However, it's more efficient to just continue on where we left off by
reusing the entries from the thenable state from before in that case.

DiffTrain build for commit 9f2eebd.
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.

5 participants