Skip to content

Commit

Permalink
Prefix owner stacks added to the console.log with the current stack
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@sebmarkbage
sebmarkbage committed Jul 23, 2024
1 parent 7048484 commit 7a614e2
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 53 deletions.
4 changes: 2 additions & 2 deletions packages/react-devtools-shared/src/__tests__/componentStacks-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ describe('component stack', () => {

expect(mockError).toHaveBeenCalledWith(
'Test error.',
(supportsOwnerStacks ? '' : '\n in Child (at **)') +
'\n in Child (at **)' +
'\n in Parent (at **)' +
'\n in Grandparent (at **)',
);
expect(mockWarn).toHaveBeenCalledWith(
'Test warning.',
(supportsOwnerStacks ? '' : '\n in Child (at **)') +
'\n in Child (at **)' +
'\n in Parent (at **)' +
'\n in Grandparent (at **)',
);
Expand Down
21 changes: 12 additions & 9 deletions packages/react-devtools-shared/src/__tests__/console-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,15 +232,15 @@ describe('console', () => {
expect(mockWarn.mock.calls[0][0]).toBe('warn');
expect(normalizeCodeLocInfo(mockWarn.mock.calls[0][1])).toEqual(
supportsOwnerStacks
? '\n in Parent (at **)'
? '\n in Child (at **)\n in Parent (at **)'
: '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
);
expect(mockError).toHaveBeenCalledTimes(1);
expect(mockError.mock.calls[0]).toHaveLength(2);
expect(mockError.mock.calls[0][0]).toBe('error');
expect(normalizeCodeLocInfo(mockError.mock.calls[0][1])).toBe(
supportsOwnerStacks
? '\n in Parent (at **)'
? '\n in Child (at **)\n in Parent (at **)'
: '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
);
});
Expand Down Expand Up @@ -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.
'\n in Parent (at **)'
: '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
);
expect(mockWarn.mock.calls[1]).toHaveLength(2);
Expand Down Expand Up @@ -497,15 +498,15 @@ describe('console', () => {
expect(mockWarn.mock.calls[0][0]).toBe('warn');
expect(normalizeCodeLocInfo(mockWarn.mock.calls[0][1])).toEqual(
supportsOwnerStacks
? '\n in Parent (at **)'
? '\n in Child (at **)\n in Parent (at **)'
: '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
);
expect(mockError).toHaveBeenCalledTimes(1);
expect(mockError.mock.calls[0]).toHaveLength(2);
expect(mockError.mock.calls[0][0]).toBe('error');
expect(normalizeCodeLocInfo(mockError.mock.calls[0][1])).toBe(
supportsOwnerStacks
? '\n in Parent (at **)'
? '\n in Child (at **)\n in Parent (at **)'
: '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
);
});
Expand Down Expand Up @@ -1032,7 +1033,7 @@ describe('console', () => {
expect(mockWarn.mock.calls[0]).toHaveLength(2);
expect(normalizeCodeLocInfo(mockWarn.mock.calls[0][1])).toEqual(
supportsOwnerStacks
? '\n in Parent (at **)'
? '\n in Child (at **)\n in Parent (at **)'
: '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
);
expect(mockWarn.mock.calls[1]).toHaveLength(3);
Expand All @@ -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.
'\n in Child (at **)\n in Parent (at **)'
: 'in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
);

expect(mockError).toHaveBeenCalledTimes(2);
expect(mockError.mock.calls[0]).toHaveLength(2);
expect(normalizeCodeLocInfo(mockError.mock.calls[0][1])).toEqual(
supportsOwnerStacks
? '\n in Parent (at **)'
? '\n in Child (at **)\n in Parent (at **)'
: '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
);
expect(mockError.mock.calls[1]).toHaveLength(3);
Expand All @@ -1060,7 +1062,8 @@ describe('console', () => {
expect(mockError.mock.calls[1][1]).toEqual('error');
expect(normalizeCodeLocInfo(mockError.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.
'\n in Child (at **)\n in Parent (at **)'
: 'in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
);
});
Expand Down
29 changes: 0 additions & 29 deletions packages/react-devtools-shared/src/backend/DevToolsFiberComponentStack.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,6 @@ export function supportsOwnerStacks(fiber: Fiber): boolean {
return fiber._debugStack !== undefined;
}

function describeFunctionComponentFrameWithoutLineNumber(fn: Function): string {
// We use this because we don't actually want to describe the line of the component
// but just the component name.
const name = fn ? fn.displayName || fn.name : '';
return name ? describeBuiltInComponentFrame(name) : '';
}

export function getOwnerStackByFiberInDev(
workTagMap: WorkTagMap,
workInProgress: Fiber,
Expand All @@ -140,10 +133,6 @@ export function getOwnerStackByFiberInDev(
HostComponent,
SuspenseComponent,
SuspenseListComponent,
FunctionComponent,
SimpleMemoComponent,
ForwardRef,
ClassComponent,
} = workTagMap;
try {
let info = '';
Expand Down Expand Up @@ -173,24 +162,6 @@ export function getOwnerStackByFiberInDev(
case SuspenseListComponent:
info += describeBuiltInComponentFrame('SuspenseList');
break;
case FunctionComponent:
case SimpleMemoComponent:
case ClassComponent:
if (!workInProgress._debugOwner && info === '') {
// Only if we have no other data about the callsite do we add
// the component name as the single stack frame.
info += describeFunctionComponentFrameWithoutLineNumber(
workInProgress.type,
);
}
break;
case ForwardRef:
if (!workInProgress._debugOwner && info === '') {
info += describeFunctionComponentFrameWithoutLineNumber(
workInProgress.type.render,
);
}
break;
}

let owner: void | null | Fiber | ReactComponentInfo = workInProgress;
Expand Down
45 changes: 32 additions & 13 deletions packages/react-devtools-shared/src/backend/console.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import {
supportsOwnerStacks,
supportsNativeConsoleTasks,
} from './DevToolsFiberComponentStack';
import {formatOwnerStack} from './DevToolsOwnerStack';
import {castBool, castBrowserTheme} from '../utils';

const OVERRIDE_CONSOLE_METHODS = ['error', 'trace', 'warn'];
Expand Down Expand Up @@ -252,17 +253,31 @@ export function patch({
consoleSettingsRef.appendComponentStack &&
!supportsNativeConsoleTasks(current)
) {
const componentStack = supportsOwnerStacks(current)
? getOwnerStackByFiberInDev(
workTagMap,
current,
(currentDispatcherRef: any),
)
: getStackByFiberInDevAndProd(
workTagMap,
current,
(currentDispatcherRef: any),
);
const enableOwnerStacks = supportsOwnerStacks(current);
let componentStack = '';
if (supportsOwnerStacks(current)) {
// Prefix the owner stack with the current stack. I.e. what called
// console.error. While this will also be part of the native stack,
// it is hidden and not presented alongside this argument so we print
// them all together.
const topStackFrames = formatOwnerStack(
new Error('react-stack-top-frame'),
);
if (topStackFrames) {
componentStack += '\n' + topStackFrames;
}
componentStack += getOwnerStackByFiberInDev(
workTagMap,
current,
(currentDispatcherRef: any),
);
} else {
componentStack = getStackByFiberInDevAndProd(
workTagMap,
current,
(currentDispatcherRef: any),
);
}
if (componentStack !== '') {
// Create a fake Error so that when we print it we get native source maps. Every
// browser will print the .stack property of the error and then parse it back for source
Expand All @@ -272,13 +287,17 @@ export function patch({
// In Chromium, only the stack property is printed but in Firefox the <name>:<message>
// gets printed so to make the colon make sense, we name it so we print Component Stack:
// and similarly Safari leave an expandable slot.
fakeError.name = 'Component Stack'; // This gets printed
fakeError.name = enableOwnerStacks
? 'Stack'
: 'Component Stack'; // This gets printed
// In Chromium, the stack property needs to start with ^[\w.]*Error\b to trigger stack
// formatting. Otherwise it is left alone. So we prefix it. Otherwise we just override it
// to our own stack.
fakeError.stack =
__IS_CHROME__ || __IS_EDGE__
? 'Error Component Stack:' + componentStack
? (enableOwnerStacks
? 'Error Stack:'
: 'Error Component Stack:') + componentStack
: componentStack;
if (alreadyHasComponentStack) {
// Only modify the component stack if it matches what we would've added anyway.
Expand Down

0 comments on commit 7a614e2

Please sign in to comment.