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(lhr): overhaul LHR details, introduce details.summary #4616

Merged
merged 4 commits into from
Mar 9, 2018
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 @@ -179,9 +179,7 @@ module.exports = [
details: {
items: {
0: {
2: {
text: /^480 x 57/,
},
displayedAspectRatio: /^480 x 57/,
},
length: 1,
},
Expand Down
7 changes: 7 additions & 0 deletions lighthouse-cli/test/smokehouse/smokehouse.js
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,13 @@ function collateResults(actual, expected) {
throw new Error(`Config did not trigger run of expected audit ${auditName}`);
}

// TODO: TERRIBLE HACK, remove asap
// Smokehouse has been asserting the AuditResult.score, whereas report cares about AuditDfn score (within reportCategories)
// A subsequent PR will change scores completely which means rebaselining all our smokehouse expectations
if (actualResult.scoringMode === 'binary') {
actualResult.score = Boolean(actualResult.score);
}

const expectedResult = expected.audits[auditName];
const diff = findDifference(auditName, actualResult, expectedResult);

Expand Down
73 changes: 30 additions & 43 deletions lighthouse-core/audits/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,54 +70,27 @@ class Audit {
});
}

/**
* Table cells will use the type specified in headings[x].itemType. However a custom type
* can be provided: results[x].propName = {type: 'code', text: '...'}
* @param {!Audit.Headings} headings
* @param {!Array<!Object<string, *>>} results
* @return {!Array<!DetailsRenderer.DetailsJSON>}
*/
static makeTableRows(headings, results) {
const tableRows = results.map(item => {
return headings.map(heading => {
const value = item[heading.key];
if (typeof value === 'object' && value && value.type) return value;

return {
type: heading.itemType,
text: value,
};
});
});
return tableRows;
}

/**
* @param {!Audit.Headings} headings
* @return {!Array<!DetailsRenderer.DetailsJSON>}
*/
static makeTableHeaders(headings) {
return headings.map(heading => ({
type: 'text',
itemKey: heading.key,
itemType: heading.itemType,
text: heading.text,
}));
}

