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 Dec 4, 2019
1 parent 685c19d commit 68e5152
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 38 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import { Compiler, compilation } from 'webpack';
import { Budget, Type } from '../../browser/schema';
import { ThresholdSeverity, checkBudgets } from '../utilities/bundle-calculator';
import { ProcessBundleResult } from '../../utils/process-bundle';

export interface BundleBudgetPluginOptions {
budgets: Budget[];
Expand All @@ -29,8 +30,12 @@ export class BundleBudgetPlugin {
}

private runChecks(budgets: Budget[], compilation: compilation.Compilation) {
// No process bundle results because this plugin is only used when differential
// builds are disabled.
const processResults: ProcessBundleResult[] = [];

const stats = compilation.getStats().toJson();
for (const { severity, message } of checkBudgets(budgets, stats)) {
for (const { severity, message } of checkBudgets(budgets, stats, processResults)) {
switch (severity) {
case ThresholdSeverity.Warning:
compilation.warnings.push(`budgets: ${message}`);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/
import * as webpack from 'webpack';
import { ProcessBundleFile, ProcessBundleResult } from '../../../src/utils/process-bundle';
import { Budget, Type } from '../../browser/schema';
import { formatSize } from '../utilities/stats';

Expand All @@ -30,6 +31,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 @@ -98,6 +104,7 @@ export function* calculateThresholds(budget: Budget): IterableIterator<Threshold
function calculateSizes(
budget: Budget,
stats: webpack.Stats.ToJsonOutput,
processResults: ProcessBundleResult[],
): Size[] {
if (budget.type === Type.AnyComponentStyle) {
// Component style size information is not available post-build, this must
Expand All @@ -124,19 +131,55 @@ function calculateSizes(
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>>;
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 @@ -149,22 +192,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}`);
}
// 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;
}

return asset;
})
.map((asset) => asset.size)
.reduce((total: number, size: number) => total + size, 0);
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 @@ -173,22 +226,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 @@ -287,13 +333,15 @@ function calculateBytes(
}

export function* checkBudgets(
budgets: Budget[], webpackStats: webpack.Stats.ToJsonOutput):
IterableIterator<{ severity: ThresholdSeverity, message: string }> {
budgets: Budget[],
webpackStats: webpack.Stats.ToJsonOutput,
processResults: ProcessBundleResult[],
): IterableIterator<{ severity: ThresholdSeverity, message: string }> {
// Ignore AnyComponentStyle budgets as these are handled in `AnyComponentStyleBudgetChecker`.
const computableBudgets = budgets.filter((budget) => budget.type !== Type.AnyComponentStyle);

for (const budget of computableBudgets) {
const sizes = calculateSizes(budget, webpackStats);
const sizes = calculateSizes(budget, webpackStats, processResults);
for (const { size, label } of sizes) {
yield* checkThresholds(calculateThresholds(budget), size, label);
}
Expand Down Expand Up @@ -339,6 +387,32 @@ export function* checkThresholds(thresholds: IterableIterator<Threshold>, size:
}
}

/** 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;
}

function assertNever(input: never): never {
throw new Error(`Unexpected call to assertNever() with input: ${
JSON.stringify(input, null /* replacer */, 4 /* tabSize */)}`);
Expand Down
3 changes: 2 additions & 1 deletion packages/angular_devkit/build_angular/src/browser/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -639,7 +639,8 @@ export function buildWebpackBrowser(

// Check for budget errors and display them to the user.
const budgets = options.budgets || [];
for (const {severity, message} of checkBudgets(budgets, webpackStats)) {
const budgetFailures = checkBudgets(budgets, webpackStats, processResults);
for (const {severity, message} of budgetFailures) {
switch (severity) {
case ThresholdSeverity.Warning:
webpackStats.warnings.push(message);
Expand Down

0 comments on commit 68e5152

Please sign in to comment.