DevTools: Scheduling Profiler: Better error message when no marks

[bvaughn]

The scheduling profiler currently shows "unsupported version" message if you import a performance trace with no marks.

It incorrectly assumes the lack of an explicit version mark indicates an earlier version of the data which didn't include marks, but in the event that there are no marks at all we should infer that a production version of React DOM was being used instead and show a different error.

[byronluk]

Hi @bvaughn, mind if I pick this up?

[bvaughn]

Sure thing, @byronluk.

[byronluk]

Thanks!

Regarding your comment-

but in the event that there are no marks at all

do you mean no marks that were generated by React? (just want to double check since I'm not too familiar with the devtools profiling 😅).

If so, I'm thinking the best way to check for this is to see if any React-related profiling data was generated in profilerData-

react/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js

Line 853 in f51579f

const profilerData: ReactProfilerData = {

Specifically checking if any these fields were populated - batchUIDToMeasuresMap, componentMeasures, schedulingEvents, suspenseEvents.

Does this sound like a sound approach?

[bvaughn]

do you mean no marks that were generated by React?

Yes. So a browser performance trace was recorded from a site running the React production bundle (no profiling features enabled) or even a website that wasn't using React at all.

Does this sound like a sound approach?

Sure.

Realistically it's probably sufficient to check schedulingEvents.length > 0 and laneToReactMeasureMap.size > 0. Seems highly unlikely that a valid profile wouldn't contain at least some of those.

[byronluk]

Thanks for the guidance!

Since laneToReactMeasureMap gets pre-initialized with values here:

react/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js

Lines 848 to 851 in 045156c

	const laneToReactMeasureMap = new Map();
	for (let lane: ReactLane = 0; lane < REACT_TOTAL_NUM_LANES; lane++) {
	laneToReactMeasureMap.set(lane, []);
	}

I ended up checking batchUIDToMeasuresMap instead. It was simpler this way and I figured it has the same effect since both maps end up being populated under the same scenario in the markWorkStarted function:

react/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js

Lines 154 to 167 in 045156c

	// This array is pre-initialized when the batchUID is generated.
	const measures = currentProfilerData.batchUIDToMeasuresMap.get(batchUID);
	if (measures != null) {
	measures.push(measure);
	} else {
	currentProfilerData.batchUIDToMeasuresMap.set(state.batchUID, [measure]);
	}
	// This array is pre-initialized before processing starts.
	lanes.forEach(lane => {
	((currentProfilerData.laneToReactMeasureMap.get(
	lane,
	): any): ReactMeasure[]).push(measure);
	});

Hopefully that's okay. I submitted a PR here: #22157

[bvaughn]

Since laneToReactMeasureMap gets pre-initialized with values here...I ended up checking batchUIDToMeasuresMap instead.

Good call

[stale]

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

[github-actions]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!