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

polyfill usage with /base is counterintuitive

Open roippi opened this issue 4 years ago • 5 comments
trafficstars

Hi there. The how the polyfill works section states:

The "standard" version of the web-vitals library includes some of the same logic found in polyfill.js

This means that you must include the polyfill, even on browsers (ie chrome) that natively support First Input Delay. Omitting the polyfill will break FID on chrome. (Cannot read property 'firstHiddenTime' of undefined)

This is rather counterintuitive: it means the polyfill is doing more than just polyfilling. It's bundling some application logic in with the polyfill. I've never seen another polyfill on the web that acts this way - a reasonable developer expectation is that polyfills replace missing browser functionality, and no more.

Can we make the polyfill pure, and leave application logic inside the app? Alternately, calling this something other than a "polyfill" since it's really a polyfill + helper script. Thanks.

roippi avatar Nov 20 '20 16:11 roippi

This means that you must include the polyfill, even on browsers (ie chrome) that natively support First Input Delay. Omitting the polyfill will break FID on chrome. (Cannot read property 'firstHiddenTime' of undefined)

You do not need to include the polyfill script or use web-vitals/base. If you do not include the polyfill, FID will work just fine in Chromium-based browsers.

The only way it will break is if you do not add the polyfill script but then you import web-vitals/base in your code. The reason this will break is because the "base" version assumes the polyfill script has already been loaded on the page. (I'm assuming that's how you're seeing the Cannot read property 'firstHiddenTime' of undefined error?)

I thought it was clear in the documentation that it had to be one or the other. If you think it's not clear, let me know.

This is rather counterintuitive: it means the polyfill is doing more than just polyfilling. It's bundling some application logic in with the polyfill.

The polyfill is just polyfilling. Where I agree it's confusing is that the "standard" version is also polyfilling—it's just not polyfilling as much stuff. What it's polyfilling is the detection of the metrics after a bfcache restore.

The only reason there are two scripts is because regular FID cannot be polyfilled without adding code to the <head> of the document, whereas bfcache-restore FID can be polyfilled as long as the script is loaded prior to the page being put in the cache (which it always will be).

And since some of the code required to detect FID cross-browser is also required to detect FID after a bfcache restore, it made sense to have two versions of the script to avoid duplication when using the polyfill. I agree it'd be simpler to have just one version, but I thought it was worth a bit of confusion to avoid the duplication.

Once there are APIs to detect bfcache versions of these metrics, then the web-vitals/base version can go away and it will then be a standard script plus an optional polyfill to support other browsers. We're kinda in a weird state right now, unfortunately.

philipwalton avatar Nov 20 '20 21:11 philipwalton

You do not need to include the polyfill script or use web-vitals/base.

I work for a large e-commerce website; we want to capture webvitals for all of our customers. 60-70% of our mobile traffic is safari.

It is not feasible to bundle different versions of webvitals for different browsers.

Thus, we need to use web-vitals/base. This breaks in chrome if we do not include the polyfill.

I thought it was clear in the documentation that it had to be one or the other. If you think it's not clear, let me know.

It is clear. It is also counterintuitive, and not everyone reads every letter of the documentation when implementing. We didn't. It caused our webvitals to break for a bit, because we omitted the polyfill for chrome.

I do not think it is unreasonable to ask for something labeled a polyfill to be a pure polyfill.

The polyfill is just polyfilling

web-vitals/base does not work without the polyfill on chrome. FID breaks with the error Cannot read property 'firstHiddenTime' of undefined.

roippi avatar Nov 21 '20 01:11 roippi

I work for a large e-commerce website; we want to capture webvitals for all of our customers. 60-70% of our mobile traffic is safari.

It is not feasible to bundle different versions of webvitals for different browsers.

Thus, we need to use web-vitals/base. This breaks in chrome if we do not include the polyfill.

Yes, that's correct. This is expected, and this is what is documented. Shipping web-vitals/base to all users and the polyfill to just Chrome users is not supported.

Also, keep in mind that even Chrome benefits from the polyfill, as it gets a (potentially) more accurate recording of the page's visibility state when it was loaded. In addition, it could be the case in the future that the polyfill is ahead of Chrome, so it's not a safe assumption that Chrome will never need the polyfill.

If you want to use the polyfill and get Safari support, then you have to use it with the web-vitals/base script. If you want to ship different versions to different browsers, then you have to do that in both places (your JS bundle and the HTML document).

It is clear. It is also counterintuitive, and not everyone reads every letter of the documentation when implementing. We didn't. It caused our webvitals to break for a bit, because we omitted the polyfill for chrome.

I do not think it is unreasonable to ask for something labeled a polyfill to be a pure polyfill.

The polyfill is a pure polyfill. Your issue was caused by using web-vitals/base in a way that's not supported.

As I said before, I agree and understand that this is not the way that libraries are typically added to a page, but there's no other way to measure FID in all browsers other than to have a separate script added to the <head> of the page.

If you have a suggestion for a better for web-vitals/base in order to make it more clear, I'd love to hear it. I originally called it web-vitals/external-polyfill (to emphasize that it was meant to be used with the external polyfill), but I got feedback from a few people on my team that it was weird to call a script "external-polyfill" if it wasn't an external polyfill.

philipwalton avatar Nov 23 '20 03:11 philipwalton

Thanks for the response. I will be on hiatus for the next week and can reply next Monday. Hope you and everyone else reading this has a happy and safe holiday.

roippi avatar Nov 23 '20 15:11 roippi

I thought it was clear in the documentation that it had to be one or the other. If you think it's not clear, let me know.

I think it can be clarified. We ran into the same issue where we operated under the faulty assumption that in the worst case, a missing polyfill would just mean no data. I assumed I would still be able to call getFID, and that the onReport callback would just never be triggered if it wasn't supported. Instead, without the polyfill, getFID raises an uncaught exception.

It'd be valuable to update the documentation to call out that without the polyfill, use of the web vitals base script may produce undefined behaviour, and not just "missing data". Alternatively, in my opinion it's more intuitive to have the various getMetric functions just become no-ops if there is not sufficient information to trigger the onReport handler.

noahnu avatar Jan 26 '21 21:01 noahnu

Closing this a the polyfill is deprecated and will be removed in a future version. Plus it's primary use case is for FID which is less relevant now that INP is due to replace it.

tunetheweb avatar Oct 18 '23 19:10 tunetheweb