From 0c9cb148c4ac6d1d55069a764200f4a572c7160c Mon Sep 17 00:00:00 2001 From: Alan Agius Date: Fri, 8 Mar 2024 10:56:08 +0000 Subject: [PATCH] fix(@angular-devkit/build-angular): ensure proper display of build logs in the presence of warnings or errors Previously, a race condition could occur due to the spinner, resulting in the deletion of the last printed log when warnings or errors were present. With this update, we ensure that logs are printed after the spinner has stopped. Fixes #27233 --- .../src/builders/application/build-action.ts | 19 +++++- .../src/builders/application/execute-build.ts | 2 +- .../src/builders/application/index.ts | 19 +++--- .../tools/esbuild/bundler-execution-result.ts | 5 ++ .../build_angular/src/tools/esbuild/utils.ts | 67 ++++++++++--------- 5 files changed, 70 insertions(+), 42 deletions(-) diff --git a/packages/angular_devkit/build_angular/src/builders/application/build-action.ts b/packages/angular_devkit/build_angular/src/builders/application/build-action.ts index 14ce3de13213..1eee42968206 100644 --- a/packages/angular_devkit/build_angular/src/builders/application/build-action.ts +++ b/packages/angular_devkit/build_angular/src/builders/application/build-action.ts @@ -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'; @@ -34,7 +39,7 @@ const packageWatchFiles = [ ]; export async function* runEsBuildBuildAction( - action: (rebuildState?: RebuildState) => ExecutionResult | Promise, + action: (rebuildState?: RebuildState) => Promise, options: { workspaceRoot: string; projectRoot: string; @@ -51,6 +56,8 @@ export async function* runEsBuildBuildAction( signal?: AbortSignal; preserveSymlinks?: boolean; clearScreen?: boolean; + colors?: boolean; + jsonLogs?: boolean; }, ): AsyncIterable<(ExecutionResult['outputWithFiles'] | ExecutionResult['output']) & BuilderOutput> { const { @@ -68,6 +75,8 @@ export async function* runEsBuildBuildAction( workspaceRoot, progress, preserveSymlinks, + colors, + jsonLogs, } = options; if (deleteOutputPath && writeToFileSystem) { @@ -84,6 +93,9 @@ export async function* runEsBuildBuildAction( try { // Perform the build action result = await withProgress('Building...', () => action()); + + // Log all diagnostic (error/warning/logs) messages + await logMessages(logger, result, colors, jsonLogs); } finally { // Ensure Sass workers are shutdown if not watching if (!watch) { @@ -179,6 +191,9 @@ export async function* runEsBuildBuildAction( action(result.createRebuildState(changes)), ); + // Log all diagnostic (error/warning/logs) messages + await logMessages(logger, result, colors, jsonLogs); + // 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. 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 ee848a90ff7f..4e8856b7c510 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 @@ -185,7 +185,7 @@ export async function executeBuild( } if (!jsonLogs) { - context.logger.info( + executionResult.addLog( logBuildStats( metafile, initialFiles, diff --git a/packages/angular_devkit/build_angular/src/builders/application/index.ts b/packages/angular_devkit/build_angular/src/builders/application/index.ts index 69d3c72f6d3e..80edf0632a2e 100644 --- a/packages/angular_devkit/build_angular/src/builders/application/index.ts +++ b/packages/angular_devkit/build_angular/src/builders/application/index.ts @@ -9,7 +9,7 @@ import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect'; import type { Plugin } from 'esbuild'; import { BuildOutputFile, BuildOutputFileType } from '../../tools/esbuild/bundler-context'; -import { logMessages } from '../../tools/esbuild/utils'; +import { createJsonBuildManifest } from '../../tools/esbuild/utils'; import { colors as ansiColors } from '../../utils/color'; import { purgeStaleBuildCache } from '../../utils/purge-cache'; import { assertCompatibleAngularVersion } from '../../utils/version'; @@ -90,29 +90,28 @@ export async function* buildApplicationInternal( const startTime = process.hrtime.bigint(); const result = await executeBuild(normalizedOptions, context, rebuildState); - if (!jsonLogs) { + if (jsonLogs) { + result.addLog(await createJsonBuildManifest(result, normalizedOptions)); + } else { if (prerenderOptions) { const prerenderedRoutesLength = result.prerenderedRoutes.length; let prerenderMsg = `Prerendered ${prerenderedRoutesLength} static route`; prerenderMsg += prerenderedRoutesLength !== 1 ? 's.' : '.'; - logger.info(ansiColors.magenta(prerenderMsg)); + result.addLog(ansiColors.magenta(prerenderMsg)); } const buildTime = Number(process.hrtime.bigint() - startTime) / 10 ** 9; const hasError = result.errors.length > 0; if (writeToFileSystem && !hasError) { - logger.info(`Output location: ${outputOptions.base}\n`); + result.addLog(`Output location: ${outputOptions.base}\n`); } - logger.info( - `Application bundle generation ${hasError ? 'failed' : 'complete'}. [${buildTime.toFixed(3)} seconds]`, + result.addLog( + `Application bundle generation ${hasError ? 'failed' : 'complete'}. [${buildTime.toFixed(3)} seconds]\n`, ); } - // Log all diagnostic (error/warning) messages - await logMessages(logger, result, normalizedOptions); - return result; }, { @@ -127,6 +126,8 @@ export async function* buildApplicationInternal( workspaceRoot: normalizedOptions.workspaceRoot, progress: normalizedOptions.progress, clearScreen: normalizedOptions.clearScreen, + colors: normalizedOptions.colors, + jsonLogs: normalizedOptions.jsonLogs, writeToFileSystem, // For app-shell and SSG server files are not required by users. // Omit these when SSR is not enabled. 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 02674c91d089..83dc7b46d584 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 @@ -40,6 +40,7 @@ export class ExecutionResult { errors: (Message | PartialMessage)[] = []; prerenderedRoutes: string[] = []; warnings: (Message | PartialMessage)[] = []; + logs: string[] = []; externalMetadata?: ExternalResultMetadata; constructor( @@ -55,6 +56,10 @@ export class ExecutionResult { this.assetFiles.push(...assets); } + addLog(value: string): void { + this.logs.push(value); + } + addError(error: PartialMessage | string): void { if (typeof error === 'string') { this.errors.push({ text: error, location: null }); diff --git a/packages/angular_devkit/build_angular/src/tools/esbuild/utils.ts b/packages/angular_devkit/build_angular/src/tools/esbuild/utils.ts index 6fd6100fa6b9..41c4c42fdb20 100644 --- a/packages/angular_devkit/build_angular/src/tools/esbuild/utils.ts +++ b/packages/angular_devkit/build_angular/src/tools/esbuild/utils.ts @@ -429,46 +429,53 @@ interface BuildManifest { prerenderedRoutes?: string[]; } -export async function logMessages( - logger: logging.LoggerApi, - executionResult: ExecutionResult, - options: NormalizedApplicationBuildOptions, -): Promise { +export async function createJsonBuildManifest( + result: ExecutionResult, + normalizedOptions: NormalizedApplicationBuildOptions, +): Promise { const { + colors: color, outputOptions: { base, server, browser }, ssrOptions, - jsonLogs, - colors: color, - } = options; - const { warnings, errors, prerenderedRoutes } = executionResult; - const warningMessages = warnings.length - ? await formatMessages(warnings, { kind: 'warning', color }) - : []; - const errorMessages = errors.length ? await formatMessages(errors, { kind: 'error', color }) : []; + } = normalizedOptions; - if (jsonLogs) { - // JSON format output - const manifest: BuildManifest = { - errors: errorMessages, - warnings: warningMessages, - outputPaths: { - root: pathToFileURL(base), - browser: pathToFileURL(join(base, browser)), - server: ssrOptions ? pathToFileURL(join(base, server)) : undefined, - }, - prerenderedRoutes, - }; + const { warnings, errors, prerenderedRoutes } = result; + + const manifest: BuildManifest = { + errors: errors.length ? await formatMessages(errors, { kind: 'error', color }) : [], + warnings: warnings.length ? await formatMessages(warnings, { kind: 'warning', color }) : [], + outputPaths: { + root: pathToFileURL(base), + browser: pathToFileURL(join(base, browser)), + server: ssrOptions ? pathToFileURL(join(base, server)) : undefined, + }, + prerenderedRoutes, + }; + + return JSON.stringify(manifest, undefined, 2); +} + +export async function logMessages( + logger: logging.LoggerApi, + executionResult: ExecutionResult, + color?: boolean, + jsonLogs?: boolean, +): Promise { + const { warnings, errors, logs } = executionResult; - logger.info(JSON.stringify(manifest, undefined, 2)); + if (logs.length) { + logger.info(logs.join('\n')); + } + if (jsonLogs) { return; } - if (warningMessages.length) { - logger.warn(warningMessages.join('\n')); + if (warnings.length) { + logger.warn((await formatMessages(warnings, { kind: 'warning', color })).join('\n')); } - if (errorMessages.length) { - logger.error(errorMessages.join('\n')); + if (errors.length) { + logger.error((await formatMessages(errors, { kind: 'error', color })).join('\n')); } }