core: allow any audit details type to be used in an opportunity

[brendankenny]

Upcoming opportunities need to include different audit details than just tables, but the existing 'opportunity' Details type is tied to just the table format. This PR proposes separating opportunities from details types so that any type of details can be included in an opportunity.

I have a followup PR that actually uses this functionality, but I wanted to get this PR posted so anyone can take a look and start thinking about other options in the solution space.

tl;dr:

• any audit in the performance category with a details.overallSavingsMs property will be rendered as an opportunity
• type: 'opportunity' kept for now to not break anyone using LHRs

Originally I wanted something more invasive (e.g. still have an opportunity details, but it could have any kind of details nested in it, or just get rid of opportunity details altogether), but of all the audits, opportunities are the ones most relied on by downstream tools (performance sites, lighthouse-ci, BQ queries, etc), so this feels like the maximum viable change that can occur without being disruptive.

For Lighthouse 11, I'd like to attempt retiring type: 'opportunity' in favor of this solution, which will get rid of the dual table type, taking care of @alexnj's goals in #14771 and make things clearer than they are after this change.

[alexnj]

I like the idea of expanding opportunities beyond just tables! A few additional considerations:

• Do we want to restrict opportunities to just time (ms) optimizations? If we have significant byte savings determined by an audit (via wastedBytes), or if fixing an audit recommendation would move one of the CWV metric significantly, wouldn't that be considered an opportunity?
• (This is sorta related to the above) Should we call out opportunities more explicitly (like isOpportunity in core: merge Details.Opportunity with Details.Table #14771) vs. inferring from the presence of a field? If opportunities are going to remain exclusive to time savings, then this is likely okay to leave as proposed.
• The change introduces some confusion between the usage of inherited overallSavingsMs vs. current Table.summary.wastedMs. Would we consolidate an audit's summary.wastedMs into overallSavingsMs at some point? For an opportunity in table format, this might introduce the same value in both places, if we don't consolidate, I think? FWIW, core: merge Details.Opportunity with Details.Table #14771 deals with this consolidation to one savings field per unit at the cost of breaking backward compatibility.

[brendankenny]

Thanks for taking a look!

• Do we want to restrict opportunities to just time (ms) optimizations? If we have significant byte savings determined by an audit (via wastedBytes), or if fixing an audit recommendation would move one of the CWV metric significantly, wouldn't that be considered an opportunity?

I think yes if we develop the unified theory of opportunities everyone kind of wants but hasn't been able to define so far, and it fits in there. If we're very sure a recommendation will improve a metric, then we should be able to make some level of estimate of how much it will help, which so far has meant time savings since that's what (almost all) the metrics are measured in.

The most likely next kind of opportunity seems like it will be for the unitless CLS. If we develop an opportunity there, it won't fit into the LH4-10 framework and will have to be shoehorned in.

Bytes saved is tricky. Definitely measurable, but the first question should be which bytes and when during load would they be saved (early bytes usually being much more impactful than very late bytes, for instance). And if we can answer that, we're usually most of the way to making a time-saved estimate...

• (This is sorta related to the above) Should we call out opportunities more explicitly (like isOpportunity in core: merge Details.Opportunity with Details.Table #14771) vs. inferring from the presence of a field? If opportunities are going to remain exclusive to time savings, then this is likely okay to leave as proposed.

Because of things like CLS opportunities and the fact that details type: 'opportunity' can't be used with non-table details, I think opportunities need a re-think in general. However, I went through like 5 iterations of this PR and I couldn't come up with anything that wouldn't be very disruptive to downstream tools, which lead me to this solution if we want to ship it in 10.1. Since this works, it seems better to hold off on any additional opportunity structural changes in case they interfere with an outright opportunity shape redesign in the future.

But for LH11+, I think we should consider anything and everything, isOpportunity, a new opportunity details type with other details nested within, moving opportunity to the audit level instead of in details (e.g. audit.isOpportunity and using numericValue as the opportunity savings, which would solve the unit issue since we already have numericUnit), etc.

• The change introduces some confusion between the usage of inherited overallSavingsMs vs. current Table.summary.wastedMs. Would we consolidate an audit's summary.wastedMs into overallSavingsMs at some point? For an opportunity in table format, this might introduce the same value in both places, if we don't consolidate, I think? FWIW, core: merge Details.Opportunity with Details.Table #14771 deals with this consolidation to one savings field per unit at the cost of breaking backward compatibility.

Personally for LH11, I would like to

• get rid of details opportunity as a table alias. Anything that wants a table should use a table, and whatever opportunities are at that point should make that as easy as including any other details type. So existing opportunities would be opportunities and have table details, as opposed to all opportunities are table-ish.
• overallSavingsBytes and summary.wastedMs/summary.wastedBytes are all actually super useful for HTTP Archive research/debugging, but the summary values are used in a total of two audits, and none of the three are ever visible in the HTML report. We should consider moving them to debugData entries to make that clearer.

The backwards compatibility story would be straightforward for any of these ideas, the main constraint possibly overriding any of them is designing to reduce churn on consumers of the LHR and having any churn that is required be as straightforward a migration as possible, with purity of LHR design a priority somewhere below that.