[Fiber] Stash usables on _debugUsables for DevTools #28295
Closed
Conversation
This file contains 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
A Flight Server can be a consumer of a stream from another Server. In this case the meta data is attached to debugInfo properties on lazy, Promises, Arrays or Elements that might in turn get forwarded to the next stream. In this case we want to forward this debug information to the client.
This lets you name the server that is producing the debug info so that you can trace the origin of where that component is executing.
Merge any debug info we got from intermediate objects until we create or update a fiber that can hold it.
use() calls. These can also include further DebugInfo on them such as what Server Component rendered the Promise or async debug info.
facebook-github-bot
added
CLA Signed
React Core Team
|
On a second thought I think maybe we don't need this because, just like Hooks, we can just replay the render to collect this information on demand. I think I'll do that instead. That wouldn't necessarily work with uncached promises though. |
|
Abandoning this in favor of #28297. One limitation of that strategy is that it's hard to find all the places something might suspend in a tree without rerendering all the fibers again. So we might need to still add something to the tree to indicate which Fibers may have further debug info / thenables. |
sebmarkbage
added a commit
that referenced
this pull request
Feb 12, 2024
Alternative to #28295. Instead of stashing all of the Usables eagerly, we can extract them by replaying the render when we need them like we do with any other hook. We already had an implementation of `use()` but it wasn't quite complete. These can also include further DebugInfo on them such as what Server Component rendered the Promise or async debug info. This is nice just to see which use() calls were made in the side-panel but it can also be used to gather everything that might have suspended. Together with #28286 we cover the case when a Promise was used a child and if it was unwrapped with use(). Notably we don't cover a Promise that was thrown (although we do support that in a Server Component which maybe we shouldn't). Throwing a Promise isn't officially supported though and that use case should move to the use() Hook. The pattern of conditionally suspending based on cache also isn't really supported with the use() pattern. You should always call use() if you previously called use() with the same input. This also ensures that we can track what might have suspended rather than what actually did. One limitation of this strategy is that it's hard to find all the places something might suspend in a tree without rerendering all the fibers again. So we might need to still add something to the tree to indicate which Fibers may have further debug info / thenables.
EdisonVan
pushed a commit
to EdisonVan/react
that referenced
this pull request
Apr 15, 2024
Alternative to facebook#28295. Instead of stashing all of the Usables eagerly, we can extract them by replaying the render when we need them like we do with any other hook. We already had an implementation of `use()` but it wasn't quite complete. These can also include further DebugInfo on them such as what Server Component rendered the Promise or async debug info. This is nice just to see which use() calls were made in the side-panel but it can also be used to gather everything that might have suspended. Together with facebook#28286 we cover the case when a Promise was used a child and if it was unwrapped with use(). Notably we don't cover a Promise that was thrown (although we do support that in a Server Component which maybe we shouldn't). Throwing a Promise isn't officially supported though and that use case should move to the use() Hook. The pattern of conditionally suspending based on cache also isn't really supported with the use() pattern. You should always call use() if you previously called use() with the same input. This also ensures that we can track what might have suspended rather than what actually did. One limitation of this strategy is that it's hard to find all the places something might suspend in a tree without rerendering all the fibers again. So we might need to still add something to the tree to indicate which Fibers may have further debug info / thenables.
bigfootjon
pushed a commit
that referenced
this pull request
Apr 18, 2024
Alternative to #28295. Instead of stashing all of the Usables eagerly, we can extract them by replaying the render when we need them like we do with any other hook. We already had an implementation of `use()` but it wasn't quite complete. These can also include further DebugInfo on them such as what Server Component rendered the Promise or async debug info. This is nice just to see which use() calls were made in the side-panel but it can also be used to gather everything that might have suspended. Together with #28286 we cover the case when a Promise was used a child and if it was unwrapped with use(). Notably we don't cover a Promise that was thrown (although we do support that in a Server Component which maybe we shouldn't). Throwing a Promise isn't officially supported though and that use case should move to the use() Hook. The pattern of conditionally suspending based on cache also isn't really supported with the use() pattern. You should always call use() if you previously called use() with the same input. This also ensures that we can track what might have suspended rather than what actually did. One limitation of this strategy is that it's hard to find all the places something might suspend in a tree without rerendering all the fibers again. So we might need to still add something to the tree to indicate which Fibers may have further debug info / thenables. DiffTrain build for commit 7a32d71.
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.
Stacked on #28286.
Stash usables on
_debugUsablesso that DevTools can pick up anyuse()calls. These can also include furtherDebugInfoon them such as what Server Component rendered the Promise or async debug info. This is nice just to see whichuse()calls were made in the side-panel but it can also be used to gather everything that might have suspended.Together with #28286 we cover the case when a Promise was used a child and if it was unwrapped with
use(). Notably we don't cover a Promise that was thrown (although we do support that in a Server Component which maybe we shouldn't). Throwing a Promise isn't officially supported though and that use case should move to theuse()Hook.The pattern of conditionally suspending based on cache also isn't really supported with the
use()pattern. You should always calluse()if you previously calleduse()with the same input. This also ensures that we can track what might have suspended rather than what actually did.