Skip to content

Commit

Permalink
fix(@angular-devkit/build-angular): ensure proper display of build lo…
Browse files Browse the repository at this point in the history
…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 #27233

(cherry picked from commit 518afd8)
  • Loading branch information
alan-agius4 committed Mar 8, 2024
1 parent 5263815 commit 3821344
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 42 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 All @@ -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;
Expand All @@ -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 {
Expand All @@ -68,6 +75,8 @@ export async function* runEsBuildBuildAction(
workspaceRoot,
progress,
preserveSymlinks,
colors,
jsonLogs,
} = options;

if (deleteOutputPath && writeToFileSystem) {
Expand All @@ -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) {
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ export async function executeBuild(
}

if (!jsonLogs) {
context.logger.info(
executionResult.addLog(
logBuildStats(
metafile,
initialFiles,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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;
},
{
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export class ExecutionResult {
errors: (Message | PartialMessage)[] = [];
prerenderedRoutes: string[] = [];
warnings: (Message | PartialMessage)[] = [];
logs: string[] = [];
externalMetadata?: ExternalResultMetadata;

constructor(
Expand All @@ -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 });
Expand Down
67 changes: 37 additions & 30 deletions packages/angular_devkit/build_angular/src/tools/esbuild/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
}
}

0 comments on commit 3821344

Please sign in to comment.