From a9da656085801608cfe164f352d4aec51969b01a Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Wed, 1 Nov 2023 16:32:20 +0000 Subject: [PATCH] fix(@angular-devkit/build-angular): fail build on non bundling error when using the esbuild based builders This change fixes an issue were non bundling related errors that happen during prerendering, index generation and bundle budget failures did not cause the build to terminate with an error. --- .../src/builders/application/execute-build.ts | 70 +++++++++++-------- .../tests/options/bundle-budgets_spec.ts | 1 + .../tools/esbuild/bundler-execution-result.ts | 6 +- .../e2e/tests/build/bundle-budgets.ts | 17 +---- 4 files changed, 48 insertions(+), 46 deletions(-) diff --git a/packages/angular_devkit/build_angular/src/builders/application/execute-build.ts b/packages/angular_devkit/build_angular/src/builders/application/execute-build.ts index 23ba4fd7be01..774e7e4ae881 100644 --- a/packages/angular_devkit/build_angular/src/builders/application/execute-build.ts +++ b/packages/angular_devkit/build_angular/src/builders/application/execute-build.ts @@ -28,7 +28,7 @@ import { logMessages, transformSupportedBrowsersToTargets, } from '../../tools/esbuild/utils'; -import { checkBudgets } from '../../utils/bundle-calculator'; +import { BudgetCalculatorResult, checkBudgets } from '../../utils/bundle-calculator'; import { colors } from '../../utils/color'; import { copyAssets } from '../../utils/copy-assets'; import { getSupportedBrowsers } from '../../utils/supported-browsers'; @@ -180,6 +180,37 @@ export async function executeBuild( executionResult.outputFiles.push(...outputFiles); + const changedFiles = + rebuildState && executionResult.findChangedFiles(rebuildState.previousOutputHashes); + + // Analyze files for bundle budget failures if present + let budgetFailures: BudgetCalculatorResult[] | undefined; + if (options.budgets) { + const compatStats = generateBudgetStats(metafile, initialFiles); + budgetFailures = [...checkBudgets(options.budgets, compatStats, true)]; + if (budgetFailures.length > 0) { + const errors = budgetFailures + .filter((failure) => failure.severity === 'error') + .map(({ message }) => message); + const warnings = budgetFailures + .filter((failure) => failure.severity !== 'error') + .map(({ message }) => message); + + await printWarningsAndErrorsToConsoleAndAddToResult( + context, + executionResult, + warnings, + errors, + ); + } + } + + // Calculate estimated transfer size if scripts are optimized + let estimatedTransferSizes; + if (optimizationOptions.scripts || optimizationOptions.styles.minify) { + estimatedTransferSizes = await calculateEstimatedTransferSizes(executionResult.outputFiles); + } + // Check metafile for CommonJS module usage if optimizing scripts if (optimizationOptions.scripts) { const messages = checkCommonJSModules(metafile, options.allowedCommonJsDependencies); @@ -202,29 +233,6 @@ export async function executeBuild( ); } - // Analyze files for bundle budget failures if present - let budgetFailures; - if (options.budgets) { - const compatStats = generateBudgetStats(metafile, initialFiles); - budgetFailures = [...checkBudgets(options.budgets, compatStats, true)]; - if (budgetFailures.length > 0) { - await logMessages(context, { - errors: budgetFailures - .filter((failure) => failure.severity === 'error') - .map((failure) => ({ text: failure.message, location: null })), - warnings: budgetFailures - .filter((failure) => failure.severity !== 'error') - .map((failure) => ({ text: failure.message, location: null })), - }); - } - } - - // Calculate estimated transfer size if scripts are optimized - let estimatedTransferSizes; - if (optimizationOptions.scripts || optimizationOptions.styles.minify) { - estimatedTransferSizes = await calculateEstimatedTransferSizes(executionResult.outputFiles); - } - // Perform i18n translation inlining if enabled let prerenderedRoutes: string[]; let errors: string[]; @@ -251,7 +259,7 @@ export async function executeBuild( executionResult.assetFiles.push(...result.additionalAssets); } - await printWarningsAndErrorsToConsole(context, warnings, errors); + await printWarningsAndErrorsToConsoleAndAddToResult(context, executionResult, warnings, errors); if (prerenderOptions) { executionResult.addOutputFile( @@ -270,8 +278,6 @@ export async function executeBuild( context.logger.info(colors.magenta(prerenderMsg) + '\n'); } - const changedFiles = - rebuildState && executionResult.findChangedFiles(rebuildState.previousOutputHashes); logBuildStats( context, metafile, @@ -293,13 +299,19 @@ export async function executeBuild( return executionResult; } -async function printWarningsAndErrorsToConsole( +async function printWarningsAndErrorsToConsoleAndAddToResult( context: BuilderContext, + executionResult: ExecutionResult, warnings: string[], errors: string[], ): Promise { + const errorMessages = errors.map((text) => ({ text, location: null })); + if (errorMessages.length) { + executionResult.addErrors(errorMessages); + } + await logMessages(context, { - errors: errors.map((text) => ({ text, location: null })), + errors: errorMessages, warnings: warnings.map((text) => ({ text, location: null })), }); } diff --git a/packages/angular_devkit/build_angular/src/builders/application/tests/options/bundle-budgets_spec.ts b/packages/angular_devkit/build_angular/src/builders/application/tests/options/bundle-budgets_spec.ts index 3d8cbf39b63c..289b0c9f1a58 100644 --- a/packages/angular_devkit/build_angular/src/builders/application/tests/options/bundle-budgets_spec.ts +++ b/packages/angular_devkit/build_angular/src/builders/application/tests/options/bundle-budgets_spec.ts @@ -42,6 +42,7 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => { }); const { result, logs } = await harness.executeOnce(); + expect(result?.success).toBeFalse(); expect(logs).toContain( jasmine.objectContaining({ level: 'error', diff --git a/packages/angular_devkit/build_angular/src/tools/esbuild/bundler-execution-result.ts b/packages/angular_devkit/build_angular/src/tools/esbuild/bundler-execution-result.ts index e7707b162312..71e61ed77e0e 100644 --- a/packages/angular_devkit/build_angular/src/tools/esbuild/bundler-execution-result.ts +++ b/packages/angular_devkit/build_angular/src/tools/esbuild/bundler-execution-result.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import type { Message } from 'esbuild'; +import type { Message, PartialMessage } from 'esbuild'; import type { ChangedFiles } from '../../tools/esbuild/watcher'; import type { SourceFileCache } from './angular/source-file-cache'; import type { BuildOutputFile, BuildOutputFileType, BundlerContext } from './bundler-context'; @@ -30,7 +30,7 @@ export interface RebuildState { export class ExecutionResult { outputFiles: BuildOutputFile[] = []; assetFiles: BuildOutputAsset[] = []; - errors: Message[] = []; + errors: (Message | PartialMessage)[] = []; externalMetadata?: { implicit: string[]; explicit?: string[] }; constructor( @@ -46,7 +46,7 @@ export class ExecutionResult { this.assetFiles.push(...assets); } - addErrors(errors: Message[]): void { + addErrors(errors: (Message | PartialMessage)[]): void { this.errors.push(...errors); } diff --git a/tests/legacy-cli/e2e/tests/build/bundle-budgets.ts b/tests/legacy-cli/e2e/tests/build/bundle-budgets.ts index f7b3caefb6c1..ee372f0ba75a 100644 --- a/tests/legacy-cli/e2e/tests/build/bundle-budgets.ts +++ b/tests/legacy-cli/e2e/tests/build/bundle-budgets.ts @@ -5,14 +5,11 @@ * 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 { getGlobalVariable } from '../../utils/env'; import { ng } from '../../utils/process'; import { updateJsonFile } from '../../utils/project'; import { expectToFail } from '../../utils/utils'; export default async function () { - const usingWebpack = !getGlobalVariable('argv')['esbuild']; - // Error await updateJsonFile('angular.json', (json) => { json.projects['test-project'].architect.build.configurations.production.budgets = [ @@ -20,17 +17,9 @@ export default async function () { ]; }); - if (usingWebpack) { - const { message: errorMessage } = await expectToFail(() => ng('build')); - if (!/Error.+budget/i.test(errorMessage)) { - throw new Error('Budget error: all, max error.'); - } - } else { - // Application builder does not generate an error exit code for budget failures - const { stderr } = await ng('build'); - if (!/Error.+budget/i.test(stderr)) { - throw new Error('Budget error: all, max error.'); - } + const { message: errorMessage } = await expectToFail(() => ng('build')); + if (!/Error.+budget/i.test(errorMessage)) { + throw new Error('Budget error: all, max error.'); } // Warning