refactor(@angular/build): avoid write file logic for internal application build action

The internal "buildApplicationInternal" function is only used by several consumers that
require writing the output files to disk. One, `browser-esbuild`, directly writes
to the disk. The two experimental unit test builders also have unique requirements
and also directly write. This leaves only the main `application` builder that relies on the
internal file writing functionality of `buildApplicationInternal`. To avoid unneeded
logic for the other usages (`dev-server`, `extract-i18n`, unit testing, etc.), the
disk writing logic is now elevated to the `application` build itself. The internal
function will now always provide the output files within the result objects generated
from a successful build. This also removes the need for the other usages to specify
that files should not be written to disk.

Author: clydin

packages/angular/build/src/builders/application/build-action.ts

@@ -9,16 +9,10 @@
 import { BuilderContext } from '@angular-devkit/architect';
 import { existsSync } from 'node:fs';
 import path from 'node:path';
-import { BuildOutputFile, BuildOutputFileType } from '../../tools/esbuild/bundler-context';
+import { BuildOutputFileType } from '../../tools/esbuild/bundler-context';
 import { ExecutionResult, RebuildState } from '../../tools/esbuild/bundler-execution-result';
 import { shutdownSassWorkerPool } from '../../tools/esbuild/stylesheets/sass-language';
-import {
-  logMessages,
-  withNoProgress,
-  withSpinner,
-  writeResultFiles,
-} from '../../tools/esbuild/utils';
-import { deleteOutputDir } from '../../utils/delete-output-dir';
+import { logMessages, withNoProgress, withSpinner } from '../../tools/esbuild/utils';
 import { shouldWatchRoot } from '../../utils/environment-options';
 import { NormalizedCachedOptions } from '../../utils/normalize-cache';
 import { NormalizedApplicationBuildOptions, NormalizedOutputOptions } from './options';
