From d763f3131e689d077a93fd45c1fdf220be796279 Mon Sep 17 00:00:00 2001 From: emily8rown Date: Wed, 10 Dec 2025 13:07:35 +0000 Subject: [PATCH 1/2] [Devtools] Navigating commits performance panel hotkey (#35238) ## Summary Add keyboard shortcuts (Cmd/Ctrl + Left/Right arrow keys) to navigate between commits in the Profiler's snapshot view. Moved `filteredCommitIndices` management and commit navigation logic (`selectNextCommitIndex`, `selectPrevCommitIndex`) from `SnapshotSelector` into `useCommitFilteringAndNavigation` used by `ProfilerContext` to enable keyboard shortcuts from the top-level Profiler component. ## How did you test this change? - New tests in ProfilerContext-tests - Built browser extension: `yarn build:` - tested in browser: `yarn run test:` - Manually verified Left/Right arrow navigation cycles through commits - Verified navigation respects commit duration filter - Verified reload-and-profile button unaffected Chrome: https://github.com/user-attachments/assets/01d2a749-13dc-4d08-8bcb-3d4d45a5f97c Edge with duration filter: https://github.com/user-attachments/assets/a7f76ff7-2a0b-4b9c-a0ce-d4449373308b firefox mixing hotkey with clicking arrow buttons: https://github.com/user-attachments/assets/48912d68-7c75-40f2-a203-5e6d7e6b2d99 --- .../src/__tests__/profilerContext-test.js | 284 ++++++++++++++++++ .../src/devtools/views/Profiler/Profiler.js | 22 +- .../views/Profiler/ProfilerContext.js | 105 +++++-- .../views/Profiler/SnapshotSelector.js | 78 +---- .../useCommitFilteringAndNavigation.js | 199 ++++++++++++ 5 files changed, 595 insertions(+), 93 deletions(-) create mode 100644 packages/react-devtools-shared/src/devtools/views/Profiler/useCommitFilteringAndNavigation.js diff --git a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js index 2cd2340fb7a..7864f9703ea 100644 --- a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js @@ -655,4 +655,288 @@ describe('ProfilerContext', () => { document.body.removeChild(profilerContainer); }); + + it('should navigate between commits when the keyboard shortcut is pressed', async () => { + const Parent = () => ; + const Child = () => null; + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + utils.act(() => root.render()); + + // Profile and record multiple commits + await utils.actAsync(() => store.profilerStore.startProfiling()); + await utils.actAsync(() => root.render()); // Commit 1 + await utils.actAsync(() => root.render()); // Commit 2 + await utils.actAsync(() => root.render()); // Commit 3 + await utils.actAsync(() => store.profilerStore.stopProfiling()); + + const Profiler = + require('react-devtools-shared/src/devtools/views/Profiler/Profiler').default; + const { + TimelineContextController, + } = require('react-devtools-timeline/src/TimelineContext'); + const { + SettingsContextController, + } = require('react-devtools-shared/src/devtools/views/Settings/SettingsContext'); + const { + ModalDialogContextController, + } = require('react-devtools-shared/src/devtools/views/ModalDialog'); + + let context: Context = ((null: any): Context); + function ContextReader() { + context = React.useContext(ProfilerContext); + return null; + } + + const profilerContainer = document.createElement('div'); + document.body.appendChild(profilerContainer); + + const profilerRoot = ReactDOMClient.createRoot(profilerContainer); + + await utils.actAsync(() => { + profilerRoot.render( + + + + + + + + + + , + ); + }); + + // Verify we have profiling data with 3 commits + expect(context.didRecordCommits).toBe(true); + expect(context.profilingData).not.toBeNull(); + const rootID = context.rootID; + expect(rootID).not.toBeNull(); + const dataForRoot = context.profilingData.dataForRoots.get(rootID); + expect(dataForRoot.commitData.length).toBe(3); + // Should start at the first commit + expect(context.selectedCommitIndex).toBe(0); + + const ownerWindow = profilerContainer.ownerDocument.defaultView; + const isMac = + typeof navigator !== 'undefined' && + navigator.platform.toUpperCase().indexOf('MAC') >= 0; + + // Test ArrowRight navigation (forward) with correct modifier + const arrowRightEvent = new KeyboardEvent('keydown', { + key: 'ArrowRight', + metaKey: isMac, + ctrlKey: !isMac, + bubbles: true, + }); + + await utils.actAsync(() => { + ownerWindow.dispatchEvent(arrowRightEvent); + }, false); + expect(context.selectedCommitIndex).toBe(1); + + await utils.actAsync(() => { + ownerWindow.dispatchEvent(arrowRightEvent); + }, false); + expect(context.selectedCommitIndex).toBe(2); + + // Test wrap-around (last -> first) + await utils.actAsync(() => { + ownerWindow.dispatchEvent(arrowRightEvent); + }, false); + expect(context.selectedCommitIndex).toBe(0); + + // Test ArrowLeft navigation (backward) with correct modifier + const arrowLeftEvent = new KeyboardEvent('keydown', { + key: 'ArrowLeft', + metaKey: isMac, + ctrlKey: !isMac, + bubbles: true, + }); + + await utils.actAsync(() => { + ownerWindow.dispatchEvent(arrowLeftEvent); + }, false); + expect(context.selectedCommitIndex).toBe(2); + + await utils.actAsync(() => { + ownerWindow.dispatchEvent(arrowLeftEvent); + }, false); + expect(context.selectedCommitIndex).toBe(1); + + await utils.actAsync(() => { + ownerWindow.dispatchEvent(arrowLeftEvent); + }, false); + expect(context.selectedCommitIndex).toBe(0); + + // Cleanup + await utils.actAsync(() => profilerRoot.unmount()); + document.body.removeChild(profilerContainer); + }); + + it('should handle commit selection edge cases when filtering commits', async () => { + const Scheduler = require('scheduler'); + + // Create components that do varying amounts of work to generate different commit durations + const Parent = ({count}) => { + Scheduler.unstable_advanceTime(10); + const items = []; + for (let i = 0; i < count; i++) { + items.push(); + } + return
{items}
; + }; + const Child = ({duration}) => { + Scheduler.unstable_advanceTime(duration); + return {duration}; + }; + + const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); + utils.act(() => root.render()); + + // Profile and record multiple commits with different amounts of work + await utils.actAsync(() => store.profilerStore.startProfiling()); + await utils.actAsync(() => root.render()); // Commit 1 - 20ms + await utils.actAsync(() => root.render()); // Commit 2 - 200ms + await utils.actAsync(() => root.render()); // Commit 3 - 1235ms + await utils.actAsync(() => root.render()); // Commit 4 - 55ms + await utils.actAsync(() => store.profilerStore.stopProfiling()); + + // Context providers + const Profiler = + require('react-devtools-shared/src/devtools/views/Profiler/Profiler').default; + const { + TimelineContextController, + } = require('react-devtools-timeline/src/TimelineContext'); + const { + SettingsContextController, + } = require('react-devtools-shared/src/devtools/views/Settings/SettingsContext'); + const { + ModalDialogContextController, + } = require('react-devtools-shared/src/devtools/views/ModalDialog'); + + let context: Context = ((null: any): Context); + function ContextReader() { + context = React.useContext(ProfilerContext); + return null; + } + + const profilerContainer = document.createElement('div'); + document.body.appendChild(profilerContainer); + + const profilerRoot = ReactDOMClient.createRoot(profilerContainer); + + await utils.actAsync(() => { + profilerRoot.render( + + + + + + + + + + , + ); + }); + + // Verify we have profiling data with 4 commits + expect(context.didRecordCommits).toBe(true); + expect(context.profilingData).not.toBeNull(); + const rootID = context.rootID; + expect(rootID).not.toBeNull(); + const dataForRoot = context.profilingData.dataForRoots.get(rootID); + expect(dataForRoot.commitData.length).toBe(4); + // Edge case 1: Should start at the first commit + expect(context.selectedCommitIndex).toBe(0); + + const ownerWindow = profilerContainer.ownerDocument.defaultView; + const isMac = + typeof navigator !== 'undefined' && + navigator.platform.toUpperCase().indexOf('MAC') >= 0; + + const arrowRightEvent = new KeyboardEvent('keydown', { + key: 'ArrowRight', + metaKey: isMac, + ctrlKey: !isMac, + bubbles: true, + }); + + await utils.actAsync(() => { + ownerWindow.dispatchEvent(arrowRightEvent); + }, false); + expect(context.selectedCommitIndex).toBe(1); + + await utils.actAsync(() => { + context.setIsCommitFilterEnabled(true); + }); + + // Edge case 2: When filtering is enabled, selected commit should remain if it's still visible + expect(context.filteredCommitIndices.length).toBe(4); + expect(context.selectedCommitIndex).toBe(1); + expect(context.selectedFilteredCommitIndex).toBe(1); + + await utils.actAsync(() => { + context.setMinCommitDuration(1000000); + }); + + // Edge case 3: When all commits are filtered out, selection should be null + expect(context.filteredCommitIndices).toEqual([]); + expect(context.selectedCommitIndex).toBe(null); + expect(context.selectedFilteredCommitIndex).toBe(null); + + await utils.actAsync(() => { + context.setMinCommitDuration(0); + }); + + // Edge case 4: After restoring commits, first commit should be auto-selected + expect(context.filteredCommitIndices.length).toBe(4); + expect(context.selectedCommitIndex).toBe(0); + expect(context.selectedFilteredCommitIndex).toBe(0); + + await utils.actAsync(() => { + ownerWindow.dispatchEvent(arrowRightEvent); + }, false); + expect(context.selectedCommitIndex).toBe(1); + + await utils.actAsync(() => { + ownerWindow.dispatchEvent(arrowRightEvent); + }, false); + expect(context.selectedCommitIndex).toBe(2); + + await utils.actAsync(() => { + ownerWindow.dispatchEvent(arrowRightEvent); + }, false); + expect(context.selectedCommitIndex).toBe(3); + + // Filter out the currently selected commit using actual commit data + const commitDurations = dataForRoot.commitData.map( + commit => commit.duration, + ); + const selectedCommitDuration = commitDurations[3]; + const filterThreshold = selectedCommitDuration + 0.001; + await utils.actAsync(() => { + context.setMinCommitDuration(filterThreshold); + }); + + // Edge case 5: Should auto-select first available commit when current one is filtered + expect(context.selectedCommitIndex).not.toBe(null); + expect(context.selectedFilteredCommitIndex).toBe(1); + + await utils.actAsync(() => { + context.setIsCommitFilterEnabled(false); + }); + + // Edge case 6: When filtering is disabled, selected commit should remain + expect(context.filteredCommitIndices.length).toBe(4); + expect(context.selectedCommitIndex).toBe(2); + expect(context.selectedFilteredCommitIndex).toBe(2); + + await utils.actAsync(() => profilerRoot.unmount()); + document.body.removeChild(profilerContainer); + }); }); diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/Profiler.js b/packages/react-devtools-shared/src/devtools/views/Profiler/Profiler.js index 5e330637fd9..6254be06d83 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/Profiler.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/Profiler.js @@ -54,6 +54,8 @@ function Profiler(_: {}) { supportsProfiling, startProfiling, stopProfiling, + selectPrevCommitIndex, + selectNextCommitIndex, } = useContext(ProfilerContext); const {file: timelineTraceEventData, searchInputContainerRef} = @@ -63,9 +65,9 @@ function Profiler(_: {}) { const isLegacyProfilerSelected = selectedTabID !== 'timeline'; - // Cmd+E to start/stop profiler recording const handleKeyDown = useEffectEvent((event: KeyboardEvent) => { const correctModifier = isMac ? event.metaKey : event.ctrlKey; + // Cmd+E to start/stop profiler recording if (correctModifier && event.key === 'e') { if (isProfiling) { stopProfiling(); @@ -74,6 +76,24 @@ function Profiler(_: {}) { } event.preventDefault(); event.stopPropagation(); + } else if ( + isLegacyProfilerSelected && + didRecordCommits && + selectedCommitIndex !== null + ) { + // Cmd+Left/Right (Mac) or Ctrl+Left/Right (Windows/Linux) to navigate commits + if ( + correctModifier && + (event.key === 'ArrowLeft' || event.key === 'ArrowRight') + ) { + if (event.key === 'ArrowLeft') { + selectPrevCommitIndex(); + } else { + selectNextCommitIndex(); + } + event.preventDefault(); + event.stopPropagation(); + } } }); 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 a9225c3c8fe..d593558dbdd 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js @@ -10,7 +10,14 @@ import type {ReactContext} from 'shared/ReactTypes'; import * as React from 'react'; -import {createContext, useCallback, useContext, useMemo, useState} from 'react'; +import { + createContext, + useCallback, + useContext, + useMemo, + useState, + useEffect, +} from 'react'; import {useLocalStorage, useSubscription} from '../hooks'; import { TreeDispatcherContext, @@ -18,8 +25,9 @@ import { } from '../Components/TreeContext'; import {StoreContext} from '../context'; import {logEvent} from 'react-devtools-shared/src/Logger'; +import {useCommitFilteringAndNavigation} from './useCommitFilteringAndNavigation'; -import type {ProfilingDataFrontend} from './types'; +import type {CommitDataFrontend, ProfilingDataFrontend} from './types'; export type TabID = 'flame-chart' | 'ranked-chart' | 'timeline'; @@ -61,6 +69,12 @@ export type Context = { // It impacts the flame graph and ranked charts. selectedCommitIndex: number | null, selectCommitIndex: (value: number | null) => void, + selectNextCommitIndex(): void, + selectPrevCommitIndex(): void, + + // Which commits are currently filtered by duration? + filteredCommitIndices: Array, + selectedFilteredCommitIndex: number | null, // Which fiber is currently selected in the Ranked or Flamegraph charts? selectedFiberID: number | null, @@ -164,6 +178,7 @@ function ProfilerContextController({children}: Props): React.Node { [setRootID, selectFiber], ); + // Sync rootID with profilingData changes. if (prevProfilingData !== profilingData) { setPrevProfilingData(profilingData); @@ -189,16 +204,6 @@ function ProfilerContextController({children}: Props): React.Node { } } - const [isCommitFilterEnabled, setIsCommitFilterEnabled] = - useLocalStorage('React::DevTools::isCommitFilterEnabled', false); - const [minCommitDuration, setMinCommitDuration] = useLocalStorage( - 'minCommitDuration', - 0, - ); - - const [selectedCommitIndex, selectCommitIndex] = useState( - null, - ); const [selectedTabID, selectTab] = useLocalStorage( 'React::DevTools::Profiler::defaultTab', 'flame-chart', @@ -212,27 +217,66 @@ function ProfilerContextController({children}: Props): React.Node { }, ); - const startProfiling = useCallback(() => { - logEvent({ - event_name: 'profiling-start', - metadata: {current_tab: selectedTabID}, - }); - store.profilerStore.startProfiling(); - }, [store, selectedTabID]); const stopProfiling = useCallback( () => store.profilerStore.stopProfiling(), [store], ); - if (isProfiling) { - if (selectedCommitIndex !== null) { - selectCommitIndex(null); + // Get commit data for the current root + // NOTE: Unlike profilerStore.getDataForRoot() which uses Suspense (throws when data unavailable), + // this uses subscription pattern and returns [] when data isn't ready. + // Always check didRecordCommits before using commitData or filteredCommitIndices. + const commitData = useMemo(() => { + if (!didRecordCommits || rootID === null || profilingData === null) { + return ([]: Array); } - if (selectedFiberID !== null) { - selectFiberID(null); - selectFiberName(null); + const dataForRoot = profilingData.dataForRoots.get(rootID); + return dataForRoot + ? dataForRoot.commitData + : ([]: Array); + }, [didRecordCommits, rootID, profilingData]); + + // Commit filtering and navigation + const { + isCommitFilterEnabled, + setIsCommitFilterEnabled, + minCommitDuration, + setMinCommitDuration, + selectedCommitIndex, + selectCommitIndex, + filteredCommitIndices, + selectedFilteredCommitIndex, + selectNextCommitIndex, + selectPrevCommitIndex, + } = useCommitFilteringAndNavigation(commitData); + + const startProfiling = useCallback(() => { + logEvent({ + event_name: 'profiling-start', + metadata: {current_tab: selectedTabID}, + }); + + // Clear selections when starting a new profiling session + selectCommitIndex(null); + selectFiberID(null); + selectFiberName(null); + + store.profilerStore.startProfiling(); + }, [store, selectedTabID, selectCommitIndex]); + + // Auto-select first commit when profiling data becomes available and no commit is selected. + useEffect(() => { + if ( + profilingData !== null && + selectedCommitIndex === null && + rootID !== null + ) { + const dataForRoot = profilingData.dataForRoots.get(rootID); + if (dataForRoot && dataForRoot.commitData.length > 0) { + selectCommitIndex(0); + } } - } + }, [profilingData, rootID, selectCommitIndex]); const value = useMemo( () => ({ @@ -257,6 +301,10 @@ function ProfilerContextController({children}: Props): React.Node { selectedCommitIndex, selectCommitIndex, + selectNextCommitIndex, + selectPrevCommitIndex, + filteredCommitIndices, + selectedFilteredCommitIndex, selectedFiberID, selectedFiberName, @@ -275,7 +323,6 @@ function ProfilerContextController({children}: Props): React.Node { supportsProfiling, rootID, - setRootID, setRootIDAndClearFiber, isCommitFilterEnabled, @@ -285,6 +332,10 @@ function ProfilerContextController({children}: Props): React.Node { selectedCommitIndex, selectCommitIndex, + selectNextCommitIndex, + selectPrevCommitIndex, + filteredCommitIndices, + selectedFilteredCommitIndex, selectedFiberID, selectedFiberName, diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/SnapshotSelector.js b/packages/react-devtools-shared/src/devtools/views/Profiler/SnapshotSelector.js index 088ca0325d0..0d470ddb000 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/SnapshotSelector.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/SnapshotSelector.js @@ -8,7 +8,7 @@ */ import * as React from 'react'; -import {Fragment, useContext, useMemo} from 'react'; +import {Fragment, useContext} from 'react'; import Button from '../Button'; import ButtonIcon from '../ButtonIcon'; import {ProfilerContext} from './ProfilerContext'; @@ -22,11 +22,13 @@ export type Props = {}; export default function SnapshotSelector(_: Props): React.Node { const { - isCommitFilterEnabled, - minCommitDuration, rootID, selectedCommitIndex, selectCommitIndex, + selectPrevCommitIndex, + selectNextCommitIndex, + filteredCommitIndices, + selectedFilteredCommitIndex, } = useContext(ProfilerContext); const {profilerStore} = useContext(StoreContext); @@ -43,47 +45,8 @@ export default function SnapshotSelector(_: Props): React.Node { commitTimes.push(commitDatum.timestamp); }); - const filteredCommitIndices = useMemo( - () => - commitData.reduce((reduced: $FlowFixMe, commitDatum, index) => { - if ( - !isCommitFilterEnabled || - commitDatum.duration >= minCommitDuration - ) { - reduced.push(index); - } - return reduced; - }, []), - [commitData, isCommitFilterEnabled, minCommitDuration], - ); - const numFilteredCommits = filteredCommitIndices.length; - // Map the (unfiltered) selected commit index to an index within the filtered data. - const selectedFilteredCommitIndex = useMemo(() => { - if (selectedCommitIndex !== null) { - for (let i = 0; i < filteredCommitIndices.length; i++) { - if (filteredCommitIndices[i] === selectedCommitIndex) { - return i; - } - } - } - return null; - }, [filteredCommitIndices, selectedCommitIndex]); - - // TODO (ProfilerContext) This should be managed by the context controller (reducer). - // It doesn't currently know about the filtered commits though (since it doesn't suspend). - // Maybe this component should pass filteredCommitIndices up? - if (selectedFilteredCommitIndex === null) { - if (numFilteredCommits > 0) { - selectCommitIndex(0); - } else { - selectCommitIndex(null); - } - } else if (selectedFilteredCommitIndex >= numFilteredCommits) { - selectCommitIndex(numFilteredCommits === 0 ? null : numFilteredCommits - 1); - } - let label = null; if (numFilteredCommits > 0) { // $FlowFixMe[missing-local-annot] @@ -110,11 +73,11 @@ export default function SnapshotSelector(_: Props): React.Node { const handleKeyDown = event => { switch (event.key) { case 'ArrowDown': - viewPrevCommit(); + selectPrevCommitIndex(); event.stopPropagation(); break; case 'ArrowUp': - viewNextCommit(); + selectNextCommitIndex(); event.stopPropagation(); break; default: @@ -147,30 +110,15 @@ export default function SnapshotSelector(_: Props): React.Node { ); } - const viewNextCommit = () => { - let nextCommitIndex = ((selectedFilteredCommitIndex: any): number) + 1; - if (nextCommitIndex === filteredCommitIndices.length) { - nextCommitIndex = 0; - } - selectCommitIndex(filteredCommitIndices[nextCommitIndex]); - }; - const viewPrevCommit = () => { - let nextCommitIndex = ((selectedFilteredCommitIndex: any): number) - 1; - if (nextCommitIndex < 0) { - nextCommitIndex = filteredCommitIndices.length - 1; - } - selectCommitIndex(filteredCommitIndices[nextCommitIndex]); - }; - // $FlowFixMe[missing-local-annot] const handleKeyDown = event => { switch (event.key) { case 'ArrowLeft': - viewPrevCommit(); + selectPrevCommitIndex(); event.stopPropagation(); break; case 'ArrowRight': - viewNextCommit(); + selectNextCommitIndex(); event.stopPropagation(); break; default: @@ -193,8 +141,8 @@ export default function SnapshotSelector(_: Props): React.Node { className={styles.Button} data-testname="SnapshotSelector-PreviousButton" disabled={numFilteredCommits === 0} - onClick={viewPrevCommit} - title="Select previous commit"> + onClick={selectPrevCommitIndex} + title="Select previous commit ←">
+ onClick={selectNextCommitIndex} + title="Select next commit →"> diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/useCommitFilteringAndNavigation.js b/packages/react-devtools-shared/src/devtools/views/Profiler/useCommitFilteringAndNavigation.js new file mode 100644 index 00000000000..e309e6a3cf3 --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/useCommitFilteringAndNavigation.js @@ -0,0 +1,199 @@ +/** + * 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 {useCallback, useMemo, useState} from 'react'; +import {useLocalStorage} from '../hooks'; + +import type {CommitDataFrontend} from './types'; + +export type CommitFilteringAndNavigation = { + isCommitFilterEnabled: boolean, + setIsCommitFilterEnabled: (value: boolean) => void, + minCommitDuration: number, + setMinCommitDuration: (value: number) => void, + + // Selection state + selectedCommitIndex: number | null, + selectCommitIndex: (value: number | null) => void, + + // Filtered data + filteredCommitIndices: Array, + selectedFilteredCommitIndex: number | null, + + // Navigation + selectNextCommitIndex: () => void, + selectPrevCommitIndex: () => void, +}; + +export function useCommitFilteringAndNavigation( + commitData: Array, +): CommitFilteringAndNavigation { + // Filter settings persisted to localStorage + const [isCommitFilterEnabled, setIsCommitFilterEnabledValue] = + useLocalStorage('React::DevTools::isCommitFilterEnabled', false); + const [minCommitDuration, setMinCommitDurationValue] = + useLocalStorage('minCommitDuration', 0); + + // Currently selected commit index (in the unfiltered list) + const [selectedCommitIndex, selectCommitIndex] = useState( + null, + ); + + const calculateFilteredIndices = useCallback( + (enabled: boolean, minDuration: number): Array => { + return commitData.reduce((reduced: Array, commitDatum, index) => { + if (!enabled || commitDatum.duration >= minDuration) { + reduced.push(index); + } + return reduced; + }, ([]: Array)); + }, + [commitData], + ); + + const findFilteredIndex = useCallback( + (commitIndex: number | null, filtered: Array): number | null => { + if (commitIndex === null) return null; + for (let i = 0; i < filtered.length; i++) { + if (filtered[i] === commitIndex) { + return i; + } + } + return null; + }, + [], + ); + + // Adjust selection when filter settings change to keep a valid selection + const adjustSelectionAfterFilterChange = useCallback( + (newFilteredIndices: Array) => { + const currentSelectedIndex = selectedCommitIndex; + const selectedFilteredIndex = findFilteredIndex( + currentSelectedIndex, + newFilteredIndices, + ); + + if (newFilteredIndices.length === 0) { + // No commits pass the filter - clear selection + selectCommitIndex(null); + } else if (currentSelectedIndex === null) { + // No commit was selected - select first available + selectCommitIndex(newFilteredIndices[0]); + } else if (selectedFilteredIndex === null) { + // Currently selected commit was filtered out - find closest commit before it + let closestBefore = null; + for (let i = newFilteredIndices.length - 1; i >= 0; i--) { + if (newFilteredIndices[i] < currentSelectedIndex) { + closestBefore = newFilteredIndices[i]; + break; + } + } + // If no commit before it, use the first available + selectCommitIndex( + closestBefore !== null ? closestBefore : newFilteredIndices[0], + ); + } else if (selectedFilteredIndex >= newFilteredIndices.length) { + // Filtered position is out of bounds - clamp to last available + selectCommitIndex(newFilteredIndices[newFilteredIndices.length - 1]); + } + // Otherwise, the current selection is still valid in the filtered list, keep it + }, + [findFilteredIndex, selectedCommitIndex, selectCommitIndex], + ); + + const filteredCommitIndices = useMemo( + () => calculateFilteredIndices(isCommitFilterEnabled, minCommitDuration), + [calculateFilteredIndices, isCommitFilterEnabled, minCommitDuration], + ); + + const selectedFilteredCommitIndex = useMemo( + () => findFilteredIndex(selectedCommitIndex, filteredCommitIndices), + [findFilteredIndex, selectedCommitIndex, filteredCommitIndices], + ); + + const selectNextCommitIndex = useCallback(() => { + if ( + selectedFilteredCommitIndex === null || + filteredCommitIndices.length === 0 + ) { + return; + } + let nextCommitIndex = selectedFilteredCommitIndex + 1; + if (nextCommitIndex === filteredCommitIndices.length) { + nextCommitIndex = 0; + } + selectCommitIndex(filteredCommitIndices[nextCommitIndex]); + }, [selectedFilteredCommitIndex, filteredCommitIndices, selectCommitIndex]); + + const selectPrevCommitIndex = useCallback(() => { + if ( + selectedFilteredCommitIndex === null || + filteredCommitIndices.length === 0 + ) { + return; + } + let prevCommitIndex = selectedFilteredCommitIndex - 1; + if (prevCommitIndex < 0) { + prevCommitIndex = filteredCommitIndices.length - 1; + } + selectCommitIndex(filteredCommitIndices[prevCommitIndex]); + }, [selectedFilteredCommitIndex, filteredCommitIndices, selectCommitIndex]); + + // Setters that also adjust selection when filter changes + const setIsCommitFilterEnabled = useCallback( + (value: boolean) => { + setIsCommitFilterEnabledValue(value); + + const newFilteredIndices = calculateFilteredIndices( + value, + minCommitDuration, + ); + + adjustSelectionAfterFilterChange(newFilteredIndices); + }, + [ + setIsCommitFilterEnabledValue, + calculateFilteredIndices, + minCommitDuration, + adjustSelectionAfterFilterChange, + ], + ); + + const setMinCommitDuration = useCallback( + (value: number) => { + setMinCommitDurationValue(value); + + const newFilteredIndices = calculateFilteredIndices( + isCommitFilterEnabled, + value, + ); + + adjustSelectionAfterFilterChange(newFilteredIndices); + }, + [ + setMinCommitDurationValue, + calculateFilteredIndices, + isCommitFilterEnabled, + adjustSelectionAfterFilterChange, + ], + ); + + return { + isCommitFilterEnabled, + setIsCommitFilterEnabled, + minCommitDuration, + setMinCommitDuration, + selectedCommitIndex, + selectCommitIndex, + filteredCommitIndices, + selectedFilteredCommitIndex, + selectNextCommitIndex, + selectPrevCommitIndex, + }; +} From eade0d0fb78e327c5624f53753126687f05c0d16 Mon Sep 17 00:00:00 2001 From: Jack Pope Date: Wed, 10 Dec 2025 13:35:20 -0500 Subject: [PATCH 2/2] Attach instance handle to DOM in DEV for enableInternalInstanceMap (#35341) Continue attaching `internalInstanceKey` to DOM nodes in DEV. This prevents breaking some internal dev tooling while we experiment with the broader change. Note that this does not reference the DOM handle within the flag, just attaches it and deletes it. Internals tracking is still done through the private map. --- .../react-dom-bindings/src/client/ReactDOMComponentTree.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponentTree.js b/packages/react-dom-bindings/src/client/ReactDOMComponentTree.js index 2063e5ef2f5..01e4fcde95c 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponentTree.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponentTree.js @@ -75,6 +75,9 @@ export function detachDeletedInstance(node: Instance): void { delete (node: any)[internalEventHandlerListenersKey]; delete (node: any)[internalEventHandlesSetKey]; delete (node: any)[internalRootNodeResourcesKey]; + if (__DEV__) { + delete (node: any)[internalInstanceKey]; + } return; } // TODO: This function is only called on host components. I don't think all of @@ -97,6 +100,9 @@ export function precacheFiberNode( ): void { if (enableInternalInstanceMap) { internalInstanceMap.set(node, hostInst); + if (__DEV__) { + (node: any)[internalInstanceKey] = hostInst; + } return; } (node: any)[internalInstanceKey] = hostInst;