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: merge Details.Opportunity with Details.Table #14771

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
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: 27 additions & 11 deletions cli/test/smokehouse/test-definitions/byte-efficiency.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,9 @@ const expectations = {
},
'unminified-css': {
details: {
overallSavingsBytes: '>17000',
summary: {
wastedBytes: '>17000',
},
items: {
length: 2,
},
Expand All @@ -182,8 +184,10 @@ const expectations = {
details: {
// the specific ms value is not meaningful for this smoketest
// *some largish amount* of savings should be reported
overallSavingsMs: '>500',
overallSavingsBytes: '>45000',
summary: {
wastedMs: '>500',
wastedBytes: '>45000',
},
items: [
{
url: 'http://localhost:10200/byte-efficiency/script.js',
Expand Down Expand Up @@ -212,7 +216,9 @@ const expectations = {
},
'unused-css-rules': {
details: {
overallSavingsBytes: '>40000',
summary: {
wastedBytes: '>40000',
},
items: {
length: 2,
},
Expand All @@ -223,8 +229,10 @@ const expectations = {
details: {
// the specific ms value here is not meaningful for this smoketest
// *some* savings should be reported
overallSavingsMs: '>0',
overallSavingsBytes: '35000 +/- 1000',
summary: {
wastedMs: '>0',
wastedBytes: '35000 +/- 1000',
},
items: [
{
url: 'http://localhost:10200/byte-efficiency/script.js',
Expand Down Expand Up @@ -269,7 +277,9 @@ const expectations = {
},
'modern-image-formats': {
details: {
overallSavingsBytes: '137000 +/- 10000',
summary: {
wastedBytes: '137000 +/- 10000',
},
items: [
{url: /lighthouse-1024x680.jpg$/},
{url: /lighthouse-unoptimized.jpg$/},
Expand All @@ -285,16 +295,20 @@ const expectations = {
details: {
// the specific ms value is not meaningful for this smoketest
// *some largish amount* of savings should be reported
overallSavingsMs: '>700',
overallSavingsBytes: '>50000',
summary: {
wastedMs: '>700',
wastedBytes: '>50000',
},
items: {
length: 3,
},
},
},
'uses-optimized-images': {
details: {
overallSavingsBytes: '>10000',
summary: {
wastedBytes: '>10000',
},
items: {
length: 1,
},
Expand All @@ -303,7 +317,9 @@ const expectations = {
// Check that images aren't TOO BIG.
'uses-responsive-images': {
details: {
overallSavingsBytes: '169000 +/- 5000',
summary: {
wastedBytes: '169000 +/- 5000',
},
items: [
{wastedPercent: '81 +/- 5', url: /lighthouse-1024x680.jpg/},
{wastedPercent: '88 +/- 5', url: /lighthouse-2048x1356.webp\?size0/},
Expand Down
4 changes: 3 additions & 1 deletion cli/test/smokehouse/test-definitions/dobetterweb.js
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,9 @@ const expectations = {
'efficient-animated-content': {
score: '<0.5',
details: {
overallSavingsMs: '>2000',
summary: {
wastedMs: '>2000',
},
items: [
{
url: 'http://localhost:10200/dobetterweb/lighthouse-rotating.gif',
Expand Down
34 changes: 6 additions & 28 deletions core/audits/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,7 @@ const DEFAULT_PASS = 'defaultPass';
* @typedef TableOptions
* @property {number=} wastedMs
* @property {number=} wastedBytes
*/

/**
* @typedef OpportunityOptions
* @property {number} overallSavingsMs
* @property {number=} overallSavingsBytes
* @property {boolean=} isOpportunity
*/

/**
Expand Down Expand Up @@ -113,8 +108,8 @@ class Audit {
* This catches typos in the `key` property of a heading definition of table/opportunity details.
* Throws an error if any of keys referenced by headings don't exist in at least one of the items.
*
* @param {LH.Audit.Details.Table['headings']|LH.Audit.Details.Opportunity['headings']} headings
* @param {LH.Audit.Details.Opportunity['items']|LH.Audit.Details.Table['items']} items
* @param {LH.Audit.Details.Table['headings']} headings
* @param {LH.Audit.Details.Table['items']} items
*/
static assertHeadingKeysExist(headings, items) {
// If there are no items, there's nothing to check.
Expand All @@ -139,14 +134,15 @@ class Audit {
* @return {LH.Audit.Details.Table}
*/
static makeTableDetails(headings, results, options = {}) {
const {wastedBytes, wastedMs} = options;
const {wastedBytes, wastedMs, isOpportunity} = options;
const summary = (wastedBytes || wastedMs) ? {wastedBytes, wastedMs} : undefined;
if (results.length === 0) {
return {
type: 'table',
headings: [],
items: [],
summary,
isOpportunity,
};
}

Expand All @@ -157,6 +153,7 @@ class Audit {
headings: headings,
items: results,
summary,
isOpportunity,
};
}

Expand Down Expand Up @@ -226,25 +223,6 @@ class Audit {
});
}

/**
* @param {LH.Audit.Details.Opportunity['headings']} headings
* @param {LH.Audit.Details.Opportunity['items']} items
* @param {OpportunityOptions} options
* @return {LH.Audit.Details.Opportunity}
*/
static makeOpportunityDetails(headings, items, options) {
Audit.assertHeadingKeysExist(headings, items);
const {overallSavingsMs, overallSavingsBytes} = options;

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

/**
* @param {LH.Artifacts.NodeDetails} node
* @return {LH.Audit.Details.NodeValue}
Expand Down
6 changes: 3 additions & 3 deletions core/audits/byte-efficiency/byte-efficiency-audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const WASTED_MS_FOR_SCORE_OF_ZERO = 5000;
* @typedef {object} ByteEfficiencyProduct
* @property {Array<LH.Audit.ByteEfficiencyItem>} items
* @property {Map<string, number>=} wastedBytesByUrl
* @property {LH.Audit.Details.Opportunity['headings']} headings
* @property {LH.Audit.Details.Table['headings']} headings
* @property {LH.IcuMessage} [displayValue]
* @property {LH.IcuMessage} [explanation]
* @property {Array<string | LH.IcuMessage>} [warnings]
Expand Down Expand Up @@ -228,8 +228,8 @@ class ByteEfficiencyAudit extends Audit {
displayValue = str_(i18n.UIStrings.displayValueByteSavings, {wastedBytes});
}

const details = Audit.makeOpportunityDetails(result.headings, results,
{overallSavingsMs: wastedMs, overallSavingsBytes: wastedBytes});
const details = Audit.makeTableDetails(result.headings, results,
{wastedMs, wastedBytes, isOpportunity: true});

return {
explanation: result.explanation,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class EfficientAnimatedContent extends ByteEfficiencyAudit {
};
});

/** @type {LH.Audit.Details.Opportunity['headings']} */
/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)},
{key: 'totalBytes', valueType: 'bytes', label: str_(i18n.UIStrings.columnResourceSize)},
Expand Down
2 changes: 1 addition & 1 deletion core/audits/byte-efficiency/modern-image-formats.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ class ModernImageFormats extends ByteEfficiencyAudit {
});
}

/** @type {LH.Audit.Details.Opportunity['headings']} */
/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'node', valueType: 'node', label: ''},
{key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)},
Expand Down
2 changes: 1 addition & 1 deletion core/audits/byte-efficiency/offscreen-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ class OffscreenImages extends ByteEfficiencyAudit {
await ProcessedTrace.request(trace, context).then(tot => tot.timestamps.traceEnd));
}

/** @type {LH.Audit.Details.Opportunity['headings']} */
/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'node', valueType: 'node', label: ''},
{key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)},
Expand Down
5 changes: 2 additions & 3 deletions core/audits/byte-efficiency/render-blocking-resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -283,15 +283,14 @@ class RenderBlockingResources extends Audit {
displayValue = str_(i18n.UIStrings.displayValueMsSavings, {wastedMs});
}

/** @type {LH.Audit.Details.Opportunity['headings']} */
/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)},
{key: 'totalBytes', valueType: 'bytes', label: str_(i18n.UIStrings.columnTransferSize)},
{key: 'wastedMs', valueType: 'timespanMs', label: str_(i18n.UIStrings.columnWastedMs)},
];

const details = Audit.makeOpportunityDetails(headings, results,
{overallSavingsMs: wastedMs});
const details = Audit.makeTableDetails(headings, results, {wastedMs, isOpportunity: true});

return {
displayValue,
Expand Down
2 changes: 1 addition & 1 deletion core/audits/byte-efficiency/unminified-css.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class UnminifiedCSS extends ByteEfficiencyAudit {
items.push(result);
}

/** @type {LH.Audit.Details.Opportunity['headings']} */
/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)},
{key: 'totalBytes', valueType: 'bytes', label: str_(i18n.UIStrings.columnTransferSize)},
Expand Down
2 changes: 1 addition & 1 deletion core/audits/byte-efficiency/unminified-javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ class UnminifiedJavaScript extends ByteEfficiencyAudit {
}
}

/** @type {LH.Audit.Details.Opportunity['headings']} */
/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)},
{key: 'totalBytes', valueType: 'bytes', label: str_(i18n.UIStrings.columnTransferSize)},
Expand Down
2 changes: 1 addition & 1 deletion core/audits/byte-efficiency/unused-css-rules.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class UnusedCSSRules extends ByteEfficiencyAudit {
const items = unusedCssItems
.filter(sheet => sheet && sheet.wastedBytes > IGNORE_THRESHOLD_IN_BYTES);

/** @type {LH.Audit.Details.Opportunity['headings']} */
/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)},
{key: 'totalBytes', valueType: 'bytes', label: str_(i18n.UIStrings.columnTransferSize)},
Expand Down
2 changes: 1 addition & 1 deletion core/audits/byte-efficiency/uses-optimized-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ class UsesOptimizedImages extends ByteEfficiencyAudit {
});
}

/** @type {LH.Audit.Details.Opportunity['headings']} */
/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'node', valueType: 'node', label: ''},
{key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)},
Expand Down
2 changes: 1 addition & 1 deletion core/audits/byte-efficiency/uses-responsive-images.js
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ class UsesResponsiveImages extends ByteEfficiencyAudit {

const items = Array.from(resultsMap.values());

/** @type {LH.Audit.Details.Opportunity['headings']} */
/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'node', valueType: 'node', label: ''},
{key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)},
Expand Down
2 changes: 1 addition & 1 deletion core/audits/byte-efficiency/uses-text-compression.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ class ResponsesAreCompressed extends ByteEfficiencyAudit {
});
});

/** @type {LH.Audit.Details.Opportunity['headings']} */
/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)},
{key: 'totalBytes', valueType: 'bytes', label: str_(i18n.UIStrings.columnTransferSize)},
Expand Down
5 changes: 2 additions & 3 deletions core/audits/dobetterweb/uses-http2.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,14 +241,13 @@ class UsesHTTP2Audit extends Audit {
const simulator = await LoadSimulator.request(simulatorOptions, context);
const wastedMs = UsesHTTP2Audit.computeWasteWithTTIGraph(resources, graph, simulator);

/** @type {LH.Audit.Details.Opportunity['headings']} */
/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)},
{key: 'protocol', valueType: 'text', label: str_(UIStrings.columnProtocol)},
];

