refactor(@angular-devkit/build-angular): improve accuracy of programmatic watch mode usage for esbuild builders

packages/angular_devkit/build_angular/src/builders/application/build-action.ts

@@ -30,6 +30,7 @@ export async function* runEsBuildBuildAction(
     progress?: boolean;
     deleteOutputPath?: boolean;
     poll?: number;
+    signal?: AbortSignal;
   },
 ): AsyncIterable<(ExecutionResult['outputWithFiles'] | ExecutionResult['output']) & BuilderOutput> {
   const {
@@ -75,75 +76,89 @@ export async function* runEsBuildBuildAction(
   let result: ExecutionResult;
   try {
     result = await withProgress('Building...', () => action());
-
-    if (writeToFileSystem) {
-      // Write output files
-      await writeResultFiles(result.outputFiles, result.assetFiles, outputPath);
-
-      yield result.output;
-    } else {
-      // Requires casting due to unneeded `JsonObject` requirement. Remove once fixed.
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      yield result.outputWithFiles as any;
-    }
-
-    // Finish if watch mode is not enabled
-    if (!watch) {
-      return;
-    }
   } finally {
     // Ensure Sass workers are shutdown if not watching
     if (!watch) {
       shutdownSassWorkerPool();
     }
   }
 
-  if (progress) {
-    logger.info('Watch mode enabled. Watching for file changes...');
+  // Setup watcher if watch mode enabled
+  let watcher: import('../../tools/esbuild/watcher').BuildWatcher | undefined;
+  if (watch) {
+    if (progress) {
+      logger.info('Watch mode enabled. Watching for file changes...');
+    }
+
+    // Setup a watcher
+    const { createWatcher } = await import('../../tools/esbuild/watcher');
+    watcher = createWatcher({
+      polling: typeof poll === 'number',
+      interval: poll,
+      ignored: [
+        // Ignore the output and cache paths to avoid infinite rebuild cycles
+        outputPath,
+        cacheOptions.basePath,
+        // Ignore all node modules directories to avoid excessive file watchers.
+        // Package changes are handled below by watching manifest and lock files.
+        '**/node_modules/**',
+        '**/.*/**',
+      ],
+    });
+
+    // Setup abort support
+    options.signal?.addEventListener('abort', () => void watcher?.close());
+
+    // Temporarily watch the entire project
+    watcher.add(projectRoot);
+
+    // Watch workspace for package manager changes
+    const packageWatchFiles = [
+      // manifest can affect module resolution
+      'package.json',
+      // npm lock file
+      'package-lock.json',
+      // pnpm lock file
+      'pnpm-lock.yaml',
+      // yarn lock file including Yarn PnP manifest files (https://yarnpkg.com/advanced/pnp-spec/)
+      'yarn.lock',
+      '.pnp.cjs',
+      '.pnp.data.json',
+    ];
+
+    watcher.add(packageWatchFiles.map((file) => path.join(workspaceRoot, file)));
+
+    // Watch locations provided by the initial build result
+    watcher.add(result.watchFiles);
   }
 
-  // Setup a watcher
-  const { createWatcher } = await import('../../tools/esbuild/watcher');
-  const watcher = createWatcher({
-    polling: typeof poll === 'number',
-    interval: poll,
-    ignored: [
-      // Ignore the output and cache paths to avoid infinite rebuild cycles
-      outputPath,
-      cacheOptions.basePath,
-      // Ignore all node modules directories to avoid excessive file watchers.
-      // Package changes are handled below by watching manifest and lock files.
-      '**/node_modules/**',
-      '**/.*/**',
-    ],
-  });
-
-  // Temporarily watch the entire project
-  watcher.add(projectRoot);
-
-  // Watch workspace for package manager changes
-  const packageWatchFiles = [
-    // manifest can affect module resolution
-    'package.json',
-    // npm lock file
-    'package-lock.json',
-    // pnpm lock file
-    'pnpm-lock.yaml',
-    // yarn lock file including Yarn PnP manifest files (https://yarnpkg.com/advanced/pnp-spec/)
-    'yarn.lock',
-    '.pnp.cjs',
-    '.pnp.data.json',
-  ];
-
-  watcher.add(packageWatchFiles.map((file) => path.join(workspaceRoot, file)));
-
-  // Watch locations provided by the initial build result
-  let previousWatchFiles = new Set(result.watchFiles);
-  watcher.add(result.watchFiles);
+  // 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.
+  if (writeToFileSystem) {
+    // Write output files
+    await writeResultFiles(result.outputFiles, result.assetFiles, outputPath);
+
+    yield result.output;
+  } else {
+    // Requires casting due to unneeded `JsonObject` requirement. Remove once fixed.
+    // eslint-disable-next-line @typescript-eslint/no-explicit-any
+    yield result.outputWithFiles as any;
+  }
+
+  // Finish if watch mode is not enabled
+  if (!watcher) {
+    return;
+  }
 
   // Wait for changes and rebuild as needed
+  let previousWatchFiles = new Set(result.watchFiles);
   try {
     for await (const changes of watcher) {
+      if (options.signal?.aborted) {
+        break;
+      }
+
       if (verbose) {
         logger.info(changes.toDebugString());
       }

packages/angular_devkit/build_angular/src/builders/application/index.ts

@@ -17,7 +17,8 @@ import { Schema as ApplicationBuilderOptions } from './schema';
 
 export async function* buildApplicationInternal(
   options: ApplicationBuilderInternalOptions,
-  context: BuilderContext,
+  // TODO: Integrate abort signal support into builder system
+  context: BuilderContext & { signal?: AbortSignal },
   infrastructureSettings?: {
     write?: boolean;
   },
@@ -73,6 +74,7 @@ export async function* buildApplicationInternal(
       progress: normalizedOptions.progress,
       writeToFileSystem: infrastructureSettings?.write,
       logger: context.logger,
+      signal: context.signal,
     },
   );
 }

packages/angular_devkit/build_angular/src/builders/application/tests/options/inline-style-language_spec.ts

@@ -75,7 +75,7 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
           harness.expectFile('dist/main.js').content.toContain('color: indianred');
         });
 
-        xit('updates produced stylesheet in watch mode', async () => {
+        it('updates produced stylesheet in watch mode', async () => {
           harness.useTarget('build', {
             ...BASE_OPTIONS,
             inlineStyleLanguage: InlineStyleLanguage.Scss,
@@ -87,8 +87,9 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
             content.replace('__STYLE_MARKER__', '$primary: indianred;\\nh1 { color: $primary; }'),
           );
 
+          const builderAbort = new AbortController();
           const buildCount = await harness
-            .execute()
+            .execute({ signal: builderAbort.signal })
             .pipe(
               timeout(30000),
               concatMap(async ({ result }, index) => {
@@ -121,10 +122,12 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
                     harness.expectFile('dist/main.js').content.not.toContain('color: indianred');
                     harness.expectFile('dist/main.js').content.not.toContain('color: aqua');
                     harness.expectFile('dist/main.js').content.toContain('color: blue');
+
+                    // Test complete - abort watch mode
+                    builderAbort.abort();
                     break;
                 }
               }),
-              take(3),
               count(),
             )
             .toPromise();

packages/angular_devkit/build_angular/src/builders/application/tests/setup.ts

@@ -28,4 +28,8 @@ export const BASE_OPTIONS = Object.freeze<Schema>({
 
   // Disable optimizations
   optimization: false,
+
+  // Enable polling (if a test enables watch mode).
+  // This is a workaround for bazel isolation file watch not triggering in tests.
+  poll: 100,
 });

packages/angular_devkit/build_angular/src/testing/builder-harness.ts

@@ -51,6 +51,7 @@ export interface BuilderHarnessExecutionOptions {
   outputLogsOnFailure: boolean;
   outputLogsOnException: boolean;
   useNativeFileWatching: boolean;
+  signal: AbortSignal;
 }
 
 /**
@@ -235,6 +236,7 @@ export class BuilderHarness<T> {
       this.builderInfo,
       this.resolvePath('.'),
       contextHost,
+      options.signal,
       useNativeFileWatching ? undefined : this.watcherNotifier,
     );
     if (this.targetName !== undefined) {
@@ -389,6 +391,7 @@ class HarnessBuilderContext implements BuilderContext {
     public builder: BuilderInfo,
     basePath: string,
     private readonly contextHost: ContextHost,
+    public readonly signal: AbortSignal | undefined,
     public readonly watcherFactory: BuilderWatcherFactory | undefined,
   ) {
     this.workspaceRoot = this.currentDirectory = basePath;
@@ -442,6 +445,7 @@ class HarnessBuilderContext implements BuilderContext {
       info,
       this.workspaceRoot,
       this.contextHost,
+      this.signal,
       this.watcherFactory,
     );
     context.target = target;