From 0197f26c439ea6a88cec4f70d0b58ce6dfab72a4 Mon Sep 17 00:00:00 2001 From: Piotr Tomczewski Date: Fri, 1 Nov 2024 11:46:10 +0100 Subject: [PATCH 1/5] refactor: move InspectedElementContext to enable reuse of hook names cache The InspectedElementContext was previously only available in the Components tab. This change moves the context provider to a higher level in the component hierarchy, making the InspectedElementContext available in both the Components and Profiler tabs. --- .../devtools/views/Components/Components.js | 5 +- .../src/devtools/views/DevTools.js | 73 ++++++++++--------- 2 files changed, 40 insertions(+), 38 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/Components/Components.js b/packages/react-devtools-shared/src/devtools/views/Components/Components.js index 995f1d92cf323..b7ce607685051 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/Components.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/Components.js @@ -19,7 +19,6 @@ import { } from 'react-devtools-shared/src/storage'; import InspectedElementErrorBoundary from './InspectedElementErrorBoundary'; import InspectedElement from './InspectedElement'; -import {InspectedElementContextController} from './InspectedElementContext'; import {ModalDialog} from '../ModalDialog'; import SettingsModal from 'react-devtools-shared/src/devtools/views/Settings/SettingsModal'; import {NativeStyleContextController} from './NativeStyleEditor/context'; @@ -162,9 +161,7 @@ function Components(_: {}) {
- - - +
diff --git a/packages/react-devtools-shared/src/devtools/views/DevTools.js b/packages/react-devtools-shared/src/devtools/views/DevTools.js index 9bee64ff95150..bd14bdda0f5ba 100644 --- a/packages/react-devtools-shared/src/devtools/views/DevTools.js +++ b/packages/react-devtools-shared/src/devtools/views/DevTools.js @@ -28,6 +28,7 @@ import {SettingsContextController} from './Settings/SettingsContext'; import {TreeContextController} from './Components/TreeContext'; import ViewElementSourceContext from './Components/ViewElementSourceContext'; import FetchFileWithCachingContext from './Components/FetchFileWithCachingContext'; +import {InspectedElementContextController} from './Components/InspectedElementContext'; import HookNamesModuleLoaderContext from 'react-devtools-shared/src/devtools/views/Components/HookNamesModuleLoaderContext'; import {ProfilerContextController} from './Profiler/ProfilerContext'; import {TimelineContextController} from 'react-devtools-timeline/src/TimelineContext'; @@ -276,43 +277,47 @@ export default function DevTools({ - -
- {showTabBar && ( -
- - - {process.env.DEVTOOLS_VERSION} - -
- + +
+ {showTabBar && ( +
+ + + {process.env.DEVTOOLS_VERSION} + +
+ +
+ )} + + - )} - - -
- + + From 03a62bc5ffb48cf87491711ccb729850e7cabf83 Mon Sep 17 00:00:00 2001 From: Piotr Tomczewski Date: Fri, 1 Nov 2024 11:46:10 +0100 Subject: [PATCH 2/5] feat(profiler): display names of changed hooks in the Profiler tab - Introduced `HookChangeSummary` component to show names of changed hooks, improving clarity and making it easier to identify specific hook updates in the Profiler tab. - Added `getAlreadyLoadedHookNames` helper for efficient retrieval of hook names. Resolves #21856 --- .../views/Profiler/HookChangeSummary.css | 59 +++++++ .../views/Profiler/HookChangeSummary.js | 152 ++++++++++++++++++ .../devtools/views/Profiler/WhatChanged.js | 25 +-- .../src/hookNamesCache.js | 8 + 4 files changed, 225 insertions(+), 19 deletions(-) create mode 100644 packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.css create mode 100644 packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.js diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.css b/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.css new file mode 100644 index 0000000000000..d1aff47bddfcf --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.css @@ -0,0 +1,59 @@ +.LoadHookNamesToggle, +.ToggleError { + padding: 2px; + background: none; + border: none; + cursor: pointer; + position: relative; + bottom: -0.2em; + margin-block: -1em; +} + +.ToggleError { + color: var(--color-error-text); +} + +.Hook { + list-style-type: none; + margin: 0; + padding-left: 0.5rem; + line-height: 1.125rem; + + font-family: var(--font-family-monospace); + font-size: var(--font-size-monospace-normal); +} + +.Hook .Hook { + padding-left: 1rem; +} + +.Name { + color: var(--color-dim); + flex: 0 0 auto; + cursor: default; +} + +.PrimitiveHookName { + color: var(--color-text); + flex: 0 0 auto; + cursor: default; +} + +.Name:after { + color: var(--color-text); + content: ': '; + margin-right: 0.5rem; +} + +.PrimitiveHookNumber { + background-color: var(--color-primitive-hook-badge-background); + color: var(--color-primitive-hook-badge-text); + font-size: var(--font-size-monospace-small); + margin-right: 0.25rem; + border-radius: 0.125rem; + padding: 0.125rem 0.25rem; +} + +.HookName { + color: var(--color-component-name); +} diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.js b/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.js new file mode 100644 index 0000000000000..cd10556f8eacb --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.js @@ -0,0 +1,152 @@ +import * as React from 'react'; +import {useContext, useMemo, useCallback, memo, useState, useEffect} from 'react'; +import styles from './HookChangeSummary.css'; +import ButtonIcon from '../ButtonIcon'; +import {InspectedElementContext} from '../Components/InspectedElementContext'; +import {StoreContext} from '../context'; +import { + getAlreadyLoadedHookNames, + getHookSourceLocationKey, +} from 'react-devtools-shared/src/hookNamesCache'; +import Toggle from '../Toggle'; + +const hookListFormatter = new Intl.ListFormat('en', { + style: 'long', + type: 'conjunction', +}); + +const Hook = memo(({hook, hookNames}) => { + const hookSource = hook.hookSource; + const hookName = useMemo(() => { + if (!hookSource || !hookNames) return null; + const key = getHookSourceLocationKey(hookSource); + return hookNames.get(key) || null; + }, [hookSource, hookNames]); + + return ( +
    +
  • + {hook.id !== null && ( + {hook.id + 1} + )} + + {hook.name} + {hookName && ({hookName})} + + {hook.subHooks?.map((subHook, index) => ( + + ))} +
    • +
+ ); +}); + +const shouldKeepHook = (hook, hooksArray) => { + if (hook.id !== null && hooksArray.includes(hook.id)) { + return true; + } + return hook.subHooks?.some(subHook => shouldKeepHook(subHook, hooksArray)) ?? false; +}; + +const filterHooks = (hook, hooksArray) => { + if (!shouldKeepHook(hook, hooksArray)) { + return null; + } + + if (hook.subHooks?.length > 0) { + const filteredSubHooks = hook.subHooks + .map(subHook => filterHooks(subHook, hooksArray)) + .filter(Boolean); + + return filteredSubHooks.length > 0 + ? { ...hook, subHooks: filteredSubHooks } + : { ...hook }; + } + + return hook; +}; + +const HookChangeSummary = memo(({hooks, fiberID, state}) => { + const {parseHookNames, toggleParseHookNames, inspectedElement} = useContext( + InspectedElementContext, + ); + const store = useContext(StoreContext); + + const [parseHookNamesOptimistic, setParseHookNamesOptimistic] = useState(parseHookNames); + + useEffect(() => { + setParseHookNamesOptimistic(parseHookNames); + }, [inspectedElement?.id, parseHookNames]); + + const handleOnChange = useCallback(() => { + setParseHookNamesOptimistic(!parseHookNames); + toggleParseHookNames(); + }, [toggleParseHookNames, parseHookNames]); + + const element = fiberID !== null ? store.getElementByID(fiberID) : null; + const hookNames = getAlreadyLoadedHookNames(element); + + const filteredHooks = useMemo(() => { + if (!hooks || !inspectedElement?.hooks) return null; + return inspectedElement.hooks + .map(hook => filterHooks(hook, hooks)) + .filter(Boolean); + }, [inspectedElement?.hooks, hooks]); + + const hookParsingFailed = useMemo(() => parseHookNames && hookNames === null, [parseHookNames, hookNames]) ; + + if (!hooks?.length) { + return No hooks changed; + } + + // Fallback to old list of ids when inspectedElement ID doesn't match element ID or when hook counts differ + if ( + inspectedElement?.id !== element?.id || + filteredHooks?.length !== hooks.length + ) { + const hookIds = hooks.map(hookId => String(hookId + 1)); + const hookWord = hookIds.length === 1 ? '• Hook' : '• Hooks'; + return ( + + {hookWord} {hookListFormatter.format(hookIds)} changed + + ); + } + + let toggleTitle; + if (hookParsingFailed) { + toggleTitle = 'Hook parsing failed'; + } else if (parseHookNamesOptimistic) { + toggleTitle = 'Parsing hook names ...'; + } else { + toggleTitle = 'Parse hook names (may be slow)'; + } + + return ( +
+ {filteredHooks.length > 1 ? '• Hooks changed:' : '• Hook changed:'} + {(!parseHookNames || hookParsingFailed) && ( + + + + )} + {filteredHooks.map(hook => ( + + ))} +
+ ); +}); + +export default HookChangeSummary; diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/WhatChanged.js b/packages/react-devtools-shared/src/devtools/views/Profiler/WhatChanged.js index 52fc756a063d1..327bb5e955223 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/WhatChanged.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/WhatChanged.js @@ -14,24 +14,7 @@ import {ProfilerContext} from './ProfilerContext'; import {StoreContext} from '../context'; import styles from './WhatChanged.css'; - -function hookIndicesToString(indices: Array): string { - // This is debatable but I think 1-based might ake for a nicer UX. - const numbers = indices.map(value => value + 1); - - switch (numbers.length) { - case 0: - return 'No hooks changed'; - case 1: - return `Hook ${numbers[0]} changed`; - case 2: - return `Hooks ${numbers[0]} and ${numbers[1]} changed`; - default: - return `Hooks ${numbers.slice(0, numbers.length - 1).join(', ')} and ${ - numbers[numbers.length - 1] - } changed`; - } -} +import HookChangeSummary from './HookChangeSummary'; type Props = { fiberID: number, @@ -106,7 +89,11 @@ export default function WhatChanged({fiberID}: Props): React.Node { if (Array.isArray(hooks)) { changes.push(
- • {hookIndicesToString(hooks)} +
, ); } else { diff --git a/packages/react-devtools-shared/src/hookNamesCache.js b/packages/react-devtools-shared/src/hookNamesCache.js index 89377eebb18fd..e0d4b1075c219 100644 --- a/packages/react-devtools-shared/src/hookNamesCache.js +++ b/packages/react-devtools-shared/src/hookNamesCache.js @@ -72,6 +72,14 @@ export function hasAlreadyLoadedHookNames(element: Element): boolean { return record != null && record.status === Resolved; } +export function getAlreadyLoadedHookNames(element: Element): HookNames | null { + const record = map.get(element); + if (record != null && record.status === Resolved) { + return record.value; + } + return null; +} + export function loadHookNames( element: Element, hooksTree: HooksTree, From e5ea078f3194b05f84fb834499cf8f30c14bc555 Mon Sep 17 00:00:00 2001 From: Piotr Tomczewski Date: Fri, 1 Nov 2024 12:45:55 +0100 Subject: [PATCH 3/5] feat(profiler): add display mode option to preserve hook change summary in tooltips - Added `displayMode` prop to `HookChangeSummary` for `detailed` and `compact` views. - Updated `WhatChanged` and `HoveredFiberInfo` to use `compact` mode in tooltips, preserving the previous concise summary display. This change maintains the original hook change summary style in tooltips while allowing a detailed view where needed. --- .../views/Profiler/HookChangeSummary.js | 185 +++++++++++------- .../views/Profiler/HoveredFiberInfo.js | 2 +- .../devtools/views/Profiler/WhatChanged.js | 7 +- 3 files changed, 116 insertions(+), 78 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.js b/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.js index cd10556f8eacb..8a5f92dfe434f 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.js @@ -1,9 +1,21 @@ import * as React from 'react'; -import {useContext, useMemo, useCallback, memo, useState, useEffect} from 'react'; +import { + useContext, + useMemo, + useCallback, + memo, + useState, + useEffect, +} from 'react'; import styles from './HookChangeSummary.css'; import ButtonIcon from '../ButtonIcon'; import {InspectedElementContext} from '../Components/InspectedElementContext'; import {StoreContext} from '../context'; +import { + HooksList, + State, +} from 'react-devtools-shared/src/devtools/views/Profiler/types'; + import { getAlreadyLoadedHookNames, getHookSourceLocationKey, @@ -35,7 +47,11 @@ const Hook = memo(({hook, hookNames}) => { {hookName && ({hookName})} {hook.subHooks?.map((subHook, index) => ( - + ))} @@ -46,7 +62,9 @@ const shouldKeepHook = (hook, hooksArray) => { if (hook.id !== null && hooksArray.includes(hook.id)) { return true; } - return hook.subHooks?.some(subHook => shouldKeepHook(subHook, hooksArray)) ?? false; + return ( + hook.subHooks?.some(subHook => shouldKeepHook(subHook, hooksArray)) ?? false + ); }; const filterHooks = (hook, hooksArray) => { @@ -60,93 +78,108 @@ const filterHooks = (hook, hooksArray) => { .filter(Boolean); return filteredSubHooks.length > 0 - ? { ...hook, subHooks: filteredSubHooks } - : { ...hook }; + ? {...hook, subHooks: filteredSubHooks} + : {...hook}; } return hook; }; -const HookChangeSummary = memo(({hooks, fiberID, state}) => { - const {parseHookNames, toggleParseHookNames, inspectedElement} = useContext( - InspectedElementContext, - ); - const store = useContext(StoreContext); +type Props = { + fiberID: number, + hooks: HooksList | null, + state: State | null, + displayMode?: 'detailed' | 'compact', +}; +const HookChangeSummary = memo( + ({hooks, fiberID, state, displayMode = 'detailed'}: Props) => { + const {parseHookNames, toggleParseHookNames, inspectedElement} = useContext( + InspectedElementContext, + ); + const store = useContext(StoreContext); - const [parseHookNamesOptimistic, setParseHookNamesOptimistic] = useState(parseHookNames); + const [parseHookNamesOptimistic, setParseHookNamesOptimistic] = + useState(parseHookNames); - useEffect(() => { - setParseHookNamesOptimistic(parseHookNames); - }, [inspectedElement?.id, parseHookNames]); + useEffect(() => { + setParseHookNamesOptimistic(parseHookNames); + }, [inspectedElement?.id, parseHookNames]); - const handleOnChange = useCallback(() => { - setParseHookNamesOptimistic(!parseHookNames); - toggleParseHookNames(); - }, [toggleParseHookNames, parseHookNames]); + const handleOnChange = useCallback(() => { + setParseHookNamesOptimistic(!parseHookNames); + toggleParseHookNames(); + }, [toggleParseHookNames, parseHookNames]); - const element = fiberID !== null ? store.getElementByID(fiberID) : null; - const hookNames = getAlreadyLoadedHookNames(element); + const element = fiberID !== null ? store.getElementByID(fiberID) : null; + const hookNames = getAlreadyLoadedHookNames(element); - const filteredHooks = useMemo(() => { - if (!hooks || !inspectedElement?.hooks) return null; - return inspectedElement.hooks - .map(hook => filterHooks(hook, hooks)) - .filter(Boolean); - }, [inspectedElement?.hooks, hooks]); + const filteredHooks = useMemo(() => { + if (!hooks || !inspectedElement?.hooks) return null; + return inspectedElement.hooks + .map(hook => filterHooks(hook, hooks)) + .filter(Boolean); + }, [inspectedElement?.hooks, hooks]); - const hookParsingFailed = useMemo(() => parseHookNames && hookNames === null, [parseHookNames, hookNames]) ; + const hookParsingFailed = useMemo( + () => parseHookNames && hookNames === null, + [parseHookNames, hookNames], + ); - if (!hooks?.length) { - return No hooks changed; - } + if (!hooks?.length) { + return No hooks changed; + } + + // Fallback to old list of ids when inspectedElement ID doesn't match element ID or when hook counts differ + if ( + inspectedElement?.id !== element?.id || + filteredHooks?.length !== hooks.length || + displayMode === 'compact' + ) { + const hookIds = hooks.map(hookId => String(hookId + 1)); + const hookWord = hookIds.length === 1 ? '• Hook' : '• Hooks'; + return ( + + {hookWord} {hookListFormatter.format(hookIds)} changed + + ); + } + + let toggleTitle; + if (hookParsingFailed) { + toggleTitle = 'Hook parsing failed'; + } else if (parseHookNamesOptimistic) { + toggleTitle = 'Parsing hook names ...'; + } else { + toggleTitle = 'Parse hook names (may be slow)'; + } - // Fallback to old list of ids when inspectedElement ID doesn't match element ID or when hook counts differ - if ( - inspectedElement?.id !== element?.id || - filteredHooks?.length !== hooks.length - ) { - const hookIds = hooks.map(hookId => String(hookId + 1)); - const hookWord = hookIds.length === 1 ? '• Hook' : '• Hooks'; return ( - - {hookWord} {hookListFormatter.format(hookIds)} changed - +
+ {filteredHooks.length > 1 ? '• Hooks changed:' : '• Hook changed:'} + {(!parseHookNames || hookParsingFailed) && ( + + + + )} + {filteredHooks.map(hook => ( + + ))} +
); - } - - let toggleTitle; - if (hookParsingFailed) { - toggleTitle = 'Hook parsing failed'; - } else if (parseHookNamesOptimistic) { - toggleTitle = 'Parsing hook names ...'; - } else { - toggleTitle = 'Parse hook names (may be slow)'; - } - - return ( -
- {filteredHooks.length > 1 ? '• Hooks changed:' : '• Hook changed:'} - {(!parseHookNames || hookParsingFailed) && ( - - - - )} - {filteredHooks.map(hook => ( - - ))} -
- ); -}); + }, +); export default HookChangeSummary; diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/HoveredFiberInfo.js b/packages/react-devtools-shared/src/devtools/views/Profiler/HoveredFiberInfo.js index f1156a05aee14..a6dfeffb3e20e 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/HoveredFiberInfo.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/HoveredFiberInfo.js @@ -95,7 +95,7 @@ export default function HoveredFiberInfo({fiberData}: Props): React.Node {
{renderDurationInfo ||
Did not client render.
} - +
diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/WhatChanged.js b/packages/react-devtools-shared/src/devtools/views/Profiler/WhatChanged.js index 327bb5e955223..dffcd03350cda 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/WhatChanged.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/WhatChanged.js @@ -18,9 +18,13 @@ import HookChangeSummary from './HookChangeSummary'; type Props = { fiberID: number, + displayMode?: 'detailed' | 'compact', }; -export default function WhatChanged({fiberID}: Props): React.Node { +export default function WhatChanged({ + fiberID, + displayMode = 'detailed', +}: Props): React.Node { const {profilerStore} = useContext(StoreContext); const {rootID, selectedCommitIndex} = useContext(ProfilerContext); @@ -93,6 +97,7 @@ export default function WhatChanged({fiberID}: Props): React.Node { hooks={hooks} fiberID={fiberID} state={state} + displayMode={displayMode} />
, ); From 47b662a5a0838bce15d557ad02400f5e6894346d Mon Sep 17 00:00:00 2001 From: Piotr Tomczewski Date: Thu, 12 Dec 2024 14:57:19 +0100 Subject: [PATCH 4/5] types(profiler): fix Flow types in HookChangeSummary --- .../views/Profiler/HookChangeSummary.js | 62 +++++++++++++------ 1 file changed, 44 insertions(+), 18 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.js b/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.js index 8a5f92dfe434f..306d5e6ec207e 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.js @@ -1,3 +1,12 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + import * as React from 'react'; import { useContext, @@ -11,23 +20,27 @@ import styles from './HookChangeSummary.css'; import ButtonIcon from '../ButtonIcon'; import {InspectedElementContext} from '../Components/InspectedElementContext'; import {StoreContext} from '../context'; -import { - HooksList, - State, -} from 'react-devtools-shared/src/devtools/views/Profiler/types'; import { getAlreadyLoadedHookNames, getHookSourceLocationKey, } from 'react-devtools-shared/src/hookNamesCache'; import Toggle from '../Toggle'; +import type {HooksNode} from 'react-debug-tools/src/ReactDebugHooks'; +import type {ChangeDescription} from './types'; +// $FlowFixMe: Flow doesn't know about Intl.ListFormat const hookListFormatter = new Intl.ListFormat('en', { style: 'long', type: 'conjunction', }); -const Hook = memo(({hook, hookNames}) => { +type HookProps = { + hook: HooksNode, + hookNames: Map | null, +}; + +const Hook: React.AbstractComponent = memo(({hook, hookNames}) => { const hookSource = hook.hookSource; const hookName = useMemo(() => { if (!hookSource || !hookNames) return null; @@ -39,7 +52,9 @@ const Hook = memo(({hook, hookNames}) => {
  • {hook.id !== null && ( - {hook.id + 1} + + {String(hook.id + 1)} + )} @@ -58,7 +73,10 @@ const Hook = memo(({hook, hookNames}) => { ); }); -const shouldKeepHook = (hook, hooksArray) => { +const shouldKeepHook = ( + hook: HooksNode, + hooksArray: Array, +): boolean => { if (hook.id !== null && hooksArray.includes(hook.id)) { return true; } @@ -67,7 +85,10 @@ const shouldKeepHook = (hook, hooksArray) => { ); }; -const filterHooks = (hook, hooksArray) => { +const filterHooks = ( + hook: HooksNode, + hooksArray: Array, +): HooksNode | null => { if (!shouldKeepHook(hook, hooksArray)) { return null; } @@ -85,13 +106,14 @@ const filterHooks = (hook, hooksArray) => { return hook; }; -type Props = { +type Props = {| fiberID: number, - hooks: HooksList | null, - state: State | null, + hooks: $PropertyType, + state: $PropertyType, displayMode?: 'detailed' | 'compact', -}; -const HookChangeSummary = memo( +|}; + +const HookChangeSummary: React.AbstractComponent = memo( ({hooks, fiberID, state, displayMode = 'detailed'}: Props) => { const {parseHookNames, toggleParseHookNames, inspectedElement} = useContext( InspectedElementContext, @@ -99,7 +121,7 @@ const HookChangeSummary = memo( const store = useContext(StoreContext); const [parseHookNamesOptimistic, setParseHookNamesOptimistic] = - useState(parseHookNames); + useState(parseHookNames); useEffect(() => { setParseHookNamesOptimistic(parseHookNames); @@ -111,7 +133,8 @@ const HookChangeSummary = memo( }, [toggleParseHookNames, parseHookNames]); const element = fiberID !== null ? store.getElementByID(fiberID) : null; - const hookNames = getAlreadyLoadedHookNames(element); + const hookNames = + element != null ? getAlreadyLoadedHookNames(element) : null; const filteredHooks = useMemo(() => { if (!hooks || !inspectedElement?.hooks) return null; @@ -129,7 +152,6 @@ const HookChangeSummary = memo( return No hooks changed; } - // Fallback to old list of ids when inspectedElement ID doesn't match element ID or when hook counts differ if ( inspectedElement?.id !== element?.id || filteredHooks?.length !== hooks.length || @@ -144,7 +166,7 @@ const HookChangeSummary = memo( ); } - let toggleTitle; + let toggleTitle: string; if (hookParsingFailed) { toggleTitle = 'Hook parsing failed'; } else if (parseHookNamesOptimistic) { @@ -153,6 +175,10 @@ const HookChangeSummary = memo( toggleTitle = 'Parse hook names (may be slow)'; } + if (filteredHooks == null) { + return null; + } + return (
    {filteredHooks.length > 1 ? '• Hooks changed:' : '• Hook changed:'} @@ -172,7 +198,7 @@ const HookChangeSummary = memo( )} {filteredHooks.map(hook => ( From e24324f032b35ae1ecf49b662150d1757e6acfd8 Mon Sep 17 00:00:00 2001 From: Piotr Tomczewski Date: Mon, 10 Mar 2025 20:50:53 +0100 Subject: [PATCH 5/5] refactor(profiler): simplify key handling, subHooks filtering, and hook parsing logic --- .../views/Profiler/HookChangeSummary.js | 38 +++++++++---------- 1 file changed, 17 insertions(+), 21 deletions(-) diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.js b/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.js index 306d5e6ec207e..ed852c85c778a 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/HookChangeSummary.js @@ -62,11 +62,7 @@ const Hook: React.AbstractComponent = memo(({hook, hookNames}) => { {hookName && ({hookName})} {hook.subHooks?.map((subHook, index) => ( - + ))}
@@ -80,9 +76,12 @@ const shouldKeepHook = ( if (hook.id !== null && hooksArray.includes(hook.id)) { return true; } - return ( - hook.subHooks?.some(subHook => shouldKeepHook(subHook, hooksArray)) ?? false - ); + const subHooks = hook.subHooks; + if (subHooks == null) { + return false; + } + + return subHooks.some(subHook => shouldKeepHook(subHook, hooksArray)); }; const filterHooks = ( @@ -93,17 +92,17 @@ const filterHooks = ( return null; } - if (hook.subHooks?.length > 0) { - const filteredSubHooks = hook.subHooks - .map(subHook => filterHooks(subHook, hooksArray)) - .filter(Boolean); - - return filteredSubHooks.length > 0 - ? {...hook, subHooks: filteredSubHooks} - : {...hook}; + const subHooks = hook.subHooks; + if (subHooks == null) { + return hook; } - return hook; + const filteredSubHooks = subHooks + .map(subHook => filterHooks(subHook, hooksArray)) + .filter(Boolean); + return filteredSubHooks.length > 0 + ? {...hook, subHooks: filteredSubHooks} + : hook; }; type Props = {| @@ -143,10 +142,7 @@ const HookChangeSummary: React.AbstractComponent = memo( .filter(Boolean); }, [inspectedElement?.hooks, hooks]); - const hookParsingFailed = useMemo( - () => parseHookNames && hookNames === null, - [parseHookNames, hookNames], - ); + const hookParsingFailed = parseHookNames && hookNames === null; if (!hooks?.length) { return No hooks changed;