/**
* @param {!Audit.Headings} headings
* @param {!Array<!Object<string, string>>} results
* @param {!DetailsRenderer.DetailsSummary} summary
* @return {!DetailsRenderer.DetailsJSON}
*/
static makeTableDetails(headings, results) {
const tableHeaders = Audit.makeTableHeaders(headings);
const tableRows = Audit.makeTableRows(headings, results);
static makeTableDetails(headings, results, summary) {
if (results.length === 0) {
return {
type: 'table',
headings: [],
items: [],
summary,
};
}

return {
type: 'table',
header: 'View Details',
itemHeaders: tableHeaders,
items: tableRows,
headings: headings,
items: results,
summary,
};
}

Expand All @@ -141,20 +114,35 @@ class Audit {
if (displayValue === score) {
displayValue = '';
}

// TODO: restore after initial 3.0 branching
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

delete?

// if (typeof score === 'boolean' || score === null) {
// score = score ? 100 : 0;
// }

// if (!Number.isFinite(score)) {
// throw new Error(`Invalid score: ${score}`);
// }

// TODO, don't consider an auditResult's scoringMode (currently applied to all ByteEfficiency)
const scoringMode = result.scoringMode || audit.meta.scoringMode || Audit.SCORING_MODES.BINARY;
delete result.scoringMode;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add note that will be removed in next PR


let auditDescription = audit.meta.description;
if (audit.meta.failureDescription) {
if (!score || (typeof score === 'number' && score < 100)) {
auditDescription = audit.meta.failureDescription;
}
}

return {
score,
displayValue: `${displayValue}`,
rawValue: result.rawValue,
error: result.error,
debugString: result.debugString,
extendedInfo: result.extendedInfo,
scoringMode: audit.meta.scoringMode || Audit.SCORING_MODES.BINARY,
scoringMode,
informative: audit.meta.informative,
manual: audit.meta.manual,
notApplicable: result.notApplicable,
Expand All @@ -172,7 +160,6 @@ module.exports = Audit;
* @typedef {Object} Audit.Heading
* @property {string} key
* @property {string} itemType
* @property {string} itemKey
* @property {string} text
*/

Expand Down
5 changes: 3 additions & 2 deletions lighthouse-core/audits/bootup-time.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,14 @@ class BootupTime extends Audit {
.filter(result => result.sum >= THRESHOLD_IN_MS)
.sort((a, b) => b.sum - a.sum);

const tableDetails = BootupTime.makeTableDetails(headings, results);
const summary = {wastedMs: totalBootupTime};
const details = BootupTime.makeTableDetails(headings, results, summary);

return {
score: totalBootupTime < 2000,
rawValue: totalBootupTime,
displayValue: Util.formatMilliseconds(totalBootupTime),
details: tableDetails,
details,
extendedInfo: {
value: extendedInfo,
},
Expand Down
36 changes: 11 additions & 25 deletions lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,31 +28,14 @@ class UnusedBytes extends Audit {
else return 0;
}

/**
* @param {number} bytes
* @return {string}
*/
static bytesDetails(bytes) {
return {
type: 'bytes',
value: bytes,
displayUnit: 'kb',
granularity: 1,
};
}

/**
* @param {number} bytes
* @param {number} networkThroughput measured in bytes/second
* @return {string}
*/
static bytesToMsDetails(bytes, networkThroughput) {
static bytesToMs(bytes, networkThroughput) {
const milliseconds = bytes / networkThroughput * 1000;
return {
type: 'ms',
value: milliseconds,
granularity: 10,
};
return milliseconds;
}

/**
Expand Down Expand Up @@ -109,10 +92,8 @@ class UnusedBytes extends Audit {
const debugString = result.debugString;
const results = result.results
.map(item => {
item.wastedKb = this.bytesDetails(item.wastedBytes);
item.wastedMs = this.bytesToMsDetails(item.wastedBytes, networkThroughput);
item.totalKb = this.bytesDetails(item.totalBytes);
item.totalMs = this.bytesToMsDetails(item.totalBytes, networkThroughput);
item.wastedMs = this.bytesToMs(item.wastedBytes, networkThroughput);
item.totalMs = this.bytesToMs(item.totalBytes, networkThroughput);
return item;
})
.sort((itemA, itemB) => itemB.wastedBytes - itemA.wastedBytes);
Expand All @@ -126,21 +107,26 @@ class UnusedBytes extends Audit {
displayValue = `Potential savings of ${wastedBytes} bytes`;
}

const tableDetails = Audit.makeTableDetails(result.headings, results);
const summary = {
wastedMs,
wastedBytes,
};
const details = Audit.makeTableDetails(result.headings, results, summary);

return {
debugString,
displayValue,
rawValue: wastedMs,
score: UnusedBytes.scoreForWastedMs(wastedMs),
scoringMode: Audit.SCORING_MODES.NUMERIC,
extendedInfo: {
value: {
wastedMs,
wastedKb,
results,
},
},
details: tableDetails,
details,
};
}

Expand Down
29 changes: 16 additions & 13 deletions lighthouse-core/audits/byte-efficiency/offscreen-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ class OffscreenImages extends ByteEfficiencyAudit {
name: 'offscreen-images',
description: 'Offscreen images',
informative: true,
helpText: 'Consider lazy-loading offscreen and hidden images to improve page load speed ' +
helpText:
'Consider lazy-loading offscreen and hidden images to improve page load speed ' +
'and time to interactive. ' +
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/offscreen-images).',
requiredArtifacts: ['ImageUsage', 'ViewportDimensions', 'traces', 'devtoolsLogs'],
Expand Down Expand Up @@ -72,11 +73,6 @@ class OffscreenImages extends ByteEfficiencyAudit {

return {
url,
preview: {
type: 'thumbnail',
url: image.networkRecord.url,
mimeType: image.networkRecord.mimeType,
},
requestStartTime: image.networkRecord.startTime,
totalBytes,
wastedBytes,
Expand Down Expand Up @@ -107,9 +103,9 @@ class OffscreenImages extends ByteEfficiencyAudit {
}

// If an image was used more than once, warn only about its least wasteful usage
const existing = results.get(processed.preview.url);
const existing = results.get(processed.url);
if (!existing || existing.wastedBytes > processed.wastedBytes) {
results.set(processed.preview.url, processed);
results.set(processed.url, processed);
}

return results;
Expand All @@ -118,17 +114,24 @@ class OffscreenImages extends ByteEfficiencyAudit {
return artifacts.requestFirstInteractive(trace).then(firstInteractive => {
const ttiTimestamp = firstInteractive.timestamp / 1000000;
const results = Array.from(resultsMap.values()).filter(item => {
const isWasteful = item.wastedBytes > IGNORE_THRESHOLD_IN_BYTES &&
item.wastedPercent > IGNORE_THRESHOLD_IN_PERCENT;
const isWasteful =
item.wastedBytes > IGNORE_THRESHOLD_IN_BYTES &&
item.wastedPercent > IGNORE_THRESHOLD_IN_PERCENT;
const loadedEarly = item.requestStartTime < ttiTimestamp;
return isWasteful && loadedEarly;
});

const headings = [
{key: 'preview', itemType: 'thumbnail', text: ''},
{key: 'url', itemType: 'thumbnail', text: ''},
{key: 'url', itemType: 'url', text: 'URL'},
{key: 'totalKb', itemType: 'text', text: 'Original'},
{key: 'wastedKb', itemType: 'text', text: 'Potential Savings'},
{key: 'totalBytes', itemType: 'bytes', displayUnit: 'kb', granularity: 1, text: 'Original'},
{
key: 'wastedBytes',
itemType: 'bytes',
displayUnit: 'kb',
granularity: 1,
text: 'Potential Savings',
},
];

return {
Expand Down
23 changes: 14 additions & 9 deletions lighthouse-core/audits/byte-efficiency/total-byte-weight.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
'use strict';

const ByteEfficiencyAudit = require('./byte-efficiency-audit');
const Util = require('../../report/v2/renderer/util');

// Parameters for log-normal CDF scoring. See https://www.desmos.com/calculator/gpmjeykbwr
// ~75th and ~90th percentiles http://httparchive.org/interesting.php?a=All&l=Feb%201%202017&s=All#bytesTotal
Expand All @@ -22,9 +23,9 @@ class TotalByteWeight extends ByteEfficiencyAudit {
description: 'Avoids enormous network payloads',
failureDescription: 'Has enormous network payloads',
helpText:
'Large network payloads cost users real money and are highly correlated with ' +
'long load times. [Learn ' +
'more](https://developers.google.com/web/tools/lighthouse/audits/network-payloads).',
'Large network payloads cost users real money and are highly correlated with ' +
'long load times. [Learn ' +
'more](https://developers.google.com/web/tools/lighthouse/audits/network-payloads).',
scoringMode: ByteEfficiencyAudit.SCORING_MODES.NUMERIC,
requiredArtifacts: ['devtoolsLogs'],
};
Expand All @@ -50,8 +51,7 @@ class TotalByteWeight extends ByteEfficiencyAudit {
const result = {
url: record.url,
totalBytes: record.transferSize,
totalKb: ByteEfficiencyAudit.bytesDetails(record.transferSize),
totalMs: ByteEfficiencyAudit.bytesToMsDetails(record.transferSize, networkThroughput),
totalMs: ByteEfficiencyAudit.bytesToMs(record.transferSize, networkThroughput),
};

totalBytes += result.totalBytes;
Expand All @@ -60,7 +60,6 @@ class TotalByteWeight extends ByteEfficiencyAudit {
const totalCompletedRequests = results.length;
results = results.sort((itemA, itemB) => itemB.totalBytes - itemA.totalBytes).slice(0, 10);


// Use the CDF of a log-normal distribution for scoring.
// <= 1600KB: score≈100
// 4000KB: score=50
Expand All @@ -73,16 +72,22 @@ class TotalByteWeight extends ByteEfficiencyAudit {

const headings = [
{key: 'url', itemType: 'url', text: 'URL'},
{key: 'totalKb', itemType: 'text', text: 'Total Size'},
{key: 'totalMs', itemType: 'text', text: 'Transfer Time'},
{
key: 'totalBytes',
itemType: 'bytes',
displayUnit: 'kb',
granularity: 1,
text: 'Total Size',
},
{key: 'totalMs', itemType: 'ms', text: 'Transfer Time'},
];

const tableDetails = ByteEfficiencyAudit.makeTableDetails(headings, results);

return {
score,
rawValue: totalBytes,
displayValue: `Total size was ${Math.round(totalBytes / 1024)} KB`,
displayValue: `Total size was ${Util.formatBytesToKB(totalBytes, 1)}`,
extendedInfo: {
value: {
results,
Expand Down
7 changes: 4 additions & 3 deletions lighthouse-core/audits/byte-efficiency/unminified-css.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class UnminifiedCSS extends ByteEfficiencyAudit {
let url = stylesheet.header.sourceURL;
if (!url || url === pageUrl) {
const contentPreview = UnusedCSSRules.determineContentPreview(stylesheet.content);
url = {type: 'code', text: contentPreview};
url = {type: 'code', value: contentPreview};
}

const totalBytes = ByteEfficiencyAudit.estimateTransferSize(networkRecord, content.length,
Expand Down Expand Up @@ -144,8 +144,9 @@ class UnminifiedCSS extends ByteEfficiencyAudit {
results,
headings: [
{key: 'url', itemType: 'url', text: 'URL'},
{key: 'totalKb', itemType: 'text', text: 'Original'},
{key: 'wastedKb', itemType: 'text', text: 'Potential Savings'},
{key: 'totalBytes', itemType: 'bytes', displayUnit: 'kb', granularity: 1, text: 'Original'},
{key: 'wastedBytes', itemType: 'bytes', displayUnit: 'kb', granularity: 1,
text: 'Potential Savings'},
],
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,9 @@ class UnminifiedJavaScript extends ByteEfficiencyAudit {
debugString,
headings: [
{key: 'url', itemType: 'url', text: 'URL'},
{key: 'totalKb', itemType: 'text', text: 'Original'},
{key: 'wastedKb', itemType: 'text', text: 'Potential Savings'},
{key: 'totalBytes', itemType: 'bytes', displayUnit: 'kb', granularity: 1, text: 'Original'},
{key: 'wastedBytes', itemType: 'bytes', displayUnit: 'kb', granularity: 1,
text: 'Potential Savings'},
],
};
}
Expand Down
Loading