Skip to content

Commit 5dad2b4

Browse files
authored
[DevTools] Fix commit index reset when switching profiler roots (#35672)
Fixes #31463, #30114. When switching between roots in the profiler flamegraph, the commit index was preserved from the previous root. This caused an error "Invalid commit X. There are only Y commits." when the new root had fewer commits than the selected index. This fix resets the commit index to 0 (or null if no commits) when the commitData changes, which happens when switching roots.
1 parent 748ee74 commit 5dad2b4

File tree

2 files changed

+86
-0
lines changed

2 files changed

+86
-0
lines changed

packages/react-devtools-shared/src/__tests__/profilerContext-test.js

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -776,6 +776,84 @@ describe('ProfilerContext', () => {
776776
document.body.removeChild(profilerContainer);
777777
});
778778

779+
it('should reset commit index when switching to a different root', async () => {
780+
const Parent = () => <Child />;
781+
const Child = () => null;
782+
783+
const containerA = document.createElement('div');
784+
const containerB = document.createElement('div');
785+
786+
const rootA = ReactDOMClient.createRoot(containerA);
787+
const rootB = ReactDOMClient.createRoot(containerB);
788+
789+
utils.act(() => rootA.render(<Parent />));
790+
utils.act(() => rootB.render(<Parent />));
791+
792+
// Profile and record different numbers of commits for each root
793+
// Root A: 5 commits, Root B: 2 commits
794+
await utils.actAsync(() => store.profilerStore.startProfiling());
795+
await utils.actAsync(() => rootA.render(<Parent />)); // Root A commit 1
796+
await utils.actAsync(() => rootA.render(<Parent />)); // Root A commit 2
797+
await utils.actAsync(() => rootA.render(<Parent />)); // Root A commit 3
798+
await utils.actAsync(() => rootA.render(<Parent />)); // Root A commit 4
799+
await utils.actAsync(() => rootA.render(<Parent />)); // Root A commit 5
800+
await utils.actAsync(() => rootB.render(<Parent />)); // Root B commit 1
801+
await utils.actAsync(() => rootB.render(<Parent />)); // Root B commit 2
802+
await utils.actAsync(() => store.profilerStore.stopProfiling());
803+
804+
let context: Context = ((null: any): Context);
805+
function ContextReader() {
806+
context = React.useContext(ProfilerContext);
807+
return null;
808+
}
809+
810+
await utils.actAsync(() =>
811+
TestRenderer.create(
812+
<Contexts>
813+
<ContextReader />
814+
</Contexts>,
815+
),
816+
);
817+
818+
// Verify we have profiling data for both roots
819+
expect(context.didRecordCommits).toBe(true);
820+
expect(context.profilingData).not.toBeNull();
821+
822+
const rootIDs = Array.from(context.profilingData.dataForRoots.keys());
823+
expect(rootIDs.length).toBe(2);
824+
825+
const [rootAID, rootBID] = rootIDs;
826+
const rootAData = context.profilingData.dataForRoots.get(rootAID);
827+
const rootBData = context.profilingData.dataForRoots.get(rootBID);
828+
829+
expect(rootAData.commitData.length).toBe(5);
830+
expect(rootBData.commitData.length).toBe(2);
831+
832+
// Select root A and navigate to commit 4 (index 3)
833+
await utils.actAsync(() => context.setRootID(rootAID));
834+
expect(context.rootID).toBe(rootAID);
835+
expect(context.selectedCommitIndex).toBe(0);
836+
837+
await utils.actAsync(() => context.selectCommitIndex(3));
838+
expect(context.selectedCommitIndex).toBe(3);
839+
840+
// Switch to root B which only has 2 commits
841+
// The commit index should be reset to 0, not stay at 3 (which would be invalid)
842+
await utils.actAsync(() => context.setRootID(rootBID));
843+
expect(context.rootID).toBe(rootBID);
844+
// Should be reset to 0 since commit 3 doesn't exist in root B
845+
expect(context.selectedCommitIndex).toBe(0);
846+
847+
// Verify we can still navigate in root B
848+
await utils.actAsync(() => context.selectCommitIndex(1));
849+
expect(context.selectedCommitIndex).toBe(1);
850+
851+
// Switch back to root A - should reset to 0
852+
await utils.actAsync(() => context.setRootID(rootAID));
853+
expect(context.rootID).toBe(rootAID);
854+
expect(context.selectedCommitIndex).toBe(0);
855+
});
856+
779857
it('should handle commit selection edge cases when filtering commits', async () => {
780858
const Scheduler = require('scheduler');
781859

packages/react-devtools-shared/src/devtools/views/Profiler/useCommitFilteringAndNavigation.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,14 @@ export function useCommitFilteringAndNavigation(
4545
null,
4646
);
4747

48+
// Reset commit index when commitData changes (e.g., when switching roots).
49+
const [previousCommitData, setPreviousCommitData] =
50+
useState<Array<CommitDataFrontend>>(commitData);
51+
if (previousCommitData !== commitData) {
52+
setPreviousCommitData(commitData);
53+
selectCommitIndex(commitData.length > 0 ? 0 : null);
54+
}
55+
4856
const calculateFilteredIndices = useCallback(
4957
(enabled: boolean, minDuration: number): Array<number> => {
5058
return commitData.reduce((reduced: Array<number>, commitDatum, index) => {

0 commit comments

Comments
 (0)