Wait until idle before starting detection

[westonruter]

Summary

Fixes #894

This rearranges the detection logic to defer doing any DOM operations until after the page has fully loaded and the CPU is idle. Additionally, it now yields to main before initiating the fetch request to submit the data to the server, additionally breaking up tasks.

Performance Profiles

With 6X CPU throttling, the plugin active with the change applied shows a marginal improvement where tasks are further broken up.

Trace without ILO

Trace with ILO before change

Trace with ILO after change

Checklist

• PR has either [Focus] or Infrastructure label.
• PR has a [Type] label.
• PR has a milestone or the no milestone label.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: westonruter <[email protected]>
Co-authored-by: adamsilverstein <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[adamsilverstein]

Additionally, it now yields to main before initiating the fetch request to submit the data to the server, additionally breaking up tasks.

I wonder if you could avoid sending here entirely with a Beacon API approach?

[adamsilverstein]

this makes me wonder if on the server side we should potentially avoid using the captured data when the link is apparently to a non scrolled to top position - ie. when it contains an anchor hash. Do you do that already or what do you think?

[westonruter]

That effectively is what this is doing, yes.

As for server-side detection of this, the URL fragment is not sent along in the HTTP request for the page, so the server won't be able to see it. (Which is why annoyingly URL fragments can be lost when having to authenticate.) It would have to be checked client-side. And often a URL fragment may not refer to an element on the page so that would require checking the DOM to see if it exists. But we only care in the end if the user is not in the first viewport, so that's what checking the scroll position does, I think in a more reliable way.

[adamsilverstein]

Looks good! Couple of small questions.

[adamsilverstein]

Superb!

[westonruter]

Additionally, it now yields to main before initiating the fetch request to submit the data to the server, additionally breaking up tasks.

I wonder if you could avoid sending here entirely with a Beacon API approach?

@adamsilverstein That's a great question! I don't have a good reason why this shouldn't be used instead, other than initially it didn't seem it could be used to send a Content-Type header, but I can see this is not the case. I'm going to open that in a separate PR.

[westonruter]

I don't have a good reason why this shouldn't be used instead, other than initially it didn't seem it could be used to send a Content-Type header, but I can see this is not the case. I'm going to open that in a separate PR.

@adamsilverstein Oh, I remember the more important reason. It is setting a client-side lock to prevent attempting to send more URL metrics when the REST API response is successful:

performance/modules/images/image-loading-optimization/detection/detect.js

Lines 363 to 374 in 7761a9b

	const response = await fetch( restApiEndpoint, {
	method: 'POST',
	headers: {
	'Content-Type': 'application/json',
	'X-WP-Nonce': restApiNonce,
	},
	body: JSON.stringify( urlMetrics ),
	} );
	if ( response.status === 200 ) {
	setStorageLock( getCurrentTime() );
	}

This isn't possible with the Beacon API because the response is not available. In any case, I did work out how to replace the fetch code with the Beacon API if that didn't end up being needed:

	const url = new URL( restApiEndpoint );
	url.searchParams.append( '_wpnonce', restApiNonce );
	navigator.sendBeacon(
		url,
		new Blob( [ JSON.stringify( urlMetrics ) ], {
			type: 'application/json',
		} )
	);