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: subRow refactor, rename to subItem #10867

Merged
merged 17 commits into from
Jun 8, 2020
Original file line number Diff line number Diff line change
Expand Up @@ -155,21 +155,13 @@ const expectations = [
url: 'http://localhost:10200/byte-efficiency/bundle.js',
totalBytes: '12913 +/- 1000',
wastedBytes: '5827 +/- 200',
sources: [
'…./b.js',
'…./c.js',
'…webpack/bootstrap',
],
sourceBytes: [
'4417 +/- 50',
'2200 +/- 50',
'2809 +/- 50',
],
sourceWastedBytes: [
'2191 +/- 50',
'2182 +/- 50',
'1259 +/- 50',
],
subItems: {
items: [
{source: '…./b.js', sourceBytes: '4417 +/- 50', sourceWastedBytes: '2191 +/- 50'},
{source: '…./c.js', sourceBytes: '2200 +/- 50', sourceWastedBytes: '2182 +/- 50'},
{source: '…webpack/bootstrap', sourceBytes: '2809 +/- 50', sourceWastedBytes: '1259 +/- 50'},
],
},
},
],
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,17 +19,21 @@ module.exports = [
items: [
{
url: 'http://localhost:10200/legacy-javascript.js',
signals: [
'@babel/plugin-transform-classes',
'@babel/plugin-transform-regenerator',
'@babel/plugin-transform-spread',
],
subItems: {
items: [
{signal: '@babel/plugin-transform-classes'},
{signal: '@babel/plugin-transform-regenerator'},
{signal: '@babel/plugin-transform-spread'},
],
},
},
{
url: 'http://localhost:10200/legacy-javascript.html',
signals: [
'String.prototype.includes',
],
subItems: {
items: [
{signal: 'String.prototype.includes'},
],
},
},
],
},
Expand Down
77 changes: 36 additions & 41 deletions lighthouse-core/audits/byte-efficiency/duplicated-javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ class DuplicatedJavascript extends ByteEfficiencyAudit {
/**
* This audit highlights JavaScript modules that appear to be duplicated across all resources,
* either within the same bundle or between different bundles. Each details item returned is
* a module with subrows for each resource that includes it. The wastedBytes for the details
* a module with subItems for each resource that includes it. The wastedBytes for the details
* item is the number of bytes occupied by the sum of all but the largest copy of the module.
* wastedBytesByUrl attributes the cost of the bytes to a specific resource, for use by lantern.
* @param {LH.Artifacts} artifacts
Expand All @@ -116,19 +116,12 @@ class DuplicatedJavascript extends ByteEfficiencyAudit {
const duplication =
await DuplicatedJavascript._getDuplicationGroupedByNodeModules(artifacts, context);

/**
* @typedef ItemSubrows
* @property {string[]} urls
* @property {number[]} sourceBytes
*/

/**
* @typedef {LH.Audit.ByteEfficiencyItem & ItemSubrows} Item
*/

/** @type {Item[]} */
/** @type {LH.Audit.ByteEfficiencyItem[]} */
const items = [];

let overflowWastedBytes = 0;
const overflowUrls = new Set();

/** @type {Map<string, number>} */
const wastedBytesByUrl = new Map();
for (const [source, sourceDatas] of duplication.entries()) {
Expand All @@ -139,52 +132,54 @@ class DuplicatedJavascript extends ByteEfficiencyAudit {
// is not present. Instead, size is used as a heuristic for latest version. This makes the
// audit conserative in its estimation.

const urls = [];
const bytesValues = [];
const subItems = [];

let wastedBytesTotal = 0;
for (let i = 0; i < sourceDatas.length; i++) {
const sourceData = sourceDatas[i];
const url = sourceData.scriptUrl;
urls.push(url);
bytesValues.push(sourceData.size);
subItems.push({
url,
sourceBytes: sourceData.size,
});
if (i === 0) continue;
wastedBytesTotal += sourceData.size;
wastedBytesByUrl.set(url, (wastedBytesByUrl.get(url) || 0) + sourceData.size);
}

if (wastedBytesTotal <= ignoreThresholdInBytes) {
overflowWastedBytes += wastedBytesTotal;
for (const subItem of subItems) {
overflowUrls.add(subItem.url);
}
continue;
}

items.push({
source,
wastedBytes: wastedBytesTotal,
// Not needed, but keeps typescript happy.
url: '',
// Not needed, but keeps typescript happy.
totalBytes: 0,
urls,
sourceBytes: bytesValues,
subItems: {
type: 'subitems',
items: subItems,
},
});
}

/** @type {Item} */
const otherItem = {
source: 'Other',
wastedBytes: 0,
url: '',
totalBytes: 0,
urls: [],
sourceBytes: [],
};
for (const item of items.filter(item => item.wastedBytes <= ignoreThresholdInBytes)) {
otherItem.wastedBytes += item.wastedBytes;
for (let i = 0; i < item.urls.length; i++) {
const url = item.urls[i];
if (!otherItem.urls.includes(url)) {
otherItem.urls.push(url);
}
}
items.splice(items.indexOf(item), 1);
}
if (otherItem.wastedBytes > ignoreThresholdInBytes) {
items.push(otherItem);
if (overflowWastedBytes > ignoreThresholdInBytes) {
items.push({
source: 'Other',
wastedBytes: overflowWastedBytes,
url: '',
totalBytes: 0,
subItems: {
type: 'subitems',
items: Array.from(overflowUrls).map(url => ({url})),
},
});
}

// Convert bytes to transfer size estimation.
Expand Down Expand Up @@ -214,8 +209,8 @@ class DuplicatedJavascript extends ByteEfficiencyAudit {
/** @type {LH.Audit.Details.OpportunityColumnHeading[]} */
const headings = [
/* eslint-disable max-len */
{key: 'source', valueType: 'code', subRows: {key: 'urls', valueType: 'url'}, label: str_(i18n.UIStrings.columnSource)},
{key: '_', valueType: 'bytes', subRows: {key: 'sourceBytes'}, granularity: 0.05, label: str_(i18n.UIStrings.columnSize)},
{key: 'source', valueType: 'code', subHeading: {key: 'url', valueType: 'url'}, label: str_(i18n.UIStrings.columnSource)},
{key: '_', valueType: 'bytes', subHeading: {key: 'sourceBytes'}, granularity: 0.05, label: str_(i18n.UIStrings.columnSize)},
{key: 'wastedBytes', valueType: 'bytes', granularity: 0.05, label: str_(i18n.UIStrings.columnWastedBytes)},
/* eslint-enable max-len */
];
Expand Down
32 changes: 19 additions & 13 deletions lighthouse-core/audits/byte-efficiency/unused-javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,13 @@ function commonPrefix(strings) {
}

/**
* @param {string[]} strings
* @param {string} string
* @param {string} commonPrefix
* @return {string[]}
* @return {string}
*/
function trimCommonPrefix(strings, commonPrefix) {
if (!commonPrefix) return strings;
return strings.map(s => s.startsWith(commonPrefix) ? '…' + s.slice(commonPrefix.length) : s);
function trimCommonPrefix(string, commonPrefix) {
if (!commonPrefix) return string;
return string.startsWith(commonPrefix) ? '…' + string.slice(commonPrefix.length) : string;
}

/**
Expand Down Expand Up @@ -94,6 +94,7 @@ class UnusedJavaScript extends ByteEfficiencyAudit {
const transfer = ByteEfficiencyAudit
.estimateTransferSize(networkRecord, unusedJsSummary.totalBytes, 'Script');
const transferRatio = transfer / unusedJsSummary.totalBytes;
/** @type {LH.Audit.ByteEfficiencyItem} */
const item = {
url: unusedJsSummary.url,
totalBytes: Math.round(transferRatio * unusedJsSummary.totalBytes),
Expand All @@ -120,11 +121,16 @@ class UnusedJavaScript extends ByteEfficiencyAudit {
.filter(d => d.unused >= bundleSourceUnusedThreshold);

const commonSourcePrefix = commonPrefix([...bundle.map._sourceInfos.keys()]);
Object.assign(item, {
sources: trimCommonPrefix(topUnusedSourceSizes.map(d => d.source), commonSourcePrefix),
sourceBytes: topUnusedSourceSizes.map(d => d.total),
sourceWastedBytes: topUnusedSourceSizes.map(d => d.unused),
});
item.subItems = {
type: 'subitems',
items: topUnusedSourceSizes.map(({source, unused, total}) => {
return {
source: trimCommonPrefix(source, commonSourcePrefix),
sourceBytes: total,
sourceWastedBytes: unused,
};
}),
};
}

items.push(item);
Expand All @@ -134,9 +140,9 @@ class UnusedJavaScript extends ByteEfficiencyAudit {
items,
headings: [
/* eslint-disable max-len */
{key: 'url', valueType: 'url', subRows: {key: 'sources', valueType: 'code'}, label: str_(i18n.UIStrings.columnURL)},
{key: 'totalBytes', valueType: 'bytes', subRows: {key: 'sourceBytes'}, label: str_(i18n.UIStrings.columnTransferSize)},
{key: 'wastedBytes', valueType: 'bytes', subRows: {key: 'sourceWastedBytes'}, label: str_(i18n.UIStrings.columnWastedBytes)},
{key: 'url', valueType: 'url', subHeading: {key: 'source', valueType: 'code'}, label: str_(i18n.UIStrings.columnURL)},
{key: 'totalBytes', valueType: 'bytes', subHeading: {key: 'sourceBytes'}, label: str_(i18n.UIStrings.columnTransferSize)},
{key: 'wastedBytes', valueType: 'bytes', subHeading: {key: 'sourceWastedBytes'}, label: str_(i18n.UIStrings.columnWastedBytes)},
/* eslint-enable max-len */
],
};
Expand Down
37 changes: 24 additions & 13 deletions lighthouse-core/audits/legacy-javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,8 @@ class LegacyJavascript extends Audit {
const networkRecords = await NetworkRecords.request(devtoolsLog, context);
const bundles = await JSBundles.request(artifacts, context);

/** @type {Array<{url: string, signals: string[], locations: LH.Audit.Details.SourceLocationValue[]}>} */
/** @typedef {{signal: string, location: LH.Audit.Details.SourceLocationValue}} SubItem */
/** @type {Array<{url: string, subItems: LH.Audit.Details.TableSubItems}>} */
const tableRows = [];
let signalCount = 0;

Expand All @@ -372,27 +373,37 @@ class LegacyJavascript extends Audit {
this.detectAcrossScripts(matcher, artifacts.ScriptElements, networkRecords, bundles);
urlToMatchResults.forEach((matches, url) => {
/** @type {typeof tableRows[number]} */
const row = {url, signals: [], locations: []};
const row = {
url,
subItems: {
type: 'subitems',
items: [],
},
};
for (const match of matches) {
const {name, line, column} = match;
row.signals.push(name);
row.locations.push({
type: 'source-location',
url,
line,
column,
urlProvider: 'network',
});
/** @type {SubItem} */
const subItem = {
signal: name,
location: {
type: 'source-location',
url,
line,
column,
urlProvider: 'network',
},
};
row.subItems.items.push(subItem);
}
tableRows.push(row);
signalCount += row.signals.length;
signalCount += row.subItems.items.length;
});

/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
/* eslint-disable max-len */
{key: 'url', itemType: 'url', subRows: {key: 'locations', itemType: 'source-location'}, text: str_(i18n.UIStrings.columnURL)},
{key: null, itemType: 'code', subRows: {key: 'signals'}, text: ''},
{key: 'url', itemType: 'url', subHeading: {key: 'location', itemType: 'source-location'}, text: str_(i18n.UIStrings.columnURL)},
{key: null, itemType: 'code', subHeading: {key: 'signal'}, text: ''},
/* eslint-enable max-len */
];
const details = Audit.makeTableDetails(headings, tableRows);
Expand Down
Loading