const details = Audit.makeOpportunityDetails(headings, resources,
{overallSavingsMs: wastedMs});
const details = Audit.makeTableDetails(headings, resources, {wastedMs, isOpportunity: true});

return {
displayValue,
Expand Down
5 changes: 2 additions & 3 deletions core/audits/prioritize-lcp-image.js
Original file line number Diff line number Diff line change
Expand Up @@ -256,14 +256,13 @@ class PrioritizeLcpImage extends Audit {
const {results, wastedMs} =
PrioritizeLcpImage.computeWasteWithGraph(lcpElement, lcpNodeToPreload, graph, simulator);

/** @type {LH.Audit.Details.Opportunity['headings']} */
/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'node', valueType: 'node', label: ''},
{key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)},
{key: 'wastedMs', valueType: 'timespanMs', label: str_(i18n.UIStrings.columnWastedMs)},
];
const details = Audit.makeOpportunityDetails(headings, results,
{overallSavingsMs: wastedMs});
const details = Audit.makeTableDetails(headings, results, {wastedMs, isOpportunity: true});

// If LCP element was an image and had valid network records (regardless of
// if it should be preloaded), it will be found first in the `initiatorPath`.
Expand Down
6 changes: 3 additions & 3 deletions core/audits/redirects.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,13 @@ class Redirects extends Audit {
});
}

