From 866184cc169e702930d3640c407980b4ce91b88e Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Tue, 4 Aug 2026 18:43:56 -0400 Subject: [PATCH] fix(@angular/build): retain watch files on error in load result cache During incremental builds (serve and build --watch), caching an error result that lacks watch files would cause previously tracked dependency files to be dropped from the file watcher dependency map. For PostCSS plugins that read external dependency files (such as Tailwind configs or theme files), a syntax or parse error in a dependency file would result in an error being cached without watch files, preventing any subsequent edits to the dependency file from clearing the error until the entry stylesheet itself was modified. This commit improves MemoryLoadResultCache to remember the last-known watch set for each cache key across invalidations and union it into any newly cached error result. Additionally, compileString() in stylesheet-plugin-factory now explicitly returns error.file in watchFiles when catching a PostCSS CssSyntaxError, mirroring Sass and Less behavior. Closes #33666 --- .../behavior/rebuild-global_styles_spec.ts | 114 ++++++++++++++++++ .../src/tools/esbuild/load-result-cache.ts | 15 +++ .../tools/esbuild/load-result-cache_spec.ts | 85 +++++++++++++ .../stylesheets/stylesheet-plugin-factory.ts | 1 + 4 files changed, 215 insertions(+) create mode 100644 packages/angular/build/src/tools/esbuild/load-result-cache_spec.ts diff --git a/packages/angular/build/src/builders/application/tests/behavior/rebuild-global_styles_spec.ts b/packages/angular/build/src/builders/application/tests/behavior/rebuild-global_styles_spec.ts index 22c4c32202bd..2de5c5f229ca 100644 --- a/packages/angular/build/src/builders/application/tests/behavior/rebuild-global_styles_spec.ts +++ b/packages/angular/build/src/builders/application/tests/behavior/rebuild-global_styles_spec.ts @@ -132,5 +132,119 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => { { outputLogsOnFailure: false }, ); }); + + it('rebuilds PostCSS stylesheet after error on rebuild from plugin dependency', async () => { + harness.useTarget('build', { + ...BASE_OPTIONS, + watch: true, + styles: ['src/styles.css'], + }); + + await harness.writeFile( + 'test-plugin.js', + ` + const fs = require('fs'); + const path = require('path'); + module.exports = () => { + return { + postcssPlugin: 'test-plugin', + Once(root, { result }) { + const themePath = path.join(path.dirname(root.source.input.file), 'theme.json'); + result.messages.push({ + type: 'dependency', + file: themePath, + }); + const data = fs.readFileSync(themePath, 'utf-8'); + const json = JSON.parse(data); + root.append('body { color: ' + json.color + '; }'); + }, + }; + }; + module.exports.postcss = true; + `, + ); + await harness.writeFile( + '.postcssrc.json', + JSON.stringify({ + plugins: { + './test-plugin.js': {}, + }, + }), + ); + await harness.writeFile('src/styles.css', '/* base */'); + await harness.writeFile('src/theme.json', '{"color": "aqua"}'); + + await harness.executeWithCases( + [ + async ({ result }) => { + 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/theme.json', 'invalid-json'); + }, + async ({ result }) => { + expect(result?.success).toBe(false); + + await harness.writeFile('src/theme.json', '{"color": "blue"}'); + }, + ({ result }) => { + 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'); + }, + ], + { outputLogsOnFailure: false }, + ); + }); + + it('rebuilds PostCSS stylesheet after CSS syntax error on initial build from import', async () => { + harness.useTarget('build', { + ...BASE_OPTIONS, + watch: true, + styles: ['src/styles.css'], + }); + + await harness.writeFile( + 'noop-plugin.js', + ` + module.exports = () => ({ postcssPlugin: 'noop-plugin' }); + module.exports.postcss = true; + `, + ); + await harness.writeFile( + '.postcssrc.json', + JSON.stringify({ + plugins: { + './noop-plugin.js': {}, + }, + }), + ); + await harness.writeFile('src/styles.css', "@import './a.css';"); + await harness.writeFile('src/a.css', "a { ' }"); + + await harness.executeWithCases( + [ + async ({ result }) => { + expect(result?.success).toBe(false); + + await harness.writeFile('src/a.css', 'body { color: aqua; }'); + }, + async ({ result }) => { + 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.css', 'body { color: blue; }'); + }, + ({ result }) => { + 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'); + }, + ], + { outputLogsOnFailure: false }, + ); + }); }); }); diff --git a/packages/angular/build/src/tools/esbuild/load-result-cache.ts b/packages/angular/build/src/tools/esbuild/load-result-cache.ts index 7a658c54240e..00f803f2fdc7 100644 --- a/packages/angular/build/src/tools/esbuild/load-result-cache.ts +++ b/packages/angular/build/src/tools/esbuild/load-result-cache.ts @@ -50,12 +50,26 @@ export function createCachedLoad( export class MemoryLoadResultCache implements LoadResultCache { #loadResults = new Map(); #fileDependencies = new Map>(); + #watchFilesPerKey = new Map>(); get(path: string): OnLoadResult | undefined { return this.#loadResults.get(path); } async put(path: string, result: OnLoadResult): Promise { + if (result.errors && result.errors.length > 0) { + const previousWatchFiles = this.#watchFilesPerKey.get(path); + if (previousWatchFiles) { + result.watchFiles = Array.from( + new Set([...(result.watchFiles ?? []), ...previousWatchFiles]), + ); + } + } else if (result.watchFiles && result.watchFiles.length > 0) { + this.#watchFilesPerKey.set(path, [...result.watchFiles]); + } else { + this.#watchFilesPerKey.delete(path); + } + this.#loadResults.set(path, result); if (result.watchFiles) { for (const watchFile of result.watchFiles) { @@ -96,5 +110,6 @@ export class MemoryLoadResultCache implements LoadResultCache { clear(): void { this.#loadResults.clear(); this.#fileDependencies.clear(); + this.#watchFilesPerKey.clear(); } } diff --git a/packages/angular/build/src/tools/esbuild/load-result-cache_spec.ts b/packages/angular/build/src/tools/esbuild/load-result-cache_spec.ts new file mode 100644 index 000000000000..fb0396aea829 --- /dev/null +++ b/packages/angular/build/src/tools/esbuild/load-result-cache_spec.ts @@ -0,0 +1,85 @@ +/** + * @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.dev/license + */ + +import { MemoryLoadResultCache } from './load-result-cache'; + +describe('MemoryLoadResultCache', () => { + let cache: MemoryLoadResultCache; + + beforeEach(() => { + cache = new MemoryLoadResultCache(); + }); + + it('should store and retrieve results', async () => { + const result = { + contents: 'body { color: red; }', + loader: 'css' as const, + }; + + await cache.put('file:/test/styles.css', result); + const cached = cache.get('file:/test/styles.css'); + + expect(cached).toBe(result); + }); + + it('should track watch files in fileDependencies', async () => { + const result = { + contents: 'body { color: red; }', + loader: 'css' as const, + watchFiles: ['/test/styles.css', '/test/theme.json'], + }; + + await cache.put('file:/test/styles.css', result); + + expect(cache.watchFiles).toContain('/test/styles.css'); + expect(cache.watchFiles).toContain('/test/theme.json'); + }); + + it('should invalidate cached results when a dependency changes', async () => { + const result = { + contents: 'body { color: red; }', + loader: 'css' as const, + watchFiles: ['/test/styles.css', '/test/theme.json'], + }; + + await cache.put('file:/test/styles.css', result); + expect(cache.get('file:/test/styles.css')).toBe(result); + + const invalidated = cache.invalidate('/test/theme.json'); + expect(invalidated).toBeTrue(); + expect(cache.get('file:/test/styles.css')).toBeUndefined(); + }); + + it('should preserve previous watch files when caching an error result', async () => { + const successResult = { + contents: 'body { color: red; }', + loader: 'css' as const, + watchFiles: ['/test/styles.css', '/test/theme.json'], + }; + + await cache.put('file:/test/styles.css', successResult); + cache.invalidate('/test/theme.json'); + + // Simulate an incremental rebuild error result that only has the entry file in watchFiles + const errorResult = { + errors: [{ text: 'Syntax error in theme.json' }], + watchFiles: ['/test/styles.css'], + }; + + await cache.put('file:/test/styles.css', errorResult); + + // Both the entry file and the previous dependency should be tracked + expect(cache.watchFiles).toContain('/test/styles.css'); + expect(cache.watchFiles).toContain('/test/theme.json'); + + // Invalidating the dependency should clear the cached error result + const invalidated = cache.invalidate('/test/theme.json'); + expect(invalidated).toBeTrue(); + expect(cache.get('file:/test/styles.css')).toBeUndefined(); + }); +}); diff --git a/packages/angular/build/src/tools/esbuild/stylesheets/stylesheet-plugin-factory.ts b/packages/angular/build/src/tools/esbuild/stylesheets/stylesheet-plugin-factory.ts index 78925f35835e..2c40e007350e 100644 --- a/packages/angular/build/src/tools/esbuild/stylesheets/stylesheet-plugin-factory.ts +++ b/packages/angular/build/src/tools/esbuild/stylesheets/stylesheet-plugin-factory.ts @@ -427,6 +427,7 @@ async function compileString( }, }, ], + watchFiles: error.file && error.file !== filename ? [filename, error.file] : [filename], }; } else { assertIsError(error);