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.

Sign up for GitHub

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

Jump to bottom

Bug: [Flight] Async server components in ai/rsc not rendered correctly #28595

Closed
unstubbable opened this issue Mar 20, 2024 · 1 comment · Fixed by #28669
Closed

Bug: [Flight] Async server components in ai/rsc not rendered correctly #28595

unstubbable opened this issue Mar 20, 2024 · 1 comment · Fixed by #28669
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@unstubbable
Copy link
Collaborator

unstubbable commented Mar 20, 2024 •
edited
Loading

When returning an async server component from an ai/rsc tool's render function, a reference to it is not serialized as a lazy node, even though it might not have been emitted as a row yet. Currently the React Flight Client is not resilient to this. This leads to the reference not being resolved correctly, and then null is rendered instead of the element.

The bug was introduced with #28283.

React version: 18.3.0-canary-6c3b8dbfe-20240226 (via Next.js v14.2.0-canary.30)

Steps To Reproduce

  1. Build ai/rsc example according to https://sdk.vercel.ai/docs/concepts/ai-rsc#build-your-app
  2. Move data fetching from render function to an async server component

(It's probably also reproducible by writing a unit test that replicates what's being done in https://github.com/vercel/ai/blob/main/packages/core/rsc/utils.tsx#L17-L50. I might try this later... done)

Link to code example: unstubbable/ai-rsc-test#1
This contains the code of step 2 from above, has more details about the expected and actual RSC responses, and also shows in the comments how I arrived at my conclusion.
@unstubbable unstubbable added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Mar 20, 2024
@unstubbable
Copy link
Collaborator Author

A potential fix is mentioned in the code comments of #28283:

We should really detect whether this already was emitted and synchronously available. In that case we can refer to it synchronously and only make it lazy otherwise. We currently don't have a data structure that lets us see that though.

unstubbable added a commit to unstubbable/react that referenced this issue Mar 20, 2024
@unstubbable
Add a test for issue facebook#28595
efdd474 
The added test, intended to fail and reproduce the reported issue,
unexpectedly passes in its current state. I see three possible reasons:

1. The bug report could be invalid.
2. How I've structured of the test might be insufficient to replicate
   what `ai/rsc` is doing.
3. Something in the test setup could be obscuring the actual error.

If the problem lies in reason 2 or 3, this test could possibly serve as
a foundation for further investigation.
unstubbable added a commit to unstubbable/react that referenced this issue Mar 20, 2024
@unstubbable
Add a test for issue facebook#28595
d50070b 
The added test, intended to fail and reproduce the [reported
issue](facebook#28595), unexpectedly
passes in its current state. I see three possible reasons:

1. The bug report could be invalid.
2. How I've structured the test might be insufficient to replicate what
   `ai/rsc` is doing.
3. Something in the test setup could be masking the actual error.

If the problem lies in reason 2 or 3, this test could possibly serve as
a foundation for further investigation.
unstubbable added a commit to unstubbable/react that referenced this issue Mar 20, 2024
@unstubbable
Add a test for issue facebook#28595
5ff5957 
The added test, intended to fail and reproduce the [reported
issue](facebook#28595), unexpectedly
passes in its current state. I see three possible reasons:

1. The bug report could be invalid.
2. How I've structured the test might be insufficient to replicate what
   `ai/rsc` is doing.
3. Something in the test setup could be masking the actual error.
   (Possibly related to fake timers?)

If the problem lies in reason 2 or 3, this test could possibly serve as
a foundation for further investigation.
unstubbable added a commit to unstubbable/react that referenced this issue Mar 20, 2024
@unstubbable
Add a test for issue facebook#28595
08444e2 
The added test, intended to fail and reproduce the [reported
issue](facebook#28595), unexpectedly
passes in its current state. I see three possible reasons:

1. The bug report could be invalid.
2. How I've structured the test might be insufficient to replicate what
   `ai/rsc` is doing.
3. Something in the test setup could be masking the actual error.
   (Possibly related to fake timers?)

If the problem lies in reason 2 or 3, this test could possibly serve as
a foundation for further investigation.
@unstubbable unstubbable mentioned this issue Mar 20, 2024
Closed
unstubbable added a commit to unstubbable/react that referenced this issue Mar 21, 2024
@unstubbable
Add a test for issue facebook#28595
d5cbcd6 
The added test, intended to fail and reproduce the [reported
issue](facebook#28595), unexpectedly
passes in its current state. I see three possible reasons:

1. The bug report could be invalid.
2. How I've structured the test might be insufficient to replicate what
   `ai/rsc` is doing.
3. Something in the test setup could be masking the actual error.
   (Maybe related to fake timers?)

If the problem lies in reason 2 or 3, this test could possibly serve as
a foundation for further investigation.
unstubbable added a commit to unstubbable/react that referenced this issue Mar 23, 2024
@unstubbable
Add a test for issue facebook#28595
4baed31 
The added test, intended to fail and reproduce the [reported
issue](facebook#28595), unexpectedly
passes in its current state. I see three possible reasons:

1. The bug report could be invalid.
2. How I've structured the test might be insufficient to replicate what
   `ai/rsc` is doing.
3. Something in the test setup could be masking the actual error.
   (Maybe related to fake timers?)

If the problem lies in reason 2 or 3, this test could possibly serve as
a foundation for further investigation.
unstubbable added a commit to unstubbable/react that referenced this issue Mar 23, 2024
@unstubbable
Choose lazy or value ID based on whether model chunk was emitted
d29a0dc 
fixes facebook#28595
@unstubbable unstubbable mentioned this issue Mar 23, 2024
Closed
unstubbable added a commit to unstubbable/react that referenced this issue Mar 23, 2024
@unstubbable
Add a test for issue facebook#28595
84a9438 
The added test, intended to fail and reproduce the [reported
issue](facebook#28595), unexpectedly
passes in its current state. I see three possible reasons:

1. The bug report could be invalid.
2. How I've structured the test might be insufficient to replicate what
   `ai/rsc` is doing.
3. Something in the test setup could be masking the actual error.
   (Maybe related to fake timers?)

If the problem lies in reason 2 or 3, this test could possibly serve as
a foundation for further investigation.
unstubbable added a commit to unstubbable/react that referenced this issue Mar 23, 2024
@unstubbable
Use lazy ID if referenced model chunk was not emitted yet
87a1799 
fixes facebook#28595
unstubbable added a commit to unstubbable/react that referenced this issue Mar 29, 2024
@unstubbable
Add a test for issue facebook#28595
7cda430 
The added test, intended to fail and reproduce the [reported
issue](facebook#28595), unexpectedly
passes in its current state. I see three possible reasons:

1. The bug report could be invalid.
2. How I've structured the test might be insufficient to replicate what
   `ai/rsc` is doing.
3. Something in the test setup could be masking the actual error.
   (Maybe related to fake timers?)

If the problem lies in reason 2 or 3, this test could possibly serve as
a foundation for further investigation.
unstubbable added a commit to unstubbable/react that referenced this issue Mar 29, 2024
@unstubbable
Add a test for issue facebook#28595
6e92c9f 
The added test, intended to fail and reproduce the [reported
issue](facebook#28595), unexpectedly
passes in its current state. I see three possible reasons:

1. The bug report could be invalid.
2. How I've structured the test might be insufficient to replicate what
   `ai/rsc` is doing.
3. Something in the test setup could be masking the actual error.
   (Maybe related to fake timers?)

If the problem lies in reason 2 or 3, this test could possibly serve as
a foundation for further investigation.
@unstubbable unstubbable changed the title Bug: [Flight] Async server components in ai/rsc not serialized correctly Bug: [Flight] Async server components in ai/rsc not rendered correctly Mar 30, 2024
@unstubbable unstubbable mentioned this issue Mar 30, 2024
Merged
sebmarkbage pushed a commit that referenced this issue Apr 1, 2024
@unstubbable
[Flight] Update stale blocked values in createModelResolver (#28669)
93f9179 
Alternative to #28620.

Instead of emitting lazy references to not-yet-emitted models in the
Flight Server, this fixes the observed issue in
unstubbable/ai-rsc-test#1 by adjusting the lazy
model resolution in the Flight Client to update stale blocked root
models, before assigning them as chunk values. In addition, the element
props are not outlined anymore in the Flight Server to avoid having to
also handle their staleness in blocked elements.

fixes #28595
github-actions bot pushed a commit that referenced this issue Apr 1, 2024
@sebmarkbage
[Flight] Update stale blocked values in createModelResolver (#28669)
b50214b 
Alternative to #28620.

Instead of emitting lazy references to not-yet-emitted models in the
Flight Server, this fixes the observed issue in
unstubbable/ai-rsc-test#1 by adjusting the lazy
model resolution in the Flight Client to update stale blocked root
models, before assigning them as chunk values. In addition, the element
props are not outlined anymore in the Flight Server to avoid having to
also handle their staleness in blocked elements.

fixes #28595

DiffTrain build for [93f9179](93f9179)
EdisonVan pushed a commit to EdisonVan/react that referenced this issue Apr 15, 2024
@unstubbable @AndyPengc12
[Flight] Update stale blocked values in createModelResolver (facebo…
084d1be 
…ok#28669)

Alternative to facebook#28620.

Instead of emitting lazy references to not-yet-emitted models in the
Flight Server, this fixes the observed issue in
unstubbable/ai-rsc-test#1 by adjusting the lazy
model resolution in the Flight Client to update stale blocked root
models, before assigning them as chunk values. In addition, the element
props are not outlined anymore in the Flight Server to avoid having to
also handle their staleness in blocked elements.

fixes facebook#28595
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Milestone
No milestone
1 participant
@unstubbable