core(uses-rel-preconnect): warn on 3+ preconnects

[piotrzarycki]

closes #9894

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[piotrzarycki]

@googlebot I signed it!

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[patrickhulce]

thanks very much @piotrzarycki!

[brendankenny]

I'm not sure if I understand the root issue in #9894. From my reading of uses-rel-preconnect, the audit will currently suggest that you preconnect to all preconnectable origins. If someone adds preconnects to all those origins, reruns Lighthouse, and gets a new set of suggested preconnect origins in the next page load, that means they're all new origins, and it seems likely the preconnected origins added in the last round aren't actually being used.

It seems like a warning that preconnects aren't being used (like we do in uses-rel-preload) would be more useful in that case, since going over a browser preconnect limit is really the second order effect and would actually be hiding the fact that the browser is wasting time connecting to origins that don't need it.

However, for a site like the one in #9894, I don't know how we avoid getting into a cycle of recommending adding preconnects and then recommending removing those preconnects in the next Lighthouse run. Are we sure they're all worthy of being preconnected?

[patrickhulce]

the audit will currently suggest that you preconnect to all preconnectable origins

We definitely don't want to say this :)

It seems like a warning that preconnects aren't being used

We do in fact do this already.

https://github.com/GoogleChrome/lighthouse/blame/6e5fc878f8cc69e00620b20092bfad1da6c1e4e2/lighthouse-core/audits/uses-rel-preconnect.js#L168-L172

Are we sure they're all worthy of being preconnected?

No we're not and that's kinda the point of showing all of them instead of just a couple. We don't really know for the particular site which origins are worth preconnecting to because if the analytics script fires like 300 ms earlier, who cares? But maybe there's another third party script thats for the customer support widget that the user actually came here for that's much more important. That's up to them, and I'm not sure we should try to distinguish anything there.

[brendankenny]

It seems like a warning that preconnects aren't being used

We do in fact do this already.

https://github.com/GoogleChrome/lighthouse/blame/6e5fc878f8cc69e00620b20092bfad1da6c1e4e2/lighthouse-core/audits/uses-rel-preconnect.js#L168-L172

That's for origins that a resource was loaded from but didn't take advantage of an attempted preconnect. But if I had a preconnect for random-origin.com and never loaded anything from it, we wouldn't flag anything in that case, right?

[patrickhulce]

But if I had a preconnect for random-origin.com and never loaded anything from it, we wouldn't flag anything in that case, right?

Oh like you completely stopped making requests to that origin entirely? We wouldn't flag that right now, but we don't wouldn't flag that in preload either and you asked for "like we do in uses-rel-preload" ;) (not sure it's even possible for preload requests?)

[piotrzarycki]

@patrickhulce @brendankenny
Any additional modifications should be done, to finish this PR?

[patrickhulce]

@piotrzarycki I think @brendankenny would feel more comfortable with this change if we still returned the details items in this case, so basically not an early return but still use the warnings and score values.

Is this right @brendankenny ?

[connorjclark]

LGTM after one last small change - just a comment change.

[connorjclark]

thanks!