Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core(unused-css-rules): no more inifinity savings #9731

Merged
merged 2 commits into from
Sep 26, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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-53238161
patrickhulce marked this conversation as resolved.
Show resolved Hide resolved
.filter(record => record.resourceSize > 0)
.reduce((indexed, record) => {
indexed[record.url] = record;
return indexed;
Expand Down
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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