fix(@angular-devkit/build-angular): ensure proper display of build lo…

…gs 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 angular#27233
alan-agius4 · Mar 8, 2024 · 0c9cb14 · 0c9cb14
1 parent b6986a7
commit 0c9cb14
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 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
@@ -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<ExecutionResult>,
+  action: (rebuildState?: RebuildState) => Promise<ExecutionResult>,
   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
@@ -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
@@ -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
@@ -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
@@ -429,46 +429,53 @@ interface BuildManifest {
   prerenderedRoutes?: string[];
 }
 
-export async function logMessages(
-  logger: logging.LoggerApi,
-  executionResult: ExecutionResult,
-  options: NormalizedApplicationBuildOptions,
-): Promise<void> {
+export async function createJsonBuildManifest(
+  result: ExecutionResult,
+  normalizedOptions: NormalizedApplicationBuildOptions,
+): Promise<string> {
   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<void> {
+  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'));
   }
 }