refactor(@angular/build): provide structured application builder resu…

…lt types The application builder now provides structured output types to its internal consumers. The architect builders themselves and the programmatic API is not changed. These output result types allow for the development server to receive additional information regarding the build and update the active browser appropriately. This functionality is not yet implemented but the additional result types provide the base infrastructure to enable future features. The result types also allow for reduced complexity inside other builders such as i18n extraction and the browser compatibility builder. The usage is not yet fully optimized and will be refined in future changes.
angular · Jul 19, 2024 · 421b6e7 · 421b6e7
1 parent 37a2138
commit 421b6e7
Show file tree

Hide file tree

Showing 18 changed files with 368 additions and 247 deletions.
diff --git a/packages/angular/build/src/builders/application/build-action.ts b/packages/angular/build/src/builders/application/build-action.ts
@@ -6,10 +6,10 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
-import { BuilderContext, BuilderOutput } from '@angular-devkit/architect';
+import { BuilderContext } from '@angular-devkit/architect';
 import { existsSync } from 'node:fs';
 import path from 'node:path';
-import { BuildOutputFile } from '../../tools/esbuild/bundler-context';
+import { BuildOutputFile, BuildOutputFileType } from '../../tools/esbuild/bundler-context';
 import { ExecutionResult, RebuildState } from '../../tools/esbuild/bundler-execution-result';
 import { shutdownSassWorkerPool } from '../../tools/esbuild/stylesheets/sass-language';
 import {
@@ -22,6 +22,7 @@ import { deleteOutputDir } from '../../utils/delete-output-dir';
 import { shouldWatchRoot } from '../../utils/environment-options';
 import { NormalizedCachedOptions } from '../../utils/normalize-cache';
 import { NormalizedApplicationBuildOptions, NormalizedOutputOptions } from './options';
+import { FullResult, Result, ResultKind, ResultMessage } from './results';
 
 // Watch workspace for package manager changes
 const packageWatchFiles = [
@@ -37,9 +38,6 @@ const packageWatchFiles = [
   '.pnp.data.json',
 ];
 
-type BuildActionOutput = (ExecutionResult['outputWithFiles'] | ExecutionResult['output']) &
-  BuilderOutput;
-
 export async function* runEsBuildBuildAction(
   action: (rebuildState?: RebuildState) => Promise<ExecutionResult>,
   options: {
@@ -61,7 +59,7 @@ export async function* runEsBuildBuildAction(
     colors?: boolean;
     jsonLogs?: boolean;
   },
-): AsyncIterable<BuildActionOutput> {
+): AsyncIterable<Result> {
   const {
     writeToFileSystemFilter,
     writeToFileSystem,
@@ -226,10 +224,24 @@ export async function* runEsBuildBuildAction(
 
 async function writeAndEmitOutput(
   writeToFileSystem: boolean,
-  { outputFiles, output, outputWithFiles, assetFiles }: ExecutionResult,
+  {
+    outputFiles,
+    outputWithFiles,
+    assetFiles,
+    externalMetadata,
+    htmlIndexPath,
+    htmlBaseHref,
+  }: ExecutionResult,
   outputOptions: NormalizedApplicationBuildOptions['outputOptions'],
   writeToFileSystemFilter: ((file: BuildOutputFile) => boolean) | undefined,
-): Promise<BuildActionOutput> {
+): Promise<Result> {
+  if (!outputWithFiles.success) {
+    return {
+      kind: ResultKind.Failure,
+      errors: outputWithFiles.errors as ResultMessage[],
+    };
+  }
+
   if (writeToFileSystem) {
     // Write output files
     const outputFilesToWrite = writeToFileSystemFilter
@@ -238,10 +250,37 @@ async function writeAndEmitOutput(
 
     await writeResultFiles(outputFilesToWrite, assetFiles, outputOptions);
 
-    return output;
+    // Currently unused other than indicating success if writing to disk.
+    return {
+      kind: ResultKind.Full,
+      files: {},
+    };
   } else {
-    // Requires casting due to unneeded `JsonObject` requirement. Remove once fixed.
-    // eslint-disable-next-line @typescript-eslint/no-explicit-any
-    return outputWithFiles as any;
+    const result: FullResult = {
+      kind: ResultKind.Full,
+      files: {},
+      detail: {
+        externalMetadata,
+        htmlIndexPath,
+        htmlBaseHref,
+      },
+    };
+    for (const file of outputWithFiles.assetFiles) {
+      result.files[file.destination] = {
+        type: BuildOutputFileType.Browser,
+        inputPath: file.source,
+        origin: 'disk',
+      };
+    }
+    for (const file of outputWithFiles.outputFiles) {
+      result.files[file.path] = {
+        type: file.type,
+        contents: file.contents,
+        origin: 'memory',
+        hash: file.hash,
+      };
+    }
+
+    return result;
   }
 }
diff --git a/packages/angular/build/src/builders/application/execute-build.ts b/packages/angular/build/src/builders/application/execute-build.ts
@@ -156,6 +156,8 @@ export async function executeBuild(
   // Watch input index HTML file if configured
   if (options.indexHtmlOptions) {
     executionResult.extraWatchFiles.push(options.indexHtmlOptions.input);
+    executionResult.htmlIndexPath = options.indexHtmlOptions.output;
+    executionResult.htmlBaseHref = options.baseHref;
   }
 
   // Perform i18n translation inlining if enabled

diff --git a/packages/angular/build/src/builders/application/index.ts b/packages/angular/build/src/builders/application/index.ts
@@ -20,6 +20,7 @@ import {
   ApplicationBuilderInternalOptions,
   normalizeOptions,
 } from './options';
+import { Result, ResultKind } from './results';
 import { Schema as ApplicationBuilderOptions } from './schema';
 
 export type { ApplicationBuilderOptions };
@@ -32,7 +33,7 @@ export async function* buildApplicationInternal(
     write?: boolean;
   },
   extensions?: ApplicationBuilderExtensions,
-): AsyncIterable<ApplicationBuilderOutput> {
+): AsyncIterable<Result> {
   const { workspaceRoot, logger, target } = context;
 
   // Check Angular version.
@@ -44,7 +45,9 @@ export async function* buildApplicationInternal(
   // Determine project name from builder context target
   const projectName = target?.project;
   if (!projectName) {
-    yield { success: false, error: `The 'application' builder requires a target to be specified.` };
+    context.logger.error(`The 'application' builder requires a target to be specified.`);
+    // Only the vite-based dev server current uses the errors value
+    yield { kind: ResultKind.Failure, errors: [] };
 
     return;
   }
@@ -57,19 +60,19 @@ export async function* buildApplicationInternal(
   if (writeServerBundles) {
     const { browser, server } = normalizedOptions.outputOptions;
     if (browser === '') {
-      yield {
-        success: false,
-        error: `'outputPath.browser' cannot be configured to an empty string when SSR is enabled.`,
-      };
+      context.logger.error(
+        `'outputPath.browser' cannot be configured to an empty string when SSR is enabled.`,
+      );
+      yield { kind: ResultKind.Failure, errors: [] };
 
       return;
     }
 
     if (browser === server) {
-      yield {
-        success: false,
-        error: `'outputPath.browser' and 'outputPath.server' cannot be configured to the same value.`,
-      };
+      context.logger.error(
+        `'outputPath.browser' and 'outputPath.server' cannot be configured to the same value.`,
+      );
+      yield { kind: ResultKind.Failure, errors: [] };
 
       return;
     }
@@ -185,7 +188,7 @@ export function buildApplication(
   extensions?: ApplicationBuilderExtensions,
 ): AsyncIterable<ApplicationBuilderOutput>;
 
-export function buildApplication(
+export async function* buildApplication(
   options: ApplicationBuilderOptions,
   context: BuilderContext,
   pluginsOrExtensions?: Plugin[] | ApplicationBuilderExtensions,
@@ -199,7 +202,9 @@ export function buildApplication(
     extensions = pluginsOrExtensions;
   }
 
-  return buildApplicationInternal(options, context, undefined, extensions);
+  for await (const result of buildApplicationInternal(options, context, undefined, extensions)) {
+    yield { success: result.kind !== ResultKind.Failure };
+  }
 }
 
 export default createBuilder(buildApplication);
diff --git a/packages/angular/build/src/builders/application/results.ts b/packages/angular/build/src/builders/application/results.ts
@@ -0,0 +1,74 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.dev/license
+ */
+
+import { BuildOutputFileType } from '../../tools/esbuild/bundler-context';
+
+export enum ResultKind {
+  Failure,
+  Full,
+  Incremental,
+  ComponentUpdate,
+}
+
+export type Result = FailureResult | FullResult | IncrementalResult | ComponentUpdateResult;
+
+export interface BaseResult {
+  kind: ResultKind;
+  warnings?: ResultMessage[];
+  duration?: number;
+  detail?: Record<string, unknown>;
+}
+
+export interface FailureResult extends BaseResult {
+  kind: ResultKind.Failure;
+  errors: ResultMessage[];
+}
+
+export interface FullResult extends BaseResult {
+  kind: ResultKind.Full;
+  files: Record<string, ResultFile>;
+}
+
+export interface IncrementalResult extends BaseResult {
+  kind: ResultKind.Incremental;
+  added: string[];
+  removed: string[];
+  modified: string[];
+  files: Record<string, ResultFile>;
+}
+
+export type ResultFile = DiskFile | MemoryFile;
+
+export interface BaseResultFile {
+  origin: 'memory' | 'disk';
+  type: BuildOutputFileType;
+}
+
+export interface DiskFile extends BaseResultFile {
+  origin: 'disk';
+  inputPath: string;
+}
+
+export interface MemoryFile extends BaseResultFile {
+  origin: 'memory';
+  hash: string;
+  contents: Uint8Array;
+}
+
+export interface ResultMessage {
+  text: string;
+  location?: { file: string; line: number; column: number } | null;
+  notes?: { text: string }[];
+}
+
+export interface ComponentUpdateResult extends BaseResult {
+  kind: ResultKind.ComponentUpdate;
+  id: string;
+  type: 'style' | 'template';
+  content: string;
+}
diff --git a/packages/angular/build/src/builders/application/tests/options/output-path_spec.ts b/packages/angular/build/src/builders/application/tests/options/output-path_spec.ts
@@ -286,10 +286,14 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
             },
           });
 
-          const { result } = await harness.executeOnce();
+          const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false });
           expect(result?.success).toBeFalse();
-          expect(result?.error).toContain(
-            `'outputPath.browser' cannot be configured to an empty string when SSR is enabled`,
+          expect(logs).toContain(
+            jasmine.objectContaining({
+              message: jasmine.stringMatching(
+                `'outputPath.browser' cannot be configured to an empty string when SSR is enabled`,
+              ),
+            }),
           );
         });
       });
@@ -349,10 +353,14 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
           },
         });
 
-        const { result } = await harness.executeOnce();
+        const { result, logs } = await harness.executeOnce({ outputLogsOnFailure: false });
         expect(result?.success).toBeFalse();
-        expect(result?.error).toContain(
-          `'outputPath.browser' and 'outputPath.server' cannot be configured to the same value`,
+        expect(logs).toContain(
+          jasmine.objectContaining({
+            message: jasmine.stringMatching(
+              `'outputPath.browser' and 'outputPath.server' cannot be configured to the same value`,
+            ),
+          }),
         );
       });
     });