Skip to content

Commit

Permalink
fix(unused-css-rules): update to support new coverage format (#2518)
Browse files Browse the repository at this point in the history
  • Loading branch information
@patrickhulce @paulirish
patrickhulce authored and paulirish committed Aug 18, 2017
1 parent 5e9a3ab commit fb19596
Show file tree
Hide file tree
Showing 7 changed files with 95 additions and 104 deletions.
3 changes: 2 additions & 1 deletion lighthouse-cli/test/smokehouse/byte-config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,14 @@
* Config file for running PWA smokehouse audits.
*/
module.exports = {
extends: 'lighthouse:default',
extends: 'lighthouse:full',
settings: {
onlyAudits: [
'offscreen-images',
'uses-webp-images',
'uses-optimized-images',
'uses-responsive-images',
'unused-css-rules',
]
}
};
22 changes: 11 additions & 11 deletions lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,17 +13,17 @@ module.exports = [
initialUrl: 'http://localhost:10200/byte-efficiency/tester.html',
url: 'http://localhost:10200/byte-efficiency/tester.html',
audits: {
// TODO: re-enable once CSS protocol has stabilized
// 'unused-css-rules': {
// score: false,
// extendedInfo: {
// value: {
// results: {
// length: 2
// }
// }
// }
// },
'unused-css-rules': {
score: '<100',
extendedInfo: {
value: {
wastedKb: 26,
results: {
length: 2
}
}
}
},
'offscreen-images': {
score: '<100',
extendedInfo: {
Expand Down
98 changes: 54 additions & 44 deletions lighthouse-core/audits/byte-efficiency/unused-css-rules.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
'use strict';

const ByteEfficiencyAudit = require('./byte-efficiency-audit');
const URL = require('../../lib/url-shim');

const PREVIEW_LENGTH = 100;

Expand Down Expand Up @@ -34,30 +33,26 @@ class UnusedCSSRules extends ByteEfficiencyAudit {
*/
static indexStylesheetsById(styles, networkRecords) {
const indexedNetworkRecords = networkRecords
.filter(record => record._resourceType && record._resourceType._name === 'stylesheet')
.reduce((indexed, record) => {
indexed[record.url] = record;
return indexed;
}, {});

return styles.reduce((indexed, stylesheet) => {
indexed[stylesheet.header.styleSheetId] = Object.assign({
used: [],
unused: [],
usedRules: [],
networkRecord: indexedNetworkRecords[stylesheet.header.sourceURL],
}, stylesheet);
return indexed;
}, {});
}

/**
* Counts the number of unused rules and adds count information to sheets.
* Adds used rules to their corresponding stylesheet.
* @param {!Array.<{styleSheetId: string, used: boolean}>} rules The output of the CSSUsage gatherer.
* @param {!Object} indexedStylesheets Stylesheet information indexed by id.
* @return {number} The number of unused rules.
*/
static countUnusedRules(rules, indexedStylesheets) {
let unused = 0;

static indexUsedRules(rules, indexedStylesheets) {
rules.forEach(rule => {
const stylesheetInfo = indexedStylesheets[rule.styleSheetId];

Expand All @@ -66,14 +61,48 @@ class UnusedCSSRules extends ByteEfficiencyAudit {
}

if (rule.used) {
stylesheetInfo.used.push(rule);
} else {
unused++;
stylesheetInfo.unused.push(rule);
stylesheetInfo.usedRules.push(rule);
}
});
}

/**
* @param {!Object} stylesheetInfo
* @return {{wastedBytes: number, totalBytes: number, wastedPercent: number}}
*/
static computeUsage(stylesheetInfo) {
let usedUncompressedBytes = 0;
const totalUncompressedBytes = stylesheetInfo.content.length;

for (const usedRule of stylesheetInfo.usedRules) {
usedUncompressedBytes += usedRule.endOffset - usedRule.startOffset;
}

const networkRecord = stylesheetInfo.networkRecord;
let totalTransferredBytes;
if (!networkRecord) {
// We don't know how many bytes this sheet used on the network, but we can guess it was
// roughly the size of the content gzipped.
// See https://discuss.httparchive.org/t/file-size-and-compression-savings/145 for multipliers
totalTransferredBytes = Math.round(totalUncompressedBytes * .5);
} else if (networkRecord._resourceType && networkRecord._resourceType._name === 'stylesheet') {
// This was a regular standalone stylesheet, just use the transfer size.
totalTransferredBytes = networkRecord.transferSize;
} else {
// This was a stylesheet 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 compressionRatio = (networkRecord._transferSize / networkRecord._resourceSize) || 1;
totalTransferredBytes = Math.round(compressionRatio * totalUncompressedBytes);
}

const percentUnused = (totalUncompressedBytes - usedUncompressedBytes) / totalUncompressedBytes;
const wastedBytes = Math.round(percentUnused * totalTransferredBytes);

return unused;
return {
wastedBytes,
wastedPercent: percentUnused * 100,
totalBytes: totalTransferredBytes,
};
}

/**
Expand Down Expand Up @@ -115,40 +144,21 @@ class UnusedCSSRules extends ByteEfficiencyAudit {
/**
* @param {!Object} stylesheetInfo The stylesheetInfo object.
* @param {string} pageUrl The URL of the page, used to identify inline styles.
* @return {{url: string, label: string, code: string}} The result for the details object.
* @return {?{url: string, wastedBytes: number, totalBytes: number}}
*/
static mapSheetToResult(stylesheetInfo, pageUrl) {
const numUsed = stylesheetInfo.used.length;
const numUnused = stylesheetInfo.unused.length;

if ((numUsed === 0 && numUnused === 0) || stylesheetInfo.isDuplicate) {
if (stylesheetInfo.isDuplicate) {
return null;
}

let url = stylesheetInfo.header.sourceURL;
if (!url || url === pageUrl) {
const contentPreview = UnusedCSSRules.determineContentPreview(stylesheetInfo.content);
url = '*inline*```' + contentPreview + '```';
} else {
url = URL.getURLDisplayName(url);
url = {type: 'code', text: contentPreview};
}

// If we don't know for sure how many bytes this sheet used on the network,
// we can guess it was roughly the size of the content gzipped.
const totalBytes = stylesheetInfo.networkRecord ?
stylesheetInfo.networkRecord.transferSize :
Math.round(stylesheetInfo.content.length / 3);

const percentUnused = numUnused / (numUsed + numUnused);
const wastedBytes = Math.round(percentUnused * totalBytes);

return {
url,
numUnused,
wastedBytes,
wastedPercent: percentUnused * 100,
totalBytes,
};
const usage = UnusedCSSRules.computeUsage(stylesheetInfo);
return Object.assign({url}, usage);
}

/**
Expand All @@ -163,21 +173,21 @@ class UnusedCSSRules extends ByteEfficiencyAudit {
const devtoolsLogs = artifacts.devtoolsLogs[ByteEfficiencyAudit.DEFAULT_PASS];
return artifacts.requestNetworkRecords(devtoolsLogs).then(networkRecords => {
const indexedSheets = UnusedCSSRules.indexStylesheetsById(styles, networkRecords);
UnusedCSSRules.countUnusedRules(usage, indexedSheets);
const results = Object.keys(indexedSheets).map(sheetId => {
return UnusedCSSRules.mapSheetToResult(indexedSheets[sheetId], pageUrl);
}).filter(sheet => sheet && sheet.wastedBytes > 1024);
UnusedCSSRules.indexUsedRules(usage, indexedSheets);

const results = Object.keys(indexedSheets)
.map(sheetId => UnusedCSSRules.mapSheetToResult(indexedSheets[sheetId], pageUrl))
.filter(sheet => sheet && sheet.wastedBytes > 1024);

const headings = [
{key: 'url', itemType: 'url', text: 'URL'},
{key: 'numUnused', itemType: 'url', text: 'Unused Rules'},
{key: 'totalKb', itemType: 'text', text: 'Original'},
{key: 'potentialSavings', itemType: 'text', text: 'Potential Savings'},
];

return {
results,
headings
headings,
};
});
}
Expand Down
3 changes: 0 additions & 3 deletions lighthouse-core/config/default.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ module.exports = {
'manifest',
'chrome-console-messages',
'image-usage',
// 'css-usage',
'accessibility',
'dobetterweb/all-event-listeners',
'dobetterweb/anchors-with-no-rel-noopener',
Expand Down Expand Up @@ -123,7 +122,6 @@ module.exports = {
'accessibility/video-caption',
'accessibility/video-description',
'byte-efficiency/total-byte-weight',
// 'byte-efficiency/unused-css-rules',
'byte-efficiency/offscreen-images',
'byte-efficiency/uses-webp-images',
'byte-efficiency/uses-optimized-images',
Expand Down Expand Up @@ -229,7 +227,6 @@ module.exports = {
{id: 'estimated-input-latency', weight: 1, group: 'perf-metric'},
{id: 'link-blocking-first-paint', weight: 0, group: 'perf-hint'},
{id: 'script-blocking-first-paint', weight: 0, group: 'perf-hint'},
// {id: 'unused-css-rules', weight: 0},
{id: 'uses-responsive-images', weight: 0, group: 'perf-hint'},
{id: 'offscreen-images', weight: 0, group: 'perf-hint'},
{id: 'uses-optimized-images', weight: 0, group: 'perf-hint'},
Expand Down
11 changes: 9 additions & 2 deletions lighthouse-core/config/full-config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,24 @@ module.exports = {
passName: 'extraPass',
gatherers: [
'styles',
'css-usage',
]
},
],
audits: [
'byte-efficiency/unused-css-rules',
'dobetterweb/no-old-flexbox',
],
categories: {
'performance': {
audits: [
{id: 'unused-css-rules', weight: 0, group: 'perf-hint'},
],
},
'best-practices': {
audits: [
{id: 'no-old-flexbox', weight: 1},
]
}
],
},
},
};
10 changes: 1 addition & 9 deletions lighthouse-core/gather/gatherers/css-usage.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,7 @@ class CSSUsage extends Gatherer {
beforePass(options) {
return options.driver.sendCommand('DOM.enable')
.then(_ => options.driver.sendCommand('CSS.enable'))
.then(_ => options.driver.sendCommand('CSS.startRuleUsageTracking'))
.catch(err => {
// TODO(phulce): Remove this once CSS usage hits stable
if (/startRuleUsageTracking/.test(err.message)) {
throw new Error('CSS Usage tracking requires Chrome \u2265 56');
}

throw err;
});
.then(_ => options.driver.sendCommand('CSS.startRuleUsageTracking'));
}

afterPass(options) {
Expand Down
52 changes: 18 additions & 34 deletions lighthouse-core/test/audits/byte-efficiency/unused-css-rules-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,25 +98,21 @@ describe('Best Practices: unused css rules audit', () => {
baseSheet = {
header: {sourceURL: baseUrl},
content: 'dummy',
used: [{dummy: 1}],
unused: [],
usedRules: [],
};
});

it('correctly computes potentialSavings', () => {
assert.equal(map({used: [], unused: [1, 2]}).wastedPercent, 100);
assert.equal(map({used: [1, 2], unused: [1, 2]}).wastedPercent, 50);
assert.equal(map({used: [1, 2], unused: []}).wastedPercent, 0);
assert.equal(map({usedRules: []}).wastedPercent, 100);
assert.equal(map({usedRules: [{startOffset: 0, endOffset: 3}]}).wastedPercent, 40);
assert.equal(map({usedRules: [{startOffset: 0, endOffset: 5}]}).wastedPercent, 0);
});

it('correctly computes url', () => {
assert.equal(map({header: {sourceURL: ''}}).url, '*inline*```dummy```');
assert.equal(map({header: {sourceURL: 'a'}}, 'http://g.co/a').url, '*inline*```dummy```');
assert.equal(map({header: {sourceURL: 'foobar'}}).url, '/foobar');
});

it('does not give content preview when url is present', () => {
assert.ok(!/dummy/.test(map({header: {sourceURL: 'foobar'}}).url));
const expectedPreview = {type: 'code', text: 'dummy'};
assert.deepEqual(map({header: {sourceURL: ''}}).url, expectedPreview);
assert.deepEqual(map({header: {sourceURL: 'a'}}, 'http://g.co/a').url, expectedPreview);
assert.equal(map({header: {sourceURL: 'foobar'}}).url, 'http://g.co/foobar');
});
});

Expand Down Expand Up @@ -175,13 +171,8 @@ describe('Best Practices: unused css rules audit', () => {
requestNetworkRecords,
URL: {finalUrl: ''},
CSSUsage: [
{styleSheetId: 'a', used: true},
{styleSheetId: 'a', used: false},
{styleSheetId: 'a', used: false},
{styleSheetId: 'a', used: false},
{styleSheetId: 'b', used: true},
{styleSheetId: 'b', used: false},
{styleSheetId: 'c', used: false},
{styleSheetId: 'a', used: true, startOffset: 0, endOffset: 11}, // 44 * 1 / 4
{styleSheetId: 'b', used: true, startOffset: 0, endOffset: 3075}, // 2050 * 3 / 2
],
Styles: [
{
Expand All @@ -200,7 +191,7 @@ describe('Best Practices: unused css rules audit', () => {
}).then(result => {
assert.equal(result.results.length, 2);
assert.equal(result.results[0].totalBytes, 10 * 1024);
assert.equal(result.results[1].totalBytes, 2050);
assert.equal(result.results[1].totalBytes, 3075);
assert.equal(result.results[0].wastedPercent, 75);
assert.equal(result.results[1].wastedPercent, 50);
});
Expand All @@ -212,10 +203,7 @@ describe('Best Practices: unused css rules audit', () => {
requestNetworkRecords,
URL: {finalUrl: ''},
CSSUsage: [
{styleSheetId: 'a', used: true},
{styleSheetId: 'a', used: true},
{styleSheetId: 'a', used: false},
{styleSheetId: 'b', used: false},
{styleSheetId: 'a', used: true, startOffset: 0, endOffset: 33}, // 44 * 3 / 4
],
Styles: [
{
Expand All @@ -239,12 +227,8 @@ describe('Best Practices: unused css rules audit', () => {
requestNetworkRecords,
URL: {finalUrl: ''},
CSSUsage: [
{styleSheetId: 'a', used: true},
{styleSheetId: 'a', used: true},
{styleSheetId: 'a', used: false},
{styleSheetId: 'b', used: true},
{styleSheetId: 'b', used: false},
{styleSheetId: 'b', used: false},
{styleSheetId: 'a', used: true, startOffset: 0, endOffset: 8000}, // 4000 * 3 / 2
{styleSheetId: 'b', used: true, startOffset: 0, endOffset: 500}, // 500 * 3 / 3
],
Styles: [
{
Expand All @@ -256,21 +240,21 @@ describe('Best Practices: unused css rules audit', () => {
content: `${generate('123', 500)}`
},
{
header: {styleSheetId: 'c', sourceURL: 'c.css'},
header: {styleSheetId: 'c', sourceURL: 'file://c.css'},
content: '@import url(http://googlefonts.com?myfont)'
},
{
header: {styleSheetId: 'd', sourceURL: 'd.css'},
header: {styleSheetId: 'd', sourceURL: 'file://d.css'},
content: '/* nothing to see here */'
},
{
header: {styleSheetId: 'e', sourceURL: 'e.css'},
header: {styleSheetId: 'e', sourceURL: 'file://e.css'},
content: ' '
},
]
}).then(result => {
assert.equal(result.results.length, 1);
assert.equal(result.results[0].numUnused, 1);
assert.equal(Math.floor(result.results[0].wastedPercent), 33);
});
});
});
Expand Down

0 comments on commit fb19596

Please sign in to comment.