fix(@angular/build): scope BundlerContext invalidation and cache one-shot bundle results

Ensure `BundlerContext.invalidate()` only clears `#esbuildResult` when a changed file is in `this.watchFiles` while still invalidating `#loadCache`, preventing edits to one component stylesheet from invalidating all other components sharing the same `MemoryLoadResultCache`.

Author: alan-agius4

packages/angular/build/src/tools/esbuild/bundler-context.ts

@@ -81,6 +81,7 @@ export class BundlerContext {
   #optionsFactory: BundlerOptionsFactory<BuildOptions & { metafile: true; write: false }>;
   #shouldCacheResult: boolean;
   #loadCache?: LoadResultCache;
+  #loadCacheKeys?: Set<string>;
   #invalidationEpoch = 0;
   readonly watchFiles = new Set<string>();
 
@@ -222,16 +223,34 @@ export class BundlerContext {
 
   // eslint-disable-next-line max-lines-per-function
   async #performBundle(): Promise<BundleContextResult> {
+    this.#loadCacheKeys?.clear();
+
     // Create esbuild options if not present
     if (this.#esbuildOptions === undefined) {
       if (this.incremental && !this.#loadCache) {
         this.#loadCache = new MemoryLoadResultCache();
       }
-      this.#esbuildOptions = this.#optionsFactory(this.#loadCache);
-    }
-
-    if (this.incremental) {
-      this.watchFiles.clear();
+      const loadCache = this.#loadCache;
+      const scopedLoadCache: LoadResultCache | undefined =
+        this.incremental && loadCache
+          ? {
+              get: (path) => {
+                (this.#loadCacheKeys ??= new Set()).add(path);
+
+                return loadCache.get(path);
+              },
+              put: (path, result) => {
+                (this.#loadCacheKeys ??= new Set()).add(path);
+
+                return loadCache.put(path, result);
+              },
+              invalidate: (path) => loadCache.invalidate(path),
+              get watchFiles() {
+                return loadCache.watchFiles;
+              },
+            }
+          : loadCache;
+      this.#esbuildOptions = this.#optionsFactory(scopedLoadCache);
     }
 
     let result: BuildResult<{ metafile: true; write: false }>;
@@ -263,7 +282,7 @@ export class BundlerContext {
       // Build failures will throw an exception which contains errors/warnings
       if (isEsBuildFailure(failure)) {
         this.#addErrorsToWatch(failure);
-        this.#addLoadCacheFilesToWatch();
+        await this.#addLoadCacheFilesToWatch();
 
         return failure;
       } else {
@@ -275,6 +294,8 @@ export class BundlerContext {
     // While this should technically not be linked to incremental mode, incremental is only
     // currently enabled with watch mode where watch files are needed.
     if (this.incremental) {
+      this.watchFiles.clear();
+
       // Add input files except virtual angular files which do not exist on disk
       for (const input of Object.keys(result.metafile.inputs)) {
         const isInternal = isInternalAngularFile(input) || isInternalBundlerFile(input);
@@ -309,7 +330,7 @@ export class BundlerContext {
     // Return if the build encountered any errors
     if (result.errors.length) {
       this.#addErrorsToWatch(result);
-      this.#addLoadCacheFilesToWatch();
+      await this.#addLoadCacheFilesToWatch();
 
       return {
         errors: result.errors,
@@ -425,12 +446,26 @@ export class BundlerContext {
   }
 
   #addErrorsToWatch(result: BuildFailure | BuildResult): void {
+    const entryPoints = this.#esbuildOptions?.entryPoints;
+    if (entryPoints) {
+      const entries = Array.isArray(entryPoints)
+        ? entryPoints.map((entry) => (typeof entry === 'string' ? entry : entry.in))
+        : Object.values(entryPoints);
+      for (const entry of entries) {
+        if (isInternalAngularFile(entry) || isInternalBundlerFile(entry)) {
+          continue;
+        }
+
+        this.watchFiles.add(isAbsolute(entry) ? normalize(entry) : join(this.workspaceRoot, entry));
+      }
+    }
+
     for (const error of result.errors) {
       let file = error.location?.file;
       if (file && !isInternalAngularFile(file)) {
         this.watchFiles.add(isAbsolute(file) ? normalize(file) : join(this.workspaceRoot, file));
       }
-      for (const note of error.notes) {
+      for (const note of error.notes ?? []) {
         file = note.location?.file;
         if (file && !isInternalAngularFile(file)) {
           this.watchFiles.add(isAbsolute(file) ? normalize(file) : join(this.workspaceRoot, file));
@@ -439,12 +474,24 @@ export class BundlerContext {
     }
   }
 
-  #addLoadCacheFilesToWatch(): void {
-    if (this.incremental && this.#loadCache) {
-      for (const file of this.#loadCache.watchFiles) {
-        if (!isInternalAngularFile(file)) {
-          this.watchFiles.add(isAbsolute(file) ? normalize(file) : join(this.workspaceRoot, file));
+  async #addLoadCacheFilesToWatch(): Promise<void> {
+    const loadCache = this.#loadCache;
+    if (!this.incremental || !loadCache || !this.#loadCacheKeys) {
+      return;
+    }
+
+    const results = await Promise.all(Array.from(this.#loadCacheKeys, (key) => loadCache.get(key)));
+    for (const cachedResult of results) {
+      if (!cachedResult?.watchFiles) {
+        continue;
+      }
+
+      for (const file of cachedResult.watchFiles) {
+        if (isInternalAngularFile(file)) {
+          continue;
         }
+
+        this.watchFiles.add(isAbsolute(file) ? normalize(file) : join(this.workspaceRoot, file));
       }
     }
   }
@@ -499,30 +546,13 @@ export class BundlerContext {
       candidateFiles = normalizedFiles;
     }
 
-    let invalid = false;
-    for (const file of candidateFiles) {
-      if (this.#loadCache?.invalidate(file)) {
-        invalid = true;
+    if (this.#loadCache) {
+      for (const file of candidateFiles) {
+        this.#loadCache.invalidate(file);
       }
     }
 
-    if (!invalid) {
-      if (this.watchFiles.size < candidateFiles.size) {
-        for (const file of this.watchFiles) {
-          if (candidateFiles.has(file)) {
-            invalid = true;
-            break;
-          }
-        }
-      } else {
-        for (const file of candidateFiles) {
-          if (this.watchFiles.has(file)) {
-            invalid = true;
-            break;
-          }
-        }
-      }
-    }
+    const invalid = !this.watchFiles.isDisjointFrom(candidateFiles);
 
     if (invalid) {
       this.#invalidationEpoch++;
@@ -544,6 +574,7 @@ export class BundlerContext {
       this.#esbuildResult = undefined;
       this.#activeBundlePromise = undefined;
       this.#loadCache = undefined;
+      this.#loadCacheKeys = undefined;
       await this.#esbuildContext?.dispose();
     } finally {
       this.#esbuildContext = undefined;

packages/angular/build/src/tools/esbuild/bundler-context_spec.ts

@@ -6,10 +6,21 @@
  * found in the LICENSE file at https://angular.dev/license
  */
 
+import assert from 'node:assert';
+import { mkdtemp, readFile, writeFile } from 'node:fs/promises';
+import { join } from 'node:path';
 import { BundlerContext } from './bundler-context';
-import { MemoryLoadResultCache } from './load-result-cache';
+import { MemoryLoadResultCache, createCachedLoad } from './load-result-cache';
 
 describe('BundlerContext', () => {
+  let tempRoot: string;
+
+  beforeAll(async () => {
+    const baseTmpDir = process.env['TEST_TMPDIR'];
+    assert(baseTmpDir, 'TEST_TMPDIR is not set');
+    tempRoot = await mkdtemp(join(baseTmpDir, 'angular-cli-bundler-context-test-'));
+  });
+
   describe('invalidate', () => {
     it('should return false when incremental is disabled', () => {
       const context = new BundlerContext('/workspace', /* incremental */ false, () => ({
@@ -83,12 +94,73 @@ describe('BundlerContext', () => {
         /* initialFilter */ undefined,
         loadCache,
       );
+      context.watchFiles.add('/workspace/src/app.css');
 
       expect(loadCache.get('file:/workspace/src/app.css')).toBeDefined();
       expect(context.invalidate(['/workspace/src/app.css'])).toBeTrue();
       expect(loadCache.get('file:/workspace/src/app.css')).toBeUndefined();
     });
 
+    it('should not invalidate unrelated contexts sharing a load cache', async () => {
+      const workspaceDir = await mkdtemp(join(tempRoot, 'shared-cache-'));
+      const compAPath = join(workspaceDir, 'comp-a.css');
+      const compBPath = join(workspaceDir, 'comp-b.css');
+      await writeFile(compAPath, '.a { color: red; }');
+      await writeFile(compBPath, '.b { color: blue; }');
+
+      const sharedCache = new MemoryLoadResultCache();
+      const createCssContext = (entry: string) =>
+        new BundlerContext(
+          workspaceDir,
+          /* incremental */ true,
+          (loadCache) => ({
+            absWorkingDir: workspaceDir,
+            bundle: true,
+            entryPoints: [entry],
+            outdir: workspaceDir,
+            plugins: [
+              {
+                name: 'test-cached-css',
+                setup(build) {
+                  build.onLoad(
+                    { filter: /\.css$/ },
+                    createCachedLoad(loadCache, async (args) => ({
+                      contents: await readFile(args.path, 'utf8'),
+                      loader: 'css',
+                      watchFiles: [args.path],
+                    })),
+                  );
+                },
+              },
+            ],
+          }),
+          /* useContext */ false,
+          /* initialFilter */ undefined,
+          sharedCache,
+        );
+
+      const contextA = createCssContext(compAPath);
+      const contextB = createCssContext(compBPath);
+
+      const resultA1 = await contextA.bundle();
+      const resultB1 = await contextB.bundle();
+      expect(resultA1.errors).toBeUndefined();
+      expect(resultB1.errors).toBeUndefined();
+
+      await writeFile(compAPath, '.a { color: green; }');
+
+      // Even if contextB.invalidate runs first against the shared cache, contextB must not be invalidated
+      expect(contextB.invalidate([compAPath])).toBeFalse();
+      expect(contextA.invalidate([compAPath])).toBeTrue();
+
+      const resultB2 = await contextB.bundle();
+      const resultA2 = await contextA.bundle();
+      expect(resultB2).toBe(resultB1);
+      expect(resultA2).not.toBe(resultA1);
+      assert(!resultA2.errors);
+      expect(resultA2.outputFiles[0].text).toContain('green');
+    });
+
     it('should work when watchFiles is smaller than changed files', () => {
       const context = new BundlerContext('/workspace', /* incremental */ true, () => ({
         entryPoints: ['main.js'],
@@ -110,4 +182,40 @@ describe('BundlerContext', () => {
       expect(context.invalidate(new Set(['/workspace/src/file50.css']))).toBeTrue();
     });
   });
+
+  describe('bundle', () => {
+    it('should recover when fixing a broken @import in plain CSS in watch mode', async () => {
+      const workspaceDir = await mkdtemp(join(tempRoot, 'css-import-recovery-'));
+      const compCssPath = join(workspaceDir, 'comp.css');
+      const subCssPath = join(workspaceDir, 'sub.css');
+      await writeFile(compCssPath, "@import './sub.css';");
+      await writeFile(subCssPath, "@import './non-existent.css';");
+
+      const context = new BundlerContext(
+        workspaceDir,
+        /* incremental */ true,
+        () => ({
+          absWorkingDir: workspaceDir,
+          bundle: true,
+          entryPoints: [compCssPath],
+          outdir: workspaceDir,
+          logLevel: 'silent',
+        }),
+        /* useContext */ false,
+      );
+
+      const failedResult = await context.bundle();
+      expect(failedResult.errors).toBeDefined();
+      expect(context.watchFiles.has(subCssPath)).toBeTrue();
+      expect(context.watchFiles.has(compCssPath)).toBeTrue();
+
+      // Fix sub.css and verify invalidation + recovery
+      await writeFile(subCssPath, '.sub { color: purple; }');
+      expect(context.invalidate([subCssPath])).toBeTrue();
+
+      const recoveredResult = await context.bundle();
+      assert(!recoveredResult.errors);
+      expect(recoveredResult.outputFiles[0].text).toContain('purple');
+    });
+  });
 });