diff --git a/packages/react-devtools-shared/src/__tests__/componentStacks-test.js b/packages/react-devtools-shared/src/__tests__/componentStacks-test.js index 6bbdc899b55bb..b99db5c540097 100644 --- a/packages/react-devtools-shared/src/__tests__/componentStacks-test.js +++ b/packages/react-devtools-shared/src/__tests__/componentStacks-test.js @@ -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 **)', ); diff --git a/packages/react-devtools-shared/src/__tests__/console-test.js b/packages/react-devtools-shared/src/__tests__/console-test.js index 56ff240170dcd..da1990921dc29 100644 --- a/packages/react-devtools-shared/src/__tests__/console-test.js +++ b/packages/react-devtools-shared/src/__tests__/console-test.js @@ -232,7 +232,7 @@ 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); @@ -240,7 +240,7 @@ describe('console', () => { 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 **)', ); }); @@ -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); @@ -497,7 +498,7 @@ 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); @@ -505,7 +506,7 @@ describe('console', () => { 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 **)', ); }); @@ -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); @@ -1042,7 +1043,8 @@ 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 **)', ); @@ -1050,7 +1052,7 @@ describe('console', () => { 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); @@ -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 **)', ); }); diff --git a/packages/react-devtools-shared/src/backend/DevToolsFiberComponentStack.js b/packages/react-devtools-shared/src/backend/DevToolsFiberComponentStack.js index 277bc2e520c62..88e23e0ae2c83 100644 --- a/packages/react-devtools-shared/src/backend/DevToolsFiberComponentStack.js +++ b/packages/react-devtools-shared/src/backend/DevToolsFiberComponentStack.js @@ -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, @@ -140,10 +133,6 @@ export function getOwnerStackByFiberInDev( HostComponent, SuspenseComponent, SuspenseListComponent, - FunctionComponent, - SimpleMemoComponent, - ForwardRef, - ClassComponent, } = workTagMap; try { let info = ''; @@ -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; diff --git a/packages/react-devtools-shared/src/backend/console.js b/packages/react-devtools-shared/src/backend/console.js index 2057727c55bca..e0217b348f453 100644 --- a/packages/react-devtools-shared/src/backend/console.js +++ b/packages/react-devtools-shared/src/backend/console.js @@ -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']; @@ -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 @@ -272,13 +287,17 @@ export function patch({ // In Chromium, only the stack property is printed but in Firefox the : // 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.