-
Notifications
You must be signed in to change notification settings - Fork 45.6k
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
[Flight] Test deduplication with streaming server components #29001
Conversation
I have a TODO to fix dedupe in ReadableStream/AsyncIterable so I first thought it was about that but this is just recursive promises. |
Yes this is about recursive promises, combined with using the same element as suspense |
Well, it is expected that the fallback and the children of a Suspense boundary cannot share state. They are two separate slots so they are two separate components. This becomes important for example with resuspending when the state in both of them can exist at the same time. The fallback is shown but the state of the children is preserved while it's suspended and can be shown again. It also doesn't really make conceptual sense to render the same thing on both slots. It would always be suspended at the same time. They'd at least need different props. In this case the extra wrapper Row is actually supposed to clear the state too. We don't in the case of Server Components as a pragmatic way to avoid the overhead/leak of wrapper. However, this is not the only problem with this approach. For example, by creating nested Suspense boundaries you can't reorder the items in the list and have them preserve state since they're not siblings but nested components that should really have keys which would clear the state on reorders. That's why the correct model for this will be AsyncIterable + SuspenseList. In fact, this was why SuspenseList was built in the first place. |
Thanks for providing the suspense context, that makes sense. I do see how AsyncIterable + SuspenseList will be useful for streamable UI that’s appended to. For the kind of streamable UI that’s updated instead, it will probably feel like misusing SuspenseList for that (e.g. only showing the last value). |
Stacked on top of #28996, because initially I thought that the deduplication by property path would make this test succeed.
I think the test might be useful for verifying a fix that can unblock the reconciliation issue in the AI SDK (see vercel/ai#1257, or vercel/ai#1210 (comment), for reference), whether this can be incorporated into #28996, or tackled separately.
The flight payload for the test shows that deduplication via property path is not applied in this scenario:
I would still need to investigate whether this is an expected limitation by the Flight Server (i.e. reference by path can only work within the same model root, and not across chunks), or whether this is actually supposed to be supported. After all, the path does include the task ID, and in fact, the Flight Client can resolve
4:"$2:props:fallback"
. But even then, this still leads to a re-mount. So maybe it's also a limitation of Fiber?