From 557c8003b095017348c7970eaab9dd11de5c8596 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Thu, 27 Nov 2025 18:18:16 +0000 Subject: [PATCH 01/27] add shortcut to navigate between commits in the performance panel --- .../src/__tests__/profilerContext-test.js | 116 +++++++++++++++++- .../src/devtools/views/Profiler/Profiler.js | 13 ++ .../views/Profiler/ProfilerContext.js | 75 +++++++++++ .../views/Profiler/SnapshotSelector.js | 67 ++-------- 4 files changed, 216 insertions(+), 55 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js index 2cd2340fb7a..9f02694fd85 100644 --- a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js @@ -655,4 +655,118 @@ 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'); + utils.act(() => legacyRender(, container)); + + // Profile and record multiple commits + await utils.actAsync(() => store.profilerStore.startProfiling()); + await utils.actAsync(() => legacyRender(, container)); // Commit 1 + await utils.actAsync(() => legacyRender(, container)); // Commit 2 + await utils.actAsync(() => legacyRender(, container)); // Commit 3 + 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); + + // Create a root for the profiler + const profilerRoot = ReactDOMClient.createRoot(profilerContainer); + + // Render the profiler with ContextReader + 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); + + // Set initial commit selection + await utils.actAsync(() => context.selectCommitIndex(0)); + expect(context.selectedCommitIndex).toBe(0); + + const ownerWindow = profilerContainer.ownerDocument.defaultView; + + // Test ArrowRight navigation (forward) + const arrowRightEvent = new KeyboardEvent('keydown', { + key: 'ArrowRight', + 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) + const arrowLeftEvent = new KeyboardEvent('keydown', { + key: 'ArrowLeft', + 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); + + 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..b7f979d2017 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} = @@ -74,9 +76,20 @@ function Profiler(_: {}) { } event.preventDefault(); event.stopPropagation(); + } else if (isLegacyProfilerSelected && didRecordCommits && selectedCommitIndex !== null) { + if (event.key === 'ArrowLeft' || event.key === 'ArrowRight') { + if (event.key === 'ArrowLeft') { + selectPrevCommitIndex(); + } else { + selectNextCommitIndex(); + } + event.preventDefault(); + event.stopPropagation(); + } } }); + useEffect(() => { const div = profilerRef.current; if (!div) { 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..d4fc906e6ea 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js @@ -61,6 +61,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, @@ -234,6 +240,67 @@ function ProfilerContextController({children}: Props): React.Node { } } + // 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 []; + } + const dataForRoot = profilingData.dataForRoots.get(rootID); + return dataForRoot ? dataForRoot.commitData : []; + }, [didRecordCommits, rootID, profilingData]); + + const filteredCommitIndices = useMemo( + () => + commitData.reduce((reduced, commitDatum, index) => { + if ( + !isCommitFilterEnabled || + commitDatum.duration >= minCommitDuration + ) { + reduced.push(index); + } + return reduced; + }, []), + [commitData, isCommitFilterEnabled, minCommitDuration], + ); + +const selectedFilteredCommitIndex = useMemo(() => { + if (selectedCommitIndex !== null) { + for (let i = 0; i < filteredCommitIndices.length; i++) { + if (filteredCommitIndices[i] === selectedCommitIndex) { + return i; + } + } + } + return null; +}, [filteredCommitIndices, selectedCommitIndex]); + + + 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]); + + const value = useMemo( () => ({ selectedTabID, @@ -257,6 +324,10 @@ function ProfilerContextController({children}: Props): React.Node { selectedCommitIndex, selectCommitIndex, + selectNextCommitIndex, + selectPrevCommitIndex, + filteredCommitIndices, + selectedFilteredCommitIndex, selectedFiberID, selectedFiberName, @@ -285,6 +356,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..ba135e217de 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,37 +45,9 @@ 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? + // Handle edge cases where selected commit becomes invalid after filtering if (selectedFilteredCommitIndex === null) { if (numFilteredCommits > 0) { selectCommitIndex(0); @@ -110,11 +84,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 +121,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,7 +152,7 @@ export default function SnapshotSelector(_: Props): React.Node { className={styles.Button} data-testname="SnapshotSelector-PreviousButton" disabled={numFilteredCommits === 0} - onClick={viewPrevCommit} + onClick={selectPrevCommitIndex} title="Select previous commit"> @@ -227,8 +186,8 @@ export default function SnapshotSelector(_: Props): React.Node { className={styles.Button} data-testname="SnapshotSelector-NextButton" disabled={numFilteredCommits === 0} - onClick={viewNextCommit} - title="Select next commit"> + onClick={selectNextCommitIndex} + title="Select next commit ->"> From 6a7dc2f05cca1655e582ad583afd1af6be5afde4 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Thu, 27 Nov 2025 18:22:30 +0000 Subject: [PATCH 02/27] add indicator into tooltips for the hotkeys --- .../src/devtools/views/Profiler/SnapshotSelector.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 ba135e217de..aad0671b84b 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/SnapshotSelector.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/SnapshotSelector.js @@ -153,7 +153,7 @@ export default function SnapshotSelector(_: Props): React.Node { data-testname="SnapshotSelector-PreviousButton" disabled={numFilteredCommits === 0} onClick={selectPrevCommitIndex} - title="Select previous commit"> + title="Select previous commit - Left arrow">
+ title="Select next commit - Right arrow"> From 99f12da251acf43f474a464d10bb8f2f0320e1bb Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Thu, 27 Nov 2025 18:24:18 +0000 Subject: [PATCH 03/27] yarn prettier --- .../src/__tests__/console-test.js | 27 +++----- .../src/__tests__/profilerContext-test.js | 1 + .../src/devtools/views/Profiler/Profiler.js | 7 +- .../views/Profiler/ProfilerContext.js | 68 ++++++++++--------- .../react-reconciler/src/ReactFiberHooks.js | 4 +- 5 files changed, 53 insertions(+), 54 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/console-test.js b/packages/react-devtools-shared/src/__tests__/console-test.js index 00d6d971267..1debfadc62c 100644 --- a/packages/react-devtools-shared/src/__tests__/console-test.js +++ b/packages/react-devtools-shared/src/__tests__/console-test.js @@ -363,8 +363,7 @@ describe('console', () => { it('should double log if hideConsoleLogsInStrictMode is disabled in Strict mode', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = false; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = - false; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = false; const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); @@ -405,8 +404,7 @@ describe('console', () => { it('should not double log if hideConsoleLogsInStrictMode is enabled in Strict mode', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = false; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = - true; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = true; const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); @@ -433,8 +431,7 @@ describe('console', () => { it('should double log from Effects if hideConsoleLogsInStrictMode is disabled in Strict mode', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = false; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = - false; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = false; const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); @@ -481,8 +478,7 @@ describe('console', () => { it('should not double log from Effects if hideConsoleLogsInStrictMode is enabled in Strict mode', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = false; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = - true; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = true; const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); @@ -518,8 +514,7 @@ describe('console', () => { it('should double log from useMemo if hideConsoleLogsInStrictMode is disabled in Strict mode', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = false; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = - false; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = false; const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); @@ -562,8 +557,7 @@ describe('console', () => { it('should not double log from useMemo fns if hideConsoleLogsInStrictMode is enabled in Strict mode', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = false; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = - true; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = true; const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); @@ -592,8 +586,7 @@ describe('console', () => { it('should double log in Strict mode initial render for extension', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = false; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = - false; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = false; // This simulates a render that happens before React DevTools have finished // their handshake to attach the React DOM renderer functions to DevTools @@ -638,8 +631,7 @@ describe('console', () => { it('should not double log in Strict mode initial render for extension', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = false; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = - true; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = true; // This simulates a render that happens before React DevTools have finished // their handshake to attach the React DOM renderer functions to DevTools @@ -670,8 +662,7 @@ describe('console', () => { it('should properly dim component stacks during strict mode double log', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = true; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = - false; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = false; const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); diff --git a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js index 9f02694fd85..999c8874aff 100644 --- a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js @@ -770,3 +770,4 @@ describe('ProfilerContext', () => { 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 b7f979d2017..8c7fbf0cad2 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/Profiler.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/Profiler.js @@ -76,7 +76,11 @@ function Profiler(_: {}) { } event.preventDefault(); event.stopPropagation(); - } else if (isLegacyProfilerSelected && didRecordCommits && selectedCommitIndex !== null) { + } else if ( + isLegacyProfilerSelected && + didRecordCommits && + selectedCommitIndex !== null + ) { if (event.key === 'ArrowLeft' || event.key === 'ArrowRight') { if (event.key === 'ArrowLeft') { selectPrevCommitIndex(); @@ -89,7 +93,6 @@ function Profiler(_: {}) { } }); - useEffect(() => { const div = profilerRef.current; if (!div) { 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 d4fc906e6ea..47a58c6c4cd 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js @@ -266,40 +266,44 @@ function ProfilerContextController({children}: Props): React.Node { [commitData, isCommitFilterEnabled, minCommitDuration], ); -const selectedFilteredCommitIndex = useMemo(() => { - if (selectedCommitIndex !== null) { - for (let i = 0; i < filteredCommitIndices.length; i++) { - if (filteredCommitIndices[i] === selectedCommitIndex) { - return i; + const selectedFilteredCommitIndex = useMemo(() => { + if (selectedCommitIndex !== null) { + for (let i = 0; i < filteredCommitIndices.length; i++) { + if (filteredCommitIndices[i] === selectedCommitIndex) { + return i; + } } } - } - return null; -}, [filteredCommitIndices, selectedCommitIndex]); - - - 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]); - + return null; + }, [filteredCommitIndices, selectedCommitIndex]); + + 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]); const value = useMemo( () => ({ diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 66a390ebb81..774c6f85407 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -3816,8 +3816,8 @@ function enqueueRenderPhaseUpdate( // This is a render phase update. Stash it in a lazily-created map of // queue -> linked list of updates. After this render pass, we'll restart // and apply the stashed updates on top of the work-in-progress hook. - didScheduleRenderPhaseUpdateDuringThisPass = didScheduleRenderPhaseUpdate = - true; + didScheduleRenderPhaseUpdateDuringThisPass = + didScheduleRenderPhaseUpdate = true; const pending = queue.pending; if (pending === null) { // This is the first update. Create a circular list. From 3ac348323d570842d3d48859792351e2760bde18 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Thu, 27 Nov 2025 18:33:42 +0000 Subject: [PATCH 04/27] changed tooltip for left and right arrow to symbols --- .../src/devtools/views/Profiler/SnapshotSelector.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 aad0671b84b..df8b43ce8b1 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/SnapshotSelector.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/SnapshotSelector.js @@ -153,7 +153,7 @@ export default function SnapshotSelector(_: Props): React.Node { data-testname="SnapshotSelector-PreviousButton" disabled={numFilteredCommits === 0} onClick={selectPrevCommitIndex} - title="Select previous commit - Left arrow"> + title="Select previous commit ←">
+ title="Select next commit →"> From d53d7a5b818d622881483fb62062d6f1f4d43d62 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Fri, 28 Nov 2025 14:15:04 +0000 Subject: [PATCH 05/27] move state management to profiler context from snapshot selector --- .../src/__tests__/profilerContext-test.js | 93 +++++++++++++++++++ .../src/devtools/views/Profiler/Profiler.js | 3 +- .../views/Profiler/ProfilerContext.js | 28 +++++- .../views/Profiler/SnapshotSelector.js | 11 --- 4 files changed, 118 insertions(+), 17 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js index 999c8874aff..4b3fc140fd2 100644 --- a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js @@ -770,4 +770,97 @@ describe('ProfilerContext', () => { document.body.removeChild(profilerContainer); }); + + it('should handle commit selection edge cases when filtering commits', async () => { + // Create components that render with different durations + const FastComponent = () => null; + const SlowComponent = () => { + // Simulate slow render + const start = performance.now(); + while (performance.now() - start < 20) { + // Busy wait + } + return null; + }; + + const container = document.createElement('div'); + + // Initial render + utils.act(() => legacyRender(, container)); + + let context: Context = ((null: any): Context); + function ContextReader() { + context = React.useContext(ProfilerContext); + return null; + } + + await utils.actAsync(() => + TestRenderer.create( + + + , + ), + ); + + // Profile with multiple commits of varying durations + await utils.actAsync(() => store.profilerStore.startProfiling()); + await utils.actAsync(() => legacyRender(, container)); // Fast commit (index 0) + await utils.actAsync(() => legacyRender(, container)); // Slow commit (index 1) + await utils.actAsync(() => legacyRender(, container)); // Fast commit (index 2) + await utils.actAsync(() => legacyRender(, container)); // Slow commit (index 3) + await utils.actAsync(() => store.profilerStore.stopProfiling()); + + // Initially, no commit is selected and no filter is enabled + expect(context.selectedCommitIndex).toBe(null); + expect(context.isCommitFilterEnabled).toBe(false); + + // Case 1: When no commit is selected and there are commits, first should auto-select + expect(context.filteredCommitIndices.length).toBe(4); + expect(context.selectedFilteredCommitIndex).toBe(null); + + // The context should auto-select the first commit when rendered with commits available + await utils.actAsync(() => { + TestRenderer.create( + + + , + ); + }); + expect(context.selectedCommitIndex).toBe(0); + + // Case 2: Select a slow commit, then enable filter to hide it + await utils.actAsync(() => context.selectCommitIndex(3)); // Select last slow commit + expect(context.selectedCommitIndex).toBe(3); + + // Enable filter with duration threshold that filters out fast commits + await utils.actAsync(() => context.setIsCommitFilterEnabled(true)); + await utils.actAsync(() => context.setMinCommitDuration(10)); // Filter for commits > 10ms + + // After filtering, only slow commits (1, 3) should remain + // Selected commit (3) should still be valid + expect(context.filteredCommitIndices).toEqual([1, 3]); + expect(context.selectedCommitIndex).toBe(3); + expect(context.selectedFilteredCommitIndex).toBe(1); // Index 1 in filtered array + + // Case 3: Select a fast commit, then filter it out + await utils.actAsync(() => context.setIsCommitFilterEnabled(false)); + await utils.actAsync(() => context.selectCommitIndex(0)); // Select first fast commit + expect(context.selectedCommitIndex).toBe(0); + + // Re-enable filter - commit 0 should be filtered out + await utils.actAsync(() => context.setIsCommitFilterEnabled(true)); + + // Context should auto-correct to last valid filtered commit + expect(context.filteredCommitIndices).toEqual([1, 3]); + expect(context.selectedCommitIndex).toBe(3); // Auto-corrected to last filtered commit + expect(context.selectedFilteredCommitIndex).toBe(1); + + // Case 4: Filter out all commits + await utils.actAsync(() => context.setMinCommitDuration(1000)); // Very high threshold + + // No commits should pass filter + expect(context.filteredCommitIndices).toEqual([]); + expect(context.selectedCommitIndex).toBe(null); // Should be null when no commits + expect(context.selectedFilteredCommitIndex).toBe(null); + }); }); 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 8c7fbf0cad2..c531c2bbaad 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/Profiler.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/Profiler.js @@ -65,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(); @@ -81,6 +81,7 @@ function Profiler(_: {}) { didRecordCommits && selectedCommitIndex !== null ) { + // Left/Right to navigate commits if (event.key === 'ArrowLeft' || event.key === 'ArrowRight') { if (event.key === 'ArrowLeft') { selectPrevCommitIndex(); 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 47a58c6c4cd..0ed94f6a0ea 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js @@ -19,7 +19,7 @@ import { import {StoreContext} from '../context'; import {logEvent} from 'react-devtools-shared/src/Logger'; -import type {ProfilingDataFrontend} from './types'; +import type {CommitDataFrontend, ProfilingDataFrontend} from './types'; export type TabID = 'flame-chart' | 'ranked-chart' | 'timeline'; @@ -205,6 +205,7 @@ function ProfilerContextController({children}: Props): React.Node { const [selectedCommitIndex, selectCommitIndex] = useState( null, ); + const [selectedTabID, selectTab] = useLocalStorage( 'React::DevTools::Profiler::defaultTab', 'flame-chart', @@ -246,15 +247,17 @@ function ProfilerContextController({children}: Props): React.Node { // Always check didRecordCommits before using commitData or filteredCommitIndices. const commitData = useMemo(() => { if (!didRecordCommits || rootID === null || profilingData === null) { - return []; + return ([]: Array); } const dataForRoot = profilingData.dataForRoots.get(rootID); - return dataForRoot ? dataForRoot.commitData : []; + return dataForRoot + ? dataForRoot.commitData + : ([]: Array); }, [didRecordCommits, rootID, profilingData]); const filteredCommitIndices = useMemo( () => - commitData.reduce((reduced, commitDatum, index) => { + commitData.reduce((reduced: Array, commitDatum, index) => { if ( !isCommitFilterEnabled || commitDatum.duration >= minCommitDuration @@ -262,7 +265,7 @@ function ProfilerContextController({children}: Props): React.Node { reduced.push(index); } return reduced; - }, []), + }, ([]: Array)), [commitData, isCommitFilterEnabled, minCommitDuration], ); @@ -305,6 +308,21 @@ function ProfilerContextController({children}: Props): React.Node { selectCommitIndex(filteredCommitIndices[prevCommitIndex]); }, [selectedFilteredCommitIndex, filteredCommitIndices, selectCommitIndex]); + // Handle edge cases where selected commit becomes invalid after filtering + const numFilteredCommits = filteredCommitIndices.length; + if (selectedFilteredCommitIndex === null && numFilteredCommits > 0) { + selectCommitIndex(filteredCommitIndices[0]); + } else if ( + selectedFilteredCommitIndex !== null && + selectedFilteredCommitIndex >= numFilteredCommits + ) { + selectCommitIndex( + numFilteredCommits === 0 + ? null + : filteredCommitIndices[numFilteredCommits - 1], + ); + } + const value = useMemo( () => ({ selectedTabID, 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 df8b43ce8b1..0d470ddb000 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/SnapshotSelector.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/SnapshotSelector.js @@ -47,17 +47,6 @@ export default function SnapshotSelector(_: Props): React.Node { const numFilteredCommits = filteredCommitIndices.length; - // Handle edge cases where selected commit becomes invalid after filtering - 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] From dc73de47f833f2e3c6a26234834d5e3c3e1d426f Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Tue, 2 Dec 2025 13:37:45 +0000 Subject: [PATCH 06/27] Refactor to avoid cascading --- .../src/__tests__/profilerContext-test.js | 128 +++++++++++++++- .../src/devtools/views/Profiler/Profiler.js | 7 +- .../views/Profiler/ProfilerContext.js | 142 ++++++++++++++---- 3 files changed, 241 insertions(+), 36 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js index 4b3fc140fd2..0a1e622f8d5 100644 --- a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js @@ -656,7 +656,9 @@ describe('ProfilerContext', () => { document.body.removeChild(profilerContainer); }); - it('should navigate between commits when the keyboard shortcut is pressed', async () => { + // @reactVersion <= 18.2 + // @reactVersion >= 18.0 + it('should navigate between commits when the keyboard shortcut is pressed (legacy render)', async () => { const Parent = () => ; const Child = () => null; @@ -771,6 +773,130 @@ describe('ProfilerContext', () => { document.body.removeChild(profilerContainer); }); + // @reactVersion >= 18 + it('should navigate between commits when the keyboard shortcut is pressed (createRoot)', 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()); + + // 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); + + // Create a root for the profiler + const profilerRoot = ReactDOMClient.createRoot(profilerContainer); + + // Render the profiler with ContextReader + 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); + + // Set initial commit selection + await utils.actAsync(() => context.selectCommitIndex(0)); + 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); + + document.body.removeChild(profilerContainer); + }); + it('should handle commit selection edge cases when filtering commits', async () => { // Create components that render with different durations const FastComponent = () => null; 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 c531c2bbaad..6254be06d83 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/Profiler.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/Profiler.js @@ -81,8 +81,11 @@ function Profiler(_: {}) { didRecordCommits && selectedCommitIndex !== null ) { - // Left/Right to navigate commits - if (event.key === 'ArrowLeft' || event.key === 'ArrowRight') { + // 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 { 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 0ed94f6a0ea..289d26cbecb 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, + useEffect, + useMemo, + useState, +} from 'react'; import {useLocalStorage, useSubscription} from '../hooks'; import { TreeDispatcherContext, @@ -195,9 +202,9 @@ function ProfilerContextController({children}: Props): React.Node { } } - const [isCommitFilterEnabled, setIsCommitFilterEnabled] = + const [isCommitFilterEnabled, setIsCommitFilterEnabledValue] = useLocalStorage('React::DevTools::isCommitFilterEnabled', false); - const [minCommitDuration, setMinCommitDuration] = useLocalStorage( + const [minCommitDuration, setMinCommitDurationValue] = useLocalStorage( 'minCommitDuration', 0, ); @@ -255,30 +262,42 @@ function ProfilerContextController({children}: Props): React.Node { : ([]: Array); }, [didRecordCommits, rootID, profilingData]); - const filteredCommitIndices = useMemo( - () => - commitData.reduce((reduced: Array, commitDatum, index) => { - if ( - !isCommitFilterEnabled || - commitDatum.duration >= minCommitDuration - ) { + // Helper to calculate filtered indices based on filter settings + 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, isCommitFilterEnabled, minCommitDuration], + }, ([]: Array)); + }, + [commitData], ); - const selectedFilteredCommitIndex = useMemo(() => { - if (selectedCommitIndex !== null) { - for (let i = 0; i < filteredCommitIndices.length; i++) { - if (filteredCommitIndices[i] === selectedCommitIndex) { + // Helper to find the index in filtered array for a given commit index + 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; - }, [filteredCommitIndices, selectedCommitIndex]); + return null; + }, + [], + ); + + const filteredCommitIndices = useMemo( + () => calculateFilteredIndices(isCommitFilterEnabled, minCommitDuration), + [calculateFilteredIndices, isCommitFilterEnabled, minCommitDuration], + ); + + const selectedFilteredCommitIndex = useMemo( + () => findFilteredIndex(selectedCommitIndex, filteredCommitIndices), + [findFilteredIndex, selectedCommitIndex, filteredCommitIndices], + ); const selectNextCommitIndex = useCallback(() => { if ( @@ -308,20 +327,77 @@ function ProfilerContextController({children}: Props): React.Node { selectCommitIndex(filteredCommitIndices[prevCommitIndex]); }, [selectedFilteredCommitIndex, filteredCommitIndices, selectCommitIndex]); - // Handle edge cases where selected commit becomes invalid after filtering - const numFilteredCommits = filteredCommitIndices.length; - if (selectedFilteredCommitIndex === null && numFilteredCommits > 0) { - selectCommitIndex(filteredCommitIndices[0]); - } else if ( - selectedFilteredCommitIndex !== null && - selectedFilteredCommitIndex >= numFilteredCommits - ) { - selectCommitIndex( - numFilteredCommits === 0 - ? null - : filteredCommitIndices[numFilteredCommits - 1], - ); - } + const setIsCommitFilterEnabled = useCallback( + (value: boolean) => { + setIsCommitFilterEnabledValue(value); + + // Calculate what the filtered indices will be with the new setting + const newFilteredIndices = calculateFilteredIndices(value, minCommitDuration); + + // Handle edge cases where selected commit becomes invalid after filtering + const currentSelectedIndex = selectedCommitIndex; + const selectedFilteredIndex = findFilteredIndex(currentSelectedIndex, newFilteredIndices); + + if (selectedFilteredIndex === null && newFilteredIndices.length > 0) { + // No valid selection but commits exist - select first + selectCommitIndex(newFilteredIndices[0]); + } else if ( + selectedFilteredIndex !== null && + selectedFilteredIndex >= newFilteredIndices.length + ) { + // Selected commit was filtered out - select last valid commit + selectCommitIndex( + newFilteredIndices.length === 0 + ? null + : newFilteredIndices[newFilteredIndices.length - 1], + ); + } + }, + [ + setIsCommitFilterEnabledValue, + calculateFilteredIndices, + findFilteredIndex, + minCommitDuration, + selectedCommitIndex, + selectCommitIndex, + ], + ); + + const setMinCommitDuration = useCallback( + (value: number) => { + setMinCommitDurationValue(value); + + // Calculate what the filtered indices will be with the new setting + const newFilteredIndices = calculateFilteredIndices(isCommitFilterEnabled, value); + + // Handle edge cases where selected commit becomes invalid after filtering + const currentSelectedIndex = selectedCommitIndex; + const selectedFilteredIndex = findFilteredIndex(currentSelectedIndex, newFilteredIndices); + + if (selectedFilteredIndex === null && newFilteredIndices.length > 0) { + // No valid selection but commits exist - select first + selectCommitIndex(newFilteredIndices[0]); + } else if ( + selectedFilteredIndex !== null && + selectedFilteredIndex >= newFilteredIndices.length + ) { + // Selected commit was filtered out - select last valid commit + selectCommitIndex( + newFilteredIndices.length === 0 + ? null + : newFilteredIndices[newFilteredIndices.length - 1], + ); + } + }, + [ + setMinCommitDurationValue, + calculateFilteredIndices, + findFilteredIndex, + isCommitFilterEnabled, + selectedCommitIndex, + selectCommitIndex, + ], + ); const value = useMemo( () => ({ From 00fe07d48b5a12c3f148211b92a346edef30032f Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Tue, 2 Dec 2025 13:39:55 +0000 Subject: [PATCH 07/27] Ran yarn prettier --- .../views/Profiler/ProfilerContext.js | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) 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 289d26cbecb..ed12c8bc2b0 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js @@ -204,10 +204,8 @@ function ProfilerContextController({children}: Props): React.Node { const [isCommitFilterEnabled, setIsCommitFilterEnabledValue] = useLocalStorage('React::DevTools::isCommitFilterEnabled', false); - const [minCommitDuration, setMinCommitDurationValue] = useLocalStorage( - 'minCommitDuration', - 0, - ); + const [minCommitDuration, setMinCommitDurationValue] = + useLocalStorage('minCommitDuration', 0); const [selectedCommitIndex, selectCommitIndex] = useState( null, @@ -275,7 +273,6 @@ function ProfilerContextController({children}: Props): React.Node { [commitData], ); - // Helper to find the index in filtered array for a given commit index const findFilteredIndex = useCallback( (commitIndex: number | null, filtered: Array): number | null => { if (commitIndex === null) return null; @@ -332,11 +329,17 @@ function ProfilerContextController({children}: Props): React.Node { setIsCommitFilterEnabledValue(value); // Calculate what the filtered indices will be with the new setting - const newFilteredIndices = calculateFilteredIndices(value, minCommitDuration); + const newFilteredIndices = calculateFilteredIndices( + value, + minCommitDuration, + ); // Handle edge cases where selected commit becomes invalid after filtering const currentSelectedIndex = selectedCommitIndex; - const selectedFilteredIndex = findFilteredIndex(currentSelectedIndex, newFilteredIndices); + const selectedFilteredIndex = findFilteredIndex( + currentSelectedIndex, + newFilteredIndices, + ); if (selectedFilteredIndex === null && newFilteredIndices.length > 0) { // No valid selection but commits exist - select first @@ -368,11 +371,17 @@ function ProfilerContextController({children}: Props): React.Node { setMinCommitDurationValue(value); // Calculate what the filtered indices will be with the new setting - const newFilteredIndices = calculateFilteredIndices(isCommitFilterEnabled, value); + const newFilteredIndices = calculateFilteredIndices( + isCommitFilterEnabled, + value, + ); // Handle edge cases where selected commit becomes invalid after filtering const currentSelectedIndex = selectedCommitIndex; - const selectedFilteredIndex = findFilteredIndex(currentSelectedIndex, newFilteredIndices); + const selectedFilteredIndex = findFilteredIndex( + currentSelectedIndex, + newFilteredIndices, + ); if (selectedFilteredIndex === null && newFilteredIndices.length > 0) { // No valid selection but commits exist - select first From f3cacf427a7603341c81ec94c56da17a0a668368 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Tue, 2 Dec 2025 13:52:07 +0000 Subject: [PATCH 08/27] removed duplication --- .../views/Profiler/ProfilerContext.js | 81 +++++++------------ 1 file changed, 30 insertions(+), 51 deletions(-) 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 ed12c8bc2b0..a05166bdde7 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js @@ -260,7 +260,6 @@ function ProfilerContextController({children}: Props): React.Node { : ([]: Array); }, [didRecordCommits, rootID, profilingData]); - // Helper to calculate filtered indices based on filter settings const calculateFilteredIndices = useCallback( (enabled: boolean, minDuration: number): Array => { return commitData.reduce((reduced: Array, commitDatum, index) => { @@ -286,6 +285,32 @@ function ProfilerContextController({children}: Props): React.Node { [], ); + const adjustSelectionAfterFilterChange = useCallback( + (newFilteredIndices: Array) => { + const currentSelectedIndex = selectedCommitIndex; + const selectedFilteredIndex = findFilteredIndex( + currentSelectedIndex, + newFilteredIndices, + ); + + if (selectedFilteredIndex === null && newFilteredIndices.length > 0) { + // No valid selection but commits exist - select first + selectCommitIndex(newFilteredIndices[0]); + } else if ( + selectedFilteredIndex !== null && + selectedFilteredIndex >= newFilteredIndices.length + ) { + // Selected commit was filtered out - select last valid commit + selectCommitIndex( + newFilteredIndices.length === 0 + ? null + : newFilteredIndices[newFilteredIndices.length - 1], + ); + } + }, + [findFilteredIndex, selectedCommitIndex, selectCommitIndex], + ); + const filteredCommitIndices = useMemo( () => calculateFilteredIndices(isCommitFilterEnabled, minCommitDuration), [calculateFilteredIndices, isCommitFilterEnabled, minCommitDuration], @@ -328,41 +353,18 @@ function ProfilerContextController({children}: Props): React.Node { (value: boolean) => { setIsCommitFilterEnabledValue(value); - // Calculate what the filtered indices will be with the new setting const newFilteredIndices = calculateFilteredIndices( value, minCommitDuration, ); - // Handle edge cases where selected commit becomes invalid after filtering - const currentSelectedIndex = selectedCommitIndex; - const selectedFilteredIndex = findFilteredIndex( - currentSelectedIndex, - newFilteredIndices, - ); - - if (selectedFilteredIndex === null && newFilteredIndices.length > 0) { - // No valid selection but commits exist - select first - selectCommitIndex(newFilteredIndices[0]); - } else if ( - selectedFilteredIndex !== null && - selectedFilteredIndex >= newFilteredIndices.length - ) { - // Selected commit was filtered out - select last valid commit - selectCommitIndex( - newFilteredIndices.length === 0 - ? null - : newFilteredIndices[newFilteredIndices.length - 1], - ); - } + adjustSelectionAfterFilterChange(newFilteredIndices); }, [ setIsCommitFilterEnabledValue, calculateFilteredIndices, - findFilteredIndex, minCommitDuration, - selectedCommitIndex, - selectCommitIndex, + adjustSelectionAfterFilterChange, ], ); @@ -370,41 +372,18 @@ function ProfilerContextController({children}: Props): React.Node { (value: number) => { setMinCommitDurationValue(value); - // Calculate what the filtered indices will be with the new setting const newFilteredIndices = calculateFilteredIndices( isCommitFilterEnabled, value, ); - // Handle edge cases where selected commit becomes invalid after filtering - const currentSelectedIndex = selectedCommitIndex; - const selectedFilteredIndex = findFilteredIndex( - currentSelectedIndex, - newFilteredIndices, - ); - - if (selectedFilteredIndex === null && newFilteredIndices.length > 0) { - // No valid selection but commits exist - select first - selectCommitIndex(newFilteredIndices[0]); - } else if ( - selectedFilteredIndex !== null && - selectedFilteredIndex >= newFilteredIndices.length - ) { - // Selected commit was filtered out - select last valid commit - selectCommitIndex( - newFilteredIndices.length === 0 - ? null - : newFilteredIndices[newFilteredIndices.length - 1], - ); - } + adjustSelectionAfterFilterChange(newFilteredIndices); }, [ setMinCommitDurationValue, calculateFilteredIndices, - findFilteredIndex, isCommitFilterEnabled, - selectedCommitIndex, - selectCommitIndex, + adjustSelectionAfterFilterChange, ], ); From 7d0407cc5870e99cf7b3255335d3cabf1aa5a083 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Tue, 2 Dec 2025 13:56:12 +0000 Subject: [PATCH 09/27] remove unused imports --- .../src/devtools/views/Profiler/ProfilerContext.js | 1 - 1 file changed, 1 deletion(-) 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 a05166bdde7..370d71d9968 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js @@ -14,7 +14,6 @@ import { createContext, useCallback, useContext, - useEffect, useMemo, useState, } from 'react'; From cee1159213733b2f2869a72251d267eb0d2b2836 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Tue, 2 Dec 2025 14:36:30 +0000 Subject: [PATCH 10/27] yarn prettier --- .../src/devtools/views/Profiler/ProfilerContext.js | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) 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 370d71d9968..40bc7364540 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js @@ -10,13 +10,7 @@ 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} from 'react'; import {useLocalStorage, useSubscription} from '../hooks'; import { TreeDispatcherContext, From 075762e9ef09bfc8b192b9532fb74780684be69a Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Tue, 2 Dec 2025 15:06:55 +0000 Subject: [PATCH 11/27] removed legacy test --- .../src/__tests__/profilerContext-test.js | 119 +----------------- 1 file changed, 1 insertion(+), 118 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js index 0a1e622f8d5..39292f9e065 100644 --- a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js @@ -656,125 +656,8 @@ describe('ProfilerContext', () => { document.body.removeChild(profilerContainer); }); - // @reactVersion <= 18.2 - // @reactVersion >= 18.0 - it('should navigate between commits when the keyboard shortcut is pressed (legacy render)', async () => { - const Parent = () => ; - const Child = () => null; - - const container = document.createElement('div'); - utils.act(() => legacyRender(, container)); - - // Profile and record multiple commits - await utils.actAsync(() => store.profilerStore.startProfiling()); - await utils.actAsync(() => legacyRender(, container)); // Commit 1 - await utils.actAsync(() => legacyRender(, container)); // Commit 2 - await utils.actAsync(() => legacyRender(, container)); // Commit 3 - 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); - - // Create a root for the profiler - const profilerRoot = ReactDOMClient.createRoot(profilerContainer); - - // Render the profiler with ContextReader - 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); - - // Set initial commit selection - await utils.actAsync(() => context.selectCommitIndex(0)); - expect(context.selectedCommitIndex).toBe(0); - - const ownerWindow = profilerContainer.ownerDocument.defaultView; - - // Test ArrowRight navigation (forward) - const arrowRightEvent = new KeyboardEvent('keydown', { - key: 'ArrowRight', - 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) - const arrowLeftEvent = new KeyboardEvent('keydown', { - key: 'ArrowLeft', - 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); - - document.body.removeChild(profilerContainer); - }); - // @reactVersion >= 18 - it('should navigate between commits when the keyboard shortcut is pressed (createRoot)', async () => { + it('should navigate between commits when the keyboard shortcut is pressed', async () => { const Parent = () => ; const Child = () => null; From b0a625779c173119dbed304a6cdd1b0a75db9dc2 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Tue, 2 Dec 2025 17:19:47 +0000 Subject: [PATCH 12/27] removed legacy testing --- .../src/__tests__/profilerContext-test.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js index 39292f9e065..1515314cc30 100644 --- a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js @@ -780,6 +780,7 @@ describe('ProfilerContext', () => { document.body.removeChild(profilerContainer); }); + // @reactVersion >= 18 it('should handle commit selection edge cases when filtering commits', async () => { // Create components that render with different durations const FastComponent = () => null; @@ -793,9 +794,10 @@ describe('ProfilerContext', () => { }; const container = document.createElement('div'); + const root = ReactDOMClient.createRoot(container); // Initial render - utils.act(() => legacyRender(, container)); + utils.act(() => root.render()); let context: Context = ((null: any): Context); function ContextReader() { @@ -813,10 +815,10 @@ describe('ProfilerContext', () => { // Profile with multiple commits of varying durations await utils.actAsync(() => store.profilerStore.startProfiling()); - await utils.actAsync(() => legacyRender(, container)); // Fast commit (index 0) - await utils.actAsync(() => legacyRender(, container)); // Slow commit (index 1) - await utils.actAsync(() => legacyRender(, container)); // Fast commit (index 2) - await utils.actAsync(() => legacyRender(, container)); // Slow commit (index 3) + await utils.actAsync(() => root.render()); // Fast commit (index 0) + await utils.actAsync(() => root.render()); // Slow commit (index 1) + await utils.actAsync(() => root.render()); // Fast commit (index 2) + await utils.actAsync(() => root.render()); // Slow commit (index 3) await utils.actAsync(() => store.profilerStore.stopProfiling()); // Initially, no commit is selected and no filter is enabled From 922fb23e3a09e857414bef53c55772c1daedb2f3 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Tue, 2 Dec 2025 20:16:42 +0000 Subject: [PATCH 13/27] fix autoselect commit bug --- .../src/__tests__/profilerContext-test.js | 99 +++++++++---------- .../views/Profiler/ProfilerContext.js | 38 ++++--- 2 files changed, 75 insertions(+), 62 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js index 1515314cc30..846f656fd2c 100644 --- a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js @@ -782,22 +782,14 @@ describe('ProfilerContext', () => { // @reactVersion >= 18 it('should handle commit selection edge cases when filtering commits', async () => { - // Create components that render with different durations - const FastComponent = () => null; - const SlowComponent = () => { - // Simulate slow render - const start = performance.now(); - while (performance.now() - start < 20) { - // Busy wait - } - return null; - }; + const Parent = () => ; + const Child = () => null; const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); // Initial render - utils.act(() => root.render()); + utils.act(() => root.render()); let context: Context = ((null: any): Context); function ContextReader() { @@ -813,61 +805,68 @@ describe('ProfilerContext', () => { ), ); - // Profile with multiple commits of varying durations + // Profile with multiple commits await utils.actAsync(() => store.profilerStore.startProfiling()); - await utils.actAsync(() => root.render()); // Fast commit (index 0) - await utils.actAsync(() => root.render()); // Slow commit (index 1) - await utils.actAsync(() => root.render()); // Fast commit (index 2) - await utils.actAsync(() => root.render()); // Slow commit (index 3) + await utils.actAsync(() => root.render()); // Commit (index 0) + await utils.actAsync(() => root.render()); // Commit (index 1) + await utils.actAsync(() => root.render()); // Commit (index 2) + await utils.actAsync(() => root.render()); // Commit (index 3) await utils.actAsync(() => store.profilerStore.stopProfiling()); - // Initially, no commit is selected and no filter is enabled - expect(context.selectedCommitIndex).toBe(null); - expect(context.isCommitFilterEnabled).toBe(false); + // Get the actual commit durations from profiling data + const rootID = context.rootID; + const dataForRoot = context.profilingData.dataForRoots.get(rootID); + const commitDurations = dataForRoot.commitData.map(c => c.duration); - // Case 1: When no commit is selected and there are commits, first should auto-select - expect(context.filteredCommitIndices.length).toBe(4); - expect(context.selectedFilteredCommitIndex).toBe(null); + // Find a threshold that will filter some commits + const maxDuration = Math.max(...commitDurations); + const minDuration = Math.min(...commitDurations); + const threshold = (maxDuration + minDuration) / 2; - // The context should auto-select the first commit when rendered with commits available - await utils.actAsync(() => { - TestRenderer.create( - - - , - ); - }); - expect(context.selectedCommitIndex).toBe(0); + // Case 1: With commits and no filter, first commit should be auto-selected + expect(context.filteredCommitIndices.length).toBe(4); + expect(context.selectedCommitIndex).toBe(0); // Auto-selected by the context + expect(context.selectedFilteredCommitIndex).toBe(0); - // Case 2: Select a slow commit, then enable filter to hide it - await utils.actAsync(() => context.selectCommitIndex(3)); // Select last slow commit + // Case 2: Select commit 3, then enable filter + await utils.actAsync(() => context.selectCommitIndex(3)); expect(context.selectedCommitIndex).toBe(3); - // Enable filter with duration threshold that filters out fast commits + // Enable filter with a threshold await utils.actAsync(() => context.setIsCommitFilterEnabled(true)); - await utils.actAsync(() => context.setMinCommitDuration(10)); // Filter for commits > 10ms - - // After filtering, only slow commits (1, 3) should remain - // Selected commit (3) should still be valid - expect(context.filteredCommitIndices).toEqual([1, 3]); - expect(context.selectedCommitIndex).toBe(3); - expect(context.selectedFilteredCommitIndex).toBe(1); // Index 1 in filtered array + await utils.actAsync(() => context.setMinCommitDuration(threshold)); + + // After filtering, check the selected commit is still valid or was auto-corrected + const filteredIndices = context.filteredCommitIndices; + const isCommit3Filtered = filteredIndices.includes(3); + + if (isCommit3Filtered) { + // Commit 3 passed filter, should still be selected + expect(context.selectedCommitIndex).toBe(3); + } else if (filteredIndices.length > 0) { + // Commit 3 was filtered out, should auto-correct to last filtered commit + expect(context.selectedCommitIndex).toBe( + filteredIndices[filteredIndices.length - 1], + ); + } - // Case 3: Select a fast commit, then filter it out + // Case 3: Select first commit, then filter it out await utils.actAsync(() => context.setIsCommitFilterEnabled(false)); - await utils.actAsync(() => context.selectCommitIndex(0)); // Select first fast commit + await utils.actAsync(() => context.selectCommitIndex(0)); expect(context.selectedCommitIndex).toBe(0); - // Re-enable filter - commit 0 should be filtered out + // Re-enable filter - commit 0 might be filtered out await utils.actAsync(() => context.setIsCommitFilterEnabled(true)); - // Context should auto-correct to last valid filtered commit - expect(context.filteredCommitIndices).toEqual([1, 3]); - expect(context.selectedCommitIndex).toBe(3); // Auto-corrected to last filtered commit - expect(context.selectedFilteredCommitIndex).toBe(1); + // Check that selection was adjusted if needed + const newFilteredIndices = context.filteredCommitIndices; + if (newFilteredIndices.length > 0 && !newFilteredIndices.includes(0)) { + // Commit 0 was filtered out, should have been auto-corrected + expect(newFilteredIndices).toContain(context.selectedCommitIndex); + } - // Case 4: Filter out all commits - await utils.actAsync(() => context.setMinCommitDuration(1000)); // Very high threshold + // Case 4: Filter out all commits with very high threshold + await utils.actAsync(() => context.setMinCommitDuration(1000000)); // Very high threshold // No commits should pass filter expect(context.filteredCommitIndices).toEqual([]); 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 40bc7364540..f401ea6e303 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js @@ -286,19 +286,15 @@ function ProfilerContextController({children}: Props): React.Node { newFilteredIndices, ); - if (selectedFilteredIndex === null && newFilteredIndices.length > 0) { - // No valid selection but commits exist - select first + if (newFilteredIndices.length === 0) { + // No commits pass the filter - clear selection + selectCommitIndex(null); + } else if (selectedFilteredIndex === null) { + // Currently selected commit was filtered out - select first available selectCommitIndex(newFilteredIndices[0]); - } else if ( - selectedFilteredIndex !== null && - selectedFilteredIndex >= newFilteredIndices.length - ) { - // Selected commit was filtered out - select last valid commit - selectCommitIndex( - newFilteredIndices.length === 0 - ? null - : newFilteredIndices[newFilteredIndices.length - 1], - ); + } else if (selectedFilteredIndex >= newFilteredIndices.length) { + // Selected index is out of bounds - select last available + selectCommitIndex(newFilteredIndices[newFilteredIndices.length - 1]); } }, [findFilteredIndex, selectedCommitIndex, selectCommitIndex], @@ -342,6 +338,24 @@ function ProfilerContextController({children}: Props): React.Node { selectCommitIndex(filteredCommitIndices[prevCommitIndex]); }, [selectedFilteredCommitIndex, filteredCommitIndices, selectCommitIndex]); + // Auto-correct invalid commit selection after filtering + // This handles cases where: + // 1. No commit is selected but commits exist (auto-select first) + // 2. Selected commit is out of bounds after filtering (auto-select last) + const numFilteredCommits = filteredCommitIndices.length; + if (selectedFilteredCommitIndex === null && numFilteredCommits > 0) { + selectCommitIndex(filteredCommitIndices[0]); + } else if ( + selectedFilteredCommitIndex !== null && + selectedFilteredCommitIndex >= numFilteredCommits + ) { + selectCommitIndex( + numFilteredCommits === 0 + ? null + : filteredCommitIndices[numFilteredCommits - 1], + ); + } + const setIsCommitFilterEnabled = useCallback( (value: boolean) => { setIsCommitFilterEnabledValue(value); From e4247bc8774562867d41c2babe3279a290b8ade7 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Thu, 4 Dec 2025 11:07:46 +0000 Subject: [PATCH 14/27] fix profiler Context test --- babel.config.js | 1 + .../memoize-value-block-value-sequence.js | 2 +- .../src/__tests__/profilerContext-test.js | 87 ++++--------------- .../views/Profiler/ProfilerContext.js | 64 ++++++++------ 4 files changed, 56 insertions(+), 98 deletions(-) diff --git a/babel.config.js b/babel.config.js index f8a28b20cc8..77654e3518e 100644 --- a/babel.config.js +++ b/babel.config.js @@ -5,6 +5,7 @@ module.exports = { '@babel/plugin-syntax-jsx', '@babel/plugin-transform-flow-strip-types', ['@babel/plugin-proposal-class-properties', {loose: true}], + '@babel/plugin-transform-classes', 'syntax-trailing-function-commas', [ '@babel/plugin-proposal-object-rest-spread', diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/memoize-value-block-value-sequence.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/memoize-value-block-value-sequence.js index 5c731aabdf1..4d1ebc617ba 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/memoize-value-block-value-sequence.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/memoize-value-block-value-sequence.js @@ -1,6 +1,6 @@ function Foo(props) { let x; - (x = []), null; + ((x = []), null); return x; } diff --git a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js index 846f656fd2c..5e18341ab26 100644 --- a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js @@ -721,8 +721,7 @@ describe('ProfilerContext', () => { const dataForRoot = context.profilingData.dataForRoots.get(rootID); expect(dataForRoot.commitData.length).toBe(3); - // Set initial commit selection - await utils.actAsync(() => context.selectCommitIndex(0)); + // Should start at the first commit expect(context.selectedCommitIndex).toBe(0); const ownerWindow = profilerContainer.ownerDocument.defaultView; @@ -780,7 +779,7 @@ describe('ProfilerContext', () => { document.body.removeChild(profilerContainer); }); - // @reactVersion >= 18 + // TODO - need to render acutal UI components to test this it('should handle commit selection edge cases when filtering commits', async () => { const Parent = () => ; const Child = () => null; @@ -791,86 +790,32 @@ describe('ProfilerContext', () => { // Initial render utils.act(() => root.render()); + // Profile with multiple commits + await utils.actAsync(() => store.profilerStore.startProfiling()); + await utils.actAsync(() => root.render()); // Commit (index 0) + await utils.actAsync(() => root.render()); // Commit (index 1) + await utils.actAsync(() => root.render()); // Commit (index 2) + await utils.actAsync(() => root.render()); // Commit (index 3) + await utils.actAsync(() => store.profilerStore.stopProfiling()); + let context: Context = ((null: any): Context); function ContextReader() { context = React.useContext(ProfilerContext); return null; } - await utils.actAsync(() => + // Render context reader to access ProfilerContext + await utils.actAsync(() => { TestRenderer.create( , - ), - ); - - // Profile with multiple commits - await utils.actAsync(() => store.profilerStore.startProfiling()); - await utils.actAsync(() => root.render()); // Commit (index 0) - await utils.actAsync(() => root.render()); // Commit (index 1) - await utils.actAsync(() => root.render()); // Commit (index 2) - await utils.actAsync(() => root.render()); // Commit (index 3) - await utils.actAsync(() => store.profilerStore.stopProfiling()); - - // Get the actual commit durations from profiling data - const rootID = context.rootID; - const dataForRoot = context.profilingData.dataForRoots.get(rootID); - const commitDurations = dataForRoot.commitData.map(c => c.duration); - - // Find a threshold that will filter some commits - const maxDuration = Math.max(...commitDurations); - const minDuration = Math.min(...commitDurations); - const threshold = (maxDuration + minDuration) / 2; - - // Case 1: With commits and no filter, first commit should be auto-selected - expect(context.filteredCommitIndices.length).toBe(4); - expect(context.selectedCommitIndex).toBe(0); // Auto-selected by the context - expect(context.selectedFilteredCommitIndex).toBe(0); - - // Case 2: Select commit 3, then enable filter - await utils.actAsync(() => context.selectCommitIndex(3)); - expect(context.selectedCommitIndex).toBe(3); - - // Enable filter with a threshold - await utils.actAsync(() => context.setIsCommitFilterEnabled(true)); - await utils.actAsync(() => context.setMinCommitDuration(threshold)); - - // After filtering, check the selected commit is still valid or was auto-corrected - const filteredIndices = context.filteredCommitIndices; - const isCommit3Filtered = filteredIndices.includes(3); - - if (isCommit3Filtered) { - // Commit 3 passed filter, should still be selected - expect(context.selectedCommitIndex).toBe(3); - } else if (filteredIndices.length > 0) { - // Commit 3 was filtered out, should auto-correct to last filtered commit - expect(context.selectedCommitIndex).toBe( - filteredIndices[filteredIndices.length - 1], ); - } + }); - // Case 3: Select first commit, then filter it out - await utils.actAsync(() => context.setIsCommitFilterEnabled(false)); - await utils.actAsync(() => context.selectCommitIndex(0)); + // Test: With commits and no filter, first commit should be auto-selected expect(context.selectedCommitIndex).toBe(0); - - // Re-enable filter - commit 0 might be filtered out - await utils.actAsync(() => context.setIsCommitFilterEnabled(true)); - - // Check that selection was adjusted if needed - const newFilteredIndices = context.filteredCommitIndices; - if (newFilteredIndices.length > 0 && !newFilteredIndices.includes(0)) { - // Commit 0 was filtered out, should have been auto-corrected - expect(newFilteredIndices).toContain(context.selectedCommitIndex); - } - - // Case 4: Filter out all commits with very high threshold - await utils.actAsync(() => context.setMinCommitDuration(1000000)); // Very high threshold - - // No commits should pass filter - expect(context.filteredCommitIndices).toEqual([]); - expect(context.selectedCommitIndex).toBe(null); // Should be null when no commits - expect(context.selectedFilteredCommitIndex).toBe(null); + expect(context.filteredCommitIndices.length).toBe(4); // All 4 commits + expect(context.selectedFilteredCommitIndex).toBe(0); // First in filtered list }); }); 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 f401ea6e303..79d845e0622 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js @@ -229,15 +229,14 @@ function ProfilerContextController({children}: Props): React.Node { [store], ); - if (isProfiling) { - if (selectedCommitIndex !== null) { + // Clear selections when starting a new profiling session + React.useEffect(() => { + if (isProfiling) { selectCommitIndex(null); - } - if (selectedFiberID !== null) { selectFiberID(null); selectFiberName(null); } - } + }, [isProfiling]); // Get commit data for the current root // NOTE: Unlike profilerStore.getDataForRoot() which uses Suspense (throws when data unavailable), @@ -265,6 +264,23 @@ function ProfilerContextController({children}: Props): React.Node { [commitData], ); + // Auto-select first commit when profiling data becomes available + // Only runs when profilingData or rootID change, NOT when selectedCommitIndex changes + // This ensures it only auto-selects when new data arrives, not when filtering clears selection + React.useEffect(() => { + // eslint-disable-next-line react-hooks/exhaustive-deps -- intentionally reading selectedCommitIndex without depending on it + if ( + profilingData !== null && + selectedCommitIndex === null && + rootID !== null + ) { + const dataForRoot = profilingData.dataForRoots.get(rootID); + if (dataForRoot && dataForRoot.commitData.length > 0) { + selectCommitIndex(0); + } + } + }, [profilingData, rootID]); + const findFilteredIndex = useCallback( (commitIndex: number | null, filtered: Array): number | null => { if (commitIndex === null) return null; @@ -289,13 +305,27 @@ function ProfilerContextController({children}: Props): React.Node { if (newFilteredIndices.length === 0) { // No commits pass the filter - clear selection selectCommitIndex(null); - } else if (selectedFilteredIndex === null) { - // Currently selected commit was filtered out - select first available + } 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) { - // Selected index is out of bounds - select last available + // 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], ); @@ -338,24 +368,6 @@ function ProfilerContextController({children}: Props): React.Node { selectCommitIndex(filteredCommitIndices[prevCommitIndex]); }, [selectedFilteredCommitIndex, filteredCommitIndices, selectCommitIndex]); - // Auto-correct invalid commit selection after filtering - // This handles cases where: - // 1. No commit is selected but commits exist (auto-select first) - // 2. Selected commit is out of bounds after filtering (auto-select last) - const numFilteredCommits = filteredCommitIndices.length; - if (selectedFilteredCommitIndex === null && numFilteredCommits > 0) { - selectCommitIndex(filteredCommitIndices[0]); - } else if ( - selectedFilteredCommitIndex !== null && - selectedFilteredCommitIndex >= numFilteredCommits - ) { - selectCommitIndex( - numFilteredCommits === 0 - ? null - : filteredCommitIndices[numFilteredCommits - 1], - ); - } - const setIsCommitFilterEnabled = useCallback( (value: boolean) => { setIsCommitFilterEnabledValue(value); From 884c16b453346336388c6fa8fcc794320c58c087 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Thu, 4 Dec 2025 12:12:05 +0000 Subject: [PATCH 15/27] check edge cases in profiler context test --- .../src/__tests__/profilerContext-test.js | 147 ++++++++++++++++-- 1 file changed, 131 insertions(+), 16 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js index 5e18341ab26..bc31ffe1e2e 100644 --- a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js @@ -656,7 +656,6 @@ describe('ProfilerContext', () => { document.body.removeChild(profilerContainer); }); - // @reactVersion >= 18 it('should navigate between commits when the keyboard shortcut is pressed', async () => { const Parent = () => ; const Child = () => null; @@ -720,7 +719,6 @@ describe('ProfilerContext', () => { 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); @@ -776,46 +774,163 @@ describe('ProfilerContext', () => { }, false); expect(context.selectedCommitIndex).toBe(0); + // Cleanup + await utils.actAsync(() => profilerRoot.unmount()); document.body.removeChild(profilerContainer); }); - // TODO - need to render acutal UI components to test this it('should handle commit selection edge cases when filtering commits', async () => { const Parent = () => ; const Child = () => null; const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); - - // Initial render utils.act(() => root.render()); - // Profile with multiple commits + // Profile and record multiple commits await utils.actAsync(() => store.profilerStore.startProfiling()); - await utils.actAsync(() => root.render()); // Commit (index 0) - await utils.actAsync(() => root.render()); // Commit (index 1) - await utils.actAsync(() => root.render()); // Commit (index 2) - await utils.actAsync(() => root.render()); // Commit (index 3) + await utils.actAsync(() => root.render()); // Commit 1 + await utils.actAsync(() => root.render()); // Commit 2 + await utils.actAsync(() => root.render()); // Commit 3 + await utils.actAsync(() => root.render()); // Commit 4 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; } - // Render context reader to access ProfilerContext + const profilerContainer = document.createElement('div'); + document.body.appendChild(profilerContainer); + + const profilerRoot = ReactDOMClient.createRoot(profilerContainer); + await utils.actAsync(() => { - TestRenderer.create( + profilerRoot.render( - + + + + + + + + , ); }); - // Test: With commits and no filter, first commit should be auto-selected + // 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); - expect(context.filteredCommitIndices.length).toBe(4); // All 4 commits - expect(context.selectedFilteredCommitIndex).toBe(0); // First in filtered list + + 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); + + // Lower the threshold to allow commits through again + 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); + + // Filter out the currently selected commit using actual commit data + const commitDurations = dataForRoot.commitData.map(commit => commit.duration); + const selectedCommitDuration = commitDurations[2]; + console.log('commitDurations', commitDurations) + 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 + // (or null if all are filtered) + if (context.filteredCommitIndices.length > 0) { + console.log('filteredCommitIndices is not empty') + expect(context.selectedCommitIndex).not.toBe(null); + expect(context.selectedFilteredCommitIndex).toBe(0); + } else { + console.log('filteredCommitIndices is empty') + expect(context.selectedCommitIndex).toBe(null); + expect(context.selectedFilteredCommitIndex).toBe(null); + } + + await utils.actAsync(() => { + context.setIsCommitFilterEnabled(false); + }); + + // Edge case 6: When filtering is disabled, selected commit should remain or be reset to first if it was null + expect(context.filteredCommitIndices.length).toBe(4); + expect(context.selectedCommitIndex).not.toBe(null); + expect(context.selectedFilteredCommitIndex).not.toBe(null); + + // Cleanup + await utils.actAsync(() => profilerRoot.unmount()); + document.body.removeChild(profilerContainer); }); }); From 466f0d9f62d8ff1b4f2a386a2a834a6515dc26d1 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Thu, 4 Dec 2025 12:27:06 +0000 Subject: [PATCH 16/27] test edge cases with varying commit durations in profiler context --- .../src/__tests__/profilerContext-test.js | 63 +++++++++++-------- 1 file changed, 36 insertions(+), 27 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js index bc31ffe1e2e..19cf5609b8b 100644 --- a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js @@ -778,21 +778,34 @@ describe('ProfilerContext', () => { await utils.actAsync(() => profilerRoot.unmount()); document.body.removeChild(profilerContainer); }); - + // @reactVersion >= 18 it('should handle commit selection edge cases when filtering commits', async () => { - const Parent = () => ; - const Child = () => null; + 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()); + utils.act(() => root.render()); - // Profile and record multiple commits + // Profile and record multiple commits with different amounts of work 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(() => root.render()); // Commit 4 + 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 @@ -879,7 +892,6 @@ describe('ProfilerContext', () => { expect(context.selectedCommitIndex).toBe(null); expect(context.selectedFilteredCommitIndex).toBe(null); - // Lower the threshold to allow commits through again await utils.actAsync(() => { context.setMinCommitDuration(0); }); @@ -899,37 +911,34 @@ describe('ProfilerContext', () => { }, 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[2]; - console.log('commitDurations', commitDurations) + 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 - // (or null if all are filtered) - if (context.filteredCommitIndices.length > 0) { - console.log('filteredCommitIndices is not empty') - expect(context.selectedCommitIndex).not.toBe(null); - expect(context.selectedFilteredCommitIndex).toBe(0); - } else { - console.log('filteredCommitIndices is empty') - expect(context.selectedCommitIndex).toBe(null); - expect(context.selectedFilteredCommitIndex).toBe(null); - } + 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 or be reset to first if it was null + // Edge case 6: When filtering is disabled, selected commit should remain expect(context.filteredCommitIndices.length).toBe(4); - expect(context.selectedCommitIndex).not.toBe(null); - expect(context.selectedFilteredCommitIndex).not.toBe(null); + expect(context.selectedCommitIndex).toBe(2); + expect(context.selectedFilteredCommitIndex).toBe(2); - // Cleanup await utils.actAsync(() => profilerRoot.unmount()); document.body.removeChild(profilerContainer); }); From c661f2489dbecaf0a28e7782b4679879006a5487 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Thu, 4 Dec 2025 13:27:51 +0000 Subject: [PATCH 17/27] remove extra babel config --- babel.config.js | 1 - 1 file changed, 1 deletion(-) diff --git a/babel.config.js b/babel.config.js index 77654e3518e..f8a28b20cc8 100644 --- a/babel.config.js +++ b/babel.config.js @@ -5,7 +5,6 @@ module.exports = { '@babel/plugin-syntax-jsx', '@babel/plugin-transform-flow-strip-types', ['@babel/plugin-proposal-class-properties', {loose: true}], - '@babel/plugin-transform-classes', 'syntax-trailing-function-commas', [ '@babel/plugin-proposal-object-rest-spread', From 5c849202433f733cec479e7d70b32999a90f0016 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Thu, 4 Dec 2025 13:32:00 +0000 Subject: [PATCH 18/27] linting and yarn prettier --- .../src/__tests__/profilerContext-test.js | 5 +---- .../src/devtools/views/Profiler/ProfilerContext.js | 1 - 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js index 19cf5609b8b..7864f9703ea 100644 --- a/packages/react-devtools-shared/src/__tests__/profilerContext-test.js +++ b/packages/react-devtools-shared/src/__tests__/profilerContext-test.js @@ -671,7 +671,6 @@ describe('ProfilerContext', () => { await utils.actAsync(() => root.render()); // Commit 3 await utils.actAsync(() => store.profilerStore.stopProfiling()); - // Context providers const Profiler = require('react-devtools-shared/src/devtools/views/Profiler/Profiler').default; const { @@ -693,10 +692,8 @@ describe('ProfilerContext', () => { const profilerContainer = document.createElement('div'); document.body.appendChild(profilerContainer); - // Create a root for the profiler const profilerRoot = ReactDOMClient.createRoot(profilerContainer); - // Render the profiler with ContextReader await utils.actAsync(() => { profilerRoot.render( @@ -778,7 +775,7 @@ describe('ProfilerContext', () => { await utils.actAsync(() => profilerRoot.unmount()); document.body.removeChild(profilerContainer); }); - // @reactVersion >= 18 + it('should handle commit selection edge cases when filtering commits', async () => { const Scheduler = require('scheduler'); 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 79d845e0622..c424c91ca09 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js @@ -268,7 +268,6 @@ function ProfilerContextController({children}: Props): React.Node { // Only runs when profilingData or rootID change, NOT when selectedCommitIndex changes // This ensures it only auto-selects when new data arrives, not when filtering clears selection React.useEffect(() => { - // eslint-disable-next-line react-hooks/exhaustive-deps -- intentionally reading selectedCommitIndex without depending on it if ( profilingData !== null && selectedCommitIndex === null && From aa0b246eb2320ed4b3e499c02ee0ab8a3e572f54 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Thu, 4 Dec 2025 13:38:20 +0000 Subject: [PATCH 19/27] undo lint formatting where it breaks expected patterns in tests --- .../fixtures/compiler/memoize-value-block-value-sequence.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/memoize-value-block-value-sequence.js b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/memoize-value-block-value-sequence.js index 4d1ebc617ba..5c731aabdf1 100644 --- a/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/memoize-value-block-value-sequence.js +++ b/compiler/packages/babel-plugin-react-compiler/src/__tests__/fixtures/compiler/memoize-value-block-value-sequence.js @@ -1,6 +1,6 @@ function Foo(props) { let x; - ((x = []), null); + (x = []), null; return x; } From e719902586140245e67ea7daf47ceb072fd22414 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Thu, 4 Dec 2025 13:42:52 +0000 Subject: [PATCH 20/27] remove formatting of react fibre hooks --- packages/react-reconciler/src/ReactFiberHooks.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 774c6f85407..66a390ebb81 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -3816,8 +3816,8 @@ function enqueueRenderPhaseUpdate( // This is a render phase update. Stash it in a lazily-created map of // queue -> linked list of updates. After this render pass, we'll restart // and apply the stashed updates on top of the work-in-progress hook. - didScheduleRenderPhaseUpdateDuringThisPass = - didScheduleRenderPhaseUpdate = true; + didScheduleRenderPhaseUpdateDuringThisPass = didScheduleRenderPhaseUpdate = + true; const pending = queue.pending; if (pending === null) { // This is the first update. Create a circular list. From 5b3d5b2dfc826eb487bfd86fa7c32c745192bdc4 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Thu, 4 Dec 2025 13:49:44 +0000 Subject: [PATCH 21/27] revert local yarn prettier changes to console-test --- .../src/__tests__/console-test.js | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/console-test.js b/packages/react-devtools-shared/src/__tests__/console-test.js index 1debfadc62c..00d6d971267 100644 --- a/packages/react-devtools-shared/src/__tests__/console-test.js +++ b/packages/react-devtools-shared/src/__tests__/console-test.js @@ -363,7 +363,8 @@ describe('console', () => { it('should double log if hideConsoleLogsInStrictMode is disabled in Strict mode', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = false; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = false; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = + false; const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); @@ -404,7 +405,8 @@ describe('console', () => { it('should not double log if hideConsoleLogsInStrictMode is enabled in Strict mode', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = false; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = true; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = + true; const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); @@ -431,7 +433,8 @@ describe('console', () => { it('should double log from Effects if hideConsoleLogsInStrictMode is disabled in Strict mode', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = false; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = false; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = + false; const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); @@ -478,7 +481,8 @@ describe('console', () => { it('should not double log from Effects if hideConsoleLogsInStrictMode is enabled in Strict mode', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = false; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = true; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = + true; const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); @@ -514,7 +518,8 @@ describe('console', () => { it('should double log from useMemo if hideConsoleLogsInStrictMode is disabled in Strict mode', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = false; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = false; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = + false; const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); @@ -557,7 +562,8 @@ describe('console', () => { it('should not double log from useMemo fns if hideConsoleLogsInStrictMode is enabled in Strict mode', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = false; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = true; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = + true; const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); @@ -586,7 +592,8 @@ describe('console', () => { it('should double log in Strict mode initial render for extension', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = false; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = false; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = + false; // This simulates a render that happens before React DevTools have finished // their handshake to attach the React DOM renderer functions to DevTools @@ -631,7 +638,8 @@ describe('console', () => { it('should not double log in Strict mode initial render for extension', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = false; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = true; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = + true; // This simulates a render that happens before React DevTools have finished // their handshake to attach the React DOM renderer functions to DevTools @@ -662,7 +670,8 @@ describe('console', () => { it('should properly dim component stacks during strict mode double log', () => { global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.appendComponentStack = true; - global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = false; + global.__REACT_DEVTOOLS_GLOBAL_HOOK__.settings.hideConsoleLogsInStrictMode = + false; const container = document.createElement('div'); const root = ReactDOMClient.createRoot(container); From 8dbeef06c227f0fbe2f7dac3b573de6f9a8ebd3c Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Thu, 4 Dec 2025 14:55:10 +0000 Subject: [PATCH 22/27] removed unecessary comments --- .../views/Profiler/ProfilerContext.js | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) 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 c424c91ca09..7312960c271 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, @@ -170,6 +177,7 @@ function ProfilerContextController({children}: Props): React.Node { [setRootID, selectFiber], ); + // why am i doing this instead of use effect if (prevProfilingData !== profilingData) { setPrevProfilingData(profilingData); @@ -230,7 +238,7 @@ function ProfilerContextController({children}: Props): React.Node { ); // Clear selections when starting a new profiling session - React.useEffect(() => { + useEffect(() => { if (isProfiling) { selectCommitIndex(null); selectFiberID(null); @@ -264,10 +272,8 @@ function ProfilerContextController({children}: Props): React.Node { [commitData], ); - // Auto-select first commit when profiling data becomes available - // Only runs when profilingData or rootID change, NOT when selectedCommitIndex changes - // This ensures it only auto-selects when new data arrives, not when filtering clears selection - React.useEffect(() => { + // Auto-select first commit when profiling data becomes available and no commit is selected + useEffect(() => { if ( profilingData !== null && selectedCommitIndex === null && @@ -302,13 +308,10 @@ function ProfilerContextController({children}: Props): React.Node { ); 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) { @@ -316,7 +319,6 @@ function ProfilerContextController({children}: Props): React.Node { break; } } - // If no commit before it, use the first available selectCommitIndex( closestBefore !== null ? closestBefore : newFilteredIndices[0], ); @@ -450,7 +452,6 @@ function ProfilerContextController({children}: Props): React.Node { supportsProfiling, rootID, - setRootID, setRootIDAndClearFiber, isCommitFilterEnabled, From 69cda169b9219e6b9a7682280c768b57f46dc314 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Thu, 4 Dec 2025 15:14:27 +0000 Subject: [PATCH 23/27] extracted filtering and navigation from profiler context --- .../views/Profiler/ProfilerContext.js | 183 +++------------- .../useCommitFilteringAndNavigation.js | 207 ++++++++++++++++++ 2 files changed, 235 insertions(+), 155 deletions(-) create mode 100644 packages/react-devtools-shared/src/devtools/views/Profiler/useCommitFilteringAndNavigation.js 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 7312960c271..ab9819d35ac 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js @@ -25,6 +25,7 @@ import { } from '../Components/TreeContext'; import {StoreContext} from '../context'; import {logEvent} from 'react-devtools-shared/src/Logger'; +import {useCommitFilteringAndNavigation} from './useCommitFilteringAndNavigation'; import type {CommitDataFrontend, ProfilingDataFrontend} from './types'; @@ -166,7 +167,7 @@ function ProfilerContextController({children}: Props): React.Node { } } }, - [dispatch, selectFiberID, selectFiberName, store, profilingData], + [dispatch, store, profilingData], ); const setRootIDAndClearFiber = useCallback( @@ -174,10 +175,10 @@ function ProfilerContextController({children}: Props): React.Node { selectFiber(null, null); setRootID(id); }, - [setRootID, selectFiber], + [selectFiber], ); - // why am i doing this instead of use effect + // Sync rootID with profilingData changes. if (prevProfilingData !== profilingData) { setPrevProfilingData(profilingData); @@ -203,15 +204,6 @@ function ProfilerContextController({children}: Props): React.Node { } } - const [isCommitFilterEnabled, setIsCommitFilterEnabledValue] = - useLocalStorage('React::DevTools::isCommitFilterEnabled', false); - const [minCommitDuration, setMinCommitDurationValue] = - useLocalStorage('minCommitDuration', 0); - - const [selectedCommitIndex, selectCommitIndex] = useState( - null, - ); - const [selectedTabID, selectTab] = useLocalStorage( 'React::DevTools::Profiler::defaultTab', 'flame-chart', @@ -237,15 +229,6 @@ function ProfilerContextController({children}: Props): React.Node { [store], ); - // Clear selections when starting a new profiling session - useEffect(() => { - if (isProfiling) { - selectCommitIndex(null); - selectFiberID(null); - selectFiberName(null); - } - }, [isProfiling]); - // 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. @@ -260,19 +243,30 @@ function ProfilerContextController({children}: Props): React.Node { : ([]: Array); }, [didRecordCommits, rootID, profilingData]); - 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], - ); + // Commit filtering and navigation + const { + isCommitFilterEnabled, + setIsCommitFilterEnabled, + minCommitDuration, + setMinCommitDuration, + selectedCommitIndex, + selectCommitIndex, + filteredCommitIndices, + selectedFilteredCommitIndex, + selectNextCommitIndex, + selectPrevCommitIndex, + } = useCommitFilteringAndNavigation(commitData); - // Auto-select first commit when profiling data becomes available and no commit is selected + // Clear selections when starting a new profiling session + useEffect(() => { + if (isProfiling) { + selectCommitIndex(null); + selectFiberID(null); + selectFiberName(null); + } + }, [isProfiling, selectCommitIndex]); + + // Auto-select first commit when profiling data becomes available and no commit is selected. useEffect(() => { if ( profilingData !== null && @@ -284,128 +278,7 @@ function ProfilerContextController({children}: Props): React.Node { selectCommitIndex(0); } } - }, [profilingData, rootID]); - - 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; - }, - [], - ); - - const adjustSelectionAfterFilterChange = useCallback( - (newFilteredIndices: Array) => { - const currentSelectedIndex = selectedCommitIndex; - const selectedFilteredIndex = findFilteredIndex( - currentSelectedIndex, - newFilteredIndices, - ); - - if (newFilteredIndices.length === 0) { - selectCommitIndex(null); - } else if (currentSelectedIndex === null) { - selectCommitIndex(newFilteredIndices[0]); - } else if (selectedFilteredIndex === null) { - let closestBefore = null; - for (let i = newFilteredIndices.length - 1; i >= 0; i--) { - if (newFilteredIndices[i] < currentSelectedIndex) { - closestBefore = newFilteredIndices[i]; - break; - } - } - 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]); - - 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, - ], - ); + }, [profilingData, rootID, selectCommitIndex]); const value = useMemo( () => ({ 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..7f8791083a0 --- /dev/null +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/useCommitFilteringAndNavigation.js @@ -0,0 +1,207 @@ +/** + * 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 = { + // Filter settings + 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, + ); + + // Calculate which commit indices pass the filter + 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], + ); + + // Find the position of a commit index within the filtered list + 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], + ); + + // Memoized filtered indices based on current filter settings + const filteredCommitIndices = useMemo( + () => calculateFilteredIndices(isCommitFilterEnabled, minCommitDuration), + [calculateFilteredIndices, isCommitFilterEnabled, minCommitDuration], + ); + + // Position of the selected commit within the filtered list + const selectedFilteredCommitIndex = useMemo( + () => findFilteredIndex(selectedCommitIndex, filteredCommitIndices), + [findFilteredIndex, selectedCommitIndex, filteredCommitIndices], + ); + + // Navigate to next commit (wraps around) + 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]); + + // Navigate to previous commit (wraps around) + 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 46ab18849b588df016e0019253b1ec82780cebb6 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Thu, 4 Dec 2025 15:15:09 +0000 Subject: [PATCH 24/27] yarn prettier --- .../devtools/views/Profiler/useCommitFilteringAndNavigation.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/useCommitFilteringAndNavigation.js b/packages/react-devtools-shared/src/devtools/views/Profiler/useCommitFilteringAndNavigation.js index 7f8791083a0..ac6f47e87bd 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/useCommitFilteringAndNavigation.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/useCommitFilteringAndNavigation.js @@ -204,4 +204,3 @@ export function useCommitFilteringAndNavigation( selectPrevCommitIndex, }; } - From 0bd30098f63039f11d8240e45d79635df238a0b9 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Thu, 4 Dec 2025 15:29:47 +0000 Subject: [PATCH 25/27] removed unecessary comments --- .../src/devtools/views/Profiler/ProfilerContext.js | 4 ++-- .../views/Profiler/useCommitFilteringAndNavigation.js | 7 ------- 2 files changed, 2 insertions(+), 9 deletions(-) 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 ab9819d35ac..10a337b7473 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js @@ -167,7 +167,7 @@ function ProfilerContextController({children}: Props): React.Node { } } }, - [dispatch, store, profilingData], + [dispatch, selectFiberID, selectFiberName, store, profilingData], ); const setRootIDAndClearFiber = useCallback( @@ -175,7 +175,7 @@ function ProfilerContextController({children}: Props): React.Node { selectFiber(null, null); setRootID(id); }, - [selectFiber], + [setRootID, selectFiber], ); // Sync rootID with profilingData changes. diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/useCommitFilteringAndNavigation.js b/packages/react-devtools-shared/src/devtools/views/Profiler/useCommitFilteringAndNavigation.js index ac6f47e87bd..e309e6a3cf3 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/useCommitFilteringAndNavigation.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/useCommitFilteringAndNavigation.js @@ -13,7 +13,6 @@ import {useLocalStorage} from '../hooks'; import type {CommitDataFrontend} from './types'; export type CommitFilteringAndNavigation = { - // Filter settings isCommitFilterEnabled: boolean, setIsCommitFilterEnabled: (value: boolean) => void, minCommitDuration: number, @@ -46,7 +45,6 @@ export function useCommitFilteringAndNavigation( null, ); - // Calculate which commit indices pass the filter const calculateFilteredIndices = useCallback( (enabled: boolean, minDuration: number): Array => { return commitData.reduce((reduced: Array, commitDatum, index) => { @@ -59,7 +57,6 @@ export function useCommitFilteringAndNavigation( [commitData], ); - // Find the position of a commit index within the filtered list const findFilteredIndex = useCallback( (commitIndex: number | null, filtered: Array): number | null => { if (commitIndex === null) return null; @@ -110,19 +107,16 @@ export function useCommitFilteringAndNavigation( [findFilteredIndex, selectedCommitIndex, selectCommitIndex], ); - // Memoized filtered indices based on current filter settings const filteredCommitIndices = useMemo( () => calculateFilteredIndices(isCommitFilterEnabled, minCommitDuration), [calculateFilteredIndices, isCommitFilterEnabled, minCommitDuration], ); - // Position of the selected commit within the filtered list const selectedFilteredCommitIndex = useMemo( () => findFilteredIndex(selectedCommitIndex, filteredCommitIndices), [findFilteredIndex, selectedCommitIndex, filteredCommitIndices], ); - // Navigate to next commit (wraps around) const selectNextCommitIndex = useCallback(() => { if ( selectedFilteredCommitIndex === null || @@ -137,7 +131,6 @@ export function useCommitFilteringAndNavigation( selectCommitIndex(filteredCommitIndices[nextCommitIndex]); }, [selectedFilteredCommitIndex, filteredCommitIndices, selectCommitIndex]); - // Navigate to previous commit (wraps around) const selectPrevCommitIndex = useCallback(() => { if ( selectedFilteredCommitIndex === null || From c790fde2825eb31c3685b182fcc9db20dba024b8 Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Wed, 10 Dec 2025 11:47:28 +0000 Subject: [PATCH 26/27] clear state synchronously to avoid cascading update in profiler context --- .../devtools/views/Profiler/ProfilerContext.js | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) 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 10a337b7473..6ea6a0c0b0b 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js @@ -222,8 +222,14 @@ function ProfilerContextController({children}: Props): React.Node { 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]); + }, [store, selectedTabID, selectCommitIndex]); const stopProfiling = useCallback( () => store.profilerStore.stopProfiling(), [store], @@ -257,15 +263,6 @@ function ProfilerContextController({children}: Props): React.Node { selectPrevCommitIndex, } = useCommitFilteringAndNavigation(commitData); - // Clear selections when starting a new profiling session - useEffect(() => { - if (isProfiling) { - selectCommitIndex(null); - selectFiberID(null); - selectFiberName(null); - } - }, [isProfiling, selectCommitIndex]); - // Auto-select first commit when profiling data becomes available and no commit is selected. useEffect(() => { if ( From 81751efbd202d6ff0a20846ca9a15606aaf34b2a Mon Sep 17 00:00:00 2001 From: Emily Brown Date: Wed, 10 Dec 2025 12:30:02 +0000 Subject: [PATCH 27/27] corrected order to clear state only after selectCommitIndex is defined --- .../views/Profiler/ProfilerContext.js | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) 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 6ea6a0c0b0b..d593558dbdd 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/ProfilerContext.js @@ -217,19 +217,6 @@ function ProfilerContextController({children}: Props): React.Node { }, ); - 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]); const stopProfiling = useCallback( () => store.profilerStore.stopProfiling(), [store], @@ -263,6 +250,20 @@ function ProfilerContextController({children}: Props): React.Node { 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 (