search

GoogleChrome / lighthouse

Automated auditing, performance metrics, and best practices for the web.
https://developer.chrome.com/docs/lighthouse/overview/
Apache License 2.0
28.45k stars 9.39k forks source link

Legacy JavaScript should filter items that have no savings #11285

Open patrickhulce opened 4 years ago

patrickhulce commented 4 years ago

Provide the steps to reproduce

  1. Run LH on https://lhci-canary.herokuapp.com/app/

What is the current behavior?

Even 0.1KB of legacy javascript shows up in the table

image

What is the expected behavior?

Items with less than some threshold of savings do not show up in the table.

Even though the audit passes due to the opportunity graph calculations recognizing this doesn't matter, programmatic uses such as the LHCI that rely on the presence of items in .details.items as a signal of something to fix fail this case (because relying on graph ms savings can be flaky). I'm hesitant to include the legacy-javascript audit in the recommended preset while this is the case.

connorjclark commented 4 years ago

alternatively: Could LHCI use the score of the audit to know ~now~ not to surface anything?

patrickhulce commented 4 years ago

Could LHCI use the score of the audit to know now to surface anything?

Even though the audit passes due to the opportunity graph calculations recognizing this doesn't matter, programmatic uses such as the LHCI that rely on the presence of items in .details.items as a signal of something to fix fail this case (because relying on graph ms savings can be flaky).

This was intended to address that question :) The score is based on whether there is savings on the performance metrics. Because the performance metrics are variable, the score of opportunities is also variable. We only include audits in the recommended assertions preset if they will not be flaky. Otherwise you end up with situations where assertions pass the bad PR and then fail later on a good one.

patrickhulce commented 4 years ago

It's also a very common pattern across most (every one?) of the other opportunities that items with trivial savings aren't reported.

I guess I'm asking, if there were many more files that had large savings such that the score isn't 1, is there a reason a user should be told about the 0.1 KB of legacy javascript in this file?

connorjclark commented 4 years ago

got it. I'm good with a (configurable) threshold, probably acting on the item (not subitems). sg?

patrickhulce commented 4 years ago

If the default is in line with our other threshold defaults in this area, sounds great :)

patrickhulce commented 4 years ago

I just noticed that duplicated javascript threshold shipped with 1KiB threshold, would you be open to increasing that too? (at the item level of course) :D