w3c / navigation-timing

Navigation Timing
https://w3c.github.io/navigation-timing/
Other
116 stars 30 forks source link

Extending the NavigationTimingType #184

Open tunetheweb opened 1 year ago

tunetheweb commented 1 year ago

Related to the discussion on #179

There are a few uses cases where the NavigationTimingType does not cover the type:

These types, can have very different performance measurements compared to other navigations and it is often recommended to measure them separately, but at the moment, this results in lots of extra code for (for example) and understanding of all these nuances to enable this.

There's a lot of different issues in this one, so happy to split this out, or continue some of the discussion in #179 but thought I'd start with the one issue for now in case we wanted to tackle together.

verlok-cn commented 1 year ago

Should we add a back_forward_cache navigation type, and treat this is as a true navigation, to explicitly make it easier to identify these?

I'm in favour 👍

Krinkle commented 1 year ago

Should we add a back_forward_cache navigation type, and treat this is as a true navigation, to explicitly make it easier to identify these?

A possible reason against, might be that it could be confusing (and possibly break compat) if we retroactively change the value of performance.navigation.type, performance.timing, and performance.getEntriesByType('navigation') during a bfcache restore that includes the original JS heap as these would already have been consumed.

I notice that in Firefox today, it already does this. The original data in performance.timing is preserved as-is with navigationStart holding the timestamp of when the original navigation started, but performance.navigation.type changes during the bfcache restore from TYPE_NAVIGATE to TYPE_BACK_FORWARD. I realize that I had incorrectly assumed that this navigation type would only be set when bfcache isn't leveraged, since no navigation got rendered. It's restored instantly from memory with no new navigation timing data being provided. I guess it doesn't cause double-counting of the original navigation, since the already-executed JS code isn't forced to consume it a second time, but it also makes it unclear what the intended use case is for seeing a new navigation type without seeing new navigation timing data.

yoavweiss commented 1 year ago

RE BFCache specifically, there's a proposal for dedicated entries for it, to avoid the issues @Krinkle pointed out. It was discussed a while back (recording)

/cc @clelland

tunetheweb commented 1 year ago

I guess it doesn't cause double-counting of the original navigation, since the already-executed JS code isn't forced to consume it a second time but it also makes it unclear what the intended use case is for seeing a new navigation type without seeing new navigation timing data.

In part it's to avoid me (and others!) having to write code like this:

https://github.com/GoogleChrome/web-vitals/blob/39f178242afbb96dca3d48b216d60e7cd4cfa633/src/lib/initMetric.ts#L26-L34

I feel it would make it more obvious to anyone firing events (e.g. LCP), without having to be NavigationTimingType experts and know ALL the edge case you need to consider (3 currently!).

For restores and prerender it does show new navigational timing data, so this is more about categorising that data appriopriately.

For bfcache, I agree it does not fire new data. There's a question if it should? Which seems to be discussed prior to me joining this WG as per Yoav's comment. Seems like from a review of that, that there was some agreement, but needs the work to be done.

Maybe we drop bfcache for now, and limit scope of my ask here to restores and prerender, since they are simpler?

Bfcache could be revisited later (perhaps with soft nav as mentioned in the discussions Yoav's linked?)

fergald commented 11 months ago

Tab duplication/cloning has the same issues. Maybe RESTORE can be extended to cover both (maybe with a name change) or we would have a new entry for it.