[Fiber] Prefix owner stacks with the current stack at the console call #29697
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
This information is available in the regular stack but since that's hidden behind an expando and our appended stack to logs is not hidden, it hides the most important frames like the name of the current component. This is closer to what happens to the native stack. We only include stacks if they're within a ReactFiberCallUserSpace call frame. This should be most that have a current fiber but this is critical to filtering out most React frames if the regular node_modules filter doesn't work. Most React warnings fire during the rendering phase and not inside a user space function but some do like hooks warnings and setState in render. This feature is more important if we port this to React DevTools appending stacks to all logs.
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
facebook-github-bot
added
CLA Signed
React Core Team
eps1lon
approved these changes
Jun 3, 2024
| @@ -1618,8 +1618,7 @@ describe('ReactHooks', () => { | |||
| ' Previous render Next render\n' + | |||
| ' ------------------------------------------------------\n' + | |||
| `1. ${formatHookNamesToMatchErrorMessage(hookNameA, hookNameB)}\n` + | |||
| ' ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n' + | |||
| ' in App (at **)', | |||
There was a problem hiding this comment.
Is the stack frame still available in browsers?
There was a problem hiding this comment.
Since these matches a subset, by removing the stack frame it matches any stack frame.
These end up having the mentioned internals in them so I didn't want to assert on those.
github-actions bot
pushed a commit
that referenced
this pull request
Jun 3, 2024
#29697) This information is available in the regular stack but since that's hidden behind an expando and our appended stack to logs is not hidden, it hides the most important frames like the name of the current component. This is closer to what happens to the native stack. We only include stacks if they're within a ReactFiberCallUserSpace call frame. This should be most that have a current fiber but this is critical to filtering out most React frames if the regular node_modules filter doesn't work. Most React warnings fire during the rendering phase and not inside a user space function but some do like hooks warnings and setState in render. This feature is more important if we port this to React DevTools appending stacks to all logs where it's likely to originate from inside a component and you want the line within that component to immediately part of the visible stack. One thing that kind sucks is that we don't have a reliable way to exclude React internal stack frames. We filter node_modules but it might not match. For other cases I try hard to only track the stack frame at the root of React (e.g. immediately inside createElement) until the ReactFiberCallUserSpace so we don't need the filtering to work. In this case it's hard to achieve the same thing though. This is easier in RDT because we have the start/end line and parsing of stack traces so we can use that to exclude internals but that's a lot of code/complexity for shipping within the library. For example in Safari: <img width="590" alt="Screenshot 2024-05-31 at 6 15 27 PM" src="https://github.com/facebook/react/assets/63648/2820c8c0-8a03-42e9-8678-8348f66b051a"> Ideally warnOnUseFormStateInDev and useFormState wouldn't be included since they're React internals. Before this change, the Counter.js line also wasn't included though which points to exactly where the error is within the user code. (Note Server Components have V8 formatted lines and Client Components have JSC formatted lines.) DiffTrain build for [4dcdf21](4dcdf21)
github-actions bot
pushed a commit
that referenced
this pull request
Jun 3, 2024
#29697) This information is available in the regular stack but since that's hidden behind an expando and our appended stack to logs is not hidden, it hides the most important frames like the name of the current component. This is closer to what happens to the native stack. We only include stacks if they're within a ReactFiberCallUserSpace call frame. This should be most that have a current fiber but this is critical to filtering out most React frames if the regular node_modules filter doesn't work. Most React warnings fire during the rendering phase and not inside a user space function but some do like hooks warnings and setState in render. This feature is more important if we port this to React DevTools appending stacks to all logs where it's likely to originate from inside a component and you want the line within that component to immediately part of the visible stack. One thing that kind sucks is that we don't have a reliable way to exclude React internal stack frames. We filter node_modules but it might not match. For other cases I try hard to only track the stack frame at the root of React (e.g. immediately inside createElement) until the ReactFiberCallUserSpace so we don't need the filtering to work. In this case it's hard to achieve the same thing though. This is easier in RDT because we have the start/end line and parsing of stack traces so we can use that to exclude internals but that's a lot of code/complexity for shipping within the library. For example in Safari: <img width="590" alt="Screenshot 2024-05-31 at 6 15 27 PM" src="https://github.com/facebook/react/assets/63648/2820c8c0-8a03-42e9-8678-8348f66b051a"> Ideally warnOnUseFormStateInDev and useFormState wouldn't be included since they're React internals. Before this change, the Counter.js line also wasn't included though which points to exactly where the error is within the user code. (Note Server Components have V8 formatted lines and Client Components have JSC formatted lines.) DiffTrain build for commit 4dcdf21.
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 information is available in the regular stack but since that's hidden behind an expando and our appended stack to logs is not hidden, it hides the most important frames like the name of the current component.
This is closer to what happens to the native stack.
We only include stacks if they're within a ReactFiberCallUserSpace call frame. This should be most that have a current fiber but this is critical to filtering out most React frames if the regular node_modules filter doesn't work.
Most React warnings fire during the rendering phase and not inside a user space function but some do like hooks warnings and setState in render. This feature is more important if we port this to React DevTools appending stacks to all logs where it's likely to originate from inside a component and you want the line within that component to immediately part of the visible stack.
One thing that kind sucks is that we don't have a reliable way to exclude React internal stack frames. We filter node_modules but it might not match. For other cases I try hard to only track the stack frame at the root of React (e.g. immediately inside createElement) until the ReactFiberCallUserSpace so we don't need the filtering to work. In this case it's hard to achieve the same thing though. This is easier in RDT because we have the start/end line and parsing of stack traces so we can use that to exclude internals but that's a lot of code/complexity for shipping within the library.
For example in Safari:
Ideally warnOnUseFormStateInDev and useFormState wouldn't be included since they're React internals. Before this change, the Counter.js line also wasn't included though which points to exactly where the error is within the user code.
(Note Server Components have V8 formatted lines and Client Components have JSC formatted lines.)