fix(@angular-devkit/build-angular): cache stylesheet load errors with…

… application builder When using the `application` and/or `browser-esbuild` builders, stylesheets that generate errors will now have the errors cached between rebuilds during watch mode (include `ng serve`). This not only avoids reprocessing files that contain errors and that have not changed but also provides file watching information from the cache to ensure the error-causing files are properly invalidated.
angular · Nov 15, 2023 · 447761e · 447761e
1 parent fd2d72a
commit 447761e
Show file tree

Hide file tree

Showing 5 changed files with 147 additions and 16 deletions.
diff --git a/...evkit/build_angular/src/builders/application/tests/behavior/rebuild-global_styles_spec.ts b/...evkit/build_angular/src/builders/application/tests/behavior/rebuild-global_styles_spec.ts
@@ -0,0 +1,121 @@
+/**
+ * @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.io/license
+ */
+
+import { concatMap, count, timeout } from 'rxjs';
+import { buildApplication } from '../../index';
+import { APPLICATION_BUILDER_INFO, BASE_OPTIONS, describeBuilder } from '../setup';
+
+/**
+ * Maximum time in milliseconds for single build/rebuild
+ * This accounts for CI variability.
+ */
+export const BUILD_TIMEOUT = 30_000;
+
+describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
+  describe('Behavior: "Rebuilds when global stylesheets change"', () => {
+    beforeEach(async () => {
+      // Application code is not needed for styles tests
+      await harness.writeFile('src/main.ts', 'console.log("TEST");');
+    });
+
+    it('rebuilds Sass stylesheet after error on rebuild from import', async () => {
+      harness.useTarget('build', {
+        ...BASE_OPTIONS,
+        watch: true,
+        styles: ['src/styles.scss'],
+      });
+
+      await harness.writeFile('src/styles.scss', "@import './a';");
+      await harness.writeFile('src/a.scss', '$primary: aqua;\\nh1 { color: $primary; }');
+
+      const builderAbort = new AbortController();
+      const buildCount = await harness
+        .execute({ signal: builderAbort.signal, outputLogsOnFailure: false })
+        .pipe(
+          timeout(30000),
+          concatMap(async ({ result }, index) => {
+            switch (index) {
+              case 0:
+                expect(result?.success).toBe(true);
+                harness.expectFile('dist/browser/styles.css').content.toContain('color: aqua');
+                harness.expectFile('dist/browser/styles.css').content.not.toContain('color: blue');
+
+                await harness.writeFile(
+                  'src/a.scss',
+                  'invalid-invalid-invalid\\nh1 { color: $primary; }',
+                );
+                break;
+              case 1:
+                expect(result?.success).toBe(false);
+
+                await harness.writeFile('src/a.scss', '$primary: blue;\\nh1 { color: $primary; }');
+                break;
+              case 2:
+                expect(result?.success).toBe(true);
+                harness.expectFile('dist/browser/styles.css').content.not.toContain('color: aqua');
+                harness.expectFile('dist/browser/styles.css').content.toContain('color: blue');
+
+                // Test complete - abort watch mode
+                builderAbort.abort();
+                break;
+            }
+          }),
+          count(),
+        )
+        .toPromise();
+
+      expect(buildCount).toBe(3);
+    });
+
+    it('rebuilds Sass stylesheet after error on initial build from import', async () => {
+      harness.useTarget('build', {
+        ...BASE_OPTIONS,
+        watch: true,
+        styles: ['src/styles.scss'],
+      });
+
+      await harness.writeFile('src/styles.scss', "@import './a';");
+      await harness.writeFile('src/a.scss', 'invalid-invalid-invalid\\nh1 { color: $primary; }');
+
+      const builderAbort = new AbortController();
+      const buildCount = await harness
+        .execute({ signal: builderAbort.signal, outputLogsOnFailure: false })
+        .pipe(
+          timeout(30000),
+          concatMap(async ({ result }, index) => {
+            switch (index) {
+              case 0:
+                expect(result?.success).toBe(false);
+
+                await harness.writeFile('src/a.scss', '$primary: aqua;\\nh1 { color: $primary; }');
+                break;
+              case 1:
+                expect(result?.success).toBe(true);
+                harness.expectFile('dist/browser/styles.css').content.toContain('color: aqua');
+                harness.expectFile('dist/browser/styles.css').content.not.toContain('color: blue');
+
+                await harness.writeFile('src/a.scss', '$primary: blue;\\nh1 { color: $primary; }');
+                break;
+              case 2:
+                expect(result?.success).toBe(true);
+                harness.expectFile('dist/browser/styles.css').content.not.toContain('color: aqua');
+                harness.expectFile('dist/browser/styles.css').content.toContain('color: blue');
+
+                // Test complete - abort watch mode
+                builderAbort.abort();
+                break;
+            }
+          }),
+          count(),
+        )
+        .toPromise();
+
+      expect(buildCount).toBe(3);
+    });
+  });
+});
diff --git a/packages/angular_devkit/build_angular/src/tools/esbuild/bundler-context.ts b/packages/angular_devkit/build_angular/src/tools/esbuild/bundler-context.ts
@@ -220,6 +220,16 @@ export class BundlerContext {
       } else {
         throw failure;
       }
