Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): update budgets to check different…
Browse files Browse the repository at this point in the history
…ial builds separately

Fixes angular#15792.

Previously, budgets would include content for both versions of a differential build. Thus the `initial` budget would count content from the ES5 **and** ES2015 bundles together. This is a very misleading statistic because no user would download both versions. I've updated the budget calculators to take this into account and generate size values for both builds which are then checked independently of each other.

The only calculators I changed are the `InitialCalculator` (for computing `initial` bundle sizes) and `BundleCalculator` (for computing named bundles). Since budgets are handled by Webpack for builds without differential loading, the `initial` bundle will always have those two sizes. The `BundleCalculator` might reference a bundle which does not have differential loading performed (such as a CSS file), so it emits sizes depending on whether or not multiple builds were found for that chunk.

Most of the other calculators don't really need to take differential loading into account. `AnyScriptCalculator` and `AnyCalculator` already apply on a file-by-file basis, so they generate sizes for both build versions already. `AnyComponentStyleCalculator` only applies to CSS which does not have differential builds.

The wierd ones here are `AllCalculator` and `AllScriptCalculator` which reference files with and without differential builds. Conceptually, they should be separated, as a "total" budget specified by an app developer probably wanted it to mean "the total resources a user would have to download", which would only be one differential build at a time. However, I don't see a good way of identifying which assets belong to which differential build. Even if an asset belongs to a chunk with differential builds, we don't know which build takes which assets into account. I decided to leave this for the time being, but it is probably something we should look into separately.

Since budgets take differential loading into account, users might reasonably want to set different budgets for different builds (ie. "initial-es2015 bundle should be capped at 100k, but initial-es5 bundle can go to 150k"). That's more of a feature request, so I also left that out for a future PR.
  • Loading branch information
dgp1130 committed Nov 22, 2019
1 parent 117b61c commit c28498f
Show file tree
Hide file tree
Showing 3 changed files with 215 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
*/
import * as webpack from 'webpack';
import { Budget } from '../../browser/schema';
import { ProcessBundleResult, ProcessBundleFile } from '@angular-devkit/build-angular/src/utils/process-bundle';

