-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Audit: false positive for preconnect/dns-prefetch suggestion #5932
Comments
Good find thanks @ebidel :) |
I've found a few things i'm not sure if we should fix it.
When going into chrome://net-internals/ and clear all caches I have the same issue as cli with devtools & extension. So the only fix is to check the DOM and HTTP-headers to see if they are marked as preconnected. This would make the audit easier to read too. WDYT to write a gatherer for this? |
I'm able to reproduce this bug from the command line if i use |
@patrickhulce what do you think of making a gatherer of preconnect tags? |
@wardpeet in general I'm not a fan of going the static analysis route for these especially since it's hard to determine what's worth the effort to be "extra safe" and what's not. Also because it's difficult to know that you've done preconnect right! i.e. if you correct I think we should try to investigate why it's not being preconnected to google-analytics and solve that bug before resorting to manual static overrides. |
@wardpeet are you working on this or can I steal it from you? :) |
FWIW, my investigation on #6676 has sufficiently convinced me your static analysis route is worth it, so if you already had something along those lines be my guest! |
@patrickhulce feel free to take over. I haven't done anything yet |
hey @ebidel FYI I think LH might be right here, it looks like we don't want the crossorigin attribute on the Without With |
@patrickhulce I think this is not solved, because I get inconsistent behaviour with the integrated Chrome "audits", Lighthouse as standalone a Chrome extension and Pagespeed Insights results. I get this inconsistent behaviour across various sites, not only one one domain. Furthermore, sometimes the hint to preconnect to required origins does show up, sometimes it does not, and it does especially not show up when the site is fast enough with out preconnects (say, 90 score or better) I think the hints should show up independently on how fast the site is. Also, Lighthouse / Google Audits should give hints on what's wrong and what's working - so if a "crossorigin" is required or not for a certain domain, because trial & error is very cumbersome and yields inconsistent reports. The way I implemented it and i think it is correct that way is like this, for example on this Domain:
Google Audits does not show a hint to preconnect, Google Lighthouse does show a hint, Pagespeed insights does not show a hint. very weird. |
This is working exactly as intended then :) The goal of the opportunities is to highlight lowest hanging fruit that will have a measurable impact on your performance, so in cases where they wouldn't have a noticeable impact, they should not be showing up.
I agree and in an ideal world this would be great addition. Unfortunately, the need for that attribute is controlled by the specific way in which it's requested/used which isn't always visible to us from the network headers alone. It might be possible to piece together the entire picture from other element information on the page though, and exploration of what is possible here would be a really cool feature request if you wanted to open a separate issue :) |
I am having the same false positive issue |
I'm having this issue. The same page can sometimes complain that a preconnect will help, other times the same page will not flag it. Same page, same code. It's inconsistent and will sometimes complain when you do put a preconnect in place that it isn't used :-/ |
Lighthouse isn't knocking your score for lack of
That seems odd. Do you have a link so we can investigate? If so, please open a new issue with it. |
I'm seeing a false positive for preconnect. Luckily, we have a great system setup to document, so you can view the lighthouse report and the live site! Could I get some eyes on this @connorjclark @patrickhulce ? Thanks for helping make a great test tool! Lighthouse report: https://everipedia-lighthouses.now.sh/2020-02-07/lighthouse.html Here's the DOM: Here's the lighthouse false positive: SourcedAll of this came from an automated test using https://www.webpagetest.org/lighthouse then doing a EDIT: I just ran a lighthouse test on the latest Brave browser via the Lighthouse chrome extension and did NOT see this warning. Perhaps this is an issue with the lighthouse version they're running over at https://www.webpagetest.org/lighthouse ? |
@dawsbot in this WebPageTest result (https://webpagetest.org/result/200207_1J_c1f01a929bb96dde1265c14cd44ffd56/1/details/#waterfall_view_step1) without Lighthouse involved, those origins were not preconnected, so it would appear that Lighthouse is correctly flagging this situation. It's worth noting that Chrome started ignoring preconnect directives in slow network conditions when there too many, so we've changed our advice to only preconnect to the 2 most important origins in the upcoming release of Lighthouse. I'd guess this is the situation we find ourselves in now and why Brave might be still preconnecting if it's a Chrome-specific trial. |
OMG. Finally moved off tumblr.com for my blog! I built a dope site but want to make The House the happiest it can be.
I'm using `link rel=preconnect" for a couple of domains, but LH re-suggests that to me as an improvement :\
Provide the steps to reproduce
What is the current behavior?
Perf > Opportunities says that I should use dns-prefetch and/or connect to reduce RTTs to origins:
What is the expected behavior?
I'm already using
preconnect
for both these origins:Not sure if the audit already does this, but we we could look at the DOM for
link rel=preconnect|dns-prefetch
and remove any origins users are already preconnecting to.Environment Information
Related issues
Report:
https://googlechrome.github.io/lighthouse/viewer/?gist=4bd3bf89ce0ac69319db6cf1e9b94a83
The text was updated successfully, but these errors were encountered: