web-vitals icon indicating copy to clipboard operation
web-vitals copied to clipboard

Experimental Soft Navigation support

Open tunetheweb opened this issue 2 years ago • 15 comments
trafficstars

Warning: This branch is likey to stay in draft for some time, and not to be merged into main until the Soft Navs experiment completes

Fixes #119

Implements experimental support of Soft Navigations to the library:

  • [x] - Updated README
  • [x] - TTFB
  • [x] - FCP (not currently emitted by Chrome at this time).
  • [x] - LCP
  • [x] - CLS
  • [x] - INP
  • [x] - Attribution (particularly navigationTiming)
  • [ ] - Test Cases (needs to wait for a supporting version of Chrome to be available in WDIO)

tunetheweb avatar Jan 19 '23 21:01 tunetheweb

That’s for the review @mmocny ! I’ve cleaned it up quite a bit further so it’s a bit smaller now. Have another look if you get a chance and see what you think now.

tunetheweb avatar Jan 20 '23 22:01 tunetheweb

Hey! I'm super excited about this work. Could you republish the branch? The current 3.1.1-soft-navs doesn't report LCP and instead prints BARRY. Screen Shot 2023-02-08 at 22 30 06

alekseykulikov avatar Feb 08 '23 21:02 alekseykulikov

Well that's embarrassing! Republished to the soft-navs tag (which is an alias to 3.1.1-soft-navs-1 now).

tunetheweb avatar Feb 08 '23 21:02 tunetheweb

Thanks, that was fast. Maybe, I'm doing something wrong, but there's still no LCP report, despite actual values are available (Chrome 110): Screen Shot 2023-02-08 at 23 00 17

alekseykulikov avatar Feb 08 '23 22:02 alekseykulikov

Can you host the site somewhere so I can have a look?

Here's a demo site where it does work: https://www.tunetheweb.com/experiments/softnavsdemo/

tunetheweb avatar Feb 08 '23 22:02 tunetheweb

Thanks for the example. Found the issue: when onLCP is called twice (like in the example), it doesn't report soft-navs. Other metrics work.

// works (your example)
onLCP(doSoftNavProcessing, { reportSoftNavs: true })

// !! doesn't report soft navs
onLCP(doTraditionalProcessing)
onLCP(doSoftNavProcessing, { reportSoftNavs: true })

// works as expected
onCLS(doTraditionalProcessing)
onCLS(doSoftNavProcessing, { reportSoftNavs: true })

alekseykulikov avatar Feb 09 '23 09:02 alekseykulikov

Oh that's interesting. Will dig into it...

tunetheweb avatar Feb 09 '23 09:02 tunetheweb

That's fixed now if you want to reinstall the soft-navs tag (or 3.1.1-soft-navs-2 if you prefer to be explicit).

tunetheweb avatar Feb 09 '23 10:02 tunetheweb

Thanks, Barry. That was faster than I finished my coffee. Another issue: the navigationType for FCP & FID is always soft-navigation. So the way to distinguish the hard nav and softs is to use navigationId !== 1. You can replicate the issue in your demo example.

alekseykulikov avatar Feb 09 '23 11:02 alekseykulikov

Fixed!

Hoping to be able to add automated tests to this branch soon now this has landed in stable and so will be available in our test automation suite. But in the meantime greatly appreciate you finding them!

tunetheweb avatar Feb 09 '23 11:02 tunetheweb

Hi @tunetheweb,

Thanks a lot for introducing support for Soft Navigations in the library!

While exploring the new features, I noticed something interesting. It seems that not all changes are being reported when reportAllChanges is activated, as the onINP module is only tracking the longest interactions.

To handle that scenario, I've slightly updated the onINP module (https://github.com/GoogleChrome/web-vitals/pull/411) to ensure that all changes are reported when reportAllChanges is turned on (demo). I've directed my PR to your branch, as my changes are somewhat related to soft navs. I'd be glad to hear your thoughts on this.

karreiro avatar Jan 08 '24 07:01 karreiro

As discussed in #411 that is working as intended. But I raised #413 to make it clearer what exactly that intention is.

tunetheweb avatar Jan 11 '24 12:01 tunetheweb