-
Notifications
You must be signed in to change notification settings - Fork 49.6k
[Fiber] Assign implicit debug info to used thenables #34146
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
Merged
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
Comparing: cf6e502...fae80da Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: (No significant changes) |
eps1lon
approved these changes
Aug 11, 2025
This lets us pick up the debug info from a raw Promise as a child which is not covered by _debugThenables. It also lets us track some approximate start/end time of use():ed promises.
fae80da
to
45dba3a
Compare
sebmarkbage
added a commit
that referenced
this pull request
Aug 11, 2025
Similar to #34144 but for `use()`. `use()` dependencies don't get added to the `fiber._debugInfo` set because that just models the things blocking the children, and not the Fiber component itself. This picks up any debug info from the thenable state that we stashed onto `_debugThenableState` so that we know it used `use()`. <img width="593" height="425" alt="Screenshot 2025-08-09 at 4 03 40 PM" src="https://github.com/user-attachments/assets/c7e06884-4efd-47fa-a76b-132935db6ddc" /> Without #34146 this doesn't pick up uninstrumented promises but after it, it'll pick those up as well. An instrumented promise that doesn't have anything in its debug info is not picked up. For example, if it didn't depend on any I/O on the server. This doesn't yet pick up the stack trace of the `use()` call. That information is in the Hooks information but needs a follow up to extract it.
sebmarkbage
added a commit
that referenced
this pull request
Aug 15, 2025
We currently only track the reason something might suspend in development mode through debug info but this excludes some cases. As a result we can end up with boundary that suspends but has no cause. This tries to detect that and show a notice for why that might be. I'm also trying to make it work with old React versions to cover everything. In production we don't track any of this meta data like `_debugInfo`, `_debugThenable` etc. so after resolution there's no information to take from. Except suspensey images / css which we can track in prod too. We could track lazy component types already. We'd have to add something that tracks after the fact if something used a lazy child, child as a promise, hooks, etc. which doesn't exist today. So that's not backwards compatible and might add some perf/memory cost. However, another strategy is also to try to replay the components after the fact which could be backwards compatible. That's tricky for child position since there's so many rules for how to do that which would have to be replicated. If you're in development you get a different error. Given that we've added instrumentation very recently. If you're on an older development version of React, then you get a different error. Unfortunately I think my feature test is not quite perfect because it's tricky to test for the instrumentation I just added. #34146 So I think for some prereleases that has `_debugOwner` but doesn't have that you'll get a misleading error. Finally, if you're in a modern development environment, the only reason we should have any gaps is because of throw-a-Promise. This will highlight it as missing. We can detect that something threw if a Suspense boundary commits with a RetryCache but since it's a WeakSet we can't look into it to see anything about what it might have been. I don't plan on doing anything to improve this since it would only apply to new versions of React anyway and it's just inherently flawed. So just deprecate it #34032. Note that nothing in here can detect that we suspended Transition. So throwing at the root or in an update won't show that anywhere.
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.
Similar to #34137 but for Promises.
This lets us pick up the debug info from a raw Promise as a child which is not covered by
_debugThenables
. Currently ChildFiber doesn't stash its thenables so we can't pick them up from devtools after the fact without some debug info added to the parent.It also lets us track some approximate start/end time of use():ed promises based on the first time we saw this particular Promise.