[Flight] Prefix owner stacks added to the console.log with the current stack #30427

sebmarkbage · 2024-07-23T02:05:59Z

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.

vercel · 2024-07-23T02:06:03Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
react-compiler-playground	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 23, 2024 7:51pm

sebmarkbage · 2024-07-23T02:08:02Z

packages/react-devtools-shared/src/backend/DevToolsFiberComponentStack.js

-            workInProgress.type.render,
-          );
-        }
-        break;


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.

sebmarkbage · 2024-07-23T02:10:59Z

packages/react-devtools-shared/src/__tests__/console-test.js

@@ -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.


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.

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.

sebmarkbage · 2024-07-23T02:18:08Z

packages/react-devtools-shared/src/__tests__/console-test.js

@@ -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.


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.

packages/react-devtools-shared/src/backend/console.js

hoxyq · 2024-07-23T12:14:09Z

packages/react-devtools-shared/src/__tests__/console-test.js

@@ -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.


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>