[Flight] Clarify Semantics for Awaiting Cached Data #33438
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
the
React Core Team
unstubbable
added a commit
to unstubbable/react
that referenced
this pull request
Jun 5, 2025
This should allow us to visualize what facebook#33438 is trying to convey.
unstubbable
added a commit
to unstubbable/react
that referenced
this pull request
Jun 5, 2025
This should allow us to visualize what facebook#33438 is trying to convey.
unstubbable
added a commit
that referenced
this pull request
Jun 5, 2025
This should allow us to visualize what #33438 is trying to convey. An uncached 3rd-party component is displayed like this in the dev tools: <img width="1072" alt="Screenshot 2025-06-05 at 12 57 32" src="https://github.com/user-attachments/assets/d418ae23-d113-4dc9-98b8-ab426710454a" /> However, when the component is restored from a cache, it looks like this: <img width="1072" alt="Screenshot 2025-06-05 at 12 56 56" src="https://github.com/user-attachments/assets/a0e34379-d8c0-4b14-8b54-b5c06211232b" /> The `Server Components ⚛` track is missing completely here, and the `Loading profile...` phase also took way longer than without caching the 3rd-party component. On `main`, the `Server Components ⚛` track is not missing: <img width="1072" alt="Screenshot 2025-06-05 at 14 31 20" src="https://github.com/user-attachments/assets/c35e405d-27ca-4b04-a34c-03bd959a7687" /> The cached 3rd-party component starts before the current render, and is also not excluded here, which is of course expected without #33438.
sebmarkbage
force-pushed
the
ioexclusion
branch
2 times, most recently
from
505f98c to
2d40d8c
Compare
unstubbable
reviewed
Jun 7, 2025
There was a problem hiding this comment.
Do you intend to fix the issue of the missing Server Components ⚛ track that's introduced here (reported in #33443) as part of this PR?
Yea I'm still looking into it. |
sebmarkbage
force-pushed
the
ioexclusion
branch
2 times, most recently
from
4ce29ea to
78f7790
Compare
We don't really need the ID here. It's the same as the task id. We pass the task instead. It's really just used as a flag. Update comments Update comments
…mponent We also ignore any await entries that finished before the request started since they're irrelevant to the loading sequence.
…aranteed to be set yet
sebmarkbage
commented
Jun 7, 2025
| } else { | ||
| request.pendingChunks++; |
There was a problem hiding this comment.
@unstubbable The bug was that this was now mismatched since one of the branches doesn't emit a chunk. We should really move all these into the emit functions.
There was a problem hiding this comment.
Yeah, it's not the first time that managing this counter has caused a bug.
unstubbable
approved these changes
Jun 7, 2025
This was referenced Jun 9, 2025
This was referenced Jun 11, 2025
17 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Technically the async call graph spans basically all the way back to the start of the app potentially, but we don't want to include everything. Similarly we don't want to include everything from previous components in every child component. So we need some heuristics for filtering out data.
We roughly want to be able to inspect is what might contribute to a Suspense loading sequence even if it didn't this time e.g. due to a race condition.
One flaw with the previous approach was that awaiting a cached promise in a sibling that happened to finish after another sibling would be excluded. However, in a different race condition that might end up being used so I wanted to include an empty "await" in that scenario to have some association from that component.
However, for data that resolved fully before the request even started, it's a little different. This can be things that are part of the start up sequence of the app or externally cached data. We decided that this should be excluded because it doesn't contribute to the loading sequence in the expected scenario. I.e. if it's cached. Things that end up being cache misses would still be included. If you want to test externally cached data misses, then it's up to you or the framework to simulate those. E.g. by dropping the cache. This also helps free up some noise since static / cached data can be excluded in visualizations.
I also apply this principle to forwarding debug info. If you reuse a cached RSC payload, then the Server Component render time and its awaits gets clamped to the caller as if it has zero render/await time. The I/O entry is still back dated but if it was fully resolved before we started then it's completely excluded.