diff --git a/packages/react-devtools-shared/src/__tests__/console-test.js b/packages/react-devtools-shared/src/__tests__/console-test.js index df56bb00b9e1d..516762132e884 100644 --- a/packages/react-devtools-shared/src/__tests__/console-test.js +++ b/packages/react-devtools-shared/src/__tests__/console-test.js @@ -54,14 +54,6 @@ describe('console', () => { fakeConsole, ); - const inject = global.__REACT_DEVTOOLS_GLOBAL_HOOK__.inject; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.inject = internals => { - rendererID = inject(internals); - - Console.registerRenderer(internals); - return rendererID; - }; - React = require('react'); if ( React.version.startsWith('19') && @@ -1100,9 +1092,17 @@ describe('console error', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.inject = internals => { inject(internals); - Console.registerRenderer(internals, () => { - throw Error('foo'); - }); + Console.registerRenderer( + () => { + throw Error('foo'); + }, + () => { + return { + enableOwnerStacks: true, + componentStack: '\n at FakeStack (fake-file)', + }; + }, + ); }; React = require('react'); @@ -1142,11 +1142,18 @@ describe('console error', () => { expect(mockLog.mock.calls[0][0]).toBe('log'); expect(mockWarn).toHaveBeenCalledTimes(1); - expect(mockWarn.mock.calls[0]).toHaveLength(1); + expect(mockWarn.mock.calls[0]).toHaveLength(2); expect(mockWarn.mock.calls[0][0]).toBe('warn'); + // An error in showInlineWarningsAndErrors doesn't need to break component stacks. + expect(normalizeCodeLocInfo(mockError.mock.calls[0][1])).toBe( + '\n in FakeStack (at **)', + ); expect(mockError).toHaveBeenCalledTimes(1); - expect(mockError.mock.calls[0]).toHaveLength(1); + expect(mockError.mock.calls[0]).toHaveLength(2); expect(mockError.mock.calls[0][0]).toBe('error'); + expect(normalizeCodeLocInfo(mockError.mock.calls[0][1])).toBe( + '\n in FakeStack (at **)', + ); }); }); diff --git a/packages/react-devtools-shared/src/backend/console.js b/packages/react-devtools-shared/src/backend/console.js index 05d9055d0b021..f1a2ee72f7009 100644 --- a/packages/react-devtools-shared/src/backend/console.js +++ b/packages/react-devtools-shared/src/backend/console.js @@ -7,14 +7,7 @@ * @flow */ -import type {Fiber} from 'react-reconciler/src/ReactInternalTypes'; -import type { - LegacyDispatcherRef, - CurrentDispatcherRef, - ReactRenderer, - WorkTagMap, - ConsolePatchSettings, -} from './types'; +import type {ConsolePatchSettings} from './types'; import { formatConsoleArguments, @@ -25,14 +18,6 @@ import { ANSI_STYLE_DIMMING_TEMPLATE, ANSI_STYLE_DIMMING_TEMPLATE_WITH_COMPONENT_STACK, } from 'react-devtools-shared/src/constants'; -import {getInternalReactConstants, getDispatcherRef} from './fiber/renderer'; -import { - getStackByFiberInDevAndProd, - getOwnerStackByFiberInDev, - supportsOwnerStacks, - supportsConsoleTasks, -} from './fiber/DevToolsFiberComponentStack'; -import {formatOwnerStack} from './shared/DevToolsOwnerStack'; import {castBool, castBrowserTheme} from '../utils'; const OVERRIDE_CONSOLE_METHODS = ['error', 'trace', 'warn']; @@ -90,21 +75,15 @@ function restorePotentiallyModifiedArgs(args: Array): Array { } } -type OnErrorOrWarning = ( - fiber: Fiber, - type: 'error' | 'warn', - args: Array, -) => void; - -const injectedRenderers: Map< - ReactRenderer, - { - currentDispatcherRef: LegacyDispatcherRef | CurrentDispatcherRef, - getCurrentFiber: () => Fiber | null, - onErrorOrWarning: ?OnErrorOrWarning, - workTagMap: WorkTagMap, - }, -> = new Map(); +type OnErrorOrWarning = (type: 'error' | 'warn', args: Array) => void; +type GetComponentStack = ( + topFrame: Error, +) => null | {enableOwnerStacks: boolean, componentStack: string}; + +const injectedRenderers: Array<{ + onErrorOrWarning: ?OnErrorOrWarning, + getComponentStack: ?GetComponentStack, +}> = []; let targetConsole: Object = console; let targetConsoleMethods: {[string]: $FlowFixMe} = {}; @@ -132,23 +111,13 @@ export function dangerous_setTargetConsoleForTesting( // These internals will be used if the console is patched. // Injecting them separately allows the console to easily be patched or un-patched later (at runtime). export function registerRenderer( - renderer: ReactRenderer, onErrorOrWarning?: OnErrorOrWarning, + getComponentStack?: GetComponentStack, ): void { - const {currentDispatcherRef, getCurrentFiber, version} = renderer; - - // currentDispatcherRef gets injected for v16.8+ to support hooks inspection. - // getCurrentFiber gets injected for v16.9+. - if (currentDispatcherRef != null && typeof getCurrentFiber === 'function') { - const {ReactTypeOfWork} = getInternalReactConstants(version); - - injectedRenderers.set(renderer, { - currentDispatcherRef, - getCurrentFiber, - workTagMap: ReactTypeOfWork, - onErrorOrWarning, - }); - } + injectedRenderers.push({ + onErrorOrWarning, + getComponentStack, + }); } const consoleSettingsRef: ConsolePatchSettings = { @@ -219,55 +188,39 @@ export function patch({ // Search for the first renderer that has a current Fiber. // We don't handle the edge case of stacks for more than one (e.g. interleaved renderers?) - // eslint-disable-next-line no-for-of-loops/no-for-of-loops - for (const renderer of injectedRenderers.values()) { - const currentDispatcherRef = getDispatcherRef(renderer); - const {getCurrentFiber, onErrorOrWarning, workTagMap} = renderer; - const current: ?Fiber = getCurrentFiber(); - if (current != null) { - try { - if (shouldShowInlineWarningsAndErrors) { - // patch() is called by two places: (1) the hook and (2) the renderer backend. - // The backend is what implements a message queue, so it's the only one that injects onErrorOrWarning. - if (typeof onErrorOrWarning === 'function') { - onErrorOrWarning( - current, - ((method: any): 'error' | 'warn'), - // Restore and copy args before we mutate them (e.g. adding the component stack) - restorePotentiallyModifiedArgs(args), - ); - } + for (let i = 0; i < injectedRenderers.length; i++) { + const renderer = injectedRenderers[i]; + const {getComponentStack, onErrorOrWarning} = renderer; + try { + if (shouldShowInlineWarningsAndErrors) { + // patch() is called by two places: (1) the hook and (2) the renderer backend. + // The backend is what implements a message queue, so it's the only one that injects onErrorOrWarning. + if (onErrorOrWarning != null) { + onErrorOrWarning( + ((method: any): 'error' | 'warn'), + // Restore and copy args before we mutate them (e.g. adding the component stack) + restorePotentiallyModifiedArgs(args), + ); } - - if ( - consoleSettingsRef.appendComponentStack && - !supportsConsoleTasks(current) - ) { - const enableOwnerStacks = supportsOwnerStacks(current); - let componentStack = ''; - if (enableOwnerStacks) { - // 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), - ); - } + } + } catch (error) { + // Don't let a DevTools or React internal error interfere with logging. + setTimeout(() => { + throw error; + }, 0); + } + try { + if ( + consoleSettingsRef.appendComponentStack && + getComponentStack != null + ) { + // This needs to be directly in the wrapper so we can pop exactly one frame. + const topFrame = Error('react-stack-top-frame'); + const match = getComponentStack(topFrame); + if (match !== null) { + const {enableOwnerStacks, componentStack} = match; + // Empty string means we have a match but no component stack. + // We don't need to look in other renderers but we also don't add anything. 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 @@ -275,7 +228,7 @@ export function patch({ // slot doesn't line up. const fakeError = new Error(''); // 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: + // gets printed so to make the colon make sense, we name it so we print Stack: // and similarly Safari leave an expandable slot. fakeError.name = enableOwnerStacks ? 'Stack' @@ -289,6 +242,7 @@ export function patch({ ? 'Error Stack:' : 'Error Component Stack:') + componentStack : componentStack; + if (alreadyHasComponentStack) { // Only modify the component stack if it matches what we would've added anyway. // Otherwise we assume it was a non-React stack. @@ -324,15 +278,15 @@ export function patch({ } } } + // Don't add stacks from other renderers. + break; } - } catch (error) { - // Don't let a DevTools or React internal error interfere with logging. - setTimeout(() => { - throw error; - }, 0); - } finally { - break; } + } catch (error) { + // Don't let a DevTools or React internal error interfere with logging. + setTimeout(() => { + throw error; + }, 0); } } diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index c9362cc84d6df..4500da6a35366 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -106,6 +106,13 @@ import {componentInfoToComponentLogsMap} from '../shared/DevToolsServerComponent import is from 'shared/objectIs'; import hasOwnProperty from 'shared/hasOwnProperty'; +import { + getStackByFiberInDevAndProd, + getOwnerStackByFiberInDev, + supportsOwnerStacks, + supportsConsoleTasks, +} from './DevToolsFiberComponentStack'; + // $FlowFixMe[method-unbinding] const toString = Object.prototype.toString; @@ -912,6 +919,7 @@ export function attach( setErrorHandler, setSuspenseHandler, scheduleUpdate, + getCurrentFiber, } = renderer; const supportsTogglingError = typeof setErrorHandler === 'function' && @@ -1067,12 +1075,70 @@ export function attach( } } + function getComponentStack( + topFrame: Error, + ): null | {enableOwnerStacks: boolean, componentStack: string} { + if (getCurrentFiber === undefined) { + // Expected this to be part of the renderer. Ignore. + return null; + } + const current = getCurrentFiber(); + if (current === null) { + // Outside of our render scope. + return null; + } + + if (supportsConsoleTasks(current)) { + // This will be handled natively by console.createTask. No need for + // DevTools to add it. + return null; + } + + const dispatcherRef = getDispatcherRef(renderer); + if (dispatcherRef === undefined) { + return null; + } + + const enableOwnerStacks = supportsOwnerStacks(current); + let componentStack = ''; + if (enableOwnerStacks) { + // 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(topFrame); + if (topStackFrames) { + componentStack += '\n' + topStackFrames; + } + componentStack += getOwnerStackByFiberInDev( + ReactTypeOfWork, + current, + dispatcherRef, + ); + } else { + componentStack = getStackByFiberInDevAndProd( + ReactTypeOfWork, + current, + dispatcherRef, + ); + } + return {enableOwnerStacks, componentStack}; + } + // Called when an error or warning is logged during render, commit, or passive (including unmount functions). function onErrorOrWarning( - fiber: Fiber, type: 'error' | 'warn', args: $ReadOnlyArray, ): void { + if (getCurrentFiber === undefined) { + // Expected this to be part of the renderer. Ignore. + return; + } + const fiber = getCurrentFiber(); + if (fiber === null) { + // Outside of our render scope. + return; + } if (type === 'error') { // if this is an error simulated by us to trigger error boundary, ignore if ( @@ -1135,7 +1201,7 @@ export function attach( // Patching the console enables DevTools to do a few useful things: // * Append component stacks to warnings and error messages // * Disable logging during re-renders to inspect hooks (see inspectHooksOfFiber) - registerRendererWithConsole(renderer, onErrorOrWarning); + registerRendererWithConsole(onErrorOrWarning, getComponentStack); // The renderer interface can't read these preferences directly, // because it is stored in localStorage within the context of the extension. diff --git a/packages/react-devtools-shared/src/backend/flight/renderer.js b/packages/react-devtools-shared/src/backend/flight/renderer.js index 566ec6ed9c4e9..567279e737685 100644 --- a/packages/react-devtools-shared/src/backend/flight/renderer.js +++ b/packages/react-devtools-shared/src/backend/flight/renderer.js @@ -21,7 +21,7 @@ export function attach( global: Object, ): RendererInterface { patchConsoleUsingWindowValues(); - registerRendererWithConsole(renderer); + registerRendererWithConsole(); // TODO: Fill in the impl return { cleanup() {}, diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js index 12899cdd0ff4a..aeef1abc0e737 100644 --- a/packages/react-devtools-shared/src/hook.js +++ b/packages/react-devtools-shared/src/hook.js @@ -367,25 +367,6 @@ export function installHook(target: any): DevToolsHook | null { ? 'deadcode' : detectReactBuildType(renderer); - // Patching the console enables DevTools to do a few useful things: - // * Append component stacks to warnings and error messages - // * Disabling or marking logs during a double render in Strict Mode - // * Disable logging during re-renders to inspect hooks (see inspectHooksOfFiber) - // - // Allow patching console early (during injection) to - // provide developers with components stacks even if they don't run DevTools. - if (target.hasOwnProperty('__REACT_DEVTOOLS_CONSOLE_FUNCTIONS__')) { - const {registerRendererWithConsole, patchConsoleUsingWindowValues} = - target.__REACT_DEVTOOLS_CONSOLE_FUNCTIONS__; - if ( - typeof registerRendererWithConsole === 'function' && - typeof patchConsoleUsingWindowValues === 'function' - ) { - registerRendererWithConsole(renderer); - patchConsoleUsingWindowValues(); - } - } - // If we have just reloaded to profile, we need to inject the renderer interface before the app loads. // Otherwise the renderer won't yet exist and we can skip this step. const attach = target.__REACT_DEVTOOLS_ATTACH__;