+    } finally {
+      if (this.incremental) {
+        // When incremental always add any files from the load result cache
+        if (this.#loadCache) {
+          this.#loadCache.watchFiles
+            .filter((file) => !isInternalAngularFile(file))
+            // watch files are fully resolved paths
+            .forEach((file) => this.watchFiles.add(file));
+        }
+      }
     }
 
     // Update files that should be watched.
@@ -228,16 +238,9 @@ export class BundlerContext {
     if (this.incremental) {
       // Add input files except virtual angular files which do not exist on disk
       Object.keys(result.metafile.inputs)
-        .filter((input) => !input.startsWith('angular:'))
+        .filter((input) => !isInternalAngularFile(input))
         // input file paths are always relative to the workspace root
         .forEach((input) => this.watchFiles.add(join(this.workspaceRoot, input)));
-      // Also add any files from the load result cache
-      if (this.#loadCache) {
-        this.#loadCache.watchFiles
-          .filter((file) => !file.startsWith('angular:'))
-          // watch files are fully resolved paths
-          .forEach((file) => this.watchFiles.add(file));
-      }
     }
 
     // Return if the build encountered any errors
@@ -349,12 +352,12 @@ export class BundlerContext {
   #addErrorsToWatch(result: BuildFailure | BuildResult): void {
     for (const error of result.errors) {
       let file = error.location?.file;
-      if (file) {
+      if (file && !isInternalAngularFile(file)) {
         this.watchFiles.add(join(this.workspaceRoot, file));
       }
       for (const note of error.notes) {
         file = note.location?.file;
-        if (file) {
+        if (file && !isInternalAngularFile(file)) {
           this.watchFiles.add(join(this.workspaceRoot, file));
         }
       }
@@ -406,3 +409,7 @@ export class BundlerContext {
     }
   }
 }
+
+function isInternalAngularFile(file: string) {
+  return file.startsWith('angular:');
+}
diff --git a/packages/angular_devkit/build_angular/src/tools/esbuild/load-result-cache.ts b/packages/angular_devkit/build_angular/src/tools/esbuild/load-result-cache.ts
@@ -30,8 +30,8 @@ export function createCachedLoad(
     if (result === undefined) {
       result = await callback(args);
 
-      // Do not cache null or undefined or results with errors
-      if (result && result.errors === undefined) {
+      // Do not cache null or undefined
+      if (result) {
         await cache.put(loadCacheKey, result);
       }
     }

diff --git a/packages/angular_devkit/build_angular/src/tools/esbuild/stylesheets/less-language.ts b/packages/angular_devkit/build_angular/src/tools/esbuild/stylesheets/less-language.ts
@@ -115,6 +115,8 @@ async function compileString(
     };
   } catch (error) {
     if (isLessException(error)) {
+      const location = convertExceptionLocation(error);
+
       // Retry with a warning for less files requiring the deprecated inline JavaScript option
       if (error.message.includes('Inline JavaScript is not enabled.')) {
         const withJsResult = await compileString(
@@ -127,7 +129,7 @@ async function compileString(
         withJsResult.warnings = [
           {
             text: 'Deprecated inline execution of JavaScript has been enabled ("javascriptEnabled")',
-            location: convertExceptionLocation(error),
+            location,
             notes: [
               {
                 location: null,
@@ -148,10 +150,11 @@ async function compileString(
         errors: [
           {
             text: error.message,
-            location: convertExceptionLocation(error),
+            location,
           },
         ],
         loader: 'css',
+        watchFiles: location.file ? [filename, location.file] : [filename],
       };
     }
 

diff --git a/packages/angular_devkit/build_angular/src/tools/esbuild/stylesheets/sass-language.ts b/packages/angular_devkit/build_angular/src/tools/esbuild/stylesheets/sass-language.ts
@@ -176,7 +176,7 @@ async function compileString(
     };
   } catch (error) {
     if (isSassException(error)) {
-      const file = error.span.url ? fileURLToPath(error.span.url) : undefined;
+      const fileWithError = error.span.url ? fileURLToPath(error.span.url) : undefined;
 
       return {
         loader: 'css',
@@ -186,7 +186,7 @@ async function compileString(
           },
         ],
         warnings,
-        watchFiles: file ? [file] : undefined,
+        watchFiles: fileWithError ? [filePath, fileWithError] : [filePath],
       };
     }