lighthouse icon indicating copy to clipboard operation
lighthouse copied to clipboard

Performance Audit Prioritization Cleanup

Open adamraine opened this issue 2 years ago • 2 comments

There are several breaking changes related to the new performance audit prioritization that we should consider to clean up our JSON results:

  • [ ] Create an explicit group to contain all performance diagnostics. All performance diagnostics have no group right now because the group is technically auto selected by the report renderer.
    • [ ] Consider renaming "Diagnostics" to "Insights"?
  • [ ] Deprecate, but keep the overallSavingsMs (as metricSavings exists now)
  • [ ] Probably remove the opportunity details type

adamraine avatar Oct 06 '23 22:10 adamraine

It's a big change to remove the old overallSavingsMs and individual item wastedMs when we could easily just keep it. We should at least do a soft deprecation with JSDOC deprecation warnings etc.

Some usages of overallSavingsMs: https://github.com/search?q=overallSavingsMs&type=code

It appears to be useful to have a single JSON ranking for some users. This brings up a lot of the concerns I've had with computing the overall impact on the JSON. I'm willing to consider this for the future though. We could do this through an implicit sort performance audits, or an overallImpact number on each audit (Similar to Brendan's suggestion in #15445)

OpportunityItem includes wastedMs number for each item, we should look into a metric savings for each item.

adamraine avatar Oct 09 '23 21:10 adamraine

Probably remove the opportunity details type

seems like a bunch of work only to break users. report-wise i guess its not useful, but...

Types wise.. overallWastedX isn't an opportunity-specific thing, so no blocker there. But url is a required OpportunityItem member that's not part of base TableItem. Don't really have a solution there unless we're okay with optional url field.

Lots of words to say.. does dropping this type help us or users out? :)

paulirish avatar Nov 28 '23 23:11 paulirish