Skip to content

Commit 770de94

Browse files
dgp1130vikerman
authored andcommitted
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.
1 parent 061c5f3 commit 770de94

File tree

4 files changed

+317
-54
lines changed

4 files changed

+317
-54
lines changed

packages/angular_devkit/build_angular/src/angular-cli-files/plugins/bundle-budget.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
*/
88
import { Compiler, compilation } from 'webpack';
99
import { Budget, Type } from '../../browser/schema';
10+
import { ProcessBundleResult } from '../../utils/process-bundle';
1011
import { ThresholdSeverity, checkBudgets } from '../utilities/bundle-calculator';
1112

1213
export interface BundleBudgetPluginOptions {
@@ -29,8 +30,12 @@ export class BundleBudgetPlugin {
2930
}
3031

3132
private runChecks(budgets: Budget[], compilation: compilation.Compilation) {
33+
// No process bundle results because this plugin is only used when differential
34+
// builds are disabled.
35+
const processResults: ProcessBundleResult[] = [];
36+
3237
const stats = compilation.getStats().toJson();
33-
for (const { severity, message } of checkBudgets(budgets, stats)) {
38+
for (const { severity, message } of checkBudgets(budgets, stats, processResults)) {
3439
switch (severity) {
3540
case ThresholdSeverity.Warning:
3641
compilation.warnings.push(`budgets: ${message}`);

packages/angular_devkit/build_angular/src/angular-cli-files/utilities/bundle-calculator.ts

Lines changed: 130 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88
import * as webpack from 'webpack';
9+
import { ProcessBundleFile, ProcessBundleResult } from '../../../src/utils/process-bundle';
910
import { Budget, Type } from '../../browser/schema';
1011
import { formatSize } from '../utilities/stats';
1112

@@ -30,6 +31,11 @@ export enum ThresholdSeverity {
3031
Error = 'error',
3132
}
3233

34+
enum DifferentialBuildType {
35+
ORIGINAL = 'es2015',
36+
DOWNLEVEL = 'es5',
37+
}
38+
3339
export function* calculateThresholds(budget: Budget): IterableIterator<Threshold> {
3440
if (budget.maximumWarning) {
3541
yield {
@@ -98,6 +104,7 @@ export function* calculateThresholds(budget: Budget): IterableIterator<Threshold
98104
function calculateSizes(
99105
budget: Budget,
100106
stats: webpack.Stats.ToJsonOutput,
107+
processResults: ProcessBundleResult[],
101108
): Size[] {
102109
if (budget.type === Type.AnyComponentStyle) {
103110
// Component style size information is not available post-build, this must
@@ -124,19 +131,55 @@ function calculateSizes(
124131
throw new Error('Webpack stats output did not include asset information.');
125132
}
126133

127-
const calculator = new ctor(budget, chunks, assets);
134+
const calculator = new ctor(budget, chunks, assets, processResults);
128135

129136
return calculator.calculate();
130137
}
131138

139+
type ArrayElement<T> = T extends Array<infer U> ? U : never;
140+
type Chunk = ArrayElement<Exclude<webpack.Stats.ToJsonOutput['chunks'], undefined>>;
141+
type Asset = ArrayElement<Exclude<webpack.Stats.ToJsonOutput['assets'], undefined>>;
132142
abstract class Calculator {
133143
constructor (
134144
protected budget: Budget,
135-
protected chunks: Exclude<webpack.Stats.ToJsonOutput['chunks'], undefined>,
136-
protected assets: Exclude<webpack.Stats.ToJsonOutput['assets'], undefined>,
145+
protected chunks: Chunk[],
146+
protected assets: Asset[],
147+
protected processResults: ProcessBundleResult[],
137148
) {}
138149

139150
abstract calculate(): Size[];
151+
152+
/** Calculates the size of the given chunk for the provided build type. */
153+
protected calculateChunkSize(
154+
chunk: Chunk,
155+
buildType: DifferentialBuildType,
156+
): number {
157+
// Look for a process result containing different builds for this chunk.
158+
const processResult = this.processResults
159+
.find((processResult) => processResult.name === chunk.id.toString());
160+
161+
if (processResult) {
162+
// Found a differential build, use the correct size information.
163+
const processResultFile = getDifferentialBuildResult(
164+
processResult, buildType);
165+
166+
return processResultFile && processResultFile.size || 0;
167+
} else {
168+
// No differential builds, get the chunk size by summing its assets.
169+
return chunk.files
170+
.filter((file) => !file.endsWith('.map'))
171+
.map((file: string) => {
172+
const asset = this.assets.find((asset) => asset.name === file);
173+
if (!asset) {
174+
throw new Error(`Could not find asset for file: ${file}`);
175+
}
176+
177+
return asset;
178+
})
179+
.map((asset) => asset.size)
180+
.reduce((l, r) => l + r);
181+
}
182+
}
140183
}
141184

142185
/**
@@ -149,22 +192,24 @@ class BundleCalculator extends Calculator {
149192
return [];
150193
}
151194

152-
const size: number = this.chunks
153-
.filter(chunk => chunk.names.indexOf(budgetName) !== -1)
154-
.reduce((files, chunk) => [...files, ...chunk.files], [])
155-
.filter((file: string) => !file.endsWith('.map'))
156-
.map((file: string) => {
157-
const asset = this.assets.find((asset) => asset.name === file);
158-
if (!asset) {
159-
throw new Error(`Could not find asset for file: ${file}`);
160-
}
161-
162-
return asset;
163-
})
164-
.map((asset) => asset.size)
165-
.reduce((total: number, size: number) => total + size, 0);
166-
167-
return [{size, label: this.budget.name}];
195+
// The chunk may or may not have differential builds. Compute the size for
196+
// each then check afterwards if they are all the same.
197+
const buildSizes = Object.values(DifferentialBuildType).map((buildType) => {
198+
const size = this.chunks
199+
.filter(chunk => chunk.names.indexOf(budgetName) !== -1)
200+
.map((chunk) => this.calculateChunkSize(chunk, buildType))
201+
.reduce((l, r) => l + r);
202+
203+
return {size, label: `${this.budget.name}-${buildType}`};
204+
});
205+
206+
// If this bundle was not actually generated by a differential build, then
207+
// merge the results into a single value.
208+
if (allEquivalent(buildSizes.map((buildSize) => buildSize.size))) {
209+
return mergeDifferentialBuildSizes(buildSizes, budgetName);
210+
} else {
211+
return buildSizes;
212+
}
168213
}
169214
}
170215

@@ -173,22 +218,23 @@ class BundleCalculator extends Calculator {
173218
*/
174219
class InitialCalculator extends Calculator {
175220
calculate() {
176-
const size = this.chunks
177-
.filter(chunk => chunk.initial)
178-
.reduce((files, chunk) => [...files, ...chunk.files], [])
179-
.filter((file: string) => !file.endsWith('.map'))
180-
.map((file: string) => {
181-
const asset = this.assets.find((asset) => asset.name === file);
182-
if (!asset) {
183-
throw new Error(`Could not find asset for file: ${file}`);
184-
}
185-
186-
return asset;
187-
})
188-
.map((asset) => asset.size)
189-
.reduce((total: number, size: number) => total + size, 0);
190-
191-
return [{size, label: 'initial'}];
221+
const buildSizes = Object.values(DifferentialBuildType).map((buildType) => {
222+
return {
223+
label: `initial-${buildType}`,
224+
size: this.chunks
225+
.filter(chunk => chunk.initial)
226+
.map((chunk) => this.calculateChunkSize(chunk, buildType))
227+
.reduce((l, r) => l + r),
228+
};
229+
});
230+
231+
// If this bundle was not actually generated by a differential build, then
232+
// merge the results into a single value.
233+
if (allEquivalent(buildSizes.map((buildSize) => buildSize.size))) {
234+
return mergeDifferentialBuildSizes(buildSizes, 'initial');
235+
} else {
236+
return buildSizes;
237+
}
192238
}
193239
}
194240

@@ -287,13 +333,15 @@ function calculateBytes(
287333
}
288334

289335
export function* checkBudgets(
290-
budgets: Budget[], webpackStats: webpack.Stats.ToJsonOutput):
291-
IterableIterator<{ severity: ThresholdSeverity, message: string }> {
336+
budgets: Budget[],
337+
webpackStats: webpack.Stats.ToJsonOutput,
338+
processResults: ProcessBundleResult[],
339+
): IterableIterator<{ severity: ThresholdSeverity, message: string }> {
292340
// Ignore AnyComponentStyle budgets as these are handled in `AnyComponentStyleBudgetChecker`.
293341
const computableBudgets = budgets.filter((budget) => budget.type !== Type.AnyComponentStyle);
294342

295343
for (const budget of computableBudgets) {
296-
const sizes = calculateSizes(budget, webpackStats);
344+
const sizes = calculateSizes(budget, webpackStats, processResults);
297345
for (const { size, label } of sizes) {
298346
yield* checkThresholds(calculateThresholds(budget), size, label);
299347
}
@@ -339,6 +387,50 @@ export function* checkThresholds(thresholds: IterableIterator<Threshold>, size:
339387
}
340388
}
341389

390+
/** Returns the {@link ProcessBundleFile} for the given {@link DifferentialBuildType}. */
391+
function getDifferentialBuildResult(
392+
processResult: ProcessBundleResult, buildType: DifferentialBuildType):
393+
ProcessBundleFile|null {
394+
switch (buildType) {
395+
case DifferentialBuildType.ORIGINAL: return processResult.original || null;
396+
case DifferentialBuildType.DOWNLEVEL: return processResult.downlevel || null;
397+
}
398+
}
399+
400+
/**
401+
* Merges the given differential builds into a single, non-differential value.
402+
*
403+
* Preconditions: All the sizes should be equivalent, or else they represent
404+
* differential builds.
405+
*/
406+
function mergeDifferentialBuildSizes(buildSizes: Size[], margeLabel: string): Size[] {
407+
if (buildSizes.length === 0) {
408+
return [];
409+
}
410+
411+
// Only one size.
412+
return [{
413+
label: margeLabel,
414+
size: buildSizes[0].size,
415+
}];
416+
}
417+
418+
/** Returns whether or not all items in the list are equivalent to each other. */
419+
function allEquivalent<T>(items: T[]): boolean {
420+
if (items.length === 0) {
421+
return true;
422+
}
423+
424+
const first = items[0];
425+
for (const item of items.slice(1)) {
426+
if (item !== first) {
427+
return false;
428+
}
429+
}
430+
431+
return true;
432+
}
433+
342434
function assertNever(input: never): never {
343435
throw new Error(`Unexpected call to assertNever() with input: ${
344436
JSON.stringify(input, null /* replacer */, 4 /* tabSize */)}`);

0 commit comments

Comments
 (0)