fix: Error: Cannot read property 'duration' of undefined

[Amine-H]

Summary

Fix for #18786 and #18798

Just a few things I should mention first, I am really new to the code base, not sure what was the intention to use the indexes from commits instead of commitData.
In other words if commitData has more commit than commits (interaction commits) then this PR would be showing more commits than whan It should.

UPDATE:

I have updated my changes to "ignore" commits that have no corresponding commitData

Test Plan

• Profile https://react-example1.s3.amazonaws.com/index.html
• Scroll down to the end of the list

Thanks @shiftyp for the clear reproduction steps

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 3c6ea8f:

Sandbox	Source
blissful-field-ej3kj	Configuration

[sizebot]

No significant bundle size changes to report.

Size changes (experimental)

Generated by 🚫 dangerJS against 3c6ea8f

[sizebot]

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against 3c6ea8f

[Amine-H]

After further investigating the issue, I have pushed some changes that would "ignore" commits that have no corresponding commitData

I found out that the changes I pushed eaerlier aren't a valid fix, so I canceled them.

the issue is caused when you stop profiling while the page still doing some work. causing either:

• missing commit in commitData
• interactionCommits has an unexisting commitIndex

[bvaughn]

Thanks for submitting this PR!

I'm a little concerned that while it fixes a symptom (out of range index) it doesn't fix the underlying cause (why is the data missing?)

Adding some logging to DevTools and trying the repro, I can see that e.g. the backend records 181 commits but then the later profiling data only contains 180 commits. So one of them is being filtered out for some reason or is not being sent across the bridge. Either way this is likely to cause bigger problems than just this specific fix, so I'm going to look into it further.

Edit 1 All of the commits are being sent over the bridge to the frontend, but it seems to be filtered from the data that's exported.

Edit 2 I think this might be what's biting us:

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

Lines 66 to 73 in fb3f0ac

	// Filter empty commits from the profiler data.
	// It is very important to keep operations and commit data arrays perfect in sync.
	// So we must use the same criteria to filter both.
	// If these two arrays were to get out of sync, the profiler would runtime error.
	// We choose to filter on commit metadata, rather than the operations array,
	// because the latter may have false positives,
	// (e.g. a commit that re-rendered a component with the same treeBaseDuration as before).
	commitData.forEach((commitDataBackend, commitIndex) => {

Edit 3 Yup, that was it. At least for this specific case, we're filtering out a commit where "nothing renders". I think this could happen if the only things that rendered were also hidden by our component filters...

Edit 4 Confirmed. Changing the filters to intentionally hide all of the components that would be part of a commit does repro this same error behavior. So that's fine. The fix, then, is to just remove this filter step!

[bvaughn]

Going to close this PR in favor of #18862!

[Amine-H]

Alright ! that should do it! thanks 👍

[bvaughn]

No problem! Thank you!