Skip to content

Commit

Permalink
refactor(@angular-devkit/build-angular): move diagnostic logging out …
Browse files Browse the repository at this point in the history
…of build execution

The logging of diagnostic (error/warning) messages from the build execution within the
`application` builder has been moved up one level. This allows the actual execution to
focus more on generating a result and leaves the enclosing builder system to handle
notification of the results.

(cherry picked from commit b18bd20)
  • Loading branch information
clydin authored and dgp1130 committed Jan 9, 2024
1 parent 433aef9 commit ca0a836
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,12 @@ import path from 'node:path';
import { BuildOutputFile } from '../../tools/esbuild/bundler-context';
import { ExecutionResult, RebuildState } from '../../tools/esbuild/bundler-execution-result';
import { shutdownSassWorkerPool } from '../../tools/esbuild/stylesheets/sass-language';
import { withNoProgress, withSpinner, writeResultFiles } from '../../tools/esbuild/utils';
import {
logMessages,
withNoProgress,
withSpinner,
writeResultFiles,
} from '../../tools/esbuild/utils';
import { deleteOutputDir } from '../../utils/delete-output-dir';
import { shouldWatchRoot } from '../../utils/environment-options';
import { NormalizedCachedOptions } from '../../utils/normalize-cache';
Expand Down Expand Up @@ -66,7 +71,11 @@ export async function* runEsBuildBuildAction(
// Initial build
let result: ExecutionResult;
try {
// Perform the build action
result = await withProgress('Building...', () => action());

// Log all diagnostic (error/warning) messages from the build
await logMessages(logger, result);
} finally {
// Ensure Sass workers are shutdown if not watching
if (!watch) {
Expand Down Expand Up @@ -171,6 +180,9 @@ export async function* runEsBuildBuildAction(
action(result.createRebuildState(changes)),
);

// Log all diagnostic (error/warning) messages from the rebuild
await logMessages(logger, result);

// Update watched locations provided by the new build result.
// Keep watching all previous files if there are any errors; otherwise consider all
// files stale until confirmed present in the new result's watch files.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@ import { BuildOutputFileType, BundlerContext } from '../../tools/esbuild/bundler
import { ExecutionResult, RebuildState } from '../../tools/esbuild/bundler-execution-result';
import { checkCommonJSModules } from '../../tools/esbuild/commonjs-checker';
import { extractLicenses } from '../../tools/esbuild/license-extractor';
import {
calculateEstimatedTransferSizes,
logBuildStats,
logMessages,
} from '../../tools/esbuild/utils';
import { calculateEstimatedTransferSizes, logBuildStats } from '../../tools/esbuild/utils';
import { BudgetCalculatorResult, checkBudgets } from '../../utils/bundle-calculator';
import { colors } from '../../utils/color';
import { copyAssets } from '../../utils/copy-assets';
Expand Down Expand Up @@ -65,10 +61,8 @@ export async function executeBuild(
rebuildState?.fileChanges.all,
);

// Log all warnings and errors generated during bundling
await logMessages(context, bundlingResult);

const executionResult = new ExecutionResult(bundlerContexts, codeBundleCache);
executionResult.addWarnings(bundlingResult.warnings);

// Return if the bundling has errors
if (bundlingResult.errors) {
Expand Down Expand Up @@ -188,16 +182,14 @@ export async function executeBuild(
}

logBuildStats(
context,
context.logger,
metafile,
initialFiles,
budgetFailures,
changedFiles,
estimatedTransferSizes,
);

await logMessages(context, executionResult);

// Write metafile if stats option is enabled
if (options.stats) {
executionResult.addOutputFile(
Expand Down
16 changes: 8 additions & 8 deletions packages/angular_devkit/build_angular/src/tools/esbuild/utils.ts
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 { BuilderContext } from '@angular-devkit/architect';
import { logging } from '@angular-devkit/core';
import { BuildOptions, Metafile, OutputFile, PartialMessage, formatMessages } from 'esbuild';
import { createHash } from 'node:crypto';
import { constants as fsConstants } from 'node:fs';
Expand All @@ -22,7 +22,7 @@ import { BuildOutputFile, BuildOutputFileType, InitialFileRecord } from './bundl
import { BuildOutputAsset } from './bundler-execution-result';

export function logBuildStats(
context: BuilderContext,
logger: logging.LoggerApi,
metafile: Metafile,
initial: Map<string, InitialFileRecord>,
budgetFailures: BudgetCalculatorResult[] | undefined,
Expand Down Expand Up @@ -71,12 +71,12 @@ export function logBuildStats(
budgetFailures,
);

context.logger.info('\n' + tableText + '\n');
logger.info('\n' + tableText + '\n');
} else if (changedFiles !== undefined) {
context.logger.info('\nNo output file changes.\n');
logger.info('\nNo output file changes.\n');
}
if (unchangedCount > 0) {
context.logger.info(`Unchanged output files: ${unchangedCount}`);
logger.info(`Unchanged output files: ${unchangedCount}`);
}
}

Expand Down Expand Up @@ -143,17 +143,17 @@ export async function withNoProgress<T>(text: string, action: () => T | Promise<
}

export async function logMessages(
context: BuilderContext,
logger: logging.LoggerApi,
{ errors, warnings }: { errors?: PartialMessage[]; warnings?: PartialMessage[] },
): Promise<void> {
if (warnings?.length) {
const warningMessages = await formatMessages(warnings, { kind: 'warning', color: true });
context.logger.warn(warningMessages.join('\n'));
logger.warn(warningMessages.join('\n'));
}

if (errors?.length) {
const errorMessages = await formatMessages(errors, { kind: 'error', color: true });
context.logger.error(errorMessages.join('\n'));
logger.error(errorMessages.join('\n'));
}
}

Expand Down

0 comments on commit ca0a836

Please sign in to comment.