From 1506685f0e6371924962a5b8a8b8bd765eb2e402 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 9 Jan 2025 10:33:44 -0500 Subject: [PATCH 01/10] Suspensey Fonts for View Transition (#32031) Fonts flickering in while loading can be disturbing to any transition but especially View Transitions. Even if they don't cause layout thrash - the paint thrash is bad enough. We might add Suspensey fonts to all Transitions in the future but it's especially a no-brainer for View Transitions. We need to apply mutations to the DOM first to know whether that will trigger new fonts to load. For general Suspensey fonts, we'd have to revert the commit by applying mutations in reverse to return to the previous state. For View Transitions, since a snapshot is already frozen, we can freeze the screen while we're waiting for the font at no extra cost. It does mean that the page isn't responsive during this time but we should only block this for a short period anyway. The timeout needs to be short enough that it doesn't cause too much of an issue when it's a new load and slow, yet long enough that you have a chance to load it. Otherwise we wait for no reason. The assumption here is that you likely have either cached the font or preloaded it earlier - or you're on an extremely fast connection. This case is for optimizing the high end experience. Before: https://github.com/user-attachments/assets/e0acfffe-fa49-40d6-82c3-5b08760175fb After: https://github.com/user-attachments/assets/615a03d3-9d6b-4eb1-8bd5-182c4c37a628 Note that since the Navigation is blocked on the font now the browser spinner shows up while the font is loading. --- .../view-transition/src/components/Chrome.js | 10 ++++++ .../view-transition/src/components/Page.css | 8 +++++ .../view-transition/src/components/Page.js | 2 +- .../src/client/ReactFiberConfigDOM.js | 35 ++++++++++++++++++- 4 files changed, 53 insertions(+), 2 deletions(-) diff --git a/fixtures/view-transition/src/components/Chrome.js b/fixtures/view-transition/src/components/Chrome.js index 0cae4a8dd1953..be3f6d1653dd5 100644 --- a/fixtures/view-transition/src/components/Chrome.js +++ b/fixtures/view-transition/src/components/Chrome.js @@ -12,6 +12,16 @@ export default class Chrome extends Component { + + + {this.props.title} diff --git a/fixtures/view-transition/src/components/Page.css b/fixtures/view-transition/src/components/Page.css index e69de29bb2d1d..5afcc33a8f4bc 100644 --- a/fixtures/view-transition/src/components/Page.css +++ b/fixtures/view-transition/src/components/Page.css @@ -0,0 +1,8 @@ +.roboto-font { + font-family: "Roboto", serif; + font-optical-sizing: auto; + font-weight: 100; + font-style: normal; + font-variation-settings: + "wdth" 100; +} diff --git a/fixtures/view-transition/src/components/Page.js b/fixtures/view-transition/src/components/Page.js index ab40790b1647b..4e2e806055615 100644 --- a/fixtures/view-transition/src/components/Page.js +++ b/fixtures/view-transition/src/components/Page.js @@ -29,7 +29,7 @@ function Component() { className={ transitions['enter-slide-right'] + ' ' + transitions['exit-slide-left'] }> -

Slide In from Left, Slide Out to Right

+

Slide In from Left, Slide Out to Right

); } diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index 8b17db1dc38a1..c9dbcdb68c50f 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -1198,6 +1198,13 @@ export function hasInstanceAffectedParent( return oldRect.height !== newRect.height || oldRect.width !== newRect.width; } +// How long to wait for new fonts to load before just committing anyway. +// This freezes the screen. It needs to be short enough that it doesn't cause too much of +// an issue when it's a new load and slow, yet long enough that you have a chance to load +// it. Otherwise we wait for no reason. The assumption here is that you likely have +// either cached the font or preloaded it earlier. +const SUSPENSEY_FONT_TIMEOUT = 500; + export function startViewTransition( rootContainer: Container, mutationCallback: () => void, @@ -1220,8 +1227,34 @@ export function startViewTransition( const ownerWindow = ownerDocument.defaultView; const pendingNavigation = ownerWindow.navigation && ownerWindow.navigation.transition; + // $FlowFixMe[prop-missing] + const previousFontLoadingStatus = ownerDocument.fonts.status; mutationCallback(); - // TODO: Wait for fonts. + if (previousFontLoadingStatus === 'loaded') { + // Force layout calculation to trigger font loading. + // eslint-disable-next-line ft-flow/no-unused-expressions + (ownerDocument.documentElement: any).clientHeight; + if ( + // $FlowFixMe[prop-missing] + ownerDocument.fonts.status === 'loading' + ) { + // The mutation lead to new fonts being loaded. We should wait on them before continuing. + // This avoids waiting for potentially unrelated fonts that were already loading before. + // Either in an earlier transition or as part of a sync optimistic state. This doesn't + // include preloads that happened earlier. + const fontsReady = Promise.race([ + // $FlowFixMe[prop-missing] + ownerDocument.fonts.ready, + new Promise(resolve => + setTimeout(resolve, SUSPENSEY_FONT_TIMEOUT), + ), + ]).then(layoutCallback, layoutCallback); + const allReady = pendingNavigation + ? Promise.allSettled([pendingNavigation.finished, fontsReady]) + : fontsReady; + return allReady.then(afterMutationCallback, afterMutationCallback); + } + } layoutCallback(); if (pendingNavigation) { return pendingNavigation.finished.then( From 74ea0c73a26d1c61609c1f2fa2e4ee3c8d432bcb Mon Sep 17 00:00:00 2001 From: Pieter De Baets Date: Thu, 9 Jan 2025 15:51:58 +0000 Subject: [PATCH 02/10] Remove enableGetInspectorDataForInstanceInProduction flag (#32033) ## Summary Callers for this method has been removed in https://github.com/facebook/react-native/commit/65bda542320a85f4627d7957e1c6f45c2776298d, so these methods no longer need to be conditionally exported and the feature flag can be removed. ## How did you test this change? Flow fabric/native --- .../react-native-renderer/src/ReactFabric.js | 4 - .../src/ReactNativeFiberInspector.js | 132 ++++++++---------- .../src/ReactNativeRenderer.js | 4 - packages/shared/ReactFeatureFlags.js | 2 - .../forks/ReactFeatureFlags.native-fb.js | 1 - .../forks/ReactFeatureFlags.native-oss.js | 1 - .../forks/ReactFeatureFlags.test-renderer.js | 1 - ...actFeatureFlags.test-renderer.native-fb.js | 1 - .../ReactFeatureFlags.test-renderer.www.js | 1 - .../shared/forks/ReactFeatureFlags.www.js | 1 - 10 files changed, 61 insertions(+), 87 deletions(-) diff --git a/packages/react-native-renderer/src/ReactFabric.js b/packages/react-native-renderer/src/ReactFabric.js index bd50d1a317896..67382f5f00d13 100644 --- a/packages/react-native-renderer/src/ReactFabric.js +++ b/packages/react-native-renderer/src/ReactFabric.js @@ -29,7 +29,6 @@ import { import {createPortal as createPortalImpl} from 'react-reconciler/src/ReactPortal'; import {setBatchingImplementation} from './legacy-events/ReactGenericBatching'; -import {getInspectorDataForInstance} from './ReactNativeFiberInspector'; import {LegacyRoot, ConcurrentRoot} from 'react-reconciler/src/ReactRootTags'; import { findHostInstance_DEPRECATED, @@ -188,9 +187,6 @@ export { unmountComponentAtNode, stopSurface, createPortal, - // This export is typically undefined in production builds. - // See the "enableGetInspectorDataForInstanceInProduction" flag. - getInspectorDataForInstance, // The public instance has a reference to the internal instance handle. // This method allows it to acess the most recent shadow node for // the instance (it's only accessible through it). diff --git a/packages/react-native-renderer/src/ReactNativeFiberInspector.js b/packages/react-native-renderer/src/ReactNativeFiberInspector.js index d0423f1d48cfa..47d588d448a18 100644 --- a/packages/react-native-renderer/src/ReactNativeFiberInspector.js +++ b/packages/react-native-renderer/src/ReactNativeFiberInspector.js @@ -21,7 +21,6 @@ import { UIManager, getNodeFromPublicInstance, } from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface'; -import {enableGetInspectorDataForInstanceInProduction} from 'shared/ReactFeatureFlags'; import {getClosestInstanceFromNode} from './ReactNativeComponentTree'; import { getNodeFromInternalInstanceHandle, @@ -29,39 +28,40 @@ import { } from './ReactNativePublicCompat'; import {getStackByFiberInDevAndProd} from 'react-reconciler/src/ReactFiberComponentStack'; -const emptyObject = {}; +let getInspectorDataForInstance: ( + closestInstance: Fiber | null, +) => InspectorData; + if (__DEV__) { - Object.freeze(emptyObject); -} + const emptyObject = Object.freeze({}); -// $FlowFixMe[missing-local-annot] -function createHierarchy(fiberHierarchy) { - return fiberHierarchy.map(fiber => ({ - name: getComponentNameFromType(fiber.type), - getInspectorData: () => { - return { - props: getHostProps(fiber), - measure: callback => { - // If this is Fabric, we'll find a shadow node and use that to measure. - const hostFiber = findCurrentHostFiber(fiber); - const node = - hostFiber != null && - hostFiber.stateNode !== null && - hostFiber.stateNode.node; + // $FlowFixMe[missing-local-annot] + const createHierarchy = function (fiberHierarchy) { + return fiberHierarchy.map(fiber => ({ + name: getComponentNameFromType(fiber.type), + getInspectorData: () => { + return { + props: getHostProps(fiber), + measure: callback => { + // If this is Fabric, we'll find a shadow node and use that to measure. + const hostFiber = findCurrentHostFiber(fiber); + const node = + hostFiber != null && + hostFiber.stateNode !== null && + hostFiber.stateNode.node; - if (node) { - nativeFabricUIManager.measure(node, callback); - } else { - return UIManager.measure(getHostNode(fiber), callback); - } - }, - }; - }, - })); -} + if (node) { + nativeFabricUIManager.measure(node, callback); + } else { + return UIManager.measure(getHostNode(fiber), callback); + } + }, + }; + }, + })); + }; -function getHostNode(fiber: Fiber | null) { - if (__DEV__ || enableGetInspectorDataForInstanceInProduction) { + const getHostNode = function (fiber: Fiber | null) { let hostNode; // look for children first for the hostNode // as composite fibers do not have a hostNode @@ -75,22 +75,19 @@ function getHostNode(fiber: Fiber | null) { fiber = fiber.child; } return null; - } -} + }; -// $FlowFixMe[missing-local-annot] -function getHostProps(fiber) { - const host = findCurrentHostFiber(fiber); - if (host) { - return host.memoizedProps || emptyObject; - } - return emptyObject; -} + const getHostProps = function (fiber: Fiber) { + const host = findCurrentHostFiber(fiber); + if (host) { + return host.memoizedProps || emptyObject; + } + return emptyObject; + }; -function getInspectorDataForInstance( - closestInstance: Fiber | null, -): InspectorData { - if (__DEV__ || enableGetInspectorDataForInstanceInProduction) { + getInspectorDataForInstance = function ( + closestInstance: Fiber | null, + ): InspectorData { // Handle case where user clicks outside of ReactNative if (!closestInstance) { return { @@ -125,36 +122,30 @@ function getInspectorDataForInstance( selectedIndex, componentStack, }; - } + }; - throw new Error( - 'getInspectorDataForInstance() is not available in production', - ); -} - -function getOwnerHierarchy(instance: Fiber) { - const hierarchy: Array<$FlowFixMe> = []; - traverseOwnerTreeUp(hierarchy, instance); - return hierarchy; -} + const getOwnerHierarchy = function (instance: Fiber) { + const hierarchy: Array<$FlowFixMe> = []; + traverseOwnerTreeUp(hierarchy, instance); + return hierarchy; + }; -// $FlowFixMe[missing-local-annot] -function lastNonHostInstance(hierarchy) { - for (let i = hierarchy.length - 1; i > 1; i--) { - const instance = hierarchy[i]; + // $FlowFixMe[missing-local-annot] + const lastNonHostInstance = function (hierarchy) { + for (let i = hierarchy.length - 1; i > 1; i--) { + const instance = hierarchy[i]; - if (instance.tag !== HostComponent) { - return instance; + if (instance.tag !== HostComponent) { + return instance; + } } - } - return hierarchy[0]; -} + return hierarchy[0]; + }; -function traverseOwnerTreeUp( - hierarchy: Array<$FlowFixMe>, - instance: Fiber, -): void { - if (__DEV__ || enableGetInspectorDataForInstanceInProduction) { + const traverseOwnerTreeUp = function ( + hierarchy: Array<$FlowFixMe>, + instance: Fiber, + ): void { hierarchy.unshift(instance); const owner = instance._debugOwner; if (owner != null && typeof owner.tag === 'number') { @@ -162,13 +153,12 @@ function traverseOwnerTreeUp( } else { // TODO: Traverse Server Components owners. } - } + }; } function getInspectorDataForViewTag(viewTag: number): InspectorData { if (__DEV__) { const closestInstance = getClosestInstanceFromNode(viewTag); - return getInspectorDataForInstance(closestInstance); } else { throw new Error( diff --git a/packages/react-native-renderer/src/ReactNativeRenderer.js b/packages/react-native-renderer/src/ReactNativeRenderer.js index 7f959184c95ef..a1a4825393dde 100644 --- a/packages/react-native-renderer/src/ReactNativeRenderer.js +++ b/packages/react-native-renderer/src/ReactNativeRenderer.js @@ -34,7 +34,6 @@ import { // Modules provided by RN: import {UIManager} from 'react-native/Libraries/ReactPrivate/ReactNativePrivateInterface'; -import {getInspectorDataForInstance} from './ReactNativeFiberInspector'; import {LegacyRoot} from 'react-reconciler/src/ReactRootTags'; import { findHostInstance_DEPRECATED, @@ -206,9 +205,6 @@ export { unmountComponentAtNodeAndRemoveContainer, createPortal, batchedUpdates as unstable_batchedUpdates, - // This export is typically undefined in production builds. - // See the "enableGetInspectorDataForInstanceInProduction" flag. - getInspectorDataForInstance, // DEV-only: isChildPublicInstance, }; diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index bebb275e1ba62..bc1ae542fa6e1 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -260,6 +260,4 @@ export const enableAsyncDebugInfo = __EXPERIMENTAL__; export const enableUpdaterTracking = __PROFILE__; // Internal only. -export const enableGetInspectorDataForInstanceInProduction = false; - export const enableDO_NOT_USE_disableStrictPassiveEffect = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index f3551ed658a44..7a01d9c9281db 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -48,7 +48,6 @@ export const enableCreateEventHandleAPI = false; export const enableDO_NOT_USE_disableStrictPassiveEffect = false; export const enableMoveBefore = true; export const enableFizzExternalRuntime = true; -export const enableGetInspectorDataForInstanceInProduction = true; export const enableHalt = false; export const enableInfiniteRenderLoopDetection = false; export const enableLegacyCache = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 11e20133950f3..26e05087c61db 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -33,7 +33,6 @@ export const enableDO_NOT_USE_disableStrictPassiveEffect = false; export const enableFabricCompleteRootInCommitPhase = false; export const enableMoveBefore = true; export const enableFizzExternalRuntime = true; -export const enableGetInspectorDataForInstanceInProduction = false; export const enableHalt = false; export const enableHiddenSubtreeInsertionEffectCleanup = false; export const enableInfiniteRenderLoopDetection = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index f878cdc5601b6..7e11aceaf7806 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -36,7 +36,6 @@ export const enableUseEffectEventHook = false; export const favorSafetyOverHydrationPerf = true; export const enableLegacyFBSupport = false; export const enableMoveBefore = false; -export const enableGetInspectorDataForInstanceInProduction = false; export const enableFabricCompleteRootInCommitPhase = false; export const enableHiddenSubtreeInsertionEffectCleanup = false; export const enableHydrationLaneScheduling = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index a75174692c525..72731529b17a3 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -27,7 +27,6 @@ export const enableCreateEventHandleAPI = false; export const enableDO_NOT_USE_disableStrictPassiveEffect = false; export const enableMoveBefore = false; export const enableFizzExternalRuntime = true; -export const enableGetInspectorDataForInstanceInProduction = false; export const enableHalt = false; export const enableInfiniteRenderLoopDetection = false; export const enableHiddenSubtreeInsertionEffectCleanup = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index e25c011828603..bfd3316019bde 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -38,7 +38,6 @@ export const enableUseEffectEventHook = false; export const favorSafetyOverHydrationPerf = true; export const enableLegacyFBSupport = false; export const enableMoveBefore = false; -export const enableGetInspectorDataForInstanceInProduction = false; export const enableRenderableContext = false; export const enableFabricCompleteRootInCommitPhase = false; export const enableHiddenSubtreeInsertionEffectCleanup = true; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index a55cec676c37a..b317fca356c92 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -69,7 +69,6 @@ export const enableSchedulingProfiler: boolean = __PROFILE__ && dynamicFeatureFlags.enableSchedulingProfiler; export const disableLegacyContext = __EXPERIMENTAL__; -export const enableGetInspectorDataForInstanceInProduction = false; export const enableLegacyCache = true; From c4595ca4c86a2a0795bf4d6d5ca4e074babf7fc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 9 Jan 2025 11:33:34 -0500 Subject: [PATCH 03/10] Add ViewTransitionComponent to Stacks and DevTools (#32034) Just adding the name so it shows up. (Note that no experimental ones are added to the list of filters atm. Including SuspenseList etc.) --- .../fiber/DevToolsFiberComponentStack.js | 7 +++++++ .../src/backend/fiber/renderer.js | 12 ++++++++++++ .../react-devtools-shared/src/backend/types.js | 1 + .../react-devtools-shared/src/frontend/types.js | 4 +++- .../src/ReactFiberComponentStack.js | 17 ++++++++++++++++- .../src/getComponentNameFromFiber.js | 7 +++++++ 6 files changed, 46 insertions(+), 2 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/fiber/DevToolsFiberComponentStack.js b/packages/react-devtools-shared/src/backend/fiber/DevToolsFiberComponentStack.js index 17475cab2860f..46e479905afbb 100644 --- a/packages/react-devtools-shared/src/backend/fiber/DevToolsFiberComponentStack.js +++ b/packages/react-devtools-shared/src/backend/fiber/DevToolsFiberComponentStack.js @@ -43,6 +43,7 @@ export function describeFiber( SimpleMemoComponent, ForwardRef, ClassComponent, + ViewTransitionComponent, } = workTagMap; switch (workInProgress.tag) { @@ -57,6 +58,8 @@ export function describeFiber( return describeBuiltInComponentFrame('Suspense'); case SuspenseListComponent: return describeBuiltInComponentFrame('SuspenseList'); + case ViewTransitionComponent: + return describeBuiltInComponentFrame('ViewTransition'); case FunctionComponent: case IndeterminateComponent: case SimpleMemoComponent: @@ -150,6 +153,7 @@ export function getOwnerStackByFiberInDev( HostComponent, SuspenseComponent, SuspenseListComponent, + ViewTransitionComponent, } = workTagMap; try { let info = ''; @@ -177,6 +181,9 @@ export function getOwnerStackByFiberInDev( case SuspenseListComponent: info += describeBuiltInComponentFrame('SuspenseList'); break; + case ViewTransitionComponent: + info += describeBuiltInComponentFrame('ViewTransition'); + break; } let owner: void | null | Fiber | ReactComponentInfo = workInProgress; diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index e1bd7881326cf..ef8bac0099a2b 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -27,6 +27,7 @@ import { ElementTypeSuspense, ElementTypeSuspenseList, ElementTypeTracingMarker, + ElementTypeViewTransition, ElementTypeVirtual, StrictMode, } from 'react-devtools-shared/src/frontend/types'; @@ -383,6 +384,7 @@ export function getInternalReactConstants(version: string): { // want to fork again so we're adding it here instead YieldComponent: -1, // Removed Throw: 29, + ViewTransitionComponent: 30, // Experimental }; } else if (gte(version, '17.0.0-alpha')) { ReactTypeOfWork = { @@ -418,6 +420,7 @@ export function getInternalReactConstants(version: string): { TracingMarkerComponent: -1, // Doesn't exist yet YieldComponent: -1, // Removed Throw: -1, // Doesn't exist yet + ViewTransitionComponent: -1, // Doesn't exist yet }; } else if (gte(version, '16.6.0-beta.0')) { ReactTypeOfWork = { @@ -453,6 +456,7 @@ export function getInternalReactConstants(version: string): { TracingMarkerComponent: -1, // Doesn't exist yet YieldComponent: -1, // Removed Throw: -1, // Doesn't exist yet + ViewTransitionComponent: -1, // Doesn't exist yet }; } else if (gte(version, '16.4.3-alpha')) { ReactTypeOfWork = { @@ -488,6 +492,7 @@ export function getInternalReactConstants(version: string): { TracingMarkerComponent: -1, // Doesn't exist yet YieldComponent: -1, // Removed Throw: -1, // Doesn't exist yet + ViewTransitionComponent: -1, // Doesn't exist yet }; } else { ReactTypeOfWork = { @@ -523,6 +528,7 @@ export function getInternalReactConstants(version: string): { TracingMarkerComponent: -1, // Doesn't exist yet YieldComponent: 9, Throw: -1, // Doesn't exist yet + ViewTransitionComponent: -1, // Doesn't exist yet }; } // ********************************************************** @@ -565,6 +571,7 @@ export function getInternalReactConstants(version: string): { SuspenseListComponent, TracingMarkerComponent, Throw, + ViewTransitionComponent, } = ReactTypeOfWork; function resolveFiberType(type: any): $FlowFixMe { @@ -673,6 +680,8 @@ export function getInternalReactConstants(version: string): { return 'Profiler'; case TracingMarkerComponent: return 'TracingMarker'; + case ViewTransitionComponent: + return 'ViewTransition'; case Throw: // This should really never be visible. return 'Error'; @@ -907,6 +916,7 @@ export function attach( SuspenseListComponent, TracingMarkerComponent, Throw, + ViewTransitionComponent, } = ReactTypeOfWork; const { ImmediatePriority, @@ -1583,6 +1593,8 @@ export function attach( return ElementTypeSuspenseList; case TracingMarkerComponent: return ElementTypeTracingMarker; + case ViewTransitionComponent: + return ElementTypeViewTransition; default: const typeSymbol = getTypeSymbol(type); diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 9d9d9a8eb5e65..7b0d801026acc 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -76,6 +76,7 @@ export type WorkTagMap = { TracingMarkerComponent: WorkTag, YieldComponent: WorkTag, Throw: WorkTag, + ViewTransitionComponent: WorkTag, }; export type HostInstance = Object; diff --git a/packages/react-devtools-shared/src/frontend/types.js b/packages/react-devtools-shared/src/frontend/types.js index 6216283301bf5..302b168bd6eb0 100644 --- a/packages/react-devtools-shared/src/frontend/types.js +++ b/packages/react-devtools-shared/src/frontend/types.js @@ -49,6 +49,7 @@ export const ElementTypeSuspense = 12; export const ElementTypeSuspenseList = 13; export const ElementTypeTracingMarker = 14; export const ElementTypeVirtual = 15; +export const ElementTypeViewTransition = 16; // Different types of elements displayed in the Elements tree. // These types may be used to visually distinguish types, @@ -66,7 +67,8 @@ export type ElementType = | 12 | 13 | 14 - | 15; + | 15 + | 16; // WARNING // The values below are referenced by ComponentFilters (which are saved via localStorage). diff --git a/packages/react-reconciler/src/ReactFiberComponentStack.js b/packages/react-reconciler/src/ReactFiberComponentStack.js index ac2cdf20a4cd6..8385cbf63e74a 100644 --- a/packages/react-reconciler/src/ReactFiberComponentStack.js +++ b/packages/react-reconciler/src/ReactFiberComponentStack.js @@ -7,7 +7,10 @@ * @flow */ -import {enableOwnerStacks} from 'shared/ReactFeatureFlags'; +import { + enableOwnerStacks, + enableViewTransition, +} from 'shared/ReactFeatureFlags'; import type {Fiber} from './ReactInternalTypes'; import type {ReactComponentInfo} from 'shared/ReactTypes'; @@ -23,6 +26,7 @@ import { SimpleMemoComponent, ClassComponent, HostText, + ViewTransitionComponent, } from './ReactWorkTags'; import { describeBuiltInComponentFrame, @@ -52,6 +56,11 @@ function describeFiber(fiber: Fiber): string { return describeFunctionComponentFrame(fiber.type.render); case ClassComponent: return describeClassComponentFrame(fiber.type); + case ViewTransitionComponent: + if (enableViewTransition) { + return describeBuiltInComponentFrame('ViewTransition'); + } + // Fallthrough default: return ''; } @@ -123,6 +132,12 @@ export function getOwnerStackByFiberInDev(workInProgress: Fiber): string { case SuspenseListComponent: info += describeBuiltInComponentFrame('SuspenseList'); break; + case ViewTransitionComponent: + if (enableViewTransition) { + info += describeBuiltInComponentFrame('SuspenseList'); + break; + } + // Fallthrough case FunctionComponent: case SimpleMemoComponent: case ClassComponent: diff --git a/packages/react-reconciler/src/getComponentNameFromFiber.js b/packages/react-reconciler/src/getComponentNameFromFiber.js index cac247f26912b..04ad072c2583f 100644 --- a/packages/react-reconciler/src/getComponentNameFromFiber.js +++ b/packages/react-reconciler/src/getComponentNameFromFiber.js @@ -14,6 +14,7 @@ import { disableLegacyMode, enableLegacyHidden, enableRenderableContext, + enableViewTransition, } from 'shared/ReactFeatureFlags'; import { @@ -45,6 +46,7 @@ import { CacheComponent, TracingMarkerComponent, Throw, + ViewTransitionComponent, } from 'react-reconciler/src/ReactWorkTags'; import getComponentNameFromType from 'shared/getComponentNameFromType'; import {REACT_STRICT_MODE_TYPE} from 'shared/ReactSymbols'; @@ -139,7 +141,12 @@ export default function getComponentNameFromFiber(fiber: Fiber): string | null { return 'SuspenseList'; case TracingMarkerComponent: return 'TracingMarker'; + case ViewTransitionComponent: + if (enableViewTransition) { + return 'ViewTransition'; + } // The display name for these tags come from the user-provided type: + // Fallthrough case IncompleteClassComponent: case IncompleteFunctionComponent: if (disableLegacyMode) { From 8932ca32f47a1441723ae30ef6828998e9587553 Mon Sep 17 00:00:00 2001 From: lauren Date: Thu, 9 Jan 2025 12:21:05 -0500 Subject: [PATCH 04/10] [playground] Partially revert #32009 (#32035) I had forgotten that our default error reporting threshold was `none` due to the fact that build pipelines should not throw errors. This resets it back to throwing on all errors which mostly is the same as the eslint plugin. Closes #32014. --- compiler/apps/playground/components/Editor/EditorImpl.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/apps/playground/components/Editor/EditorImpl.tsx b/compiler/apps/playground/components/Editor/EditorImpl.tsx index c475a1585a9d8..8c782efe54d4e 100644 --- a/compiler/apps/playground/components/Editor/EditorImpl.tsx +++ b/compiler/apps/playground/components/Editor/EditorImpl.tsx @@ -78,6 +78,7 @@ function invokeCompiler( logEvent: () => {}, }, environment, + panicThreshold: 'all_errors', }); const ast = parseInput(source, language); let result = transformFromAstSync(ast, source, { From d16fe4be5b9b5eee0cfb0f602ad62d6b0842d253 Mon Sep 17 00:00:00 2001 From: mofeiZ <34200447+mofeiZ@users.noreply.github.com> Date: Thu, 9 Jan 2025 12:38:16 -0500 Subject: [PATCH 05/10] [compiler] Playground qol: shared compilation option directives with tests (#32012) - Adds @compilationMode(all|infer|syntax|annotation) and @panicMode(none) directives. This is now shared with our test infra - Playground still defaults to `infer` mode while tests default to `all` mode - See added fixture tests --- .../compilationMode-all-output.txt | 13 +++ .../compilationMode-infer-output.txt | 4 + .../playground/__tests__/e2e/page.spec.ts | 18 +++ .../components/Editor/EditorImpl.tsx | 108 +++++++++--------- .../src/HIR/Environment.ts | 54 ++++++++- .../src/__tests__/parseConfigPragma-test.ts | 16 ++- compiler/packages/snap/src/compiler.ts | 37 +----- 7 files changed, 154 insertions(+), 96 deletions(-) create mode 100644 compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/compilationMode-all-output.txt create mode 100644 compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/compilationMode-infer-output.txt diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/compilationMode-all-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/compilationMode-all-output.txt new file mode 100644 index 0000000000000..5633cf0b0f75a --- /dev/null +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/compilationMode-all-output.txt @@ -0,0 +1,13 @@ +import { c as _c } from "react/compiler-runtime"; //  +        @compilationMode(all) +function nonReactFn() { +  const $ = _c(1); +  let t0; +  if ($[0] === Symbol.for("react.memo_cache_sentinel")) { +    t0 = {}; +    $[0] = t0; +  } else { +    t0 = $[0]; +  } +  return t0; +} \ No newline at end of file diff --git a/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/compilationMode-infer-output.txt b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/compilationMode-infer-output.txt new file mode 100644 index 0000000000000..b50c37fc4e48f --- /dev/null +++ b/compiler/apps/playground/__tests__/e2e/__snapshots__/page.spec.ts/compilationMode-infer-output.txt @@ -0,0 +1,4 @@ +// @compilationMode(infer) +function nonReactFn() { +  return {}; +} \ No newline at end of file diff --git a/compiler/apps/playground/__tests__/e2e/page.spec.ts b/compiler/apps/playground/__tests__/e2e/page.spec.ts index 05fe96d4b9048..3ba082cf62972 100644 --- a/compiler/apps/playground/__tests__/e2e/page.spec.ts +++ b/compiler/apps/playground/__tests__/e2e/page.spec.ts @@ -79,6 +79,24 @@ function Foo() { // @flow function useFoo(propVal: {+baz: number}) { return
{(propVal.baz as number)}
; +} + `, + noFormat: true, + }, + { + name: 'compilationMode-infer', + input: `// @compilationMode(infer) +function nonReactFn() { + return {}; +} + `, + noFormat: true, + }, + { + name: 'compilationMode-all', + input: `// @compilationMode(all) +function nonReactFn() { + return {}; } `, noFormat: true, diff --git a/compiler/apps/playground/components/Editor/EditorImpl.tsx b/compiler/apps/playground/components/Editor/EditorImpl.tsx index 8c782efe54d4e..8c386116865af 100644 --- a/compiler/apps/playground/components/Editor/EditorImpl.tsx +++ b/compiler/apps/playground/components/Editor/EditorImpl.tsx @@ -20,7 +20,6 @@ import BabelPluginReactCompiler, { CompilerPipelineValue, parsePluginOptions, } from 'babel-plugin-react-compiler/src'; -import {type EnvironmentConfig} from 'babel-plugin-react-compiler/src/HIR/Environment'; import clsx from 'clsx'; import invariant from 'invariant'; import {useSnackbar} from 'notistack'; @@ -69,23 +68,14 @@ function parseInput( function invokeCompiler( source: string, language: 'flow' | 'typescript', - environment: EnvironmentConfig, - logIR: (pipelineValue: CompilerPipelineValue) => void, + options: PluginOptions, ): CompilerTransformOutput { - const opts: PluginOptions = parsePluginOptions({ - logger: { - debugLogIRs: logIR, - logEvent: () => {}, - }, - environment, - panicThreshold: 'all_errors', - }); const ast = parseInput(source, language); let result = transformFromAstSync(ast, source, { filename: '_playgroundFile.js', highlightCode: false, retainLines: true, - plugins: [[BabelPluginReactCompiler, opts]], + plugins: [[BabelPluginReactCompiler, options]], ast: true, sourceType: 'module', configFile: false, @@ -171,51 +161,59 @@ function compile(source: string): [CompilerOutput, 'flow' | 'typescript'] { try { // Extract the first line to quickly check for custom test directives const pragma = source.substring(0, source.indexOf('\n')); - const config = parseConfigPragmaForTests(pragma); - - transformOutput = invokeCompiler( - source, - language, - {...config, customHooks: new Map([...COMMON_HOOKS])}, - result => { - switch (result.kind) { - case 'ast': { - break; - } - case 'hir': { - upsert({ - kind: 'hir', - fnName: result.value.id, - name: result.name, - value: printFunctionWithOutlined(result.value), - }); - break; - } - case 'reactive': { - upsert({ - kind: 'reactive', - fnName: result.value.id, - name: result.name, - value: printReactiveFunctionWithOutlined(result.value), - }); - break; - } - case 'debug': { - upsert({ - kind: 'debug', - fnName: null, - name: result.name, - value: result.value, - }); - break; - } - default: { - const _: never = result; - throw new Error(`Unhandled result ${result}`); - } + const logIR = (result: CompilerPipelineValue): void => { + switch (result.kind) { + case 'ast': { + break; + } + case 'hir': { + upsert({ + kind: 'hir', + fnName: result.value.id, + name: result.name, + value: printFunctionWithOutlined(result.value), + }); + break; + } + case 'reactive': { + upsert({ + kind: 'reactive', + fnName: result.value.id, + name: result.name, + value: printReactiveFunctionWithOutlined(result.value), + }); + break; } + case 'debug': { + upsert({ + kind: 'debug', + fnName: null, + name: result.name, + value: result.value, + }); + break; + } + default: { + const _: never = result; + throw new Error(`Unhandled result ${result}`); + } + } + }; + const parsedOptions = parseConfigPragmaForTests(pragma, { + compilationMode: 'infer', + }); + const opts: PluginOptions = parsePluginOptions({ + ...parsedOptions, + environment: { + ...parsedOptions.environment, + customHooks: new Map([...COMMON_HOOKS]), }, - ); + logger: { + debugLogIRs: logIR, + logEvent: () => {}, + }, + }); + transformOutput = invokeCompiler(source, language, opts); } catch (err) { /** * error might be an invariant violation or other runtime error diff --git a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts index f3f426df56e44..f1db567660ec9 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/HIR/Environment.ts @@ -9,7 +9,13 @@ import * as t from '@babel/types'; import {ZodError, z} from 'zod'; import {fromZodError} from 'zod-validation-error'; import {CompilerError} from '../CompilerError'; -import {Logger} from '../Entrypoint'; +import { + CompilationMode, + Logger, + PanicThresholdOptions, + parsePluginOptions, + PluginOptions, +} from '../Entrypoint'; import {Err, Ok, Result} from '../Utils/Result'; import { DEFAULT_GLOBALS, @@ -683,7 +689,9 @@ const testComplexConfigDefaults: PartialEnvironmentConfig = { /** * For snap test fixtures and playground only. */ -export function parseConfigPragmaForTests(pragma: string): EnvironmentConfig { +function parseConfigPragmaEnvironmentForTest( + pragma: string, +): EnvironmentConfig { const maybeConfig: any = {}; // Get the defaults to programmatically check for boolean properties const defaultConfig = EnvironmentConfigSchema.parse({}); @@ -749,6 +757,48 @@ export function parseConfigPragmaForTests(pragma: string): EnvironmentConfig { suggestions: null, }); } +export function parseConfigPragmaForTests( + pragma: string, + defaults: { + compilationMode: CompilationMode; + }, +): PluginOptions { + const environment = parseConfigPragmaEnvironmentForTest(pragma); + let compilationMode: CompilationMode = defaults.compilationMode; + let panicThreshold: PanicThresholdOptions = 'all_errors'; + for (const token of pragma.split(' ')) { + if (!token.startsWith('@')) { + continue; + } + switch (token) { + case '@compilationMode(annotation)': { + compilationMode = 'annotation'; + break; + } + case '@compilationMode(infer)': { + compilationMode = 'infer'; + break; + } + case '@compilationMode(all)': { + compilationMode = 'all'; + break; + } + case '@compilationMode(syntax)': { + compilationMode = 'syntax'; + break; + } + case '@panicThreshold(none)': { + panicThreshold = 'none'; + break; + } + } + } + return parsePluginOptions({ + environment, + compilationMode, + panicThreshold, + }); +} export type PartialEnvironmentConfig = Partial; diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/parseConfigPragma-test.ts b/compiler/packages/babel-plugin-react-compiler/src/__tests__/parseConfigPragma-test.ts index dc4d5d25a47e2..903afe4c20b9f 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/parseConfigPragma-test.ts +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/parseConfigPragma-test.ts @@ -6,6 +6,7 @@ */ import {parseConfigPragmaForTests, validateEnvironmentConfig} from '..'; +import {defaultOptions} from '../Entrypoint'; describe('parseConfigPragmaForTests()', () => { it('parses flags in various forms', () => { @@ -19,13 +20,18 @@ describe('parseConfigPragmaForTests()', () => { const config = parseConfigPragmaForTests( '@enableUseTypeAnnotations @validateNoSetStateInPassiveEffects:true @validateNoSetStateInRender:false', + {compilationMode: defaultOptions.compilationMode}, ); expect(config).toEqual({ - ...defaultConfig, - enableUseTypeAnnotations: true, - validateNoSetStateInPassiveEffects: true, - validateNoSetStateInRender: false, - enableResetCacheOnSourceFileChanges: false, + ...defaultOptions, + panicThreshold: 'all_errors', + environment: { + ...defaultOptions.environment, + enableUseTypeAnnotations: true, + validateNoSetStateInPassiveEffects: true, + validateNoSetStateInRender: false, + enableResetCacheOnSourceFileChanges: false, + }, }); }); }); diff --git a/compiler/packages/snap/src/compiler.ts b/compiler/packages/snap/src/compiler.ts index a95c61450d840..6e59276c1cbb2 100644 --- a/compiler/packages/snap/src/compiler.ts +++ b/compiler/packages/snap/src/compiler.ts @@ -11,12 +11,9 @@ import {transformFromAstSync} from '@babel/core'; import * as BabelParser from '@babel/parser'; import {NodePath} from '@babel/traverse'; import * as t from '@babel/types'; -import assert from 'assert'; import type { - CompilationMode, Logger, LoggerEvent, - PanicThresholdOptions, PluginOptions, CompilerReactTarget, CompilerPipelineValue, @@ -51,31 +48,13 @@ function makePluginOptions( ValueKindEnum: typeof ValueKind, ): [PluginOptions, Array<{filename: string | null; event: LoggerEvent}>] { let gating = null; - let compilationMode: CompilationMode = 'all'; - let panicThreshold: PanicThresholdOptions = 'all_errors'; let hookPattern: string | null = null; // TODO(@mofeiZ) rewrite snap fixtures to @validatePreserveExistingMemo:false let validatePreserveExistingMemoizationGuarantees = false; let customMacros: null | Array = null; let validateBlocklistedImports = null; - let enableFire = false; let target: CompilerReactTarget = '19'; - if (firstLine.indexOf('@compilationMode(annotation)') !== -1) { - assert( - compilationMode === 'all', - 'Cannot set @compilationMode(..) more than once', - ); - compilationMode = 'annotation'; - } - if (firstLine.indexOf('@compilationMode(infer)') !== -1) { - assert( - compilationMode === 'all', - 'Cannot set @compilationMode(..) more than once', - ); - compilationMode = 'infer'; - } - if (firstLine.includes('@gating')) { gating = { source: 'ReactForgetFeatureFlag', @@ -96,10 +75,6 @@ function makePluginOptions( } } - if (firstLine.includes('@panicThreshold(none)')) { - panicThreshold = 'none'; - } - let eslintSuppressionRules: Array | null = null; const eslintSuppressionMatch = /@eslintSuppressionRules\(([^)]+)\)/.exec( firstLine, @@ -130,10 +105,6 @@ function makePluginOptions( validatePreserveExistingMemoizationGuarantees = true; } - if (firstLine.includes('@enableFire')) { - enableFire = true; - } - const hookPatternMatch = /@hookPattern:"([^"]+)"/.exec(firstLine); if ( hookPatternMatch && @@ -199,10 +170,11 @@ function makePluginOptions( debugLogIRs: debugIRLogger, }; - const config = parseConfigPragmaFn(firstLine); + const config = parseConfigPragmaFn(firstLine, {compilationMode: 'all'}); const options = { + ...config, environment: { - ...config, + ...config.environment, moduleTypeProvider: makeSharedRuntimeTypeProvider({ EffectEnum, ValueKindEnum, @@ -212,12 +184,9 @@ function makePluginOptions( hookPattern, validatePreserveExistingMemoizationGuarantees, validateBlocklistedImports, - enableFire, }, - compilationMode, logger, gating, - panicThreshold, noEmit: false, eslintSuppressionRules, flowSuppressions, From 79dcc47191b9e4bb8b767d7371cefe1d21579f3f Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Thu, 9 Jan 2025 18:00:09 +0000 Subject: [PATCH 06/10] chore[DevTools/TraceUpdates]: display names by default (#32019) Feature was added in https://github.com/facebook/react/pull/31577, lets enable it by default. Note: for gradual rollout with React Native, we will continue to emit different event, requires some changes on React Native side to support this. I have plans to make this feature to be accessible via browser context menu, which has really limited API. In order to minimize potential divergence, lets make this the default state for the feature. --- .../react-devtools-shared/src/backend/agent.js | 12 ------------ .../src/backend/views/TraceUpdates/index.js | 15 ++------------- packages/react-devtools-shared/src/bridge.js | 1 - packages/react-devtools-shared/src/constants.js | 2 -- .../devtools/views/Settings/GeneralSettings.js | 15 --------------- .../devtools/views/Settings/SettingsContext.js | 16 ---------------- 6 files changed, 2 insertions(+), 59 deletions(-) diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js index 111c458bdbda1..e883724f49765 100644 --- a/packages/react-devtools-shared/src/backend/agent.js +++ b/packages/react-devtools-shared/src/backend/agent.js @@ -148,7 +148,6 @@ export default class Agent extends EventEmitter<{ getIfHasUnsupportedRendererVersion: [], updateHookSettings: [$ReadOnly], getHookSettings: [], - showNamesWhenTracing: [boolean], }> { _bridge: BackendBridge; _isProfiling: boolean = false; @@ -159,7 +158,6 @@ export default class Agent extends EventEmitter<{ _onReloadAndProfile: | ((recordChangeDescriptions: boolean, recordTimeline: boolean) => void) | void; - _showNamesWhenTracing: boolean = true; constructor( bridge: BackendBridge, @@ -204,7 +202,6 @@ export default class Agent extends EventEmitter<{ bridge.addListener('reloadAndProfile', this.reloadAndProfile); bridge.addListener('renamePath', this.renamePath); bridge.addListener('setTraceUpdatesEnabled', this.setTraceUpdatesEnabled); - bridge.addListener('setShowNamesWhenTracing', this.setShowNamesWhenTracing); bridge.addListener('startProfiling', this.startProfiling); bridge.addListener('stopProfiling', this.stopProfiling); bridge.addListener('storeAsGlobal', this.storeAsGlobal); @@ -727,7 +724,6 @@ export default class Agent extends EventEmitter<{ this._traceUpdatesEnabled = traceUpdatesEnabled; setTraceUpdatesEnabled(traceUpdatesEnabled); - this.emit('showNamesWhenTracing', this._showNamesWhenTracing); for (const rendererID in this._rendererInterfaces) { const renderer = ((this._rendererInterfaces[ @@ -737,14 +733,6 @@ export default class Agent extends EventEmitter<{ } }; - setShowNamesWhenTracing: (show: boolean) => void = show => { - if (this._showNamesWhenTracing === show) { - return; - } - this._showNamesWhenTracing = show; - this.emit('showNamesWhenTracing', show); - }; - syncSelectionFromBuiltinElementsPanel: () => void = () => { const target = window.__REACT_DEVTOOLS_GLOBAL_HOOK__.$0; if (target == null) { diff --git a/packages/react-devtools-shared/src/backend/views/TraceUpdates/index.js b/packages/react-devtools-shared/src/backend/views/TraceUpdates/index.js index 48e504eddbb1d..5b274e8a2f606 100644 --- a/packages/react-devtools-shared/src/backend/views/TraceUpdates/index.js +++ b/packages/react-devtools-shared/src/backend/views/TraceUpdates/index.js @@ -50,20 +50,11 @@ const nodeToData: Map = new Map(); let agent: Agent = ((null: any): Agent); let drawAnimationFrameID: AnimationFrameID | null = null; let isEnabled: boolean = false; -let showNames: boolean = false; let redrawTimeoutID: TimeoutID | null = null; export function initialize(injectedAgent: Agent): void { agent = injectedAgent; agent.addListener('traceUpdates', traceUpdates); - agent.addListener('showNamesWhenTracing', (shouldShowNames: boolean) => { - showNames = shouldShowNames; - if (isEnabled) { - if (drawAnimationFrameID === null) { - drawAnimationFrameID = requestAnimationFrame(prepareToDraw); - } - } - }); } export function toggleEnabled(value: boolean): void { @@ -101,9 +92,7 @@ function traceUpdates(nodes: Set): void { rect = measureNode(node); } - let displayName = showNames - ? agent.getComponentNameForHostInstance(node) - : null; + let displayName = agent.getComponentNameForHostInstance(node); if (displayName) { const {baseComponentName, hocNames} = extractHOCNames(displayName); @@ -127,7 +116,7 @@ function traceUpdates(nodes: Set): void { : now + DISPLAY_DURATION, lastMeasuredAt, rect, - displayName: showNames ? displayName : null, + displayName, }); }); diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index e00ba5518a1a9..cb494e1b3c1ba 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -234,7 +234,6 @@ type FrontendEvents = { renamePath: [RenamePath], savedPreferences: [SavedPreferencesParams], setTraceUpdatesEnabled: [boolean], - setShowNamesWhenTracing: [boolean], shutdown: [], startInspectingHost: [], startProfiling: [StartProfilingParams], diff --git a/packages/react-devtools-shared/src/constants.js b/packages/react-devtools-shared/src/constants.js index bb8d9f5b130f9..b08738165906c 100644 --- a/packages/react-devtools-shared/src/constants.js +++ b/packages/react-devtools-shared/src/constants.js @@ -50,8 +50,6 @@ export const LOCAL_STORAGE_TRACE_UPDATES_ENABLED_KEY = 'React::DevTools::traceUpdatesEnabled'; export const LOCAL_STORAGE_SUPPORTS_PROFILING_KEY = 'React::DevTools::supportsProfiling'; -export const LOCAL_STORAGE_SHOW_NAMES_WHEN_TRACING_KEY = - 'React::DevTools::showNamesWhenTracing'; export const PROFILER_EXPORT_VERSION = 5; diff --git a/packages/react-devtools-shared/src/devtools/views/Settings/GeneralSettings.js b/packages/react-devtools-shared/src/devtools/views/Settings/GeneralSettings.js index 69f4ec737581a..d56f32f81142c 100644 --- a/packages/react-devtools-shared/src/devtools/views/Settings/GeneralSettings.js +++ b/packages/react-devtools-shared/src/devtools/views/Settings/GeneralSettings.js @@ -34,10 +34,8 @@ export default function GeneralSettings(_: {}): React.Node { setDisplayDensity, setTheme, setTraceUpdatesEnabled, - setShowNamesWhenTracing, theme, traceUpdatesEnabled, - showNamesWhenTracing, } = useContext(SettingsContext); const {backendVersion, supportsTraceUpdates} = useContext(StoreContext); @@ -85,19 +83,6 @@ export default function GeneralSettings(_: {}): React.Node { />{' '} Highlight updates when components render. -
- -
)} diff --git a/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js b/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js index ec7b3ba9c9da4..196ea806f6aac 100644 --- a/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Settings/SettingsContext.js @@ -21,7 +21,6 @@ import { LOCAL_STORAGE_BROWSER_THEME, LOCAL_STORAGE_PARSE_HOOK_NAMES_KEY, LOCAL_STORAGE_TRACE_UPDATES_ENABLED_KEY, - LOCAL_STORAGE_SHOW_NAMES_WHEN_TRACING_KEY, } from 'react-devtools-shared/src/constants'; import { COMFORTABLE_LINE_HEIGHT, @@ -54,9 +53,6 @@ type Context = { traceUpdatesEnabled: boolean, setTraceUpdatesEnabled: (value: boolean) => void, - - showNamesWhenTracing: boolean, - setShowNamesWhenTracing: (showNames: boolean) => void, }; const SettingsContext: ReactContext = createContext( @@ -115,11 +111,6 @@ function SettingsContextController({ LOCAL_STORAGE_TRACE_UPDATES_ENABLED_KEY, false, ); - const [showNamesWhenTracing, setShowNamesWhenTracing] = - useLocalStorageWithLog( - LOCAL_STORAGE_SHOW_NAMES_WHEN_TRACING_KEY, - true, - ); const documentElements = useMemo(() => { const array: Array = [ @@ -173,10 +164,6 @@ function SettingsContextController({ bridge.send('setTraceUpdatesEnabled', traceUpdatesEnabled); }, [bridge, traceUpdatesEnabled]); - useEffect(() => { - bridge.send('setShowNamesWhenTracing', showNamesWhenTracing); - }, [bridge, showNamesWhenTracing]); - const value: Context = useMemo( () => ({ displayDensity, @@ -192,8 +179,6 @@ function SettingsContextController({ theme, browserTheme, traceUpdatesEnabled, - showNamesWhenTracing, - setShowNamesWhenTracing, }), [ displayDensity, @@ -205,7 +190,6 @@ function SettingsContextController({ theme, browserTheme, traceUpdatesEnabled, - showNamesWhenTracing, ], ); From d5f3c50f584ab0212e472fc4f4d599194a0286a7 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Thu, 9 Jan 2025 18:00:30 +0000 Subject: [PATCH 07/10] chore[DevTools/Tree]: don't pre-select root element and remove unused code (#32015) In this PR: 1. Removed unused code in `Tree.js` 2. Removed logic for pre-selecting first element in the tree by default. This is a bit clowny, because it steals focus and resets scroll, when user attempts to expand / collapse some subtree. 3. Updated comments around https://github.com/facebook/react/commit/1c381c588aed1ed6814f1be04fbe42cd069ce174. To expand on 3-rd point, for someone who might be reading this in the future: We can't guarantee focus of RDT browser extension panels, because they are hosted in an `iframe`. Attempting to fire any events won't have any result, user action with the corresponding `iframe` is required in order for this `iframe` to obtain focus. The only reason why built-in Elements panel in Chrome works correctly is because it is supported natively somewhere in Chrome / Chrome DevTools. Also, when you select an element on the application page, Chrome will make sure that Elements panel opened, which technically guarantees focus inside DevTools window and Elements panel subview. As of today, we can't navigate user to third-party extensions panels, there is no API for this, hence no ability to guarantee focused RDT panels. --- .../src/devtools/views/Components/Tree.js | 35 +++---------------- 1 file changed, 5 insertions(+), 30 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Tree.js b/packages/react-devtools-shared/src/devtools/views/Components/Tree.js index db1bf9a98c135..4a69ddbd99214 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Tree.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/Tree.js @@ -42,16 +42,12 @@ import {logEvent} from 'react-devtools-shared/src/Logger'; const DEFAULT_INDENTATION_SIZE = 12; export type ItemData = { - numElements: number, isNavigatingWithKeyboard: boolean, - lastScrolledIDRef: {current: number | null, ...}, onElementMouseEnter: (id: number) => void, treeFocused: boolean, }; -type Props = {}; - -export default function Tree(props: Props): React.Node { +export default function Tree(): React.Node { const dispatch = useContext(TreeDispatcherContext); const { numElements, @@ -96,7 +92,8 @@ export default function Tree(props: Props): React.Node { ); // Picking an element in the inspector should put focus into the tree. - // This ensures that keyboard navigation works right after picking a node. + // If possible, navigation works right after picking a node. + // NOTE: This is not guaranteed to work, because browser extension panels are hosted inside an iframe. useEffect(() => { function handleStopInspectingHost(didSelectNode: boolean) { if (didSelectNode && focusTargetRef.current !== null) { @@ -112,11 +109,6 @@ export default function Tree(props: Props): React.Node { bridge.removeListener('stopInspectingHost', handleStopInspectingHost); }, [bridge]); - // This ref is passed down the context to elements. - // It lets them avoid autoscrolling to the same item many times - // when a selected virtual row goes in and out of the viewport. - const lastScrolledIDRef = useRef(null); - // Navigate the tree with up/down arrow keys. useEffect(() => { if (treeRef.current === null) { @@ -214,16 +206,7 @@ export default function Tree(props: Props): React.Node { // Focus management. const handleBlur = useCallback(() => setTreeFocused(false), []); - const handleFocus = useCallback(() => { - setTreeFocused(true); - - if (selectedElementIndex === null && numElements > 0) { - dispatch({ - type: 'SELECT_ELEMENT_AT_INDEX', - payload: 0, - }); - } - }, [dispatch, numElements, selectedElementIndex]); + const handleFocus = useCallback(() => setTreeFocused(true), []); const handleKeyPress = useCallback( (event: $FlowFixMe) => { @@ -294,19 +277,11 @@ export default function Tree(props: Props): React.Node { // This includes the owner context, since it controls a filtered view of the tree. const itemData = useMemo( () => ({ - numElements, isNavigatingWithKeyboard, onElementMouseEnter: handleElementMouseEnter, - lastScrolledIDRef, treeFocused, }), - [ - numElements, - isNavigatingWithKeyboard, - handleElementMouseEnter, - lastScrolledIDRef, - treeFocused, - ], + [isNavigatingWithKeyboard, handleElementMouseEnter, treeFocused], ); const itemKey = useCallback( From 54cfa95d3a83328868f9fba00d8213e6de6c7d2f Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Thu, 9 Jan 2025 18:01:07 +0000 Subject: [PATCH 08/10] DevTools: fix initial host instance selection (#31892) Related: https://github.com/facebook/react/pull/31342 This fixes RDT behaviour when some DOM element was pre-selected in built-in browser's Elements panel, and then Components panel of React DevTools was opened for the first time. With this change, React DevTools will correctly display the initial state of the Components Tree with the corresponding React Element (if possible) pre-selected. Previously, we would only subscribe listener when `TreeContext` is mounted, but this only happens when user opens one of React DevTools panels for the first time. With this change, we keep state inside `Store`, which is created when Browser DevTools are opened. Later, `TreeContext` will use it for initial state value. Planned next changes: 1. Merge `inspectedElementID` and `selectedElementID`, I have no idea why we need both. 2. Fix issue with `AutoSizer` rendering a blank container. --- .../src/main/index.js | 2 -- .../src/devtools/store.js | 20 ++++++++++++++ .../devtools/views/Components/TreeContext.js | 26 ++++++++++++------- 3 files changed, 37 insertions(+), 11 deletions(-) diff --git a/packages/react-devtools-extensions/src/main/index.js b/packages/react-devtools-extensions/src/main/index.js index a2758567138c8..49930f9c7ddcf 100644 --- a/packages/react-devtools-extensions/src/main/index.js +++ b/packages/react-devtools-extensions/src/main/index.js @@ -345,8 +345,6 @@ function mountReactDevTools() { createBridgeAndStore(); - setReactSelectionFromBrowser(bridge); - createComponentsPanel(); createProfilerPanel(); } diff --git a/packages/react-devtools-shared/src/devtools/store.js b/packages/react-devtools-shared/src/devtools/store.js index 9f4343beabf43..0a3fbe82bb6a0 100644 --- a/packages/react-devtools-shared/src/devtools/store.js +++ b/packages/react-devtools-shared/src/devtools/store.js @@ -96,6 +96,7 @@ export default class Store extends EventEmitter<{ componentFilters: [], error: [Error], hookSettings: [$ReadOnly], + hostInstanceSelected: [Element['id']], settingsUpdated: [$ReadOnly], mutated: [[Array, Map]], recordChangeDescriptions: [], @@ -190,6 +191,9 @@ export default class Store extends EventEmitter<{ _hookSettings: $ReadOnly | null = null; _shouldShowWarningsAndErrors: boolean = false; + // Only used in browser extension for synchronization with built-in Elements panel. + _lastSelectedHostInstanceElementId: Element['id'] | null = null; + constructor(bridge: FrontendBridge, config?: Config) { super(); @@ -265,6 +269,7 @@ export default class Store extends EventEmitter<{ bridge.addListener('saveToClipboard', this.onSaveToClipboard); bridge.addListener('hookSettings', this.onHookSettings); bridge.addListener('backendInitialized', this.onBackendInitialized); + bridge.addListener('selectElement', this.onHostInstanceSelected); } // This is only used in tests to avoid memory leaks. @@ -481,6 +486,10 @@ export default class Store extends EventEmitter<{ return this._unsupportedRendererVersionDetected; } + get lastSelectedHostInstanceElementId(): Element['id'] | null { + return this._lastSelectedHostInstanceElementId; + } + containsElement(id: number): boolean { return this._idToElement.has(id); } @@ -1431,6 +1440,7 @@ export default class Store extends EventEmitter<{ bridge.removeListener('backendVersion', this.onBridgeBackendVersion); bridge.removeListener('bridgeProtocol', this.onBridgeProtocol); bridge.removeListener('saveToClipboard', this.onSaveToClipboard); + bridge.removeListener('selectElement', this.onHostInstanceSelected); if (this._onBridgeProtocolTimeoutID !== null) { clearTimeout(this._onBridgeProtocolTimeoutID); @@ -1507,6 +1517,16 @@ export default class Store extends EventEmitter<{ this._bridge.send('getHookSettings'); // Warm up cached hook settings }; + onHostInstanceSelected: (elementId: number) => void = elementId => { + if (this._lastSelectedHostInstanceElementId === elementId) { + return; + } + + this._lastSelectedHostInstanceElementId = elementId; + // By the time we emit this, there is no guarantee that TreeContext is rendered. + this.emit('hostInstanceSelected', elementId); + }; + getHookSettings: () => void = () => { if (this._hookSettings != null) { this.emit('hookSettings', this._hookSettings); diff --git a/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js b/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js index 041d122c57d34..1d2ed380b9a1f 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js @@ -39,7 +39,7 @@ import { startTransition, } from 'react'; import {createRegExp} from '../utils'; -import {BridgeContext, StoreContext} from '../context'; +import {StoreContext} from '../context'; import Store from '../../store'; import type {Element} from 'react-devtools-shared/src/frontend/types'; @@ -836,7 +836,6 @@ function TreeContextController({ defaultSelectedElementID, defaultSelectedElementIndex, }: Props): React.Node { - const bridge = useContext(BridgeContext); const store = useContext(StoreContext); const initialRevision = useMemo(() => store.revision, [store]); @@ -899,9 +898,15 @@ function TreeContextController({ numElements: store.numElements, ownerSubtreeLeafElementID: null, selectedElementID: - defaultSelectedElementID == null ? null : defaultSelectedElementID, + defaultSelectedElementID != null + ? defaultSelectedElementID + : store.lastSelectedHostInstanceElementId, selectedElementIndex: - defaultSelectedElementIndex == null ? null : defaultSelectedElementIndex, + defaultSelectedElementIndex != null + ? defaultSelectedElementIndex + : store.lastSelectedHostInstanceElementId + ? store.getIndexOfElementID(store.lastSelectedHostInstanceElementId) + : null, // Search searchIndex: null, @@ -914,7 +919,9 @@ function TreeContextController({ // Inspection element panel inspectedElementID: - defaultInspectedElementID == null ? null : defaultInspectedElementID, + defaultInspectedElementID != null + ? defaultInspectedElementID + : store.lastSelectedHostInstanceElementId, }); const dispatchWrapper = useCallback( @@ -929,11 +936,12 @@ function TreeContextController({ // Listen for host element selections. useEffect(() => { - const handleSelectElement = (id: number) => + const handler = (id: Element['id']) => dispatchWrapper({type: 'SELECT_ELEMENT_BY_ID', payload: id}); - bridge.addListener('selectElement', handleSelectElement); - return () => bridge.removeListener('selectElement', handleSelectElement); - }, [bridge, dispatchWrapper]); + + store.addListener('hostInstanceSelected', handler); + return () => store.removeListener('hostInstanceSelected', handler); + }, [store, dispatchWrapper]); // If a newly-selected search result or inspection selection is inside of a collapsed subtree, auto expand it. // This needs to be a layout effect to avoid temporarily flashing an incorrect selection. From d6345482430952306fc83e62d4c14e2622fb1752 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Thu, 9 Jan 2025 18:13:24 +0000 Subject: [PATCH 09/10] DevTools: merge element fields in TreeStateContext (#31956) Stacked on https://github.com/facebook/react/pull/31892, see commit on top. For some reason, there were 2 fields different fields for essentially same thing: `selectedElementID` and `inspectedElementID`. Basically, the change is: ``` selectedElementID -> inspectedElementID selectedElementIndex -> inspectedElementIndex ``` I have a theory that it was due to previously used async approach around element inspection, and the whole `InspectedElementView` was wrapped in `Suspense`. --- .../__tests__/__e2e__/components.test.js | 20 +- .../__tests__/__e2e__/devtools-utils.js | 21 +- .../src/__tests__/inspectedElement-test.js | 25 +- .../src/__tests__/profilerContext-test.js | 61 ++-- .../src/devtools/utils.js | 2 +- .../src/devtools/views/Components/Element.js | 4 +- .../Components/InspectedElementSourcePanel.js | 10 +- .../Components/NativeStyleEditor/context.js | 18 +- .../views/Components/SelectedTreeHighlight.js | 10 +- .../src/devtools/views/Components/Tree.js | 32 +- .../devtools/views/Components/TreeContext.js | 334 ++++++++---------- .../views/Profiler/ProfilerContext.js | 6 +- 12 files changed, 265 insertions(+), 278 deletions(-) diff --git a/packages/react-devtools-inline/__tests__/__e2e__/components.test.js b/packages/react-devtools-inline/__tests__/__e2e__/components.test.js index b92fcb57e6d5d..71c2e603ccb27 100644 --- a/packages/react-devtools-inline/__tests__/__e2e__/components.test.js +++ b/packages/react-devtools-inline/__tests__/__e2e__/components.test.js @@ -119,7 +119,7 @@ test.describe('Components', () => { runOnlyForReactRange('>=16.8'); // Select the first list item in DevTools. - await devToolsUtils.selectElement(page, 'ListItem', 'List\nApp'); + await devToolsUtils.selectElement(page, 'ListItem', 'List\nApp', true); // Then read the inspected values. const sourceText = await page.evaluate(() => { @@ -127,7 +127,7 @@ test.describe('Components', () => { const container = document.getElementById('devtools'); const source = findAllNodes(container, [ - createTestNameSelector('InspectedElementView-Source'), + createTestNameSelector('InspectedElementView-FormattedSourceString'), ])[0]; return source.innerText; @@ -237,35 +237,35 @@ test.describe('Components', () => { } await focusComponentSearch(); - page.keyboard.insertText('List'); + await page.keyboard.insertText('List'); let count = await getComponentSearchResultsCount(); expect(count).toBe('1 | 4'); - page.keyboard.insertText('Item'); + await page.keyboard.insertText('Item'); count = await getComponentSearchResultsCount(); expect(count).toBe('1 | 3'); - page.keyboard.press('Enter'); + await page.keyboard.press('Enter'); count = await getComponentSearchResultsCount(); expect(count).toBe('2 | 3'); - page.keyboard.press('Enter'); + await page.keyboard.press('Enter'); count = await getComponentSearchResultsCount(); expect(count).toBe('3 | 3'); - page.keyboard.press('Enter'); + await page.keyboard.press('Enter'); count = await getComponentSearchResultsCount(); expect(count).toBe('1 | 3'); - page.keyboard.press('Shift+Enter'); + await page.keyboard.press('Shift+Enter'); count = await getComponentSearchResultsCount(); expect(count).toBe('3 | 3'); - page.keyboard.press('Shift+Enter'); + await page.keyboard.press('Shift+Enter'); count = await getComponentSearchResultsCount(); expect(count).toBe('2 | 3'); - page.keyboard.press('Shift+Enter'); + await page.keyboard.press('Shift+Enter'); count = await getComponentSearchResultsCount(); expect(count).toBe('1 | 3'); }); diff --git a/packages/react-devtools-inline/__tests__/__e2e__/devtools-utils.js b/packages/react-devtools-inline/__tests__/__e2e__/devtools-utils.js index 333b47309a3c9..fe2bb3f6f222e 100644 --- a/packages/react-devtools-inline/__tests__/__e2e__/devtools-utils.js +++ b/packages/react-devtools-inline/__tests__/__e2e__/devtools-utils.js @@ -27,7 +27,12 @@ async function getElementCount(page, displayName) { }, displayName); } -async function selectElement(page, displayName, waitForOwnersText) { +async function selectElement( + page, + displayName, + waitForOwnersText, + waitForSourceLoaded = false +) { await page.evaluate(listItemText => { const {createTestNameSelector, createTextSelector, findAllNodes} = window.REACT_DOM_DEVTOOLS; @@ -69,6 +74,20 @@ async function selectElement(page, displayName, waitForOwnersText) { {titleText: displayName, ownersListText: waitForOwnersText} ); } + + if (waitForSourceLoaded) { + await page.waitForFunction(() => { + const {createTestNameSelector, findAllNodes} = window.REACT_DOM_DEVTOOLS; + const container = document.getElementById('devtools'); + + const sourceStringBlock = findAllNodes(container, [ + createTestNameSelector('InspectedElementView-FormattedSourceString'), + ])[0]; + + // Wait for a new source line to be fetched + return sourceStringBlock != null && sourceStringBlock.innerText != null; + }); + } } module.exports = { diff --git a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js index 65c7e12bccf34..3374224cca0fc 100644 --- a/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js +++ b/packages/react-devtools-shared/src/__tests__/inspectedElement-test.js @@ -115,16 +115,15 @@ describe('InspectedElement', () => { const Contexts = ({ children, - defaultSelectedElementID = null, - defaultSelectedElementIndex = null, + defaultInspectedElementID = null, + defaultInspectedElementIndex = null, }) => ( + defaultInspectedElementID={defaultInspectedElementID} + defaultInspectedElementIndex={defaultInspectedElementIndex}> {children} @@ -167,8 +166,8 @@ describe('InspectedElement', () => { testRendererInstance.update( + defaultInspectedElementID={id} + defaultInspectedElementIndex={index}> @@ -355,7 +354,7 @@ describe('InspectedElement', () => { const {index, shouldHaveLegacyContext} = cases[i]; // HACK: Recreate TestRenderer instance because we rely on default state values - // from props like defaultSelectedElementID and it's easier to reset here than + // from props like defaultInspectedElementID and it's easier to reset here than // to read the TreeDispatcherContext and update the selected ID that way. // We're testing the inspected values here, not the context wiring, so that's ok. withErrorsOrWarningsIgnored( @@ -2069,7 +2068,7 @@ describe('InspectedElement', () => { }, false); // HACK: Recreate TestRenderer instance because we rely on default state values - // from props like defaultSelectedElementID and it's easier to reset here than + // from props like defaultInspectedElementID and it's easier to reset here than // to read the TreeDispatcherContext and update the selected ID that way. // We're testing the inspected values here, not the context wiring, so that's ok. withErrorsOrWarningsIgnored( @@ -2129,7 +2128,7 @@ describe('InspectedElement', () => { }, false); // HACK: Recreate TestRenderer instance because we rely on default state values - // from props like defaultSelectedElementID and it's easier to reset here than + // from props like defaultInspectedElementID and it's easier to reset here than // to read the TreeDispatcherContext and update the selected ID that way. // We're testing the inspected values here, not the context wiring, so that's ok. withErrorsOrWarningsIgnored( @@ -2408,8 +2407,8 @@ describe('InspectedElement', () => { await utils.actAsync(() => { root = TestRenderer.create( + defaultInspectedElementID={id} + defaultInspectedElementIndex={index}> @@ -3101,6 +3100,7 @@ describe('InspectedElement', () => { await utils.actAsync(() => { store.componentFilters = [utils.createDisplayNameFilter('Wrapper')]; + jest.runOnlyPendingTimers(); }, false); expect(state).toMatchInlineSnapshot(` @@ -3120,6 +3120,7 @@ describe('InspectedElement', () => { await utils.actAsync(() => { store.componentFilters = []; + jest.runOnlyPendingTimers(); }, false); expect(state).toMatchInlineSnapshot(` ✕ 0, ⚠ 2 diff --git a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js index 17f758b612216..9a9bb14c0ba0c 100644 --- a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js @@ -69,14 +69,14 @@ describe('ProfilerContext', () => { const Contexts = ({ children = null, - defaultSelectedElementID = null, - defaultSelectedElementIndex = null, + defaultInspectedElementID = null, + defaultInspectedElementIndex = null, }: any) => ( + defaultInspectedElementID={defaultInspectedElementID} + defaultInspectedElementIndex={defaultInspectedElementIndex}> {children} @@ -225,8 +225,8 @@ describe('ProfilerContext', () => { await utils.actAsync(() => TestRenderer.create( + defaultInspectedElementID={store.getElementIDAtIndex(3)} + defaultInspectedElementIndex={3}> , ), @@ -276,8 +276,8 @@ describe('ProfilerContext', () => { await utils.actAsync(() => TestRenderer.create( + defaultInspectedElementID={store.getElementIDAtIndex(3)} + defaultInspectedElementIndex={3}> , ), @@ -323,8 +323,8 @@ describe('ProfilerContext', () => { await utils.actAsync(() => TestRenderer.create( + defaultInspectedElementID={store.getElementIDAtIndex(3)} + defaultInspectedElementIndex={3}> , ), @@ -374,8 +374,8 @@ describe('ProfilerContext', () => { await utils.actAsync(() => TestRenderer.create( + defaultInspectedElementID={store.getElementIDAtIndex(3)} + defaultInspectedElementIndex={3}> , ), @@ -415,11 +415,12 @@ describe('ProfilerContext', () => { let context: Context = ((null: any): Context); let dispatch: DispatcherContext = ((null: any): DispatcherContext); - let selectedElementID = null; + let inspectedElementID = null; function ContextReader() { context = React.useContext(ProfilerContext); dispatch = React.useContext(TreeDispatcherContext); - selectedElementID = React.useContext(TreeStateContext).selectedElementID; + inspectedElementID = + React.useContext(TreeStateContext).inspectedElementID; return null; } @@ -428,13 +429,15 @@ describe('ProfilerContext', () => { // Select an element within the second root. await utils.actAsync(() => TestRenderer.create( - + , ), ); - expect(selectedElementID).toBe(id); + expect(inspectedElementID).toBe(id); // Profile and record more updates to both roots await utils.actAsync(() => store.profilerStore.startProfiling()); @@ -448,7 +451,7 @@ describe('ProfilerContext', () => { utils.act(() => dispatch({type: 'SELECT_ELEMENT_AT_INDEX', payload: 0})); // Verify that the initial Profiler root selection is maintained. - expect(selectedElementID).toBe(otherID); + expect(inspectedElementID).toBe(otherID); expect(context).not.toBeNull(); expect(context.rootID).toBe(store.getRootIDForElement(id)); }); @@ -484,11 +487,12 @@ describe('ProfilerContext', () => { let context: Context = ((null: any): Context); let dispatch: DispatcherContext = ((null: any): DispatcherContext); - let selectedElementID = null; + let inspectedElementID = null; function ContextReader() { context = React.useContext(ProfilerContext); dispatch = React.useContext(TreeDispatcherContext); - selectedElementID = React.useContext(TreeStateContext).selectedElementID; + inspectedElementID = + React.useContext(TreeStateContext).inspectedElementID; return null; } @@ -497,13 +501,15 @@ describe('ProfilerContext', () => { // Select an element within the second root. await utils.actAsync(() => TestRenderer.create( - + , ), ); - expect(selectedElementID).toBe(id); + expect(inspectedElementID).toBe(id); // Profile and record more updates to both roots await utils.actAsync(() => store.profilerStore.startProfiling()); @@ -517,7 +523,7 @@ describe('ProfilerContext', () => { utils.act(() => dispatch({type: 'SELECT_ELEMENT_AT_INDEX', payload: 0})); // Verify that the initial Profiler root selection is maintained. - expect(selectedElementID).toBe(otherID); + expect(inspectedElementID).toBe(otherID); expect(context).not.toBeNull(); expect(context.rootID).toBe(store.getRootIDForElement(id)); }); @@ -553,10 +559,11 @@ describe('ProfilerContext', () => { `); let context: Context = ((null: any): Context); - let selectedElementID = null; + let inspectedElementID = null; function ContextReader() { context = React.useContext(ProfilerContext); - selectedElementID = React.useContext(TreeStateContext).selectedElementID; + inspectedElementID = + React.useContext(TreeStateContext).inspectedElementID; return null; } @@ -567,14 +574,14 @@ describe('ProfilerContext', () => { , ), ); - expect(selectedElementID).toBeNull(); + expect(inspectedElementID).toBeNull(); // Select an element in the Profiler tab and verify that the selection is synced to the Components tab. await utils.actAsync(() => context.selectFiber(parentID, 'Parent')); - expect(selectedElementID).toBe(parentID); + expect(inspectedElementID).toBe(parentID); // Select an unmounted element and verify no Components tab selection doesn't change. await utils.actAsync(() => context.selectFiber(childID, 'Child')); - expect(selectedElementID).toBe(parentID); + expect(inspectedElementID).toBe(parentID); }); }); diff --git a/packages/react-devtools-shared/src/devtools/utils.js b/packages/react-devtools-shared/src/devtools/utils.js index 7de23d50c9e0a..b6814f73f8d0f 100644 --- a/packages/react-devtools-shared/src/devtools/utils.js +++ b/packages/react-devtools-shared/src/devtools/utils.js @@ -67,7 +67,7 @@ export function printStore( if (state === null) { return ''; } - return state.selectedElementIndex === index ? `→` : ' '; + return state.inspectedElementIndex === index ? `→` : ' '; } function printErrorsAndWarnings(element: Element): string { diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Element.js b/packages/react-devtools-shared/src/devtools/views/Components/Element.js index b58451cb51d23..7854f4c99b3ae 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Element.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/Element.js @@ -33,7 +33,7 @@ type Props = { export default function Element({data, index, style}: Props): React.Node { const store = useContext(StoreContext); - const {ownerFlatTree, ownerID, selectedElementID} = + const {ownerFlatTree, ownerID, inspectedElementID} = useContext(TreeStateContext); const dispatch = useContext(TreeDispatcherContext); @@ -46,7 +46,7 @@ export default function Element({data, index, style}: Props): React.Node { const {isNavigatingWithKeyboard, onElementMouseEnter, treeFocused} = data; const id = element === null ? null : element.id; - const isSelected = selectedElementID === id; + const isSelected = inspectedElementID === id; const errorsAndWarningsSubscription = useMemo( () => ({ diff --git a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSourcePanel.js b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSourcePanel.js index a5f0c7c869550..16ca5d1bfe589 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSourcePanel.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/InspectedElementSourcePanel.js @@ -28,7 +28,7 @@ function InspectedElementSourcePanel({ symbolicatedSourcePromise, }: Props): React.Node { return ( -
+
source
@@ -84,7 +84,9 @@ function FormattedSourceString({source, symbolicatedSourcePromise}: Props) { const {sourceURL, line} = source; return ( -
+
{formatSourceForDisplay(sourceURL, line)}
); @@ -93,7 +95,9 @@ function FormattedSourceString({source, symbolicatedSourcePromise}: Props) { const {sourceURL, line} = symbolicatedSource; return ( -
+
{formatSourceForDisplay(sourceURL, line)}
); diff --git a/packages/react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/context.js b/packages/react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/context.js index d037ca56add54..7170864ab35fe 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/context.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/NativeStyleEditor/context.js @@ -100,10 +100,10 @@ function NativeStyleContextController({children}: Props): React.Node { [store], ); - // It's very important that this context consumes selectedElementID and not NativeStyleID. + // It's very important that this context consumes inspectedElementID and not NativeStyleID. // Otherwise the effect that sends the "inspect" message across the bridge- // would itself be blocked by the same render that suspends (waiting for the data). - const {selectedElementID} = useContext(TreeStateContext); + const {inspectedElementID} = useContext(TreeStateContext); const [currentStyleAndLayout, setCurrentStyleAndLayout] = useState(null); @@ -128,7 +128,7 @@ function NativeStyleContextController({children}: Props): React.Node { resource.write(element, styleAndLayout); // Schedule update with React if the currently-selected element has been invalidated. - if (id === selectedElementID) { + if (id === inspectedElementID) { setCurrentStyleAndLayout(styleAndLayout); } } @@ -141,15 +141,15 @@ function NativeStyleContextController({children}: Props): React.Node { 'NativeStyleEditor_styleAndLayout', onStyleAndLayout, ); - }, [bridge, currentStyleAndLayout, selectedElementID, store]); + }, [bridge, currentStyleAndLayout, inspectedElementID, store]); // This effect handler polls for updates on the currently selected element. useEffect(() => { - if (selectedElementID === null) { + if (inspectedElementID === null) { return () => {}; } - const rendererID = store.getRendererIDForElement(selectedElementID); + const rendererID = store.getRendererIDForElement(inspectedElementID); let timeoutID: TimeoutID | null = null; @@ -158,7 +158,7 @@ function NativeStyleContextController({children}: Props): React.Node { if (rendererID !== null) { bridge.send('NativeStyleEditor_measure', { - id: selectedElementID, + id: inspectedElementID, rendererID, }); } @@ -170,7 +170,7 @@ function NativeStyleContextController({children}: Props): React.Node { const onStyleAndLayout = ({id}: StyleAndLayoutBackend) => { // If this is the element we requested, wait a little bit and then ask for another update. - if (id === selectedElementID) { + if (id === inspectedElementID) { if (timeoutID !== null) { clearTimeout(timeoutID); } @@ -190,7 +190,7 @@ function NativeStyleContextController({children}: Props): React.Node { clearTimeout(timeoutID); } }; - }, [bridge, selectedElementID, store]); + }, [bridge, inspectedElementID, store]); const value = useMemo( () => ({getStyleAndLayout}), diff --git a/packages/react-devtools-shared/src/devtools/views/Components/SelectedTreeHighlight.js b/packages/react-devtools-shared/src/devtools/views/Components/SelectedTreeHighlight.js index 44183f6575234..16035a13d65f9 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/SelectedTreeHighlight.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/SelectedTreeHighlight.js @@ -28,19 +28,19 @@ export default function SelectedTreeHighlight(_: {}): React.Node { const {lineHeight} = useContext(SettingsContext); const store = useContext(StoreContext); const treeFocused = useContext(TreeFocusedContext); - const {ownerID, selectedElementID} = useContext(TreeStateContext); + const {ownerID, inspectedElementID} = useContext(TreeStateContext); const subscription = useMemo( () => ({ getCurrentValue: () => { if ( - selectedElementID === null || - store.isInsideCollapsedSubTree(selectedElementID) + inspectedElementID === null || + store.isInsideCollapsedSubTree(inspectedElementID) ) { return null; } - const element = store.getElementByID(selectedElementID); + const element = store.getElementByID(inspectedElementID); if ( element === null || element.isCollapsed || @@ -83,7 +83,7 @@ export default function SelectedTreeHighlight(_: {}): React.Node { }; }, }), - [selectedElementID, store], + [inspectedElementID, store], ); const data = useSubscription(subscription); diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Tree.js b/packages/react-devtools-shared/src/devtools/views/Components/Tree.js index 4a69ddbd99214..c0fd842abc4eb 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Tree.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/Tree.js @@ -54,8 +54,8 @@ export default function Tree(): React.Node { ownerID, searchIndex, searchResults, - selectedElementID, - selectedElementIndex, + inspectedElementID, + inspectedElementIndex, } = useContext(TreeStateContext); const bridge = useContext(BridgeContext); const store = useContext(StoreContext); @@ -84,11 +84,11 @@ export default function Tree(): React.Node { // Using a callback ref accounts for this case... const listCallbackRef = useCallback( (list: $FlowFixMe) => { - if (list != null && selectedElementIndex !== null) { - list.scrollToItem(selectedElementIndex, 'smart'); + if (list != null && inspectedElementIndex !== null) { + list.scrollToItem(inspectedElementIndex, 'smart'); } }, - [selectedElementIndex], + [inspectedElementIndex], ); // Picking an element in the inspector should put focus into the tree. @@ -133,8 +133,8 @@ export default function Tree(): React.Node { case 'ArrowLeft': event.preventDefault(); element = - selectedElementID !== null - ? store.getElementByID(selectedElementID) + inspectedElementID !== null + ? store.getElementByID(inspectedElementID) : null; if (element !== null) { if (event.altKey) { @@ -153,8 +153,8 @@ export default function Tree(): React.Node { case 'ArrowRight': event.preventDefault(); element = - selectedElementID !== null - ? store.getElementByID(selectedElementID) + inspectedElementID !== null + ? store.getElementByID(inspectedElementID) : null; if (element !== null) { if (event.altKey) { @@ -202,7 +202,7 @@ export default function Tree(): React.Node { return () => { container.removeEventListener('keydown', handleKeyDown); }; - }, [dispatch, selectedElementID, store]); + }, [dispatch, inspectedElementID, store]); // Focus management. const handleBlur = useCallback(() => setTreeFocused(false), []); @@ -213,15 +213,15 @@ export default function Tree(): React.Node { switch (event.key) { case 'Enter': case ' ': - if (selectedElementID !== null) { - dispatch({type: 'SELECT_OWNER', payload: selectedElementID}); + if (inspectedElementID !== null) { + dispatch({type: 'SELECT_OWNER', payload: inspectedElementID}); } break; default: break; } }, - [dispatch, selectedElementID], + [dispatch, inspectedElementID], ); // If we switch the selected element while using the keyboard, @@ -238,8 +238,8 @@ export default function Tree(): React.Node { didSelectNewSearchResult = true; } if (isNavigatingWithKeyboard || didSelectNewSearchResult) { - if (selectedElementID !== null) { - highlightHostInstance(selectedElementID); + if (inspectedElementID !== null) { + highlightHostInstance(inspectedElementID); } else { clearHighlightHostInstance(); } @@ -250,7 +250,7 @@ export default function Tree(): React.Node { highlightHostInstance, searchIndex, searchResults, - selectedElementID, + inspectedElementID, ]); // Highlight last hovered element. diff --git a/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js b/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js index 1d2ed380b9a1f..c9013e8011f68 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/TreeContext.js @@ -29,14 +29,12 @@ import type {ReactContext} from 'shared/ReactTypes'; import * as React from 'react'; import { createContext, - useCallback, useContext, useEffect, useLayoutEffect, useMemo, useReducer, useRef, - startTransition, } from 'react'; import {createRegExp} from '../utils'; import {StoreContext} from '../context'; @@ -48,8 +46,6 @@ export type StateContext = { // Tree numElements: number, ownerSubtreeLeafElementID: number | null, - selectedElementID: number | null, - selectedElementIndex: number | null, // Search searchIndex: number | null, @@ -62,6 +58,7 @@ export type StateContext = { // Inspection element panel inspectedElementID: number | null, + inspectedElementIndex: number | null, }; type ACTION_GO_TO_NEXT_SEARCH_RESULT = { @@ -123,9 +120,6 @@ type ACTION_SET_SEARCH_TEXT = { type: 'SET_SEARCH_TEXT', payload: string, }; -type ACTION_UPDATE_INSPECTED_ELEMENT_ID = { - type: 'UPDATE_INSPECTED_ELEMENT_ID', -}; type Action = | ACTION_GO_TO_NEXT_SEARCH_RESULT @@ -145,8 +139,7 @@ type Action = | ACTION_SELECT_PREVIOUS_SIBLING_IN_TREE | ACTION_SELECT_OWNER_LIST_NEXT_ELEMENT_IN_TREE | ACTION_SELECT_OWNER_LIST_PREVIOUS_ELEMENT_IN_TREE - | ACTION_SET_SEARCH_TEXT - | ACTION_UPDATE_INSPECTED_ELEMENT_ID; + | ACTION_SET_SEARCH_TEXT; export type DispatcherContext = (action: Action) => void; @@ -162,8 +155,6 @@ type State = { // Tree numElements: number, ownerSubtreeLeafElementID: number | null, - selectedElementID: number | null, - selectedElementIndex: number | null, // Search searchIndex: number | null, @@ -176,14 +167,15 @@ type State = { // Inspection element panel inspectedElementID: number | null, + inspectedElementIndex: number | null, }; function reduceTreeState(store: Store, state: State, action: Action): State { let { numElements, ownerSubtreeLeafElementID, - selectedElementIndex, - selectedElementID, + inspectedElementID, + inspectedElementIndex, } = state; const ownerID = state.ownerID; @@ -201,34 +193,33 @@ function reduceTreeState(store: Store, state: State, action: Action): State { // We deduce the parent-child mapping from removedIDs (id -> parentID) // because by now it's too late to read them from the store. while ( - selectedElementID !== null && - removedIDs.has(selectedElementID) + inspectedElementID !== null && + removedIDs.has(inspectedElementID) ) { - selectedElementID = ((removedIDs.get( - selectedElementID, - ): any): number); + // $FlowExpectedError[incompatible-type] + inspectedElementID = removedIDs.get(inspectedElementID); } - if (selectedElementID === 0) { + if (inspectedElementID === 0) { // The whole root was removed. - selectedElementIndex = null; + inspectedElementIndex = null; } break; case 'SELECT_CHILD_ELEMENT_IN_TREE': ownerSubtreeLeafElementID = null; - if (selectedElementIndex !== null) { - const selectedElement = store.getElementAtIndex( - ((selectedElementIndex: any): number), + if (inspectedElementIndex !== null) { + const inspectedElement = store.getElementAtIndex( + inspectedElementIndex, ); if ( - selectedElement !== null && - selectedElement.children.length > 0 && - !selectedElement.isCollapsed + inspectedElement !== null && + inspectedElement.children.length > 0 && + !inspectedElement.isCollapsed ) { - const firstChildID = selectedElement.children[0]; + const firstChildID = inspectedElement.children[0]; const firstChildIndex = store.getIndexOfElementID(firstChildID); if (firstChildIndex !== null) { - selectedElementIndex = firstChildIndex; + inspectedElementIndex = firstChildIndex; } } } @@ -236,7 +227,8 @@ function reduceTreeState(store: Store, state: State, action: Action): State { case 'SELECT_ELEMENT_AT_INDEX': ownerSubtreeLeafElementID = null; - selectedElementIndex = (action: ACTION_SELECT_ELEMENT_AT_INDEX).payload; + inspectedElementIndex = (action: ACTION_SELECT_ELEMENT_AT_INDEX) + .payload; break; case 'SELECT_ELEMENT_BY_ID': ownerSubtreeLeafElementID = null; @@ -245,30 +237,30 @@ function reduceTreeState(store: Store, state: State, action: Action): State { // It might also cause problems if the specified element was inside of a (not yet expanded) subtree. lookupIDForIndex = false; - selectedElementID = (action: ACTION_SELECT_ELEMENT_BY_ID).payload; - selectedElementIndex = - selectedElementID === null + inspectedElementID = (action: ACTION_SELECT_ELEMENT_BY_ID).payload; + inspectedElementIndex = + inspectedElementID === null ? null - : store.getIndexOfElementID(selectedElementID); + : store.getIndexOfElementID(inspectedElementID); break; case 'SELECT_NEXT_ELEMENT_IN_TREE': ownerSubtreeLeafElementID = null; if ( - selectedElementIndex === null || - selectedElementIndex + 1 >= numElements + inspectedElementIndex === null || + inspectedElementIndex + 1 >= numElements ) { - selectedElementIndex = 0; + inspectedElementIndex = 0; } else { - selectedElementIndex++; + inspectedElementIndex++; } break; case 'SELECT_NEXT_SIBLING_IN_TREE': ownerSubtreeLeafElementID = null; - if (selectedElementIndex !== null) { + if (inspectedElementIndex !== null) { const selectedElement = store.getElementAtIndex( - ((selectedElementIndex: any): number), + ((inspectedElementIndex: any): number), ); if (selectedElement !== null && selectedElement.parentID !== 0) { const parent = store.getElementByID(selectedElement.parentID); @@ -279,23 +271,23 @@ function reduceTreeState(store: Store, state: State, action: Action): State { selectedChildIndex < children.length - 1 ? children[selectedChildIndex + 1] : children[0]; - selectedElementIndex = store.getIndexOfElementID(nextChildID); + inspectedElementIndex = store.getIndexOfElementID(nextChildID); } } } break; case 'SELECT_OWNER_LIST_NEXT_ELEMENT_IN_TREE': - if (selectedElementIndex !== null) { + if (inspectedElementIndex !== null) { if ( ownerSubtreeLeafElementID !== null && - ownerSubtreeLeafElementID !== selectedElementID + ownerSubtreeLeafElementID !== inspectedElementID ) { const leafElement = store.getElementByID(ownerSubtreeLeafElementID); if (leafElement !== null) { let currentElement: null | Element = leafElement; while (currentElement !== null) { - if (currentElement.ownerID === selectedElementID) { - selectedElementIndex = store.getIndexOfElementID( + if (currentElement.ownerID === inspectedElementID) { + inspectedElementIndex = store.getIndexOfElementID( currentElement.id, ); break; @@ -308,23 +300,23 @@ function reduceTreeState(store: Store, state: State, action: Action): State { } break; case 'SELECT_OWNER_LIST_PREVIOUS_ELEMENT_IN_TREE': - if (selectedElementIndex !== null) { + if (inspectedElementIndex !== null) { if (ownerSubtreeLeafElementID === null) { // If this is the first time we're stepping through the owners tree, // pin the current component as the owners list leaf. // This will enable us to step back down to this component. - ownerSubtreeLeafElementID = selectedElementID; + ownerSubtreeLeafElementID = inspectedElementID; } const selectedElement = store.getElementAtIndex( - ((selectedElementIndex: any): number), + ((inspectedElementIndex: any): number), ); if (selectedElement !== null && selectedElement.ownerID !== 0) { const ownerIndex = store.getIndexOfElementID( selectedElement.ownerID, ); if (ownerIndex !== null) { - selectedElementIndex = ownerIndex; + inspectedElementIndex = ownerIndex; } } } @@ -332,16 +324,16 @@ function reduceTreeState(store: Store, state: State, action: Action): State { case 'SELECT_PARENT_ELEMENT_IN_TREE': ownerSubtreeLeafElementID = null; - if (selectedElementIndex !== null) { + if (inspectedElementIndex !== null) { const selectedElement = store.getElementAtIndex( - ((selectedElementIndex: any): number), + ((inspectedElementIndex: any): number), ); if (selectedElement !== null && selectedElement.parentID !== 0) { const parentIndex = store.getIndexOfElementID( selectedElement.parentID, ); if (parentIndex !== null) { - selectedElementIndex = parentIndex; + inspectedElementIndex = parentIndex; } } } @@ -349,18 +341,18 @@ function reduceTreeState(store: Store, state: State, action: Action): State { case 'SELECT_PREVIOUS_ELEMENT_IN_TREE': ownerSubtreeLeafElementID = null; - if (selectedElementIndex === null || selectedElementIndex === 0) { - selectedElementIndex = numElements - 1; + if (inspectedElementIndex === null || inspectedElementIndex === 0) { + inspectedElementIndex = numElements - 1; } else { - selectedElementIndex--; + inspectedElementIndex--; } break; case 'SELECT_PREVIOUS_SIBLING_IN_TREE': ownerSubtreeLeafElementID = null; - if (selectedElementIndex !== null) { + if (inspectedElementIndex !== null) { const selectedElement = store.getElementAtIndex( - ((selectedElementIndex: any): number), + ((inspectedElementIndex: any): number), ); if (selectedElement !== null && selectedElement.parentID !== 0) { const parent = store.getElementByID(selectedElement.parentID); @@ -371,7 +363,7 @@ function reduceTreeState(store: Store, state: State, action: Action): State { selectedChildIndex > 0 ? children[selectedChildIndex - 1] : children[children.length - 1]; - selectedElementIndex = store.getIndexOfElementID(nextChildID); + inspectedElementIndex = store.getIndexOfElementID(nextChildID); } } } @@ -384,7 +376,7 @@ function reduceTreeState(store: Store, state: State, action: Action): State { } let flatIndex = 0; - if (selectedElementIndex !== null) { + if (inspectedElementIndex !== null) { // Resume from the current position in the list. // Otherwise step to the previous item, relative to the current selection. for ( @@ -393,7 +385,7 @@ function reduceTreeState(store: Store, state: State, action: Action): State { i-- ) { const {index} = elementIndicesWithErrorsOrWarnings[i]; - if (index >= selectedElementIndex) { + if (index >= inspectedElementIndex) { flatIndex = i; } else { break; @@ -407,12 +399,12 @@ function reduceTreeState(store: Store, state: State, action: Action): State { elementIndicesWithErrorsOrWarnings[ elementIndicesWithErrorsOrWarnings.length - 1 ]; - selectedElementID = prevEntry.id; - selectedElementIndex = prevEntry.index; + inspectedElementID = prevEntry.id; + inspectedElementIndex = prevEntry.index; } else { prevEntry = elementIndicesWithErrorsOrWarnings[flatIndex - 1]; - selectedElementID = prevEntry.id; - selectedElementIndex = prevEntry.index; + inspectedElementID = prevEntry.id; + inspectedElementIndex = prevEntry.index; } lookupIDForIndex = false; @@ -426,12 +418,12 @@ function reduceTreeState(store: Store, state: State, action: Action): State { } let flatIndex = -1; - if (selectedElementIndex !== null) { + if (inspectedElementIndex !== null) { // Resume from the current position in the list. // Otherwise step to the next item, relative to the current selection. for (let i = 0; i < elementIndicesWithErrorsOrWarnings.length; i++) { const {index} = elementIndicesWithErrorsOrWarnings[i]; - if (index <= selectedElementIndex) { + if (index <= inspectedElementIndex) { flatIndex = i; } else { break; @@ -442,12 +434,12 @@ function reduceTreeState(store: Store, state: State, action: Action): State { let nextEntry; if (flatIndex >= elementIndicesWithErrorsOrWarnings.length - 1) { nextEntry = elementIndicesWithErrorsOrWarnings[0]; - selectedElementID = nextEntry.id; - selectedElementIndex = nextEntry.index; + inspectedElementID = nextEntry.id; + inspectedElementIndex = nextEntry.index; } else { nextEntry = elementIndicesWithErrorsOrWarnings[flatIndex + 1]; - selectedElementID = nextEntry.id; - selectedElementIndex = nextEntry.index; + inspectedElementID = nextEntry.id; + inspectedElementIndex = nextEntry.index; } lookupIDForIndex = false; @@ -460,12 +452,15 @@ function reduceTreeState(store: Store, state: State, action: Action): State { } // Keep selected item ID and index in sync. - if (lookupIDForIndex && selectedElementIndex !== state.selectedElementIndex) { - if (selectedElementIndex === null) { - selectedElementID = null; + if ( + lookupIDForIndex && + inspectedElementIndex !== state.inspectedElementIndex + ) { + if (inspectedElementIndex === null) { + inspectedElementID = null; } else { - selectedElementID = store.getElementIDAtIndex( - ((selectedElementIndex: any): number), + inspectedElementID = store.getElementIDAtIndex( + ((inspectedElementIndex: any): number), ); } } @@ -475,8 +470,8 @@ function reduceTreeState(store: Store, state: State, action: Action): State { numElements, ownerSubtreeLeafElementID, - selectedElementIndex, - selectedElementID, + inspectedElementIndex, + inspectedElementID, }; } @@ -485,8 +480,8 @@ function reduceSearchState(store: Store, state: State, action: Action): State { searchIndex, searchResults, searchText, - selectedElementID, - selectedElementIndex, + inspectedElementID, + inspectedElementIndex, } = state; const ownerID = state.ownerID; @@ -594,11 +589,11 @@ function reduceSearchState(store: Store, state: State, action: Action): State { }); if (searchResults.length > 0) { if (prevSearchIndex === null) { - if (selectedElementIndex !== null) { + if (inspectedElementIndex !== null) { searchIndex = getNearestResultIndex( store, searchResults, - selectedElementIndex, + inspectedElementIndex, ); } else { searchIndex = 0; @@ -619,7 +614,7 @@ function reduceSearchState(store: Store, state: State, action: Action): State { } if (searchText !== prevSearchText) { - const newSearchIndex = searchResults.indexOf(selectedElementID); + const newSearchIndex = searchResults.indexOf(inspectedElementID); if (newSearchIndex === -1) { // Only move the selection if the new query // doesn't match the current selection anymore. @@ -631,17 +626,17 @@ function reduceSearchState(store: Store, state: State, action: Action): State { } } if (didRequestSearch && searchIndex !== null) { - selectedElementID = ((searchResults[searchIndex]: any): number); - selectedElementIndex = store.getIndexOfElementID( - ((selectedElementID: any): number), + inspectedElementID = ((searchResults[searchIndex]: any): number); + inspectedElementIndex = store.getIndexOfElementID( + ((inspectedElementID: any): number), ); } return { ...state, - selectedElementID, - selectedElementIndex, + inspectedElementID, + inspectedElementIndex, searchIndex, searchResults, @@ -652,14 +647,14 @@ function reduceSearchState(store: Store, state: State, action: Action): State { function reduceOwnersState(store: Store, state: State, action: Action): State { let { numElements, - selectedElementID, - selectedElementIndex, ownerID, ownerFlatTree, + inspectedElementID, + inspectedElementIndex, } = state; const {searchIndex, searchResults, searchText} = state; - let prevSelectedElementIndex = selectedElementIndex; + let prevInspectedElementIndex = inspectedElementIndex; switch (action.type) { case 'HANDLE_STORE_MUTATION': @@ -667,75 +662,76 @@ function reduceOwnersState(store: Store, state: State, action: Action): State { if (!store.containsElement(ownerID)) { ownerID = null; ownerFlatTree = null; - selectedElementID = null; + prevInspectedElementIndex = null; } else { ownerFlatTree = store.getOwnersListForElement(ownerID); - if (selectedElementID !== null) { + if (inspectedElementID !== null) { // Mutation might have caused the index of this ID to shift. - selectedElementIndex = ownerFlatTree.findIndex( - element => element.id === selectedElementID, + prevInspectedElementIndex = ownerFlatTree.findIndex( + element => element.id === inspectedElementID, ); } } } else { - if (selectedElementID !== null) { + if (inspectedElementID !== null) { // Mutation might have caused the index of this ID to shift. - selectedElementIndex = store.getIndexOfElementID(selectedElementID); + inspectedElementIndex = store.getIndexOfElementID(inspectedElementID); } } - if (selectedElementIndex === -1) { + if (inspectedElementIndex === -1) { // If we couldn't find this ID after mutation, unselect it. - selectedElementIndex = null; - selectedElementID = null; + inspectedElementIndex = null; + inspectedElementID = null; } break; case 'RESET_OWNER_STACK': ownerID = null; ownerFlatTree = null; - selectedElementIndex = - selectedElementID !== null - ? store.getIndexOfElementID(selectedElementID) + inspectedElementIndex = + inspectedElementID !== null + ? store.getIndexOfElementID(inspectedElementID) : null; break; case 'SELECT_ELEMENT_AT_INDEX': if (ownerFlatTree !== null) { - selectedElementIndex = (action: ACTION_SELECT_ELEMENT_AT_INDEX).payload; + inspectedElementIndex = (action: ACTION_SELECT_ELEMENT_AT_INDEX) + .payload; } break; case 'SELECT_ELEMENT_BY_ID': if (ownerFlatTree !== null) { const payload = (action: ACTION_SELECT_ELEMENT_BY_ID).payload; if (payload === null) { - selectedElementIndex = null; + inspectedElementIndex = null; } else { - selectedElementIndex = ownerFlatTree.findIndex( + inspectedElementIndex = ownerFlatTree.findIndex( element => element.id === payload, ); // If the selected element is outside of the current owners list, // exit the list and select the element in the main tree. // This supports features like toggling Suspense. - if (selectedElementIndex !== null && selectedElementIndex < 0) { + if (inspectedElementIndex !== null && inspectedElementIndex < 0) { ownerID = null; ownerFlatTree = null; - selectedElementIndex = store.getIndexOfElementID(payload); + inspectedElementIndex = store.getIndexOfElementID(payload); } } } break; case 'SELECT_NEXT_ELEMENT_IN_TREE': if (ownerFlatTree !== null && ownerFlatTree.length > 0) { - if (selectedElementIndex === null) { - selectedElementIndex = 0; - } else if (selectedElementIndex + 1 < ownerFlatTree.length) { - selectedElementIndex++; + if (inspectedElementIndex === null) { + inspectedElementIndex = 0; + } else if (inspectedElementIndex + 1 < ownerFlatTree.length) { + inspectedElementIndex++; } } break; case 'SELECT_PREVIOUS_ELEMENT_IN_TREE': if (ownerFlatTree !== null && ownerFlatTree.length > 0) { - if (selectedElementIndex !== null && selectedElementIndex > 0) { - selectedElementIndex--; + if (inspectedElementIndex !== null && inspectedElementIndex > 0) { + inspectedElementIndex--; } } break; @@ -747,8 +743,8 @@ function reduceOwnersState(store: Store, state: State, action: Action): State { ownerFlatTree = store.getOwnersListForElement(ownerID); // Always force reset selection to be the top of the new owner tree. - selectedElementIndex = 0; - prevSelectedElementIndex = null; + inspectedElementIndex = 0; + prevInspectedElementIndex = null; } break; default: @@ -769,12 +765,12 @@ function reduceOwnersState(store: Store, state: State, action: Action): State { } // Keep selected item ID and index in sync. - if (selectedElementIndex !== prevSelectedElementIndex) { - if (selectedElementIndex === null) { - selectedElementID = null; + if (inspectedElementIndex !== prevInspectedElementIndex) { + if (inspectedElementIndex === null) { + inspectedElementID = null; } else { if (ownerFlatTree !== null) { - selectedElementID = ownerFlatTree[selectedElementIndex].id; + inspectedElementID = ownerFlatTree[inspectedElementIndex].id; } } } @@ -783,8 +779,6 @@ function reduceOwnersState(store: Store, state: State, action: Action): State { ...state, numElements, - selectedElementID, - selectedElementIndex, searchIndex, searchResults, @@ -792,49 +786,27 @@ function reduceOwnersState(store: Store, state: State, action: Action): State { ownerID, ownerFlatTree, - }; -} -function reduceSuspenseState( - store: Store, - state: State, - action: Action, -): State { - const {type} = action; - switch (type) { - case 'UPDATE_INSPECTED_ELEMENT_ID': - if (state.inspectedElementID !== state.selectedElementID) { - return { - ...state, - inspectedElementID: state.selectedElementID, - }; - } - break; - default: - break; - } - - // React can bailout of no-op updates. - return state; + inspectedElementID, + inspectedElementIndex, + }; } type Props = { children: React$Node, // Used for automated testing - defaultInspectedElementID?: ?number, defaultOwnerID?: ?number, - defaultSelectedElementID?: ?number, - defaultSelectedElementIndex?: ?number, + defaultInspectedElementID?: ?number, + defaultInspectedElementIndex?: ?number, }; // TODO Remove TreeContextController wrapper element once global Context.write API exists. function TreeContextController({ children, - defaultInspectedElementID, defaultOwnerID, - defaultSelectedElementID, - defaultSelectedElementIndex, + defaultInspectedElementID, + defaultInspectedElementIndex, }: Props): React.Node { const store = useContext(StoreContext); @@ -865,23 +837,22 @@ function TreeContextController({ case 'SELECT_PREVIOUS_ELEMENT_WITH_ERROR_OR_WARNING_IN_TREE': case 'SELECT_PREVIOUS_SIBLING_IN_TREE': case 'SELECT_OWNER': - case 'UPDATE_INSPECTED_ELEMENT_ID': case 'SET_SEARCH_TEXT': state = reduceTreeState(store, state, action); state = reduceSearchState(store, state, action); state = reduceOwnersState(store, state, action); - state = reduceSuspenseState(store, state, action); + // TODO(hoxyq): review // If the selected ID is in a collapsed subtree, reset the selected index to null. // We'll know the correct index after the layout effect will toggle the tree, // and the store tree is mutated to account for that. if ( - state.selectedElementID !== null && - store.isInsideCollapsedSubTree(state.selectedElementID) + state.inspectedElementID !== null && + store.isInsideCollapsedSubTree(state.inspectedElementID) ) { return { ...state, - selectedElementIndex: null, + inspectedElementIndex: null, }; } @@ -897,16 +868,6 @@ function TreeContextController({ // Tree numElements: store.numElements, ownerSubtreeLeafElementID: null, - selectedElementID: - defaultSelectedElementID != null - ? defaultSelectedElementID - : store.lastSelectedHostInstanceElementId, - selectedElementIndex: - defaultSelectedElementIndex != null - ? defaultSelectedElementIndex - : store.lastSelectedHostInstanceElementId - ? store.getIndexOfElementID(store.lastSelectedHostInstanceElementId) - : null, // Search searchIndex: null, @@ -922,42 +883,38 @@ function TreeContextController({ defaultInspectedElementID != null ? defaultInspectedElementID : store.lastSelectedHostInstanceElementId, + inspectedElementIndex: + defaultInspectedElementIndex != null + ? defaultInspectedElementIndex + : store.lastSelectedHostInstanceElementId + ? store.getIndexOfElementID(store.lastSelectedHostInstanceElementId) + : null, }); - const dispatchWrapper = useCallback( - (action: Action) => { - dispatch(action); - startTransition(() => { - dispatch({type: 'UPDATE_INSPECTED_ELEMENT_ID'}); - }); - }, - [dispatch], - ); - // Listen for host element selections. useEffect(() => { const handler = (id: Element['id']) => - dispatchWrapper({type: 'SELECT_ELEMENT_BY_ID', payload: id}); + dispatch({type: 'SELECT_ELEMENT_BY_ID', payload: id}); store.addListener('hostInstanceSelected', handler); return () => store.removeListener('hostInstanceSelected', handler); - }, [store, dispatchWrapper]); + }, [store, dispatch]); // If a newly-selected search result or inspection selection is inside of a collapsed subtree, auto expand it. // This needs to be a layout effect to avoid temporarily flashing an incorrect selection. - const prevSelectedElementID = useRef(null); + const prevInspectedElementID = useRef(null); useLayoutEffect(() => { - if (state.selectedElementID !== prevSelectedElementID.current) { - prevSelectedElementID.current = state.selectedElementID; + if (state.inspectedElementID !== prevInspectedElementID.current) { + prevInspectedElementID.current = state.inspectedElementID; - if (state.selectedElementID !== null) { - const element = store.getElementByID(state.selectedElementID); + if (state.inspectedElementID !== null) { + const element = store.getElementByID(state.inspectedElementID); if (element !== null && element.parentID > 0) { store.toggleIsCollapsed(element.parentID, false); } } } - }, [state.selectedElementID, store]); + }, [state.inspectedElementID, store]); // Mutations to the underlying tree may impact this context (e.g. search results, selection state). useEffect(() => { @@ -965,7 +922,7 @@ function TreeContextController({ Array, Map, ]) => { - dispatchWrapper({ + dispatch({ type: 'HANDLE_STORE_MUTATION', payload: [addedElementIDs, removedElementIDs], }); @@ -976,20 +933,19 @@ function TreeContextController({ // At the moment, we can treat this as a mutation. // We don't know which Elements were newly added/removed, but that should be okay in this case. // It would only impact the search state, which is unlikely to exist yet at this point. - dispatchWrapper({ + dispatch({ type: 'HANDLE_STORE_MUTATION', payload: [[], new Map()], }); } store.addListener('mutated', handleStoreMutated); - return () => store.removeListener('mutated', handleStoreMutated); - }, [dispatchWrapper, initialRevision, store]); + }, [dispatch, initialRevision, store]); return ( - + {children} @@ -1028,11 +984,11 @@ function recursivelySearchTree( function getNearestResultIndex( store: Store, searchResults: Array, - selectedElementIndex: number, + inspectedElementIndex: number, ): number { const index = searchResults.findIndex(id => { const innerIndex = store.getIndexOfElementID(id); - return innerIndex !== null && innerIndex >= selectedElementIndex; + return innerIndex !== null && innerIndex >= inspectedElementIndex; }); return index === -1 ? 0 : index; diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js index 1ef7bb2b7e5c7..38203175136b0 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js @@ -88,7 +88,7 @@ type Props = { function ProfilerContextController({children}: Props): React.Node { const store = useContext(StoreContext); - const {selectedElementID} = useContext(TreeStateContext); + const {inspectedElementID} = useContext(TreeStateContext); const dispatch = useContext(TreeDispatcherContext); const {profilerStore} = store; @@ -176,9 +176,9 @@ function ProfilerContextController({children}: Props): React.Node { if (rootID === null || !dataForRoots.has(rootID)) { let selectedElementRootID = null; - if (selectedElementID !== null) { + if (inspectedElementID !== null) { selectedElementRootID = - store.getRootIDForElement(selectedElementID); + store.getRootIDForElement(inspectedElementID); } if ( selectedElementRootID !== null && From f2813ee33d37e20029c6698f34946d7f08eb7a95 Mon Sep 17 00:00:00 2001 From: Ruslan Lesiutin Date: Thu, 9 Jan 2025 18:21:55 +0000 Subject: [PATCH 10/10] [DevTools] feat[Tree]: set initial scroll offset when inspected element index is set (#31968) Stacked on https://github.com/facebook/react/pull/31956. See [commit on top](https://github.com/facebook/react/pull/31968/commits/ecb8df4175342cde7669cd4a6b008b3782eb5b61). Use `initialScrollOffset` prop for `FixedSizeList` from `react-window`. This happens when user selects an element in built-in Elements panel in DevTools, and then opens Components panel from React DevTools - elements will be synced and corresponding React Element will be pre-selected, we just have to scroll to its position now. --- .../src/devtools/views/Components/Tree.js | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Tree.js b/packages/react-devtools-shared/src/devtools/views/Components/Tree.js index c0fd842abc4eb..2a1569bb52e1e 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Tree.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/Tree.js @@ -47,6 +47,22 @@ export type ItemData = { treeFocused: boolean, }; +function calculateInitialScrollOffset( + inspectedElementIndex: number | null, + elementHeight: number, +): number | void { + if (inspectedElementIndex === null) { + return undefined; + } + + if (inspectedElementIndex < 3) { + return undefined; + } + + // Make 3 elements on top of the inspected one visible + return (inspectedElementIndex - 3) * elementHeight; +} + export default function Tree(): React.Node { const dispatch = useContext(TreeDispatcherContext); const { @@ -401,6 +417,10 @@ export default function Tree(): React.Node {