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): migrate opportunity details to new format #5296

Merged
merged 11 commits into from
May 25, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
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
38 changes: 11 additions & 27 deletions lighthouse-cli/test/smokehouse/byte-efficiency/expectations.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ module.exports = [
audits: {
'unminified-css': {
details: {
summary: {
wastedBytes: '>17000',
},
overallSavingsBytes: '>17000',
items: {
length: 1,
},
Expand All @@ -29,20 +27,16 @@ module.exports = [
'unminified-javascript': {
score: '<1',
details: {
summary: {
wastedBytes: '>45000',
wastedMs: '>500',
},
overallSavingsBytes: '>45000',
overallSavingsMs: '>500',
items: {
length: 1,
},
},
},
'unused-css-rules': {
details: {
summary: {
wastedBytes: '>39000',
},
overallSavingsBytes: '>39000',
items: {
length: 2,
},
Expand All @@ -51,10 +45,8 @@ module.exports = [
'unused-javascript': {
score: '<1',
details: {
summary: {
wastedBytes: '>=25000',
wastedMs: '>300',
},
overallSavingsBytes: '>=25000',
overallSavingsMs: '>300',
items: {
length: 2,
},
Expand All @@ -77,9 +69,7 @@ module.exports = [
},
'uses-webp-images': {
details: {
summary: {
wastedBytes: '>60000',
},
overallSavingsBytes: '>60000',
items: {
length: 4,
},
Expand All @@ -88,20 +78,16 @@ module.exports = [
'uses-text-compression': {
score: '<1',
details: {
summary: {
wastedMs: '>700',
wastedBytes: '>50000',
},
overallSavingsMs: '>700',
overallSavingsBytes: '>50000',
items: {
length: 2,
},
},
},
'uses-optimized-images': {
details: {
summary: {
wastedBytes: '>10000',
},
overallSavingsBytes: '>10000',
items: {
length: 1,
},
Expand All @@ -113,9 +99,7 @@ module.exports = [
75,
],
details: {
summary: {
wastedBytes: '>75000',
},
overallSavingsBytes: '>75000',
items: [
{wastedPercent: '<60'},
{wastedPercent: '<60'},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,7 @@ module.exports = [
'efficient-animated-content': {
score: 0,
details: {
summary: {
wastedMs: '>2000',
},
overallSavingsMs: '>2000',
items: [
{
url: 'http://localhost:10200/dobetterweb/lighthouse-rotating.gif',
Expand Down
17 changes: 17 additions & 0 deletions lighthouse-core/audits/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,23 @@ class Audit {
};
}

/**
* @param {Array<LH.ResultLite.Audit.ColumnHeading>} headings
* @param {Array<LH.ResultLite.Audit.WastedBytesDetailsItem>|Array<LH.ResultLite.Audit.WastedTimeDetailsItem>} items
* @param {number} overallSavingsMs
* @param {number=} overallSavingsBytes
* @return {LH.Result.Audit.OpportunityDetails}
*/
static makeOpportunityDetails(headings, items, overallSavingsMs, overallSavingsBytes) {
Copy link
Member Author

Choose a reason for hiding this comment

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

there's no real reason this needs to be a thing, we just need a place to check that audits are returning the correct shape. If we start enforcing the return type of audits based on the details they provide (e.g. @return {LH.Audit.Result<OpportunityDetails>} or whatever) that could work just as well.

return {
type: 'opportunity',
headings: items.length === 0 ? [] : headings,
items,
overallSavingsMs,
overallSavingsBytes,
};
}

/**
* @param {typeof Audit} audit
* @param {LH.Audit.Product} result
Expand Down
27 changes: 15 additions & 12 deletions lighthouse-core/audits/byte-efficiency/byte-efficiency-audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,15 @@ const KB_IN_BYTES = 1024;
const WASTED_MS_FOR_AVERAGE = 300;
const WASTED_MS_FOR_POOR = 750;

/**
* @typedef {object} ByteEfficiencyProduct
* @property {Array<LH.Audit.ByteEfficiencyItem>} items
* @property {LH.Result.Audit.OpportunityDetails['headings']} headings
* @property {string} [displayValue]
* @property {string} [explanation]
* @property {Array<string>} [warnings]
*/

/**
* @overview Used as the base for all byte efficiency audits. Computes total bytes
* and estimated time saved. Subclass and override `audit_` to return results.
Expand Down Expand Up @@ -104,7 +113,7 @@ class UnusedBytes extends Audit {
* - end time of the last long task in the provided graph
* - (if includeLoad is true or not provided) end time of the last node in the graph
*
* @param {Array<LH.Audit.ByteEfficiencyResult>} results The array of byte savings results per resource
* @param {Array<LH.Audit.ByteEfficiencyItem>} results The array of byte savings results per resource
* @param {Node} graph
* @param {Simulator} simulator
* @param {{includeLoad?: boolean}=} options
Expand All @@ -114,7 +123,7 @@ class UnusedBytes extends Audit {
options = Object.assign({includeLoad: true}, options);

const simulationBeforeChanges = simulator.simulate(graph);
/** @type {Map<LH.Audit.ByteEfficiencyResult['url'], LH.Audit.ByteEfficiencyResult>} */
/** @type {Map<string, LH.Audit.ByteEfficiencyItem>} */
const resultsByUrl = new Map();
for (const result of results) {
resultsByUrl.set(result.url, result);
Expand Down Expand Up @@ -159,13 +168,13 @@ class UnusedBytes extends Audit {
}

/**
* @param {LH.Audit.ByteEfficiencyProduct} result
* @param {ByteEfficiencyProduct} result
* @param {Node} graph
* @param {Simulator} simulator
* @return {LH.Audit.Product}
*/
static createAuditProduct(result, graph, simulator) {
const results = result.results.sort((itemA, itemB) => itemB.wastedBytes - itemA.wastedBytes);
const results = result.items.sort((itemA, itemB) => itemB.wastedBytes - itemA.wastedBytes);

const wastedBytes = results.reduce((sum, item) => sum + item.wastedBytes, 0);
const wastedKb = Math.round(wastedBytes / KB_IN_BYTES);
Expand All @@ -177,13 +186,7 @@ class UnusedBytes extends Audit {
displayValue = ['Potential savings of %d\xa0KB', wastedKb];
}

const summary = {
wastedMs,
wastedBytes,
};

// @ts-ignore - TODO(bckenny): unify details types. items shouldn't be an indexed type.
const details = Audit.makeTableDetails(result.headings, results, summary);
const details = Audit.makeOpportunityDetails(result.headings, results, wastedMs, wastedBytes);

return {
explanation: result.explanation,
Expand All @@ -208,7 +211,7 @@ class UnusedBytes extends Audit {
* @param {LH.Artifacts} artifacts
* @param {Array<LH.WebInspector.NetworkRequest>} networkRecords
* @param {LH.Audit.Context} context
* @return {LH.Audit.ByteEfficiencyProduct|Promise<LH.Audit.ByteEfficiencyProduct>}
* @return {ByteEfficiencyProduct|Promise<ByteEfficiencyProduct>}
*/
static audit_(artifacts, networkRecords, context) {
throw new Error('audit_ unimplemented');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class EfficientAnimatedContent extends ByteEfficiencyAudit {
/**
* @param {LH.Artifacts} artifacts
* @param {Array<LH.WebInspector.NetworkRequest>} networkRecords
* @return {LH.Audit.ByteEfficiencyProduct}
* @return {ByteEfficiencyAudit.ByteEfficiencyProduct}
*/
static audit_(artifacts, networkRecords) {
const unoptimizedContent = networkRecords.filter(
Expand All @@ -54,7 +54,7 @@ class EfficientAnimatedContent extends ByteEfficiencyAudit {
);

/** @type {Array<{url: string, totalBytes: number, wastedBytes: number}>}*/
const results = unoptimizedContent.map(record => {
const items = unoptimizedContent.map(record => {
const resourceSize = record._resourceSize || 0;
return {
url: record.url,
Expand All @@ -64,26 +64,15 @@ class EfficientAnimatedContent extends ByteEfficiencyAudit {
};
});

/** @type {LH.Result.Audit.OpportunityDetails['headings']} */
const headings = [
{key: 'url', itemType: 'url', text: 'URL'},
{
key: 'totalBytes',
itemType: 'bytes',
displayUnit: 'kb',
granularity: 1,
text: 'Transfer Size',
},
{
key: 'wastedBytes',
itemType: 'bytes',
displayUnit: 'kb',
granularity: 1,
text: 'Byte Savings',
},
{key: 'url', valueType: 'url', label: 'URL'},
{key: 'totalBytes', valueType: 'bytes', label: 'Transfer Size'},
{key: 'wastedBytes', valueType: 'bytes', label: 'Byte Savings'},
];

return {
results,
items,
headings,
};
}
Expand Down
23 changes: 9 additions & 14 deletions lighthouse-core/audits/byte-efficiency/offscreen-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class OffscreenImages extends ByteEfficiencyAudit {
* images won't reduce the overall time and the wasted bytes are really only "wasted" for TTI,
* override the function to just look at TTI savings.
*
* @param {Array<LH.Audit.ByteEfficiencyResult>} results
* @param {Array<LH.Audit.ByteEfficiencyItem>} results
* @param {LH.Gatherer.Simulation.GraphNode} graph
* @param {LH.Gatherer.Simulation.Simulator} simulator
* @return {number}
Expand All @@ -105,7 +105,7 @@ class OffscreenImages extends ByteEfficiencyAudit {
* @param {LH.Artifacts} artifacts
* @param {Array<LH.WebInspector.NetworkRequest>} networkRecords
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.ByteEfficiencyProduct>}
* @return {Promise<ByteEfficiencyAudit.ByteEfficiencyProduct>}
*/
static audit_(artifacts, networkRecords, context) {
const images = artifacts.ImageUsage;
Expand Down Expand Up @@ -144,30 +144,25 @@ class OffscreenImages extends ByteEfficiencyAudit {
// graph simulation doing the right thing.
const ttiTimestamp = firstInteractive.timestamp ? firstInteractive.timestamp / 1e6 : Infinity;

const results = Array.from(resultsMap.values()).filter(item => {
const items = Array.from(resultsMap.values()).filter(item => {
const isWasteful =
item.wastedBytes > IGNORE_THRESHOLD_IN_BYTES &&
item.wastedPercent > IGNORE_THRESHOLD_IN_PERCENT;
const loadedEarly = item.requestStartTime < ttiTimestamp;
return isWasteful && loadedEarly;
});

/** @type {LH.Result.Audit.OpportunityDetails['headings']} */
const headings = [
{key: 'url', itemType: 'thumbnail', text: ''},
{key: 'url', itemType: 'url', text: 'URL'},
{key: 'totalBytes', itemType: 'bytes', displayUnit: 'kb', granularity: 1, text: 'Original'},
{
key: 'wastedBytes',
itemType: 'bytes',
displayUnit: 'kb',
granularity: 1,
text: 'Potential Savings',
},
{key: 'url', valueType: 'thumbnail', label: ''},
{key: 'url', valueType: 'url', label: 'URL'},
{key: 'totalBytes', valueType: 'bytes', label: 'Original'},
{key: 'wastedBytes', valueType: 'bytes', label: 'Potential Savings'},
];

return {
warnings,
results,
items,
headings,
};
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,20 +207,14 @@ class RenderBlockingResources extends Audit {
displayValue = `${results.length} resource delayed first paint by ${wastedMs}ms`;
}

/** @type {LH.Result.Audit.OpportunityDetails['headings']} */
const headings = [
{key: 'url', itemType: 'url', text: 'URL'},
{
key: 'totalBytes',
itemType: 'bytes',
displayUnit: 'kb',
granularity: 0.01,
text: 'Size (KB)',
},
{key: 'wastedMs', itemType: 'ms', text: 'Download Time (ms)', granularity: 1},
{key: 'url', valueType: 'url', label: 'URL'},
{key: 'totalBytes', valueType: 'bytes', label: 'Size (KB)'},
{key: 'wastedMs', valueType: 'timespanMs', label: 'Download Time (ms)'},
Copy link
Member

Choose a reason for hiding this comment

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

we'll be dropping granularity of 1 and getting default of 10 afaict.
that is probably fine, but just FYI

];

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

return {
displayValue,
Expand Down
20 changes: 9 additions & 11 deletions lighthouse-core/audits/byte-efficiency/unminified-css.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,17 +94,16 @@ class UnminifiedCSS extends ByteEfficiencyAudit {
* @param {LH.Artifacts.CSSStyleSheetInfo} stylesheet
* @param {LH.WebInspector.NetworkRequest=} networkRecord
* @param {string} pageUrl
* @return {{url: string|LH.Audit.DetailsRendererCodeDetailJSON, totalBytes: number, wastedBytes: number, wastedPercent: number}}
* @return {{url: string, totalBytes: number, wastedBytes: number, wastedPercent: number}}
*/
static computeWaste(stylesheet, networkRecord, pageUrl) {
const content = stylesheet.content;
const totalTokenLength = UnminifiedCSS.computeTokenLength(content);

/** @type {LH.Audit.ByteEfficiencyResult['url']} */
let url = stylesheet.header.sourceURL;
if (!url || url === pageUrl) {
const contentPreview = UnusedCSSRules.determineContentPreview(stylesheet.content);
url = {type: 'code', value: contentPreview};
url = contentPreview;
}

const totalBytes = ByteEfficiencyAudit.estimateTransferSize(networkRecord, content.length,
Expand All @@ -123,11 +122,11 @@ class UnminifiedCSS extends ByteEfficiencyAudit {
/**
* @param {LH.Artifacts} artifacts
* @param {Array<LH.WebInspector.NetworkRequest>} networkRecords
* @return {LH.Audit.ByteEfficiencyProduct}
* @return {ByteEfficiencyAudit.ByteEfficiencyProduct}
*/
static audit_(artifacts, networkRecords) {
const pageUrl = artifacts.URL.finalUrl;
const results = [];
const items = [];
for (const stylesheet of artifacts.CSSUsage.stylesheets) {
const networkRecord = networkRecords
.find(record => record.url === stylesheet.header.sourceURL);
Expand All @@ -140,16 +139,15 @@ class UnminifiedCSS extends ByteEfficiencyAudit {
if (result.wastedPercent < IGNORE_THRESHOLD_IN_PERCENT ||
result.wastedBytes < IGNORE_THRESHOLD_IN_BYTES ||
!Number.isFinite(result.wastedBytes)) continue;
results.push(result);
items.push(result);
}

return {
results,
items,
headings: [
{key: 'url', itemType: 'url', text: 'URL'},
{key: 'totalBytes', itemType: 'bytes', displayUnit: 'kb', granularity: 1, text: 'Original'},
{key: 'wastedBytes', itemType: 'bytes', displayUnit: 'kb', granularity: 1,
text: 'Potential Savings'},
{key: 'url', valueType: 'url', label: 'URL'},
{key: 'totalBytes', valueType: 'bytes', label: 'Original'},
{key: 'wastedBytes', valueType: 'bytes', label: 'Potential Savings'},
],
};
}
Expand Down
Loading