fix(@angular-devkit/build-angular): ensure all related stylesheets are rebuilt when an import changes

clydin · clydin · commit ce2c3ef0e9b9 · 2024-02-23T07:11:09.000-05:00
This fixes a logic error wherein some stylesheets could potentially not be
rebuilt if a shared import was edited and triggered an application rebuild.
diff --git a/packages/angular_devkit/build_angular/src/builders/application/tests/behavior/rebuild-global_styles_spec.ts b/packages/angular_devkit/build_angular/src/builders/application/tests/behavior/rebuild-global_styles_spec.ts
@@ -112,5 +112,60 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
 
       expect(buildCount).toBe(3);
     });
+
+    it('rebuilds dependent Sass stylesheets after error on initial build from import', async () => {
+      harness.useTarget('build', {
+        ...BASE_OPTIONS,
+        watch: true,
+        styles: [
+          { bundleName: 'styles', input: 'src/styles.scss' },
+          { bundleName: 'other', input: 'src/other.scss' },
+        ],
+      });
+
+      await harness.writeFile('src/styles.scss', "@import './a';");
+      await harness.writeFile('src/other.scss', "@import './a'; h1 { color: green; }");
+      await harness.writeFile('src/a.scss', 'invalid-invalid-invalid\\nh1 { color: $primary; }');
+
+      const buildCount = await harness
+        .execute({ 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');
+
+                harness.expectFile('dist/browser/other.css').content.toContain('color: green');
+                harness.expectFile('dist/browser/other.css').content.toContain('color: aqua');
+                harness.expectFile('dist/browser/other.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');
+
+                harness.expectFile('dist/browser/other.css').content.toContain('color: green');
+                harness.expectFile('dist/browser/other.css').content.not.toContain('color: aqua');
+                harness.expectFile('dist/browser/other.css').content.toContain('color: blue');
+                break;
+            }
+          }),
+          take(3),
+          count(),
+        )
+        .toPromise();
+
+      expect(buildCount).toBe(3);
+    });
   });
 });
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
@@ -70,16 +70,18 @@ export class MemoryLoadResultCache implements LoadResultCache {
   }
 
   invalidate(path: string): boolean {
-    const affected = this.#fileDependencies.get(path);
+    const affectedPaths = this.#fileDependencies.get(path);
     let found = false;
 
-    if (affected) {
-      affected.forEach((a) => (found ||= this.#loadResults.delete(a)));
+    if (affectedPaths) {
+      for (const affected of affectedPaths) {
+        if (this.#loadResults.delete(affected)) {
+          found = true;
+        }
+      }
       this.#fileDependencies.delete(path);
     }
 
-    found ||= this.#loadResults.delete(path);
-
     return found;
   }
 
