Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): fail build on non bundling error …
Browse files Browse the repository at this point in the history
…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.

(cherry picked from commit a9da656)
  • Loading branch information
alan-agius4 committed Nov 1, 2023
1 parent bece843 commit 9eb58cf
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 46 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand All @@ -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[];
Expand All @@ -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(
Expand All @@ -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,
Expand All @@ -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<void> {
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 })),
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<logging.LogEntry>({
level: 'error',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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(
Expand All @@ -46,7 +46,7 @@ export class ExecutionResult {
this.assetFiles.push(...assets);
}

addErrors(errors: Message[]): void {
addErrors(errors: (Message | PartialMessage)[]): void {
this.errors.push(...errors);
}

Expand Down
17 changes: 3 additions & 14 deletions tests/legacy-cli/e2e/tests/build/bundle-budgets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,32 +5,21 @@
* 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 = [
{ type: 'all', maximumError: '100b' },
];
});

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
Expand Down

0 comments on commit 9eb58cf

Please sign in to comment.