From fe377360c0672821707b101b112c0e979e0fd8e8 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 6 Sep 2024 22:27:13 -0400 Subject: [PATCH] Track component logs --- .../src/backend/fiber/renderer.js | 70 +++++++++++++------ .../shared/DevToolsServerComponentLogs.js | 29 ++++++++ 2 files changed, 76 insertions(+), 23 deletions(-) create mode 100644 packages/react-devtools-shared/src/backend/shared/DevToolsServerComponentLogs.js diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 189da54c8603b..36b2b97039bed 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -100,6 +100,9 @@ import { SERVER_CONTEXT_SYMBOL_STRING, } from '../shared/ReactSymbols'; import {enableStyleXFeatures} from 'react-devtools-feature-flags'; + +import {componentInfoToComponentLogsMap} from '../shared/DevToolsServerComponentLogs'; + import is from 'shared/objectIs'; import hasOwnProperty from 'shared/hasOwnProperty'; @@ -1001,7 +1004,8 @@ export function attach( // Note, this only clears logs for Fibers that have instances. If they're filtered // and then mount, the logs are there. Ensuring we only clear what you've seen. // If we wanted to clear the whole set, we'd replace fiberToComponentLogsMap with a - // new WeakMap. + // new WeakMap. It's unclear whether we should clear componentInfoToComponentLogsMap + // since it's shared by other renderers but presumably it would. // eslint-disable-next-line no-for-of-loops/no-for-of-loops for (const devtoolsInstance of idToDevToolsInstanceMap.values()) { @@ -1012,7 +1016,7 @@ export function attach( fiberToComponentLogsMap.delete(fiber.alternate); } } else { - // TODO: Handle VirtualInstance. + componentInfoToComponentLogsMap.delete(devtoolsInstance.data); } const changed = recordConsoleLogs(devtoolsInstance, undefined); if (changed) { @@ -1025,28 +1029,27 @@ export function attach( function clearConsoleLogsHelper(instanceID: number, type: 'error' | 'warn') { const devtoolsInstance = idToDevToolsInstanceMap.get(instanceID); if (devtoolsInstance !== undefined) { + let componentLogsEntry; if (devtoolsInstance.kind === FIBER_INSTANCE) { const fiber = devtoolsInstance.data; - const componentLogsEntry = fiberToComponentLogsMap.get(fiber); - if (componentLogsEntry !== undefined) { - if (type === 'error') { - componentLogsEntry.errors.clear(); - componentLogsEntry.errorsCount = 0; - } else { - componentLogsEntry.warnings.clear(); - componentLogsEntry.warningsCount = 0; - } - const changed = recordConsoleLogs( - devtoolsInstance, - componentLogsEntry, - ); - if (changed) { - flushPendingEvents(); - updateMostRecentlyInspectedElementIfNecessary(devtoolsInstance.id); - } - } + componentLogsEntry = fiberToComponentLogsMap.get(fiber); } else { - // TODO: Handle VirtualInstance. + const componentInfo = devtoolsInstance.data; + componentLogsEntry = componentInfoToComponentLogsMap.get(componentInfo); + } + if (componentLogsEntry !== undefined) { + if (type === 'error') { + componentLogsEntry.errors.clear(); + componentLogsEntry.errorsCount = 0; + } else { + componentLogsEntry.warnings.clear(); + componentLogsEntry.warningsCount = 0; + } + const changed = recordConsoleLogs(devtoolsInstance, componentLogsEntry); + if (changed) { + flushPendingEvents(); + updateMostRecentlyInspectedElementIfNecessary(devtoolsInstance.id); + } } } } @@ -2207,6 +2210,10 @@ export function attach( pushOperation(ownerID); pushOperation(displayNameStringID); pushOperation(keyStringID); + + const componentLogsEntry = + componentInfoToComponentLogsMap.get(componentInfo); + recordConsoleLogs(instance, componentLogsEntry); } function recordUnmount(fiberInstance: FiberInstance): void { @@ -2886,6 +2893,14 @@ export function attach( ) { recordResetChildren(virtualInstance); } + // Update the errors/warnings count. If this Instance has switched to a different + // ReactComponentInfo instance, such as when refreshing Server Components, then + // we replace all the previous logs with the ones associated with the new ones rather + // than merging. Because deduping is expected to happen at the request level. + const componentLogsEntry = componentInfoToComponentLogsMap.get( + virtualInstance.data, + ); + recordConsoleLogs(virtualInstance, componentLogsEntry); // Must be called after all children have been appended. recordVirtualProfilingDurations(virtualInstance); } finally { @@ -4335,6 +4350,9 @@ export function attach( stylex: null, }; + const componentLogsEntry = + componentInfoToComponentLogsMap.get(componentInfo); + return { id: virtualInstance.id, @@ -4368,8 +4386,14 @@ export function attach( hooks: null, props: props, state: null, - errors: [], // TODO: Handle errors on Virtual Instances. - warnings: [], // TODO: Handle warnings on Virtual Instances. + errors: + componentLogsEntry === undefined + ? [] + : Array.from(componentLogsEntry.errors.entries()), + warnings: + componentLogsEntry === undefined + ? [] + : Array.from(componentLogsEntry.warnings.entries()), // List of owners owners, diff --git a/packages/react-devtools-shared/src/backend/shared/DevToolsServerComponentLogs.js b/packages/react-devtools-shared/src/backend/shared/DevToolsServerComponentLogs.js new file mode 100644 index 0000000000000..0eee05b536492 --- /dev/null +++ b/packages/react-devtools-shared/src/backend/shared/DevToolsServerComponentLogs.js @@ -0,0 +1,29 @@ +/** + * 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 keeps track of Server Component logs which may come from. +// This is in a shared module because Server Component logs don't come from a specific renderer +// but can become associated with a Virtual Instance of any renderer. + +import type {ReactComponentInfo} from 'shared/ReactTypes'; + +type ComponentLogs = { + errors: Map, + errorsCount: number, + warnings: Map, + warningsCount: number, +}; + +// This keeps it around as long as the ComponentInfo is alive which +// lets the Fiber get reparented/remounted and still observe the previous errors/warnings. +// Unless we explicitly clear the logs from a Fiber. +export const componentInfoToComponentLogsMap: WeakMap< + ReactComponentInfo, + ComponentLogs, +> = new WeakMap();