@@ -46,12 +40,9 @@ export async function* runEsBuildBuildAction(
     outputOptions: NormalizedOutputOptions;
     logger: BuilderContext['logger'];
     cacheOptions: NormalizedCachedOptions;
-    writeToFileSystem: boolean;
-    writeToFileSystemFilter: ((file: BuildOutputFile) => boolean) | undefined;
     watch?: boolean;
     verbose?: boolean;
     progress?: boolean;
-    deleteOutputPath?: boolean;
     poll?: number;
     signal?: AbortSignal;
     preserveSymlinks?: boolean;
@@ -61,13 +52,10 @@ export async function* runEsBuildBuildAction(
   },
 ): AsyncIterable<Result> {
   const {
-    writeToFileSystemFilter,
-    writeToFileSystem,
     watch,
     poll,
     clearScreen,
     logger,
-    deleteOutputPath,
     cacheOptions,
     outputOptions,
     verbose,
@@ -79,13 +67,6 @@ export async function* runEsBuildBuildAction(
     jsonLogs,
   } = options;
 
-  if (deleteOutputPath && writeToFileSystem) {
-    await deleteOutputDir(workspaceRoot, outputOptions.base, [
-      outputOptions.browser,
-      outputOptions.server,
-    ]);
-  }
-
   const withProgress: typeof withSpinner = progress ? withSpinner : withNoProgress;
 
   // Initial build
@@ -154,7 +135,7 @@ export async function* runEsBuildBuildAction(
   // Output the first build results after setting up the watcher to ensure that any code executed
   // higher in the iterator call stack will trigger the watcher. This is particularly relevant for
   // unit tests which execute the builder and modify the file system programmatically.
-  yield await writeAndEmitOutput(writeToFileSystem, result, outputOptions, writeToFileSystemFilter);
+  yield await emitOutputResult(result, outputOptions);
 
   // Finish if watch mode is not enabled
   if (!watcher) {
@@ -207,12 +188,7 @@ export async function* runEsBuildBuildAction(
         watcher.remove([...staleWatchFiles]);
       }
 
-      yield await writeAndEmitOutput(
-        writeToFileSystem,
-        result,
-        outputOptions,
-        writeToFileSystemFilter,
-      );
+      yield await emitOutputResult(result, outputOptions);
     }
   } finally {
     // Stop the watcher and cleanup incremental rebuild state
@@ -222,65 +198,55 @@ export async function* runEsBuildBuildAction(
   }
 }
 
-async function writeAndEmitOutput(
-  writeToFileSystem: boolean,
+async function emitOutputResult(
   {
     outputFiles,
-    outputWithFiles,
     assetFiles,
+    errors,
+    warnings,
     externalMetadata,
     htmlIndexPath,
     htmlBaseHref,
   }: ExecutionResult,
   outputOptions: NormalizedApplicationBuildOptions['outputOptions'],
-  writeToFileSystemFilter: ((file: BuildOutputFile) => boolean) | undefined,
 ): Promise<Result> {
-  if (!outputWithFiles.success) {
+  if (errors.length > 0) {
     return {
       kind: ResultKind.Failure,
-      errors: outputWithFiles.errors as ResultMessage[],
+      errors: errors as ResultMessage[],
+      warnings: warnings as ResultMessage[],
+      detail: {
+        outputOptions,
+      },
     };
   }
 
-  if (writeToFileSystem) {
-    // Write output files
-    const outputFilesToWrite = writeToFileSystemFilter
-      ? outputFiles.filter(writeToFileSystemFilter)
-      : outputFiles;
-
-    await writeResultFiles(outputFilesToWrite, assetFiles, outputOptions);
-
-    // Currently unused other than indicating success if writing to disk.
-    return {
-      kind: ResultKind.Full,
-      files: {},
+  const result: FullResult = {
+    kind: ResultKind.Full,
+    warnings: warnings as ResultMessage[],
+    files: {},
+    detail: {
+      externalMetadata,
+      htmlIndexPath,
+      htmlBaseHref,
+      outputOptions,
+    },
+  };
+  for (const file of assetFiles) {
+    result.files[file.destination] = {
+      type: BuildOutputFileType.Browser,
+      inputPath: file.source,
+      origin: 'disk',
     };
-  } else {
-    const result: FullResult = {
-      kind: ResultKind.Full,
-      files: {},
-      detail: {
-        externalMetadata,
-        htmlIndexPath,
-        htmlBaseHref,
-      },
+  }
+  for (const file of outputFiles) {
+    result.files[file.path] = {
+      type: file.type,
+      contents: file.contents,
+      origin: 'memory',
+      hash: file.hash,
     };
-    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;
   }
+
+  return result;
 }

packages/angular/build/src/builders/application/index.ts

@@ -8,16 +8,22 @@
 
 import { BuilderContext, BuilderOutput, createBuilder } from '@angular-devkit/architect';
 import type { Plugin } from 'esbuild';
+import assert from 'node:assert';
+import fs from 'node:fs/promises';
+import path from 'node:path';
 import { BuildOutputFile, BuildOutputFileType } from '../../tools/esbuild/bundler-context';
-import { createJsonBuildManifest } from '../../tools/esbuild/utils';
+import { createJsonBuildManifest, emitFilesToDisk } from '../../tools/esbuild/utils';
 import { colors as ansiColors } from '../../utils/color';
+import { deleteOutputDir } from '../../utils/delete-output-dir';
+import { useJSONBuildLogs } from '../../utils/environment-options';
 import { purgeStaleBuildCache } from '../../utils/purge-cache';
 import { assertCompatibleAngularVersion } from '../../utils/version';
 import { runEsBuildBuildAction } from './build-action';
 import { executeBuild } from './execute-build';
 import {
   ApplicationBuilderExtensions,
   ApplicationBuilderInternalOptions,
+  NormalizedOutputOptions,
   normalizeOptions,
 } from './options';
 import { Result, ResultKind } from './results';
@@ -29,9 +35,6 @@ export async function* buildApplicationInternal(
   options: ApplicationBuilderInternalOptions,
   // TODO: Integrate abort signal support into builder system
   context: BuilderContext & { signal?: AbortSignal },
-  infrastructureSettings?: {
-    write?: boolean;
-  },
   extensions?: ApplicationBuilderExtensions,
 ): AsyncIterable<Result> {
   const { workspaceRoot, logger, target } = context;
@@ -53,11 +56,8 @@ export async function* buildApplicationInternal(
   }
 
   const normalizedOptions = await normalizeOptions(context, projectName, options, extensions);
-  const writeToFileSystem = infrastructureSettings?.write ?? true;
-  const writeServerBundles =
-    writeToFileSystem && !!(normalizedOptions.ssrOptions && normalizedOptions.serverEntryPoint);
 
-  if (writeServerBundles) {
+  if (!normalizedOptions.outputOptions.ignoreServer) {
     const { browser, server } = normalizedOptions.outputOptions;
     if (browser === '') {
       context.logger.error(
@@ -88,7 +88,7 @@ export async function* buildApplicationInternal(
 
   yield* runEsBuildBuildAction(
     async (rebuildState) => {
-      const { prerenderOptions, outputOptions, jsonLogs } = normalizedOptions;
+      const { prerenderOptions, jsonLogs } = normalizedOptions;
 
       const startTime = process.hrtime.bigint();
       const result = await executeBuild(normalizedOptions, context, rebuildState);
@@ -106,9 +106,6 @@ export async function* buildApplicationInternal(
 
         const buildTime = Number(process.hrtime.bigint() - startTime) / 10 ** 9;
         const hasError = result.errors.length > 0;
-        if (writeToFileSystem && !hasError) {
-          result.addLog(`Output location: ${outputOptions.base}\n`);
-        }
 
         result.addLog(
           `Application bundle generation ${hasError ? 'failed' : 'complete'}. [${buildTime.toFixed(3)} seconds]\n`,
@@ -121,7 +118,6 @@ export async function* buildApplicationInternal(
       watch: normalizedOptions.watch,
       preserveSymlinks: normalizedOptions.preserveSymlinks,
       poll: normalizedOptions.poll,
-      deleteOutputPath: normalizedOptions.deleteOutputPath,
       cacheOptions: normalizedOptions.cacheOptions,
       outputOptions: normalizedOptions.outputOptions,
       verbose: normalizedOptions.verbose,
@@ -131,12 +127,6 @@ export async function* buildApplicationInternal(
       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.
-      writeToFileSystemFilter: writeServerBundles
-        ? undefined
-        : (file) => file.type !== BuildOutputFileType.Server,
       logger,
       signal,
     },
@@ -202,8 +192,80 @@ export async function* buildApplication(
     extensions = pluginsOrExtensions;
   }
 
-  for await (const result of buildApplicationInternal(options, context, undefined, extensions)) {
-    yield { success: result.kind !== ResultKind.Failure };
+  let initial = true;
+  for await (const result of buildApplicationInternal(options, context, extensions)) {
+    const outputOptions = result.detail?.['outputOptions'] as NormalizedOutputOptions | undefined;
+
+    if (initial) {
+      initial = false;
+
+      // Clean the output location if requested.
+      // Output options may not be present if the build failed.
+      if (outputOptions?.clean) {
+        await deleteOutputDir(context.workspaceRoot, outputOptions.base, [
+          outputOptions.browser,
+          outputOptions.server,
+        ]);
+      }
+    }
+
+    if (result.kind === ResultKind.Failure) {
+      yield { success: false };
+      continue;
+    }
+
+    assert(outputOptions, 'Application output options are required for builder usage.');
+    assert(result.kind === ResultKind.Full, 'Application build did not provide a full output.');
+
+    // TODO: Restructure output logging to better handle stdout JSON piping
+    if (!useJSONBuildLogs) {
+      context.logger.info(`Output location: ${outputOptions.base}\n`);
+    }
+
+    // Writes the output files to disk and ensures the containing directories are present
+    const directoryExists = new Set<string>();
+    await emitFilesToDisk(Object.entries(result.files), async ([filePath, file]) => {
+      if (outputOptions.ignoreServer && file.type === BuildOutputFileType.Server) {
+        return;
+      }
+
+      let typeDirectory: string;
+      switch (file.type) {
+        case BuildOutputFileType.Browser:
+        case BuildOutputFileType.Media:
+          typeDirectory = outputOptions.browser;
+          break;
+        case BuildOutputFileType.Server:
+          typeDirectory = outputOptions.server;
+          break;
+        case BuildOutputFileType.Root:
+          typeDirectory = '';
+          break;
+        default:
+          throw new Error(
+            `Unhandled write for file "${filePath}" with type "${BuildOutputFileType[file.type]}".`,
+          );
+      }
+      // NOTE: 'base' is a fully resolved path at this point
+      const fullFilePath = path.join(outputOptions.base, typeDirectory, filePath);
+
+      // Ensure output subdirectories exist
+      const fileBasePath = path.dirname(fullFilePath);
+      if (fileBasePath && !directoryExists.has(fileBasePath)) {
+        await fs.mkdir(fileBasePath, { recursive: true });
+        directoryExists.add(fileBasePath);
+      }
+
+      if (file.origin === 'memory') {
+        // Write file contents
+        await fs.writeFile(fullFilePath, file.contents);
+      } else {
+        // Copy file contents
+        await fs.copyFile(file.inputPath, fullFilePath, fs.constants.COPYFILE_FICLONE);
+      }
+    });
+
+    yield { success: true };
   }
 }