export interface Size {
size: number;
Expand All @@ -29,6 +30,11 @@ export enum ThresholdSeverity {
ERROR = 'error',
}

enum DifferentialBuildType {
ORIGINAL = 'es2015',
DOWNLEVEL = 'es5',
}

export function* calculateThresholds(budget: Budget): IterableIterator<Threshold> {
if (budget.maximumWarning) {
yield {
Expand Down Expand Up @@ -91,7 +97,16 @@ export function* calculateThresholds(budget: Budget): IterableIterator<Threshold
}
}

export function calculateSizes(budget: Budget, stats: webpack.Stats.ToJsonOutput): Size[] {
/**
* Calculates the sizes for bundles in the budget type provided.
*
* Precondition: Differential builds are enabled.
*/
export function calculateSizes(
budget: Budget,
stats: webpack.Stats.ToJsonOutput,
processResults: ProcessBundleResult[],
): Size[] {
const calculatorMap: Record<Budget['type'], { new(...args: any[]): Calculator }> = {
all: AllCalculator,
allScript: AllScriptCalculator,
Expand All @@ -111,19 +126,54 @@ export function calculateSizes(budget: Budget, stats: webpack.Stats.ToJsonOutput
throw new Error('Webpack stats output did not include asset information.');
}

const calculator = new ctor(budget, chunks, assets);
const calculator = new ctor(budget, chunks, assets, processResults);

return calculator.calculate();
}

type ArrayElement<T> = T extends Array<infer U> ? U : never;
type Chunk = ArrayElement<Exclude<webpack.Stats.ToJsonOutput['chunks'], undefined>>;
type Asset = ArrayElement<Exclude<webpack.Stats.ToJsonOutput['assets'], undefined>>;
export abstract class Calculator {
constructor (
protected budget: Budget,
protected chunks: Exclude<webpack.Stats.ToJsonOutput['chunks'], undefined>,
protected assets: Exclude<webpack.Stats.ToJsonOutput['assets'], undefined>,
protected chunks: Chunk[],
protected assets: Asset[],
protected processResults: ProcessBundleResult[],
) {}

abstract calculate(): Size[];

/** Calculates the size of the given chunk for the provided build type. */
protected calculateChunkSize(
chunk: Chunk,
buildType: DifferentialBuildType,
): number {
// Look for a process result containing different builds for this chunk.
const processResult = this.processResults
.find((processResult) => processResult.name === chunk.id.toString());

if (processResult) {
// Found a differential build, use the correct size information.
const processResultFile = getDifferentialBuildResult(
processResult, buildType);
return processResultFile && processResultFile.size || 0;
} else {
// No differential builds, get the chunk size by summing its assets.
return chunk.files
.filter((file) => !file.endsWith('.map'))
.map((file: string) => {
const asset = this.assets.find((asset) => asset.name === file);
if (!asset) {
throw new Error(`Could not find asset for file: ${file}`);
}

return asset;
})
.map((asset) => asset.size)
.reduce((l, r) => l + r);
}
}
}

/**
Expand All @@ -136,22 +186,32 @@ class BundleCalculator extends Calculator {
return [];
}

const size: number = this.chunks
.filter(chunk => chunk.names.indexOf(budgetName) !== -1)
.reduce((files, chunk) => [...files, ...chunk.files], [])
.filter((file: string) => !file.endsWith('.map'))
.map((file: string) => {
const asset = this.assets.find((asset) => asset.name === file);
if (!asset) {
throw new Error(`Could not find asset for file: ${file}`);
}

return asset;
})
.map((asset) => asset.size)
.reduce((total: number, size: number) => total + size, 0);
// The chunk may or may not have differential builds. Compute the size for
// each then check afterwards if they are all the same.
const buildSizes = Object.values(DifferentialBuildType).map((buildType) => {
const size = this.chunks
.filter(chunk => chunk.names.indexOf(budgetName) !== -1)
.map((chunk) => this.calculateChunkSize(chunk, buildType))
.reduce((l, r) => l + r);

return {size, label: `${this.budget.name}-${buildType}`};
});

// If there are multiple sizes, then there are differential builds. Display
// them all.
if (!allEquivalent(buildSizes.map((buildSize) => buildSize.size))) {
return buildSizes;
}

if (buildSizes.length === 0) {
return [];
}

return [{size, label: this.budget.name}];
// Only one size, must not be a differential build.
return [{
label: this.budget.name,
size: buildSizes[0].size,
}];
}
}

Expand All @@ -160,22 +220,15 @@ class BundleCalculator extends Calculator {
*/
class InitialCalculator extends Calculator {
calculate() {
const size = this.chunks
.filter(chunk => chunk.initial)
.reduce((files, chunk) => [...files, ...chunk.files], [])
.filter((file: string) => !file.endsWith('.map'))
.map((file: string) => {
const asset = this.assets.find((asset) => asset.name === file);
if (!asset) {
throw new Error(`Could not find asset for file: ${file}`);
}

return asset;
})
.map((asset) => asset.size)
.reduce((total: number, size: number) => total + size, 0);

return [{size, label: 'initial'}];
return Object.values(DifferentialBuildType).map((buildType) => {
return {
label: `initial-${buildType}`,
size: this.chunks
.filter(chunk => chunk.initial)
.map((chunk) => this.calculateChunkSize(chunk, buildType))
.reduce((l, r) => l + r),
};
});
}
}

Expand Down Expand Up @@ -286,3 +339,29 @@ export function calculateBytes(

return baselineBytes + value * factor;
}

/** Returns the {@link ProcessBundleFile} for the given {@link DifferentialBuildType}. */
function getDifferentialBuildResult(
processResult: ProcessBundleResult, buildType: DifferentialBuildType):
ProcessBundleFile|null {
switch (buildType) {
case DifferentialBuildType.ORIGINAL: return processResult.original || null;
case DifferentialBuildType.DOWNLEVEL: return processResult.downlevel || null;
}
}

/** Returns whether or not all items in the list are equivalent to each other. */
function allEquivalent<T>(items: T[]): boolean {
if (items.length === 0) {
return true;
}

const first = items[0];
for (const item of items.slice(1)) {
if (item !== first) {
return false;
}
}

return true;
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,10 @@
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import { calculateBytes, calculateThresholds, ThresholdType, ThresholdSeverity } from './bundle-calculator';
import { Type } from '../../browser/schema';
import { calculateBytes, calculateThresholds, ThresholdType, ThresholdSeverity, calculateSizes } from './bundle-calculator';
import { Type, Budget } from '../../browser/schema';
import * as webpack from 'webpack';
import {ProcessBundleResult} from '@angular-devkit/build-angular/src/utils/process-bundle';

describe('bundle-calculator', () => {
describe('calculateThresholds()', () => {
Expand Down Expand Up @@ -66,6 +68,102 @@ describe('bundle-calculator', () => {
});
});

describe('calculateSizes()', () => {
it('calculates initial bundle sizes', () => {
const budget: Budget = { type: Type.Initial };
const webpackStats = {
chunks: [
{ id: 0, initial: true },
{ id: 1, initial: false },
],
assets: [],
} as unknown as webpack.Stats.ToJsonOutput;
const processResults: ProcessBundleResult[] = [
{
name: '0',
original: { filename: 'main-es2015.js', size: 10 },
downlevel: { filename: 'main-es5.js', size: 20 },
},
{
name: '1',
original: { filename: 'unrelated.js', size: 30 },
},
];

const sizes = calculateSizes(budget, webpackStats, processResults);

expect(sizes.length).toBe(2);
expect(sizes).toContain({ label: 'initial-es2015', size: 10 });
expect(sizes).toContain({ label: 'initial-es5', size: 20 });
});

it('calculates named bundle differential sizes', () => {
const budget: Budget = { type: Type.Bundle, name: 'custom-budget' };
const webpackStats = {
chunks: [
{ id: 0, names: ['custom-budget'] },
{ id: 1, names: ['unrelated'] },
],
assets: [],
} as unknown as webpack.Stats.ToJsonOutput;
const processResults: ProcessBundleResult[] = [
{
name: '0',
original: {
filename: 'custom-budget-es2015.js',
size: 10,
},
downlevel: {
filename: 'custom-budget-es5.js',
size: 20,
},
},
{
name: '1',
original: {
filename: 'unrelated.js',
size: 30,
},
},
];

const sizes = calculateSizes(budget, webpackStats, processResults);

expect(sizes.length).toBe(2);
expect(sizes).toContain({ label: 'custom-budget-es2015', size: 10 });
expect(sizes).toContain({ label: 'custom-budget-es5', size: 20 });
});

it('calculates named bundle non-differential sizes', () => {
const budget: Budget = { type: Type.Bundle, name: 'custom-budget' };
const webpackStats = {
chunks: [
{ id: 0, names: ['custom-budget'], files: [ 'first.js', 'second.js' ] },
{ id: 1, names: ['unrelated'] },
],
assets: [
{ name: 'first.js', size: 10 },
{ name: 'second.js', size: 20 },
],
} as unknown as webpack.Stats.ToJsonOutput;
const processResults: ProcessBundleResult[] = [
// No process result for custom-budget.
{
name: '1',
original: {
filename: 'unrelated.js',
size: 40,
},
},
];

const sizes = calculateSizes(budget, webpackStats, processResults);

expect(sizes.length).toBe(1);
expect(sizes).toContain({ label: 'custom-budget', size: 30 });
});
});

describe('calculateBytes()', () => {
it('converts an integer with no postfix', () => {
expect(calculateBytes('0')).toBe(0);
Expand Down
2 changes: 1 addition & 1 deletion packages/angular_devkit/build_angular/src/browser/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -625,7 +625,7 @@ export function buildWebpackBrowser(

const budgets = options.budgets || [];
for (const budget of budgets) {
const sizes = calculateSizes(budget, webpackStats);
const sizes = calculateSizes(budget, webpackStats, processResults);
for (const threshold of calculateThresholds(budget)) {
for (const {size, label} of sizes) {
if (exceededThreshold(size, threshold)) {
Expand Down

0 comments on commit c28498f

Please sign in to comment.