-
Notifications
You must be signed in to change notification settings - Fork 46.4k
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
[Flight] Prefix owner stacks added to the console.log with the current stack #30427
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
workInProgress.type.render, | ||
); | ||
} | ||
break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the actual stack usually has a frame inside the current component, we don't have to add it even if there's no owner since otherwise it'll end up show up twice. I'll do a separate PR to align this with captureOwnerStack() which should also exclude these.
@@ -1042,15 +1043,16 @@ describe('console', () => { | |||
expect(mockWarn.mock.calls[1][1]).toMatch('warn'); | |||
expect(normalizeCodeLocInfo(mockWarn.mock.calls[1][2]).trim()).toEqual( | |||
supportsOwnerStacks | |||
? 'in Parent (at **)' | |||
? 'in Object.overrideMethod (at **)' + // TODO: This leading frame is due to our extra wrapper that shouldn't exist. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This happens because the patchForStrictMode
methods add an extra stack frame that we don't know we need to pop off.
We should really just get rid of that extra patching because it runs late and is therefore running after potential user space patching.
We should instead reuse the early patching that we already do for the purpose of strict mode printing. That way whatever user spaces wrappers do will be before the dimming and we don't add an extra stack frame here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should instead reuse the early patching that we already do for the purpose of strict mode printing. That way whatever user spaces wrappers do will be before the dimming and we don't add an extra stack frame here.
Yup. I am planning to work on it this week.
@@ -279,7 +279,8 @@ describe('console', () => { | |||
expect(mockWarn.mock.calls[0][0]).toBe('active warn'); | |||
expect(normalizeCodeLocInfo(mockWarn.mock.calls[0][1])).toEqual( | |||
supportsOwnerStacks | |||
? '\n in Parent (at **)' | |||
? // TODO: It would be nice to have a Child stack frame here since it's just the effect function. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only has the useEffect(() =>
inner function on the stack trace which don't currently have the react-bottom-stack-frame
which means they're filtered out since we don't know which frames to include.
Even if we did it wouldn't have a name. It might be nice to automatically name the effect functions based on the component/hook that they're part of. Similar to how class methods are named based on both the class instance and the method.
The current stack is available in the native UI but that's hidden by default so you don't see the actual current component on the stack. This is unlike the native async stacks UI where they're all together. So we prefix the stack with the current stack first.
7a614e2
to
ac9322e
Compare
@@ -1042,15 +1043,16 @@ describe('console', () => { | |||
expect(mockWarn.mock.calls[1][1]).toMatch('warn'); | |||
expect(normalizeCodeLocInfo(mockWarn.mock.calls[1][2]).trim()).toEqual( | |||
supportsOwnerStacks | |||
? 'in Parent (at **)' | |||
? 'in Object.overrideMethod (at **)' + // TODO: This leading frame is due to our extra wrapper that shouldn't exist. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should instead reuse the early patching that we already do for the purpose of strict mode printing. That way whatever user spaces wrappers do will be before the dimming and we don't add an extra stack frame here.
Yup. I am planning to work on it this week.
Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>
…30429) Stacked on #30427. Most hooks and such are called inside renders which already have these on the stack but life-cycles that call out on them are useful to cut off too. Typically we don't create JSX in here so they wouldn't be part of owner stacks anyway but they can be apart of plain stacks such as the ones prefixes to console logs or printed by error dialogs. This lets us cut off any React internals below. This should really be possible using just ignore listing too ideally. At this point we should maybe just build a Babel plugin that lets us annotate a function to need to have this name.
…30429) Stacked on #30427. Most hooks and such are called inside renders which already have these on the stack but life-cycles that call out on them are useful to cut off too. Typically we don't create JSX in here so they wouldn't be part of owner stacks anyway but they can be apart of plain stacks such as the ones prefixes to console logs or printed by error dialogs. This lets us cut off any React internals below. This should really be possible using just ignore listing too ideally. At this point we should maybe just build a Babel plugin that lets us annotate a function to need to have this name. DiffTrain build for commit da4abf0.
…30429) Stacked on #30427. Most hooks and such are called inside renders which already have these on the stack but life-cycles that call out on them are useful to cut off too. Typically we don't create JSX in here so they wouldn't be part of owner stacks anyway but they can be apart of plain stacks such as the ones prefixes to console logs or printed by error dialogs. This lets us cut off any React internals below. This should really be possible using just ignore listing too ideally. At this point we should maybe just build a Babel plugin that lets us annotate a function to need to have this name. DiffTrain build for [da4abf0](da4abf0)
…acebook#30429) Stacked on facebook#30427. Most hooks and such are called inside renders which already have these on the stack but life-cycles that call out on them are useful to cut off too. Typically we don't create JSX in here so they wouldn't be part of owner stacks anyway but they can be apart of plain stacks such as the ones prefixes to console logs or printed by error dialogs. This lets us cut off any React internals below. This should really be possible using just ignore listing too ideally. At this point we should maybe just build a Babel plugin that lets us annotate a function to need to have this name.
…t stack (facebook#30427) The current stack is available in the native UI but that's hidden by default so you don't see the actual current component on the stack. This is unlike the native async stacks UI where they're all together. So we prefix the stack with the current stack first. <img width="279" alt="Screenshot 2024-07-22 at 10 05 13 PM" src="https://github.com/user-attachments/assets/8f568fda-6493-416d-a0be-661caf44d808"> --------- Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>
…acebook#30429) Stacked on facebook#30427. Most hooks and such are called inside renders which already have these on the stack but life-cycles that call out on them are useful to cut off too. Typically we don't create JSX in here so they wouldn't be part of owner stacks anyway but they can be apart of plain stacks such as the ones prefixes to console logs or printed by error dialogs. This lets us cut off any React internals below. This should really be possible using just ignore listing too ideally. At this point we should maybe just build a Babel plugin that lets us annotate a function to need to have this name.
…t stack (facebook#30427) The current stack is available in the native UI but that's hidden by default so you don't see the actual current component on the stack. This is unlike the native async stacks UI where they're all together. So we prefix the stack with the current stack first. <img width="279" alt="Screenshot 2024-07-22 at 10 05 13 PM" src="https://github.com/user-attachments/assets/8f568fda-6493-416d-a0be-661caf44d808"> --------- Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>
…acebook#30429) Stacked on facebook#30427. Most hooks and such are called inside renders which already have these on the stack but life-cycles that call out on them are useful to cut off too. Typically we don't create JSX in here so they wouldn't be part of owner stacks anyway but they can be apart of plain stacks such as the ones prefixes to console logs or printed by error dialogs. This lets us cut off any React internals below. This should really be possible using just ignore listing too ideally. At this point we should maybe just build a Babel plugin that lets us annotate a function to need to have this name.
…t stack (facebook#30427) The current stack is available in the native UI but that's hidden by default so you don't see the actual current component on the stack. This is unlike the native async stacks UI where they're all together. So we prefix the stack with the current stack first. <img width="279" alt="Screenshot 2024-07-22 at 10 05 13 PM" src="https://github.com/user-attachments/assets/8f568fda-6493-416d-a0be-661caf44d808"> --------- Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>
…acebook#30429) Stacked on facebook#30427. Most hooks and such are called inside renders which already have these on the stack but life-cycles that call out on them are useful to cut off too. Typically we don't create JSX in here so they wouldn't be part of owner stacks anyway but they can be apart of plain stacks such as the ones prefixes to console logs or printed by error dialogs. This lets us cut off any React internals below. This should really be possible using just ignore listing too ideally. At this point we should maybe just build a Babel plugin that lets us annotate a function to need to have this name.
…t stack (facebook#30427) The current stack is available in the native UI but that's hidden by default so you don't see the actual current component on the stack. This is unlike the native async stacks UI where they're all together. So we prefix the stack with the current stack first. <img width="279" alt="Screenshot 2024-07-22 at 10 05 13 PM" src="https://github.com/user-attachments/assets/8f568fda-6493-416d-a0be-661caf44d808"> --------- Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>
…t stack (facebook#30427) The current stack is available in the native UI but that's hidden by default so you don't see the actual current component on the stack. This is unlike the native async stacks UI where they're all together. So we prefix the stack with the current stack first. <img width="279" alt="Screenshot 2024-07-22 at 10 05 13 PM" src="https://github.com/user-attachments/assets/8f568fda-6493-416d-a0be-661caf44d808"> --------- Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>
…acebook#30429) Stacked on facebook#30427. Most hooks and such are called inside renders which already have these on the stack but life-cycles that call out on them are useful to cut off too. Typically we don't create JSX in here so they wouldn't be part of owner stacks anyway but they can be apart of plain stacks such as the ones prefixes to console logs or printed by error dialogs. This lets us cut off any React internals below. This should really be possible using just ignore listing too ideally. At this point we should maybe just build a Babel plugin that lets us annotate a function to need to have this name.
…t stack (facebook#30427) The current stack is available in the native UI but that's hidden by default so you don't see the actual current component on the stack. This is unlike the native async stacks UI where they're all together. So we prefix the stack with the current stack first. <img width="279" alt="Screenshot 2024-07-22 at 10 05 13 PM" src="https://github.com/user-attachments/assets/8f568fda-6493-416d-a0be-661caf44d808"> --------- Co-authored-by: Ruslan Lesiutin <rdlesyutin@gmail.com>
…acebook#30429) Stacked on facebook#30427. Most hooks and such are called inside renders which already have these on the stack but life-cycles that call out on them are useful to cut off too. Typically we don't create JSX in here so they wouldn't be part of owner stacks anyway but they can be apart of plain stacks such as the ones prefixes to console logs or printed by error dialogs. This lets us cut off any React internals below. This should really be possible using just ignore listing too ideally. At this point we should maybe just build a Babel plugin that lets us annotate a function to need to have this name.
The current stack is available in the native UI but that's hidden by default so you don't see the actual current component on the stack.
This is unlike the native async stacks UI where they're all together.
So we prefix the stack with the current stack first.