fix(@angular-devkit/build-angular): update budgets to check differential builds separately

dgp1130 · dgp1130 · commit 4fe4c9bae2c0 · 2019-11-22T14:07:38.000-08:00
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.
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
@@ -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 } from '../../browser/schema';
 
 export interface Size {
@@ -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 {
@@ -91,8 +97,17 @@ export function* calculateThresholds(budget: Budget): IterableIterator<Threshold
   }
 }
 
-export function calculateSizes(budget: Budget, stats: webpack.Stats.ToJsonOutput): Size[] {
-  const calculatorMap: Record<Budget['type'], { new(...args: any[]): Calculator }> = {
+/**
+ * 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: unknown[]): Calculator }> = {
     all: AllCalculator,
     allScript: AllScriptCalculator,
     any: AnyCalculator,
@@ -111,19 +126,55 @@ 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);
+    }
+  }
 }
 
 /**
@@ -136,22 +187,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 +221,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 +340,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;
+}
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
@@ -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 * as webpack from 'webpack';
+import { Budget, Type } from '../../browser/schema';
+import { ProcessBundleResult } from '../../utils/process-bundle';
+import { ThresholdSeverity, ThresholdType, calculateBytes, calculateSizes, calculateThresholds } from './bundle-calculator';
 
 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
@@ -28,7 +28,7 @@ import {
   normalizeExtraEntryPoints,
 } from '../angular-cli-files/models/webpack-configs';
 import { markAsyncChunksNonInitial } from '../angular-cli-files/utilities/async-chunks';
-import { ThresholdSeverity, calculateSizes, calculateThresholds, ThresholdType, Threshold } from '../angular-cli-files/utilities/bundle-calculator';
+import { Threshold, ThresholdSeverity, ThresholdType, calculateSizes, calculateThresholds } from '../angular-cli-files/utilities/bundle-calculator';
 import {
   IndexHtmlTransform,
   writeIndexHtml,
@@ -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)) {
