WICG / soft-navigations

Heuristics to detect Single Page Apps soft navigations
https://wicg.github.io/soft-navigations/
Other
46 stars 6 forks source link

Unintented impact of enabling SoftNavigations to existing PerformanceObservers #11

Closed tunetheweb closed 1 year ago

tunetheweb commented 1 year ago

When SoftNavs are enabled with the Chrome flags or the upcoming Origin Trail, we will start to emit LCP and FCP entries for new pages, and also possibly new FID entries.

This might have unintended consequences for existing PerformanceObservers of these events and they may, incorrectly, process these entries as applying to the initial navigation leading to larger FCP and LCP times and incorrect FID measurements. This is especially the case for buffered events which may replay several soft nav's entries.

Should we have an opt-in to the Performance Observers to prevent these being surfaced unexpectedly and to allow the existing FCP, LCP, and FID to continue to be measured as they currently are?

yoavweiss commented 1 year ago

I got a CL going that adds a PerformanceObserverInit dictionary member named triggeredBySoftNavigation. @tunetheweb tells me this is not a great name :)

What would be a better name? Does an init option make sense as the API shape here?

tunetheweb commented 1 year ago

My concern (ignoring the length!) is it's normal entries AND soft nav entries. triggeredBySoftNavigation could be read as "only events triggered by soft navigations and not hard navigation entries".

Brainstorming here:

Some of them potentially are confusing as to include the soft-navigation entries themselves, rather than "allow additional metrics of this type to be emitted after a soft nav".

Maybe resetOnSoftNavigation is the most accurate?

yoavweiss commented 1 year ago

I like the includes part. Maybe includes{,Post}SoftNavigationMeasurements?

yoavweiss commented 1 year ago

Maybe resetOnSoftNavigation is the most accurate?

Are you suggesting that to be "true" by default, and have folks that want to softnav data to add resetAfterSoftNavigation: false?

tunetheweb commented 1 year ago

No I was suggesting resetOnSoftNavigation is false by default and setting it to true allows the LCP (for example) to be reset, and re-emitted.

But the fact you didn't grok that (and worse - read it the opposite way!) tells me that's not a good name.

yoavweiss commented 1 year ago

OK, changing the CL to includeSoftNavigationObservations. It's a lot to type, but seems descriptive enough..

tunetheweb commented 1 year ago

includeSoftNavObs is a bit shorter but maybe more obscure.

yoavweiss commented 1 year ago

Definitely more obscure..

yoavweiss commented 1 year ago

Landed the CL and an explainer PR, so closing, but feel free to reopen if more bikeshedding is required.

tunetheweb commented 1 year ago

FYI, I've updated the web-vitals soft navs branch in preparation for this landing in a future Chrome