[Flight] Prefix owner stacks added to the console.log with the current stack (#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>
Sebastian Markbåge committed
Jul 23, 2024 at 16:48 UTC
e2cac6753340f950c592d97184e8c0bdd9d3a475
4 files changed
+46
-55
packages/react-devtools-shared/src/__tests__/componentStacks-test.js
+2
-2
@@ -56,13 +56,13 @@ describe('component stack', () => {
56
57
expect(mockError).toHaveBeenCalledWith(
58
'Test error.',
59
- (supportsOwnerStacks ? '' : '\n in Child (at **)') +
59
+ '\n in Child (at **)' +
60
'\n in Parent (at **)' +
61
'\n in Grandparent (at **)',
62
);
63
expect(mockWarn).toHaveBeenCalledWith(
64
'Test warning.',
65
- (supportsOwnerStacks ? '' : '\n in Child (at **)') +
65
+ '\n in Child (at **)' +
66
'\n in Parent (at **)' +
67
'\n in Grandparent (at **)',
68
);
packages/react-devtools-shared/src/__tests__/console-test.js
+12
-9
@@ -232,7 +232,7 @@ describe('console', () => {
232
expect(mockWarn.mock.calls[0][0]).toBe('warn');
233
expect(normalizeCodeLocInfo(mockWarn.mock.calls[0][1])).toEqual(
234
supportsOwnerStacks
235
- ? '\n in Parent (at **)'
235
+ ? '\n in Child (at **)\n in Parent (at **)'
236
: '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
237
);
238
expect(mockError).toHaveBeenCalledTimes(1);
@@ -240,7 +240,7 @@ describe('console', () => {
240
expect(mockError.mock.calls[0][0]).toBe('error');
241
expect(normalizeCodeLocInfo(mockError.mock.calls[0][1])).toBe(
242
supportsOwnerStacks
243
- ? '\n in Parent (at **)'
243
+ ? '\n in Child (at **)\n in Parent (at **)'
244
: '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
245
);
246
});
@@ -279,7 +279,8 @@ describe('console', () => {
279
expect(mockWarn.mock.calls[0][0]).toBe('active warn');
280
expect(normalizeCodeLocInfo(mockWarn.mock.calls[0][1])).toEqual(
281
supportsOwnerStacks
282
- ? '\n in Parent (at **)'
282
+ ? // TODO: It would be nice to have a Child stack frame here since it's just the effect function.
283
+ '\n in Parent (at **)'
284
: '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
285
);
286
expect(mockWarn.mock.calls[1]).toHaveLength(2);
@@ -497,7 +498,7 @@ describe('console', () => {
498
expect(mockWarn.mock.calls[0][0]).toBe('warn');
499
expect(normalizeCodeLocInfo(mockWarn.mock.calls[0][1])).toEqual(
500
supportsOwnerStacks
500
- ? '\n in Parent (at **)'
501
+ ? '\n in Child (at **)\n in Parent (at **)'
502
: '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
503
);
504
expect(mockError).toHaveBeenCalledTimes(1);
@@ -505,7 +506,7 @@ describe('console', () => {
506
expect(mockError.mock.calls[0][0]).toBe('error');
507
expect(normalizeCodeLocInfo(mockError.mock.calls[0][1])).toBe(
508
supportsOwnerStacks
508
- ? '\n in Parent (at **)'
509
+ ? '\n in Child (at **)\n in Parent (at **)'
510
: '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
511
);
512
});
@@ -1032,7 +1033,7 @@ describe('console', () => {
1033
expect(mockWarn.mock.calls[0]).toHaveLength(2);
1034
expect(normalizeCodeLocInfo(mockWarn.mock.calls[0][1])).toEqual(
1035
supportsOwnerStacks
1035
- ? '\n in Parent (at **)'
1036
+ ? '\n in Child (at **)\n in Parent (at **)'
1037
: '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
1038
);
1039
expect(mockWarn.mock.calls[1]).toHaveLength(3);
@@ -1042,7 +1043,8 @@ describe('console', () => {
1043
expect(mockWarn.mock.calls[1][1]).toMatch('warn');
1044
expect(normalizeCodeLocInfo(mockWarn.mock.calls[1][2]).trim()).toEqual(
1045
supportsOwnerStacks
1045
- ? 'in Parent (at **)'
1046
+ ? 'in Object.overrideMethod (at **)' + // TODO: This leading frame is due to our extra wrapper that shouldn't exist.
1047
+ '\n in Child (at **)\n in Parent (at **)'
1048
: 'in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
1049
);
1050
@@ -1050,7 +1052,7 @@ describe('console', () => {
1052
expect(mockError.mock.calls[0]).toHaveLength(2);
1053
expect(normalizeCodeLocInfo(mockError.mock.calls[0][1])).toEqual(
1054
supportsOwnerStacks
1053
- ? '\n in Parent (at **)'
1055
+ ? '\n in Child (at **)\n in Parent (at **)'
1056
: '\n in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
1057
);
1058
expect(mockError.mock.calls[1]).toHaveLength(3);
@@ -1060,7 +1062,8 @@ describe('console', () => {
1062
expect(mockError.mock.calls[1][1]).toEqual('error');
1063
expect(normalizeCodeLocInfo(mockError.mock.calls[1][2]).trim()).toEqual(
1064
supportsOwnerStacks
1063
- ? 'in Parent (at **)'
1065
+ ? 'in Object.overrideMethod (at **)' + // TODO: This leading frame is due to our extra wrapper that shouldn't exist.
1066
+ '\n in Child (at **)\n in Parent (at **)'
1067
: 'in Child (at **)\n in Intermediate (at **)\n in Parent (at **)',
1068
);
1069
});
packages/react-devtools-shared/src/backend/DevToolsFiberComponentStack.js
-31
@@ -121,13 +121,6 @@ export function supportsOwnerStacks(fiber: Fiber): boolean {
121
return fiber._debugStack !== undefined;
122
}
123
124
-function describeFunctionComponentFrameWithoutLineNumber(fn: Function): string {
125
- // We use this because we don't actually want to describe the line of the component
126
- // but just the component name.
127
- const name = fn ? fn.displayName || fn.name : '';
128
- return name ? describeBuiltInComponentFrame(name) : '';
129
-}
130
-
124
export function getOwnerStackByFiberInDev(
125
workTagMap: WorkTagMap,
126
workInProgress: Fiber,
@@ -140,10 +133,6 @@ export function getOwnerStackByFiberInDev(
133
HostComponent,
134
SuspenseComponent,
135
SuspenseListComponent,
143
- FunctionComponent,
144
- SimpleMemoComponent,
145
- ForwardRef,
146
- ClassComponent,
136
} = workTagMap;
137
try {
138
let info = '';
@@ -159,8 +148,6 @@ export function getOwnerStackByFiberInDev(
148
// on the regular stack that's currently executing. However, for built-ins there is no such
149
// named stack frame and it would be ignored as being internal anyway. Therefore we add
150
// add one extra frame just to describe the "current" built-in component by name.
162
- // Similarly, if there is no owner at all, then there's no stack frame so we add the name
163
- // of the root component to the stack to know which component is currently executing.
151
switch (workInProgress.tag) {
152
case HostHoistable:
153
case HostSingleton:
@@ -173,24 +160,6 @@ export function getOwnerStackByFiberInDev(
160
case SuspenseListComponent:
161
info += describeBuiltInComponentFrame('SuspenseList');
162
break;
176
- case FunctionComponent:
177
- case SimpleMemoComponent:
178
- case ClassComponent:
179
- if (!workInProgress._debugOwner && info === '') {
180
- // Only if we have no other data about the callsite do we add
181
- // the component name as the single stack frame.
182
- info += describeFunctionComponentFrameWithoutLineNumber(
183
- workInProgress.type,
184
- );
185
- }
186
- break;
187
- case ForwardRef:
188
- if (!workInProgress._debugOwner && info === '') {
189
- info += describeFunctionComponentFrameWithoutLineNumber(
190
- workInProgress.type.render,
191
- );
192
- }
193
- break;
163
}
164
165
let owner: void | null | Fiber | ReactComponentInfo = workInProgress;
packages/react-devtools-shared/src/backend/console.js
+32
-13
@@ -32,6 +32,7 @@ import {
32
supportsOwnerStacks,
33
supportsNativeConsoleTasks,
34
} from './DevToolsFiberComponentStack';
35
+import {formatOwnerStack} from './DevToolsOwnerStack';
36
import {castBool, castBrowserTheme} from '../utils';
37
38
const OVERRIDE_CONSOLE_METHODS = ['error', 'trace', 'warn'];
@@ -252,17 +253,31 @@ export function patch({
253
consoleSettingsRef.appendComponentStack &&
254
!supportsNativeConsoleTasks(current)
255
) {
255
- const componentStack = supportsOwnerStacks(current)
256
- ? getOwnerStackByFiberInDev(
257
- workTagMap,
258
- current,
259
- (currentDispatcherRef: any),
260
- )
261
- : getStackByFiberInDevAndProd(
262
- workTagMap,
263
- current,
264
- (currentDispatcherRef: any),
265
- );
256
+ const enableOwnerStacks = supportsOwnerStacks(current);
257
+ let componentStack = '';
258
+ if (enableOwnerStacks) {
259
+ // Prefix the owner stack with the current stack. I.e. what called
260
+ // console.error. While this will also be part of the native stack,
261
+ // it is hidden and not presented alongside this argument so we print
262
+ // them all together.
263
+ const topStackFrames = formatOwnerStack(
264
+ new Error('react-stack-top-frame'),
265
+ );
266
+ if (topStackFrames) {
267
+ componentStack += '\n' + topStackFrames;
268
+ }
269
+ componentStack += getOwnerStackByFiberInDev(
270
+ workTagMap,
271
+ current,
272
+ (currentDispatcherRef: any),
273
+ );
274
+ } else {
275
+ componentStack = getStackByFiberInDevAndProd(
276
+ workTagMap,
277
+ current,
278
+ (currentDispatcherRef: any),
279
+ );
280
+ }
281
if (componentStack !== '') {
282
// Create a fake Error so that when we print it we get native source maps. Every
283
// browser will print the .stack property of the error and then parse it back for source
@@ -272,13 +287,17 @@ export function patch({
287
// In Chromium, only the stack property is printed but in Firefox the <name>:<message>
288
// gets printed so to make the colon make sense, we name it so we print Component Stack:
289
// and similarly Safari leave an expandable slot.
275
- fakeError.name = 'Component Stack'; // This gets printed
290
+ fakeError.name = enableOwnerStacks
291
+ ? 'Stack'
292
+ : 'Component Stack'; // This gets printed
293
// In Chromium, the stack property needs to start with ^[\w.]*Error\b to trigger stack
294
// formatting. Otherwise it is left alone. So we prefix it. Otherwise we just override it
295
// to our own stack.
296
fakeError.stack =
297
__IS_CHROME__ || __IS_EDGE__
281
- ? 'Error Component Stack:' + componentStack
298
+ ? (enableOwnerStacks
299
+ ? 'Error Stack:'
300
+ : 'Error Component Stack:') + componentStack
301
: componentStack;
302
if (alreadyHasComponentStack) {
303
// Only modify the component stack if it matches what we would've added anyway.