Add unknown location information to component stacks #30290
Merged
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
CLA Signed
React Core Team
sebmarkbage
commented
Jul 7, 2024
| suffix = | ||
| x.stack.indexOf('\n at') > -1 | ||
| ? // V8 | ||
| ' (<anonymous>)' |
There was a problem hiding this comment.
At least in Chrome DevTools this is a hardcoded special value that means this doesn't have a location.
sebmarkbage
commented
Jul 7, 2024
| ' (<anonymous>)' | ||
| : // JSC/Spidermonkey | ||
| x.stack.indexOf('@') > -1 | ||
| ? '@unknown:0:0' |
There was a problem hiding this comment.
This can really be anything include empty '@' but then Firefox can still present it as :0 which is maybe confusing. So the string unknown is just used here to communicate that we don't know the location.
sebmarkbage
commented
Jul 7, 2024
| } else { | ||
| return describeComponentFrame(name); | ||
| } | ||
| } | ||
|
|
||
| export function describeDebugInfoFrame(name: string, env: ?string): string { | ||
| return describeBuiltInComponentFrame(name + (env ? ' (' + env + ')' : '')); | ||
| return describeBuiltInComponentFrame(name + (env ? ' [' + env + ']' : '')); |
There was a problem hiding this comment.
This was my original reasoning for parenthesis #28415 (comment)
However, after further research it seems like e.g. (native) is no longer used by V8 and the only non-location is (<anonymous>) so this pattern doesn't follow and it actually gets parsed as if it's a real location.
Meanwhile brackets doesn't seem to have any meaning in any format as far as I can tell from further research so just becomes a name.
All formats are ambiguous when the actual name contains meaningful punctuations like ( or @.
sebmarkbage
force-pushed
the
stacklocations
branch
from
1728cff
to
7cb1999
Compare
hoxyq
approved these changes
Jul 8, 2024
github-actions bot
pushed a commit
that referenced
this pull request
Jul 8, 2024
This is the same change as in #30289 but for the main runtime - e.g. parent stacks in errorInfo.componentStack, appended stacks to console.error coming from React itself and when we add virtual frames to owner stacks. Since we don't add location information these frames look weird to some stack parsers - such as the native one. This is an existing issue when you want to use some off-the-shelf parsers to parse production component stacks for example. While we won't add Error objects to logs ourselves necessarily, some third party could want to do the same thing we do in DevTools and so we should provide the same capability to just take this trace and print it using an Error object. DiffTrain build for [df783f9](df783f9)
github-actions bot
pushed a commit
that referenced
this pull request
Jul 8, 2024
This is the same change as in #30289 but for the main runtime - e.g. parent stacks in errorInfo.componentStack, appended stacks to console.error coming from React itself and when we add virtual frames to owner stacks. Since we don't add location information these frames look weird to some stack parsers - such as the native one. This is an existing issue when you want to use some off-the-shelf parsers to parse production component stacks for example. While we won't add Error objects to logs ourselves necessarily, some third party could want to do the same thing we do in DevTools and so we should provide the same capability to just take this trace and print it using an Error object. DiffTrain build for commit df783f9.
huozhi
added a commit
to vercel/next.js
that referenced
this pull request
Jul 9, 2024
### What Change the display of error trace in error overlay. * Replace unknown location `<anonymous>` from the stack trace but still keep the content before the location. * Filter out all `node:` prefixes (including node:async_hooks etc.) ### Why Regarding to the chaneg in facebook/react#30290 We only stripped the `(<anonymous>)` part but still keep the first part of stack trace, e.g. in react it could be the component name. cc @sebmarkbage
This was referenced Jul 25, 2024
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.
This is the same change as in #30289 but for the main runtime - e.g. parent stacks in errorInfo.componentStack, appended stacks to console.error coming from React itself and when we add virtual frames to owner stacks.
Since we don't add location information these frames look weird to some stack parsers - such as the native one. This is an existing issue when you want to use some off-the-shelf parsers to parse production component stacks for example.
While we won't add Error objects to logs ourselves necessarily, some third party could want to do the same thing we do in DevTools and so we should provide the same capability to just take this trace and print it using an Error object.