From c28498f0d9552ad1e769948d902e988d4a1f009a Mon Sep 17 00:00:00 2001 From: Doug Parker Date: Fri, 22 Nov 2019 12:50:00 -0800 Subject: [PATCH] fix(@angular-devkit/build-angular): update budgets to check differential builds separately Fixes #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. --- .../utilities/bundle-calculator.ts | 149 ++++++++++++++---- .../utilities/bundle-calculator_spec.ts | 102 +++++++++++- .../build_angular/src/browser/index.ts | 2 +- 3 files changed, 215 insertions(+), 38 deletions(-) diff --git a/packages/angular_devkit/build_angular/src/angular-cli-files/utilities/bundle-calculator.ts b/packages/angular_devkit/build_angular/src/angular-cli-files/utilities/bundle-calculator.ts index b66df06ee5dc..93867e001ce8 100644 --- a/packages/angular_devkit/build_angular/src/angular-cli-files/utilities/bundle-calculator.ts +++ b/packages/angular_devkit/build_angular/src/angular-cli-files/utilities/bundle-calculator.ts @@ -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; @@ -29,6 +30,11 @@ export enum ThresholdSeverity { ERROR = 'error', } +enum DifferentialBuildType { + ORIGINAL = 'es2015', + DOWNLEVEL = 'es5', +} + export function* calculateThresholds(budget: Budget): IterableIterator { if (budget.maximumWarning) { yield { @@ -91,7 +97,16 @@ export function* calculateThresholds(budget: Budget): IterableIterator = { all: AllCalculator, allScript: AllScriptCalculator, @@ -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 extends Array ? U : never; +type Chunk = ArrayElement>; +type Asset = ArrayElement>; export abstract class Calculator { constructor ( protected budget: Budget, - protected chunks: Exclude, - protected assets: Exclude, + 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); + } + } } /** @@ -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, + }]; } } @@ -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), + }; + }); } } @@ -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(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; +} diff --git a/packages/angular_devkit/build_angular/src/angular-cli-files/utilities/bundle-calculator_spec.ts b/packages/angular_devkit/build_angular/src/angular-cli-files/utilities/bundle-calculator_spec.ts index b7d816188542..19cf8fb4f364 100644 --- a/packages/angular_devkit/build_angular/src/angular-cli-files/utilities/bundle-calculator_spec.ts +++ b/packages/angular_devkit/build_angular/src/angular-cli-files/utilities/bundle-calculator_spec.ts @@ -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()', () => { @@ -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); diff --git a/packages/angular_devkit/build_angular/src/browser/index.ts b/packages/angular_devkit/build_angular/src/browser/index.ts index e8d7ab8679ff..37da7b25cf31 100644 --- a/packages/angular_devkit/build_angular/src/browser/index.ts +++ b/packages/angular_devkit/build_angular/src/browser/index.ts @@ -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)) {