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`.

Also scope `#addLoadCacheFilesToWatch()` to the load cache keys accessed by the current `BundlerContext`, retain `watchFiles` across failed rebuilds while tracking entry points and unresolved relative imports on build failures, and cache `#esbuildResult` in non-incremental builds so multiple components sharing the same `styleUrl` do not re-run esbuild.

Author: alan-agius4

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

@@ -17,7 +17,7 @@ import {
   context,
 } from 'esbuild';
 import assert from 'node:assert';
-import { basename, extname, isAbsolute, join, normalize, relative } from 'node:path';
+import { basename, dirname, extname, isAbsolute, join, normalize, relative } from 'node:path';
 import { SERVER_GENERATED_EXTERNALS } from '../../utils/server-rendering/manifest';
 import {
   type BuildOutputFile,
@@ -81,6 +81,7 @@ export class BundlerContext {
   #optionsFactory: BundlerOptionsFactory<BuildOptions & { metafile: true; write: false }>;
   #shouldCacheResult: boolean;
   #loadCache?: LoadResultCache;
+  readonly #loadCacheKeys = new Set<string>();
   #invalidationEpoch = 0;
   readonly watchFiles = new Set<string>();
 
@@ -93,8 +94,9 @@ export class BundlerContext {
     sharedLoadCache?: LoadResultCache,
   ) {
     this.#loadCache = sharedLoadCache;
-    // To cache the results an option factory is needed to capture the full set of dependencies
-    this.#shouldCacheResult = incremental && typeof options === 'function';
+    // In incremental mode, an option factory is needed to capture the full set of dependencies.
+    // In non-incremental mode, results can always be cached for the duration of the build.
+    this.#shouldCacheResult = !incremental || typeof options === 'function';
     this.#optionsFactory = (...args) => {
       const baseOptions = typeof options === 'function' ? options(...args) : options;
 
@@ -227,11 +229,24 @@ export class BundlerContext {
       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 = loadCache && {
+        get: (path) => {
+          this.#loadCacheKeys.add(path);
+
+          return loadCache.get(path);
+        },
+        put: (path, result) => {
+          this.#loadCacheKeys.add(path);
+
+          return loadCache.put(path, result);
+        },
+        invalidate: (path) => loadCache.invalidate(path),
+        get watchFiles() {
+          return loadCache.watchFiles;
+        },
+      };
+      this.#esbuildOptions = this.#optionsFactory(scopedLoadCache);
     }
 
     let result: BuildResult<{ metafile: true; write: false }>;
@@ -263,7 +278,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 +290,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 +326,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 +442,31 @@ 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)) {
+          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));
+        const errorFile = isAbsolute(file) ? normalize(file) : join(this.workspaceRoot, file);
+        this.watchFiles.add(errorFile);
+        const match = /^Could not resolve "(\.\.?\/[^"]+)"/.exec(error.text);
+        if (match) {
+          this.watchFiles.add(join(dirname(errorFile), match[1]));
+        }
       }
-      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,11 +475,18 @@ export class BundlerContext {
     }
   }
 
-  #addLoadCacheFilesToWatch(): void {
+  async #addLoadCacheFilesToWatch(): Promise<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));
+      for (const key of this.#loadCacheKeys) {
+        const cachedResult = await this.#loadCache.get(key);
+        if (cachedResult?.watchFiles) {
+          for (const file of cachedResult.watchFiles) {
+            if (!isInternalAngularFile(file)) {
+              this.watchFiles.add(
+                isAbsolute(file) ? normalize(file) : join(this.workspaceRoot, file),
+              );
+            }
+          }
         }
       }
     }
@@ -499,27 +542,25 @@ 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;
-          }
+    let invalid = false;
+    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;
-          }
+      }
+    } else {
+      for (const file of candidateFiles) {
+        if (this.watchFiles.has(file)) {
+          invalid = true;
+          break;
         }
       }
     }
@@ -544,6 +585,7 @@ export class BundlerContext {
       this.#esbuildResult = undefined;
       this.#activeBundlePromise = undefined;
       this.#loadCache = undefined;
+      this.#loadCacheKeys.clear();
       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, 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,76 @@ 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 import('node:fs/promises').then((f) =>
+                        f.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);
+      if (!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 +185,80 @@ 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();
+      expect(recoveredResult.errors).toBeUndefined();
+      if (!recoveredResult.errors) {
+        expect(recoveredResult.outputFiles[0].text).toContain('purple');
+      }
+    });
+
+    it('should reuse cached result when calling bundle() twice on a non-incremental context', async () => {
+      const workspaceDir = await mkdtemp(join(tempRoot, 'non-incremental-cache-'));
+      const stylePath = join(workspaceDir, 'shared.css');
+      await writeFile(stylePath, '.shared { color: orange; }');
+
+      let loadCount = 0;
+      const context = new BundlerContext(
+        workspaceDir,
+        /* incremental */ false,
+        () => ({
+          absWorkingDir: workspaceDir,
+          bundle: true,
+          entryPoints: [stylePath],
+          outdir: workspaceDir,
+          plugins: [
+            {
+              name: 'count-loads',
+              setup(build) {
+                build.onLoad({ filter: /\.css$/ }, async () => {
+                  loadCount++;
+
+                  return null;
+                });
+              },
+            },
+          ],
+        }),
+        /* useContext */ false,
+      );
+
+      const firstResult = await context.bundle();
+      const secondResult = await context.bundle();
+
+      expect(firstResult.errors).toBeUndefined();
+      expect(secondResult).toBe(firstResult);
+      expect(loadCount).toBe(1);
+    });
+  });
 });