Skip to content

Commit

Permalink
core(unused-css-rules): no more inifinity savings (#9731)
Browse files Browse the repository at this point in the history
  • Loading branch information
patrickhulce authored and brendankenny committed Sep 26, 2019
1 parent 6ff02e4 commit 39305be
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 2 deletions.
Expand Up @@ -93,8 +93,10 @@ class UnusedBytes extends Audit {
// This was an asset that was inlined in a different resource type (e.g. HTML document).
// Use the compression ratio of the resource to estimate the total transferred bytes.
const transferSize = networkRecord.transferSize || 0;
const resourceSize = networkRecord.resourceSize;
const compressionRatio = resourceSize !== undefined ? (transferSize / resourceSize) : 1;
const resourceSize = networkRecord.resourceSize || 0;
// Get the compression ratio, if it's an invalid number, assume no compression.
const compressionRatio = Number.isFinite(resourceSize) && resourceSize > 0 ?
(transferSize / resourceSize) : 1;
return Math.round(totalBytes * compressionRatio);
}
}
Expand Down
4 changes: 4 additions & 0 deletions lighthouse-core/audits/byte-efficiency/unused-css-rules.js
Expand Up @@ -47,6 +47,10 @@ class UnusedCSSRules extends ByteEfficiencyAudit {
*/
static indexStylesheetsById(styles, networkRecords) {
const indexedNetworkRecords = networkRecords
// Some phantom network records appear with a 0 resourceSize that aren't real.
// A network record that has no size data is just as good as no network record at all for our
// purposes, so we'll just filter them out. https://github.com/GoogleChrome/lighthouse/issues/9684#issuecomment-532381611
.filter(record => record.resourceSize > 0)
.reduce((indexed, record) => {
indexed[record.url] = record;
return indexed;
Expand Down
Expand Up @@ -77,6 +77,12 @@ describe('Byte efficiency base audit', () => {
const result = estimate({transferSize: 1000, resourceType}, 100);
assert.equal(result, 100);
});

it('should not error when resource size is 0', () => {
const resourceType = 'Other';
const result = estimate({transferSize: 1000, resourceSize: 0, resourceType}, 100);
assert.equal(result, 100);
});
});

it('should format details', () => {
Expand Down
Expand Up @@ -122,6 +122,7 @@ describe('Best Practices: unused css rules audit', () => {
{
url: 'file://a.css',
transferSize: 100 * 1024,
resourceSize: 100 * 1024,
resourceType: 'Stylesheet',
},
];
Expand Down Expand Up @@ -191,6 +192,36 @@ describe('Best Practices: unused css rules audit', () => {
});
});

it('handles phantom network records without size data', async () => {
const result = await UnusedCSSAudit.audit_(getArtifacts({
CSSUsage: {rules: [
{styleSheetId: 'a', used: true, startOffset: 0, endOffset: 60000}, // 40000 * 3 * 50% = 60000
], stylesheets: [
{
header: {styleSheetId: 'a', sourceURL: 'file://a.html'},
content: `${generate('123', 40000)}`, // stylesheet size of 40000 * 3 uncompressed bytes
},
]},
}), [
{
url: 'file://a.html',
transferSize: 100 * 1024 * 0.5, // compression ratio of 0.5
resourceSize: 100 * 1024,
resourceType: 'Document', // this is a document so it'll use the compressionRatio but not the raw size
},
{
url: 'file://a.html',
transferSize: 0,
resourceSize: 0,
resourceType: 'Document',
},
]);

expect(result.items).toMatchObject([
{totalBytes: 40000 * 3 * 0.5, wastedPercent: 50},
]);
});

it('does not include empty or small sheets', () => {
return UnusedCSSAudit.audit_(getArtifacts({
CSSUsage: {rules: [
Expand Down

0 comments on commit 39305be

Please sign in to comment.