Skip to content
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
557c800
add shortcut to navigate between commits in the performance panel
emily8rown Nov 27, 2025
6a7dc2f
add indicator into tooltips for the hotkeys
emily8rown Nov 27, 2025
99f12da
yarn prettier
emily8rown Nov 27, 2025
3ac3483
changed tooltip for left and right arrow to symbols
emily8rown Nov 27, 2025
d53d7a5
move state management to profiler context from snapshot selector
emily8rown Nov 28, 2025
dc73de4
Refactor to avoid cascading
emily8rown Dec 2, 2025
00fe07d
Ran yarn prettier
emily8rown Dec 2, 2025
f3cacf4
removed duplication
emily8rown Dec 2, 2025
7d0407c
remove unused imports
emily8rown Dec 2, 2025
cee1159
yarn prettier
emily8rown Dec 2, 2025
075762e
removed legacy test
emily8rown Dec 2, 2025
694abec
Merge branch 'facebook:main' into devtools-navigating-commits-perform…
emily8rown Dec 2, 2025
b0a6257
removed legacy testing
emily8rown Dec 2, 2025
922fb23
fix autoselect commit bug
emily8rown Dec 2, 2025
ebb9e16
Merge branch 'facebook:main' into devtools-navigating-commits-perform…
emily8rown Dec 3, 2025
e4247bc
fix profiler Context test
emily8rown Dec 4, 2025
884c16b
check edge cases in profiler context test
emily8rown Dec 4, 2025
466f0d9
test edge cases with varying commit durations in profiler context
emily8rown Dec 4, 2025
9ec729c
Merge branch 'devtools-navigating-commits-performance-panel-hot-key' …
emily8rown Dec 4, 2025
c661f24
remove extra babel config
emily8rown Dec 4, 2025
5c84920
linting and yarn prettier
emily8rown Dec 4, 2025
aa0b246
undo lint formatting where it breaks expected patterns in tests
emily8rown Dec 4, 2025
e719902
remove formatting of react fibre hooks
emily8rown Dec 4, 2025
5b3d5b2
revert local yarn prettier changes to console-test
emily8rown Dec 4, 2025
8dbeef0
removed unecessary comments
emily8rown Dec 4, 2025
69cda16
extracted filtering and navigation from profiler context
emily8rown Dec 4, 2025
46ab188
yarn prettier
emily8rown Dec 4, 2025
0bd3009
removed unecessary comments
emily8rown Dec 4, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 9 additions & 18 deletions packages/react-devtools-shared/src/__tests__/console-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
208 changes: 208 additions & 0 deletions packages/react-devtools-shared/src/__tests__/profilerContext-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -655,4 +655,212 @@ describe('ProfilerContext', () => {

document.body.removeChild(profilerContainer);
});

it('should navigate between commits when the keyboard shortcut is pressed', async () => {
const Parent = () => <Child />;
const Child = () => null;

const container = document.createElement('div');
utils.act(() => legacyRender(<Parent />, container));

// Profile and record multiple commits
await utils.actAsync(() => store.profilerStore.startProfiling());
await utils.actAsync(() => legacyRender(<Parent />, container)); // Commit 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For new tests, we should try to use ReactDOMClient.createRoot and root.render(), unless we are testing against React with version prior to 18.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be a test for both?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because this feature is independent from the version of React we are debugging.

await utils.actAsync(() => legacyRender(<Parent />, container)); // Commit 2
await utils.actAsync(() => legacyRender(<Parent />, 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(
<Contexts>
<SettingsContextController browserTheme="light">
<ModalDialogContextController>
<TimelineContextController>
<Profiler />
<ContextReader />
</TimelineContextController>
</ModalDialogContextController>
</SettingsContextController>
</Contexts>,
);
});

// 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);
});

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(<FastComponent />, container));

let context: Context = ((null: any): Context);
function ContextReader() {
context = React.useContext(ProfilerContext);
return null;
}

await utils.actAsync(() =>
TestRenderer.create(
<Contexts>
<ContextReader />
</Contexts>,
),
);

// Profile with multiple commits of varying durations
await utils.actAsync(() => store.profilerStore.startProfiling());
await utils.actAsync(() => legacyRender(<FastComponent />, container)); // Fast commit (index 0)
await utils.actAsync(() => legacyRender(<SlowComponent />, container)); // Slow commit (index 1)
await utils.actAsync(() => legacyRender(<FastComponent />, container)); // Fast commit (index 2)
await utils.actAsync(() => legacyRender(<SlowComponent />, 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(
<Contexts>
<ContextReader />
</Contexts>,
);
});
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);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@ function Profiler(_: {}) {
supportsProfiling,
startProfiling,
stopProfiling,
selectPrevCommitIndex,
selectNextCommitIndex,
} = useContext(ProfilerContext);

const {file: timelineTraceEventData, searchInputContainerRef} =
Expand All @@ -63,9 +65,9 @@ function Profiler(_: {}) {

const isLegacyProfilerSelected = selectedTabID !== 'timeline';

// Cmd+E to start/stop profiler recording
const handleKeyDown = useEffectEvent((event: KeyboardEvent) => {
const correctModifier = isMac ? event.metaKey : event.ctrlKey;
// Cmd+E to start/stop profiler recording
if (correctModifier && event.key === 'e') {
if (isProfiling) {
stopProfiling();
Expand All @@ -74,6 +76,21 @@ function Profiler(_: {}) {
}
event.preventDefault();
event.stopPropagation();
} else if (
isLegacyProfilerSelected &&
didRecordCommits &&
selectedCommitIndex !== null
) {
// Left/Right to navigate commits
if (event.key === 'ArrowLeft' || event.key === 'ArrowRight') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably check for Cmd key as well. I think we should keep just arrow navigations for changes between nodes in tree view (outside of the scope for this PR), and Cmd + ->, Cmd + <- for navigation between commits. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it's nicer to have them unique but the arrow navigation for the tree view nodes only works when one has just been selected and when that has happened even having them as different shortcuts you can't use a shortcut to navigate between commits. I'm not convinced how much the tab navigation one will be used if your mouse has already selected a tab in the area there just before, and therefore it could be nicer to leave the shortcut as arrow key for simplicity

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it would confuse people they are switching tabs when they expect to switch commits if they don't realise they need to select it

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure I am following. We don't have keyboard navigation for the flamegraph in Profiler tree, right? But if we would add one, it would probably be done by using only arrow keys.

If so, then it would conflict with the navigation between commits. That's my current thinking, what is the alternative?

if (event.key === 'ArrowLeft') {
selectPrevCommitIndex();
} else {
selectNextCommitIndex();
}
event.preventDefault();
event.stopPropagation();
}
}
});

Expand Down
Loading