/** @type {LH.Audit.Details.Opportunity['headings']} */
/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)},
{key: 'wastedMs', valueType: 'timespanMs', label: str_(i18n.UIStrings.columnTimeSpent)},
];
const details = Audit.makeOpportunityDetails(headings, tableRows,
{overallSavingsMs: totalWastedMs});
const details = Audit.makeTableDetails(headings, tableRows,
{wastedMs: totalWastedMs, isOpportunity: true});

return {
// We award a passing grade if you only have 1 redirect
Expand Down
6 changes: 3 additions & 3 deletions core/audits/server-response-time.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,16 +64,16 @@ class ServerResponseTime extends Audit {
const passed = responseTime < TOO_SLOW_THRESHOLD_MS;
const displayValue = str_(UIStrings.displayValue, {timeInMs: responseTime});

/** @type {LH.Audit.Details.Opportunity['headings']} */
/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)},
{key: 'responseTime', valueType: 'timespanMs', label: str_(i18n.UIStrings.columnTimeSpent)},
];

const details = Audit.makeOpportunityDetails(
const details = Audit.makeTableDetails(
headings,
[{url: mainResource.url, responseTime}],
{overallSavingsMs: responseTime - TARGET_MS}
{wastedMs: responseTime - TARGET_MS, isOpportunity: true}
);

return {
Expand Down
6 changes: 3 additions & 3 deletions core/audits/uses-rel-preconnect.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,14 +227,14 @@ class UsesRelPreconnectAudit extends Audit {
};
}

/** @type {LH.Audit.Details.Opportunity['headings']} */
/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)},
{key: 'wastedMs', valueType: 'timespanMs', label: str_(i18n.UIStrings.columnWastedMs)},
];

const details = Audit.makeOpportunityDetails(headings, results,
{overallSavingsMs: maxWasted});
const details = Audit.makeTableDetails(headings, results,
{wastedMs: maxWasted, isOpportunity: true});

return {
score: ByteEfficiencyAudit.scoreForWastedMs(maxWasted),
Expand Down
Loading