Skip to content

Commit

Permalink
core(js-bundles): return error object when sizes cannot be determined (
Browse files Browse the repository at this point in the history
  • Loading branch information
connorjclark committed Oct 6, 2020
1 parent 054722a commit e0b16db
Show file tree
Hide file tree
Showing 6 changed files with 67 additions and 24 deletions.
13 changes: 8 additions & 5 deletions lighthouse-core/audits/byte-efficiency/unused-javascript.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,20 @@ class UnusedJavaScript extends ByteEfficiencyAudit {
};

if (item.wastedBytes <= unusedThreshold) continue;
items.push(item);

// If there was an error calculating the bundle sizes, we can't
// create any sub-items.
if (!bundle || 'errorMessage' in bundle.sizes) continue;
const sizes = bundle.sizes;

// Augment with bundle data.
if (bundle && unusedJsSummary.sourcesWastedBytes) {
if (unusedJsSummary.sourcesWastedBytes) {
const topUnusedSourceSizes = Object.entries(unusedJsSummary.sourcesWastedBytes)
.sort((a, b) => b[1] - a[1])
.slice(0, 5)
.map(([source, unused]) => {
const total =
source === '(unmapped)' ? bundle.sizes.unmappedBytes : bundle.sizes.files[source];
const total = source === '(unmapped)' ? sizes.unmappedBytes : sizes.files[source];
return {
source,
unused: Math.round(unused * transferRatio),
Expand All @@ -133,8 +138,6 @@ class UnusedJavaScript extends ByteEfficiencyAudit {
}),
};
}

items.push(item);
}

return {
Expand Down
24 changes: 12 additions & 12 deletions lighthouse-core/computed/js-bundles.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ function computeGeneratedFileSizes(map, content) {
const totalBytes = content.length;
let unmappedBytes = totalBytes;

// If the map + contents don't line up, return a result that
// denotes nothing is mapped.
const failureResult = {files: {}, unmappedBytes, totalBytes};

// @ts-expect-error: This function is added in SDK.js. This will eventually be added to CDT.
map.computeLastGeneratedColumns();

Expand All @@ -43,23 +39,27 @@ function computeGeneratedFileSizes(map, content) {
// Lines and columns are zero-based indices. Visually, lines are shown as a 1-based index.

const line = lines[lineNum];
if (line === null) {
log.error('JSBundles', `${map.url()} mapping for line out of bounds: ${lineNum + 1}`);
return failureResult;
if (line === null || line === undefined) {
const errorMessage = `${map.url()} mapping for line out of bounds: ${lineNum + 1}`;
log.error('JSBundles', errorMessage);
return {errorMessage};
}

if (colNum > line.length) {
// eslint-disable-next-line max-len
log.error('JSBundles', `${map.url()} mapping for column out of bounds: ${lineNum + 1}:${colNum}`);
return failureResult;
const errorMessage =
`${map.url()} mapping for column out of bounds: ${lineNum + 1}:${colNum}`;
log.error('JSBundles', errorMessage);
return {errorMessage};
}

let mappingLength = 0;
if (lastColNum !== undefined) {
if (lastColNum > line.length) {
// eslint-disable-next-line max-len
log.error('JSBundles', `${map.url()} mapping for last column out of bounds: ${lineNum + 1}:${lastColNum}`);
return failureResult;
const errorMessage =
`${map.url()} mapping for last column out of bounds: ${lineNum + 1}:${lastColNum}`;
log.error('JSBundles', errorMessage);
return {errorMessage};
}
mappingLength = lastColNum - colNum;
} else {
Expand Down
2 changes: 2 additions & 0 deletions lighthouse-core/computed/module-duplication.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ class ModuleDuplication {

// Determine size of each `sources` entry.
for (const {rawMap, sizes} of bundles) {
if ('errorMessage' in sizes) continue;

/** @type {SourceData[]} */
const sourceDataArray = [];
sourceDatasMap.set(rawMap, sourceDataArray);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ async function main() {
SourceMaps: [{scriptUrl: '', map: bundleMap}],
};
const bundles = await JsBundles.compute_(artifacts);
if ('errorMessage' in bundles[0].sizes) throw new Error(bundles[0].sizes.errorMessage);
const bundleFileSizes = bundles[0].sizes.files;

const allModules = Object.keys(bundleFileSizes).filter(s => s.startsWith('node_modules'));
Expand Down
49 changes: 43 additions & 6 deletions lighthouse-core/test/computed/js-bundles-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,7 @@ describe('JsBundles computed artifact', () => {
"sourceURL": "src/foo.js",
},
"sizes": Object {
"files": Object {},
"totalBytes": 13,
"unmappedBytes": 13,
"errorMessage": "compiled.js.map mapping for last column out of bounds: 1:14",
},
}
`);
Expand All @@ -314,9 +312,7 @@ describe('JsBundles computed artifact', () => {
"sourceURL": "src/foo.js",
},
"sizes": Object {
"files": Object {},
"totalBytes": 0,
"unmappedBytes": 0,
"errorMessage": "compiled.js.map mapping for column out of bounds: 1:1",
},
}
`);
Expand Down Expand Up @@ -347,5 +343,46 @@ describe('JsBundles computed artifact', () => {
}
`);
});

it('emits error when column out of bounds', async () => {
const newMappings = map.mappings.split(',');
expect(newMappings[1]).toBe('SAAAA');
// Make the column offset very big, force out of bounds.
// See https://www.mattzeunert.com/2016/02/14/how-do-source-maps-work.html
newMappings[1] = 'kD' + 'AAAA';
map.mappings = newMappings.join(',');
expect(await test()).toMatchInlineSnapshot(`
Object {
"entry": SourceMapEntry {
"columnNumber": 642,
"lastColumnNumber": 651,
"lineNumber": 0,
"name": undefined,
"sourceColumnNumber": 18,
"sourceLineNumber": 1,
"sourceURL": "src/foo.js",
},
"sizes": Object {
"errorMessage": "compiled.js.map mapping for last column out of bounds: 1:685",
},
}
`);
});

it('emits error when line out of bounds', async () => {
const newMappings = map.mappings.split(',');
expect(newMappings[1]).toBe('SAAAA');
// Make the line offset very big, force out of bounds.
// See https://sourcemaps.info/spec.html#:~:text=broken%20down%20as%20follows
map.mappings = ';'.repeat(10) + map.mappings;
expect(await test()).toMatchInlineSnapshot(`
Object {
"entry": null,
"sizes": Object {
"errorMessage": "compiled.js.map mapping for line out of bounds: 11",
},
}
`);
});
});
});
2 changes: 1 addition & 1 deletion types/artifacts.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,7 +326,7 @@ declare global {
files: Record<string, number>;
unmappedBytes: number;
totalBytes: number;
};
} | {errorMessage: string};
}

/** @see https://developer.mozilla.org/en-US/docs/Web/HTML/Element/a#Attributes */
Expand Down

0 comments on commit e0b16db

Please sign in to comment.