From 87b6437efd87fdb2f7cec9c723dff0b71339613c Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 9 Sep 2024 17:17:56 -0400 Subject: [PATCH 1/3] Don't let Fiber renderer take precedence --- packages/react-devtools-shared/src/backend/index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/index.js b/packages/react-devtools-shared/src/backend/index.js index 03a8f0755d4..943c0360ffc 100644 --- a/packages/react-devtools-shared/src/backend/index.js +++ b/packages/react-devtools-shared/src/backend/index.js @@ -75,7 +75,10 @@ export function initBackend( // Inject any not-yet-injected renderers (if we didn't reload-and-profile) if (rendererInterface == null) { - if ( + if (typeof renderer.getCurrentComponentInfo === 'function') { + // react-flight/client + rendererInterface = attachFlight(hook, id, renderer, global); + } else if ( // v16-19 typeof renderer.findFiberByHostInstance === 'function' || // v16.8+ @@ -83,9 +86,6 @@ export function initBackend( ) { // react-reconciler v16+ rendererInterface = attachFiber(hook, id, renderer, global); - } else if (typeof renderer.getCurrentComponentInfo === 'function') { - // react-flight/client - rendererInterface = attachFlight(hook, id, renderer, global); } else if (renderer.ComponentTree) { // react-dom v15 rendererInterface = attachLegacy(hook, id, renderer, global); From 88241b7bf5df70e348b86b2b8c54b443b8471e0f Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 9 Sep 2024 16:38:49 -0400 Subject: [PATCH 2/3] Implement getComponentStack and onErrorOrWarning for replayed Flight logs This adds owner stacks to replayed Server Component logs in environments that don't support native console.createTask. It also tracks the logs in the global componentInfoToComponentLogsMap which lets us associate those logs with Server Components when they later commit into the fiber tree. --- .../flight/DevToolsComponentInfoStack.js | 55 +++++++++ .../src/backend/flight/renderer.js | 106 +++++++++++++++++- 2 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 packages/react-devtools-shared/src/backend/flight/DevToolsComponentInfoStack.js diff --git a/packages/react-devtools-shared/src/backend/flight/DevToolsComponentInfoStack.js b/packages/react-devtools-shared/src/backend/flight/DevToolsComponentInfoStack.js new file mode 100644 index 00000000000..05864ed2816 --- /dev/null +++ b/packages/react-devtools-shared/src/backend/flight/DevToolsComponentInfoStack.js @@ -0,0 +1,55 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +// This is a DevTools fork of ReactComponentInfoStack. +// This fork enables DevTools to use the same "native" component stack format, +// while still maintaining support for multiple renderer versions +// (which use different values for ReactTypeOfWork). + +import type {ReactComponentInfo} from 'shared/ReactTypes'; + +import {describeBuiltInComponentFrame} from '../shared/DevToolsComponentStackFrame'; + +import {formatOwnerStack} from '../shared/DevToolsOwnerStack'; + +export function getOwnerStackByComponentInfoInDev( + componentInfo: ReactComponentInfo, +): string { + try { + let info = ''; + + // The owner stack of the current component will be where it was created, i.e. inside its owner. + // There's no actual name of the currently executing component. Instead, that is available + // on the regular stack that's currently executing. However, if there is no owner at all, then + // there's no stack frame so we add the name of the root component to the stack to know which + // component is currently executing. + if (!componentInfo.owner && typeof componentInfo.name === 'string') { + return describeBuiltInComponentFrame(componentInfo.name); + } + + let owner: void | null | ReactComponentInfo = componentInfo; + + while (owner) { + const ownerStack: ?Error = owner.debugStack; + if (ownerStack != null) { + // Server Component + owner = owner.owner; + if (owner) { + // TODO: Should we stash this somewhere for caching purposes? + info += '\n' + formatOwnerStack(ownerStack); + } + } else { + break; + } + } + return info; + } catch (x) { + return '\nError generating stack: ' + x.message + '\n' + x.stack; + } +} diff --git a/packages/react-devtools-shared/src/backend/flight/renderer.js b/packages/react-devtools-shared/src/backend/flight/renderer.js index 567279e7376..9a519f0c2d8 100644 --- a/packages/react-devtools-shared/src/backend/flight/renderer.js +++ b/packages/react-devtools-shared/src/backend/flight/renderer.js @@ -7,21 +7,125 @@ * @flow */ +import type {ReactComponentInfo} from 'shared/ReactTypes'; + import type {DevToolsHook, ReactRenderer, RendererInterface} from '../types'; +import {getOwnerStackByComponentInfoInDev} from './DevToolsComponentInfoStack'; + +import {formatOwnerStack} from '../shared/DevToolsOwnerStack'; + +import {componentInfoToComponentLogsMap} from '../shared/DevToolsServerComponentLogs'; + +import {formatConsoleArgumentsToSingleString} from 'react-devtools-shared/src/backend/utils'; + import { patchConsoleUsingWindowValues, registerRenderer as registerRendererWithConsole, } from '../console'; +function supportsConsoleTasks(componentInfo: ReactComponentInfo): boolean { + // If this ReactComponentInfo supports native console.createTask then we are already running + // inside a native async stack trace if it's active - meaning the DevTools is open. + // Ideally we'd detect if this task was created while the DevTools was open or not. + return !!componentInfo.debugTask; +} + export function attach( hook: DevToolsHook, rendererID: number, renderer: ReactRenderer, global: Object, ): RendererInterface { + const {getCurrentComponentInfo} = renderer; + + function getComponentStack( + topFrame: Error, + ): null | {enableOwnerStacks: boolean, componentStack: string} { + if (getCurrentComponentInfo === undefined) { + // Expected this to be part of the renderer. Ignore. + return null; + } + const current = getCurrentComponentInfo(); + 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 enableOwnerStacks = current.debugStack != null; + 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 += getOwnerStackByComponentInfoInDev(current); + } + return {enableOwnerStacks, componentStack}; + } + + // Called when an error or warning is logged during render, commit, or passive (including unmount functions). + function onErrorOrWarning( + type: 'error' | 'warn', + args: $ReadOnlyArray, + ): void { + if (getCurrentComponentInfo === undefined) { + // Expected this to be part of the renderer. Ignore. + return; + } + const componentInfo = getCurrentComponentInfo(); + if (componentInfo === null) { + // Outside of our render scope. + return; + } + + // We can't really use this message as a unique key, since we can't distinguish + // different objects in this implementation. We have to delegate displaying of the objects + // to the environment, the browser console, for example, so this is why this should be kept + // as an array of arguments, instead of the plain string. + // [Warning: %o, {...}] and [Warning: %o, {...}] will be considered as the same message, + // even if objects are different + const message = formatConsoleArgumentsToSingleString(...args); + + // Track the warning/error for later. + let componentLogsEntry = componentInfoToComponentLogsMap.get(componentInfo); + if (componentLogsEntry === undefined) { + componentLogsEntry = { + errors: new Map(), + errorsCount: 0, + warnings: new Map(), + warningsCount: 0, + }; + componentInfoToComponentLogsMap.set(componentInfo, componentLogsEntry); + } + + const messageMap = + type === 'error' + ? componentLogsEntry.errors + : componentLogsEntry.warnings; + const count = messageMap.get(message) || 0; + messageMap.set(message, count + 1); + if (type === 'error') { + componentLogsEntry.errorsCount++; + } else { + componentLogsEntry.warningsCount++; + } + + // The changes will be flushed later when we commit this tree to Fiber. + } + patchConsoleUsingWindowValues(); - registerRendererWithConsole(); // TODO: Fill in the impl + registerRendererWithConsole(onErrorOrWarning, getComponentStack); return { cleanup() {}, From 14a1ce983e952ec0b3de8a836b01c6f500199893 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 9 Sep 2024 21:33:37 -0400 Subject: [PATCH 3/3] Remove badge prefix --- .../src/backend/flight/renderer.js | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/react-devtools-shared/src/backend/flight/renderer.js b/packages/react-devtools-shared/src/backend/flight/renderer.js index 9a519f0c2d8..fb4a9901a96 100644 --- a/packages/react-devtools-shared/src/backend/flight/renderer.js +++ b/packages/react-devtools-shared/src/backend/flight/renderer.js @@ -89,6 +89,27 @@ export function attach( return; } + if ( + args.length > 3 && + typeof args[0] === 'string' && + args[0].startsWith('%c%s%c ') && + typeof args[1] === 'string' && + typeof args[2] === 'string' && + typeof args[3] === 'string' + ) { + // This looks like the badge we prefixed to the log. Our UI doesn't support formatted logs. + // We remove the formatting. If the environment of the log is the same as the environment of + // the component (the common case) we remove the badge completely otherwise leave it plain + const format = args[0].slice(7); + const env = args[2].trim(); + args = args.slice(4); + if (env !== componentInfo.env) { + args.unshift('[' + env + '] ' + format); + } else { + args.unshift(format); + } + } + // We can't really use this message as a unique key, since we can't distinguish // different objects in this implementation. We have to delegate displaying of the objects // to the environment, the browser console, for example, so this is why this should be kept