Skip to content

Commit

Permalink
core(third-party-summary): expands the audit to include all urls (#15611
Browse files Browse the repository at this point in the history 
)
  • Loading branch information
@alexnj
alexnj authored Nov 27, 2023
1 parent ec66e19 commit 511718a
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 52 deletions.
51 changes: 3 additions & 48 deletions core/audits/third-party-summary.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,6 @@ const PASS_THRESHOLD_IN_MS = 250;
* @property {Map<LH.Artifacts.Entity, string[]>} urls Map of URLs under each entity.
*/

/**
* Don't bother showing resources smaller than 4KiB since they're likely to be pixels, which isn't
* too actionable.
*/
const MIN_TRANSFER_SIZE_FOR_SUBITEMS = 4096;

/** Show at most 5 sub items in the resource breakdown. */
const MAX_SUBITEMS = 5;

class ThirdPartySummary extends Audit {
/**
* @return {LH.Audit.Meta}
Expand Down Expand Up @@ -146,51 +137,15 @@ class ThirdPartySummary extends Audit {
/**
* @param {LH.Artifacts.Entity} entity
* @param {SummaryMaps} summaries
* @param {Summary} stats
* @return {Array<URLSummary>}
*/
static makeSubItems(entity, summaries, stats) {
static makeSubItems(entity, summaries) {
const entityURLs = summaries.urls.get(entity) || [];
let items = entityURLs
const items = entityURLs
.map(url => /** @type {URLSummary} */ ({url, ...summaries.byURL.get(url)}))
// Filter out any cases where byURL was missing entries.
.filter((stat) => stat.transferSize > 0)
// Sort by blocking time first, then transfer size to break ties.
.sort((a, b) => (b.blockingTime - a.blockingTime) || (b.transferSize - a.transferSize));

const subitemSummary = {transferSize: 0, blockingTime: 0, tbtImpact: 0};
const minTransferSize = Math.max(MIN_TRANSFER_SIZE_FOR_SUBITEMS, stats.transferSize / 20);
const maxSubItems = Math.min(MAX_SUBITEMS, items.length);
let numSubItems = 0;
while (numSubItems < maxSubItems) {
const nextSubItem = items[numSubItems];
if (nextSubItem.blockingTime === 0 && nextSubItem.transferSize < minTransferSize) {
// Don't include the resource in the sub-item breakdown because it didn't have a big
// enough impact on its own.
break;
}

numSubItems++;
subitemSummary.transferSize += nextSubItem.transferSize;
subitemSummary.blockingTime += nextSubItem.blockingTime;
subitemSummary.tbtImpact += nextSubItem.tbtImpact;
}
if (!subitemSummary.blockingTime && !subitemSummary.transferSize) {
// Don't bother breaking down if there are no large resources.
return [];
}
// Only show the top N entries for brevity. If there is more than one remaining entry
// we'll replace the tail entries with single remainder entry.
items = items.slice(0, numSubItems);
const remainder = {
url: str_(i18n.UIStrings.otherResourcesLabel),
transferSize: stats.transferSize - subitemSummary.transferSize,
blockingTime: stats.blockingTime - subitemSummary.blockingTime,
tbtImpact: stats.tbtImpact - subitemSummary.tbtImpact,
};
if (remainder.transferSize > minTransferSize) {
items.push(remainder);
}
return items;
}

Expand Down Expand Up @@ -231,7 +186,7 @@ class ThirdPartySummary extends Audit {
entity: entity.name,
subItems: {
type: /** @type {const} */ ('subitems'),
items: ThirdPartySummary.makeSubItems(entity, summaries, stats),
items: ThirdPartySummary.makeSubItems(entity, summaries),
},
};
})
Expand Down
27 changes: 24 additions & 3 deletions core/test/audits/third-party-summary-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,37 @@ describe('Third party summary', () => {
},
{
blockingTime: 0,
mainThreadTime: 5.094,
tbtImpact: 0,
transferSize: 8007,
url: expect.toBeDisplayString('Other resources'),
transferSize: 3161,
url: 'https://www.google-analytics.com/cx/api.js?experiment=qvpc5qIfRC2EMnbn6bbN5A',
},
{
blockingTime: 0,
mainThreadTime: 7.01,
tbtImpact: 0,
transferSize: 2949,
url: 'https://www.google-analytics.com/cx/api.js?experiment=jdCfRmudTmy-0USnJ8xPbw',
},
{
blockingTime: 0,
mainThreadTime: 27.805000000000007,
tbtImpact: 0,
transferSize: 1425,
url: 'https://www.google-analytics.com/plugins/ua/linkid.js',
},
{
blockingTime: 0,
mainThreadTime: 0,
tbtImpact: 0,
transferSize: 472,
url: 'https://www.google-analytics.com/r/collect?v=1&_v=j53&aip=1&a=749319045&t=pageview&_s=1&dl=https%3A%2F%2Fpwa.rocks%2F&ul=en-us&de=UTF-8&dt=A%20selection%20of%20Progressive%20Web%20Apps&sd=24-bit&sr=412x732&vp=412x732&je=0&_u=aGAAAAAjI~&jid=2098926753&gjid=1553792979&cid=601220267.1494017431&tid=UA-58419726-4&_gid=445409680.1494017431&_r=1&gtm=GTM-Q5SW&z=1395339825',
},
],
type: 'subitems',
},
},
]);
expect(results.details.items[1].subItems.items[1].url).toBeDisplayString('Other resources');
});

it('account for simulated throttling', async () => {
Expand Down
31 changes: 30 additions & 1 deletion core/test/fixtures/user-flows/reports/sample-flow-result.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1479,6 +1479,13 @@
"blockingTime": 0,
"transferSize": 8273,
"tbtImpact": 0
},
{
"url": "https://fonts.googleapis.com/css2?family=Poppins:wght@400;500;700&display=swap",
"mainThreadTime": 4.868,
"blockingTime": 0,
"transferSize": 2506,
"tbtImpact": 0
}
]
}
Expand Down Expand Up @@ -19056,7 +19063,29 @@
"entity": "Google Fonts",
"subItems": {
"type": "subitems",
"items": []
"items": [
{
"url": "https://fonts.googleapis.com/css2?family=Poppins:wght@400;500;700&display=swap",
"mainThreadTime": 2.98,
"blockingTime": 0,
"transferSize": 0,
"tbtImpact": 0
},
{
"url": "https://fonts.gstatic.com/s/poppins/v20/pxiByp8kv8JHgFVrLCz7Z1xlFd2JQEk.woff2",
"mainThreadTime": 0,
"blockingTime": 0,
"transferSize": 0,
"tbtImpact": 0
},
{
"url": "https://fonts.gstatic.com/s/poppins/v20/pxiEyp8kv8JHgFVrJJfecnFHGPc.woff2",
"mainThreadTime": 0,
"blockingTime": 0,
"transferSize": 0,
"tbtImpact": 0
}
]
}
}
],
Expand Down

0 comments on commit 511718a

Please sign in to comment.