fix(server): handle subItemsHeading in audit detail pane

fixes #618

packages/server/src/ui/routes/build-view/audit-detail/audit-detail-pane.stories.jsx

@@ -18,6 +18,8 @@ import lhr641A_ from '../../../../../test/fixtures/lh-6-4-1-coursehero-a.json';
 import lhr641B_ from '../../../../../test/fixtures/lh-6-4-1-coursehero-b.json';
 import lhr700A_ from '../../../../../test/fixtures/lh-7-0-0-coursehero-a.json';
 import lhr700B_ from '../../../../../test/fixtures/lh-7-0-0-coursehero-b.json';
+import lhrSubitemsA_ from '../../../../../test/fixtures/lh-subitems-a.json';
+import lhrSubitemsB_ from '../../../../../test/fixtures/lh-subitems-b.json';
 
 export default {
  title: 'Build View/Audit Detail Pane',
@@ -35,12 +37,24 @@ const lhr641A = /** @type {any} */ (lhr641A_);
 const lhr641B = /** @type {any} */ (lhr641B_);
 const lhr700A = /** @type {any} */ (lhr700A_);
 const lhr700B = /** @type {any} */ (lhr700B_);
+const lhrSubitemsA = /** @type {any} */ (lhrSubitemsA_);
+const lhrSubitemsB = /** @type {any} */ (lhrSubitemsB_);
 
 const auditPairs5 = createAuditPairs(lhr5A, lhr5B);
 const auditPairs6 = createAuditPairs(lhr6A, lhr6B);
 const auditPairs62 = createAuditPairs(lhr62A, lhr62B);
 const auditPairs641 = createAuditPairs(lhr641A, lhr641B);
 const auditPairs700 = createAuditPairs(lhr700A, lhr700B);
+const auditPairsSubitems = createAuditPairs(lhrSubitemsA, lhrSubitemsB, {
+ filter: pair =>
+ [
+ 'third-party-summary',
+ 'third-party-facades',
+ 'valid-source-maps',
+ 'unused-javascript',
+ 'legacy-javascript',
+ ].includes(pair.audit.id || ''),
+});
 
 export const Default = () => (
  <AuditDetailPane
@@ -87,18 +101,34 @@ export const Version700 = () => (
  />
 );
 
+export const VersionSubitems = () => (
+ <AuditDetailPane
+ selectedAuditId={auditPairsSubitems[1].audit.id || ''}
+ setSelectedAuditId={action('setSelectedAuditId')}
+ pairs={auditPairsSubitems}
+ baseLhr={lhrSubitemsA}
+ />
+);
+
 /**
  * @param {LH.Result} lhrA
  * @param {LH.Result} lhrB
+ * @param {{sample?: boolean, filter?: (pair: LHCI.AuditPair, i: number) => boolean}} [options]
  * @return {Array<LHCI.AuditPair>}
  */
-function createAuditPairs(lhrA, lhrB) {
- return (
- computeAuditGroups(lhrA, lhrB, {percentAbsoluteDeltaThreshold: 0.05})
- .filter(group => !group.showAsUnchanged)
- .map(group => group.pairs)
- .reduce((a, b) => a.concat(b))
- // We don't need *all* the audits, so sample ~1/2 of them.
- .filter((pair, i) => i % 2 === 0 && pair.audit.id !== 'uses-long-cache-ttl')
- );
+function createAuditPairs(lhrA, lhrB, options) {
+ const {sample = true, filter} = options || {};
+ return computeAuditGroups(lhrA, lhrB, {percentAbsoluteDeltaThreshold: 0.05})
+ .filter(group => !group.showAsUnchanged)
+ .map(group => group.pairs)
+ .reduce((a, b) => a.concat(b))
+ .filter((pair, i) => {
+ if (filter) return filter(pair, i);
+ // A superlong set of details that breaks diff comparisons, always discard.
+ if (pair.audit.id === 'uses-long-cache-ttl') return false;
+ // If we're sampling, then keep half of them.
+ if (sample) return i % 2 === 0;
+ // Otherwise return them all.
+ return true;
+ });
 }

packages/server/src/ui/routes/build-view/audit-detail/table-details.jsx

@@ -84,15 +84,23 @@ export const TableDetails = props => {
  diff => diff.type === 'itemDelta' && diff.itemKey === heading.key
  );
 
+ // With the introduction of subitems, not every item will have a value for every heading.
+ const compareItem = heading.key && compare && compare.item[heading.key];
+ const baseItem = heading.key && base && base.item[heading.key];
+
  return (
  <Fragment key={j}>
  <td className={`table-column--${itemType}`}>
- <SimpleDetails
- type={itemType}
- compareValue={compare && compare.item[heading.key]}
- baseValue={base && base.item[heading.key]}
- diff={diff}
- />
+ {compareItem || baseItem ? (
+ <SimpleDetails
+ type={itemType}
+ compareValue={compareItem}
+ baseValue={baseItem}
+ diff={diff}
+ />
+ ) : (
+ <Fragment />
+ )}
  </td>
  {insertRowLabelAfterIndex === j ? (
  <td className="table-column--row-label">

packages/utils/test/audit-diff-finder.test.js

@@ -358,6 +358,29 @@ describe('#findAuditDiffs', () => {
  ]);
  });
 
+ it('should not find a details item property diff for subItems', () => {
+ const detailsItem = {url: 'http://example.com/foo.js'};
+ const baseAudit = {
+ id: 'audit',
+ score: 0.5,
+ details: {
+ headings: [{key: null, subItemsHeading: {key: 'label'}}, {key: 'timeSpent'}],
+ items: [{...detailsItem, timeSpent: 1000, subItems: [{label: 'Label 1'}]}],
+ },
+ };
+
+ const compareAudit = {
+ id: 'audit',
+ score: 0.5,
+ details: {
+ headings: [{key: null, subItemsHeading: {key: 'label'}}, {key: 'timeSpent'}],
+ items: [{...detailsItem, timeSpent: 1000, subItems: [{label: 'Label 2'}]}],
+ },
+ };
+
+ expect(findAuditDiffs(baseAudit, compareAudit)).toEqual([]);
+ });
+
  it('should find a details item addition/removal diff', () => {
  const baseAudit = {
  id: 'audit',

types/lighthouse.d.ts

@@ -40,7 +40,8 @@ declare global {
  overallSavingsMs?: number;
  overallSavingsBytes?: number;
  headings?: Array<{
- key: string;
+ key: string | null;
+ subItemsHeading?: {key: string};
  valueType?: DetailsType;
  itemType?: DetailsType;
  label?: string;