core: allow any audit details type to be used in an opportunity (#14903)

flow-report/src/summary/category.tsx

@@ -42,7 +42,6 @@ function getScoreToBeGained(audit: ScoredAuditRef): number {
 function getOverallSavings(audit: LH.ReportResult.AuditRef): number {
   return (
     audit.result.details &&
-    audit.result.details.type === 'opportunity' &&
     audit.result.details.overallSavingsMs
   ) || 0;
 }

report/renderer/performance-category-renderer.js

@@ -68,7 +68,7 @@ export class PerformanceCategoryRenderer extends CategoryRenderer {
       return element;
     }
     const details = audit.result.details;
-    if (details.type !== 'opportunity') {
+    if (details.overallSavingsMs === undefined) {
       return element;
     }
 
@@ -98,7 +98,7 @@ export class PerformanceCategoryRenderer extends CategoryRenderer {
    * @return {number}
    */
   _getWastedMs(audit) {
-    if (audit.result.details && audit.result.details.type === 'opportunity') {
+    if (audit.result.details) {
       const details = audit.result.details;
       if (typeof details.overallSavingsMs !== 'number') {
         throw new Error('non-opportunity details passed to _getWastedMs');
@@ -165,7 +165,7 @@ export class PerformanceCategoryRenderer extends CategoryRenderer {
    */
   _classifyPerformanceAudit(audit) {
     if (audit.group) return null;
-    if (audit.result.details && audit.result.details.type === 'opportunity') {
+    if (audit.result.details?.overallSavingsMs !== undefined) {
       return 'load-opportunity';
     }
     return 'diagnostic';

report/test/renderer/performance-category-renderer-test.js

@@ -150,12 +150,35 @@ describe('PerfCategoryRenderer', () => {
     expect(matchingElements).toHaveLength(0);
   });
 
+  it('renders an audit as an opportunity if overallSavingMs is present', () => {
+    // Make a non-opportunity into an opportunity.
+    const cloneCategory = JSON.parse(JSON.stringify(category));
+    const crcAudit = cloneCategory.auditRefs.find(a => a.id === 'critical-request-chains');
+    expect(crcAudit.result.details.overallSavingsMs).toBe(undefined);
+    crcAudit.result.details.overallSavingsMs = 5555;
+    crcAudit.result.details.score = 0.5;
+
+    const categoryDOM = renderer.render(cloneCategory, sampleResults.categoryGroups);
+
+    const crcElem = categoryDOM.querySelector('.lh-audit-group--load-opportunities #critical-request-chains.lh-audit--load-opportunity'); // eslint-disable-line max-len
+
+    const crcSparklineBarElem = crcElem.querySelector('.lh-sparkline__bar');
+    expect(crcSparklineBarElem).toBeTruthy();
+
+    const crcWastedElem = crcElem.querySelector('.lh-audit__display-text');
+    expect(crcWastedElem.textContent).toBe('5.56s');
+    expect(crcWastedElem.title).toMatch(/\d+ chains found/);
+
+    const crcDetailsElem = crcElem.querySelector('.lh-crc-container.lh-details');
+    expect(crcDetailsElem.textContent).toMatch('Maximum critical path latency');
+  });
+
   it('renders the failing performance opportunities', () => {
     const categoryDOM = renderer.render(category, sampleResults.categoryGroups);
 
     const oppAudits = category.auditRefs.filter(audit =>
       audit.result.details &&
-      audit.result.details.type === 'opportunity' &&
+      audit.result.details.overallSavingsMs !== undefined &&
       !ReportUtils.showAsPassed(audit.result));
     const oppElements = [...categoryDOM.querySelectorAll('.lh-audit--load-opportunity')];
     expect(oppElements.map(e => e.id).sort()).toEqual(oppAudits.map(a => a.id).sort());
@@ -224,7 +247,7 @@ describe('PerfCategoryRenderer', () => {
 
     const diagnosticAuditIds = category.auditRefs.filter(audit => {
       return !audit.group &&
-        !(audit.result.details && audit.result.details.type === 'opportunity') &&
+        !(audit.result.details?.overallSavingsMs !== undefined) &&
         !ReportUtils.showAsPassed(audit.result);
     }).map(audit => audit.id).sort();
     assert.ok(diagnosticAuditIds.length > 0);

types/lhr/audit-details.d.ts

@@ -7,6 +7,16 @@
 import {IcuMessage} from './i18n.js';
 import Treemap from './treemap.js';
 
+/** Common properties for all details types. */
+interface BaseDetails {
+  /** If present and audit is part of the performance category, audit is treated as an Opportunity. */
+  overallSavingsMs?: number;
+  /** Optional additional Opportunity information. */
+  overallSavingsBytes?: number;
+  /** Additional information, usually used for including debug or meta information in the LHR */
+  debugData?: Details.DebugData;
+}
+
 type Details =
   Details.CriticalRequestChain |
   Details.DebugData |
@@ -19,7 +29,7 @@ type Details =
 
 // Details namespace.
 declare module Details {
-  interface CriticalRequestChain {
+  interface CriticalRequestChain extends BaseDetails {
     type: 'criticalrequestchain';
     longestChain: {
       duration: number;
@@ -42,7 +52,7 @@ declare module Details {
     }
   }
 
-  interface Filmstrip {
+  interface Filmstrip extends BaseDetails {
     type: 'filmstrip';
     scale: number;
     items: {
@@ -55,20 +65,17 @@ declare module Details {
     }[];
   }
 
-  interface List {
+  interface List extends BaseDetails {
     type: 'list';
     // NOTE: any `Details` type *should* be usable in `items`, but check
     // styles/report-ui-features are good before adding.
     items: Array<Table | DebugData>;
   }
 
-  interface Opportunity {
+  interface Opportunity extends BaseDetails {
     type: 'opportunity';
-    overallSavingsMs: number;
-    overallSavingsBytes?: number;
     headings: TableColumnHeading[];
     items: OpportunityItem[];
-    debugData?: DebugData;
     /**
      * Columns to sort the items by, during grouping.
      * If omitted, entity groups will be sorted by the audit ordering vs. the new totals.
@@ -80,7 +87,7 @@ declare module Details {
     skipSumming?: Array<string>;
   }
 
-  interface Screenshot {
+  interface Screenshot extends BaseDetails {
     type: 'screenshot';
     timing: number;
     timestamp: number;
@@ -96,15 +103,14 @@ declare module Details {
     left: number;
   }
 
-  interface Table {
+  interface Table extends BaseDetails {
     type: 'table';
     headings: TableColumnHeading[];
     items: TableItem[];
     summary?: {
       wastedMs?: number;
       wastedBytes?: number;
     };
-    debugData?: DebugData;
     /**
      * Columns to sort the items by, during grouping.
      * If omitted, entity groups will be sorted by the audit ordering vs. the new totals.
@@ -131,7 +137,7 @@ declare module Details {
     [p: string]: any;
   }
 
-  interface TreemapData {
+  interface TreemapData extends BaseDetails {
     type: 'treemap-data';
     nodes: Treemap.Node[];
   }