From 0174a195b962f5e1bcc9a76c173b26c4e718e923 Mon Sep 17 00:00:00 2001 From: Charles Lyding <19598772+clydin@users.noreply.github.com> Date: Wed, 21 Oct 2026 12:55:27 -0400 Subject: [PATCH] fix(@angular/build): preserve asset precedence and expand glob-prefixed ignores in watch mode Previously in library watch mode, ignore patterns containing glob characters that did not end with `*` (such as `**/` or `{a,b}`) were not expanded with `/**` because `normalizeGlobPattern` checked `picomatch.scan(result).isGlob`. Update `normalizeGlobPattern` to append `/**` whenever the stripped pattern does not end with `*`, matching `tinyglobby` directory expansion semantics. In `collectAssetsToEmit`, incremental watch builds previously only deduplicated colliding output paths among files present in the current `modifiedFiles` set. Modifying a lower-priority asset could therefore overwrite an existing higher-priority asset on disk, and deleting a higher-priority asset did not restore an existing lower-priority fallback asset. Resolve candidate sources across all asset matchers, evict overridden asset sources in `BuilderState.recordEmittedOutputs`, and re-emit lower-priority fallback assets when an overriding asset is deleted. Also preserve cached `statSync` results during deleted-directory checks so a newly created file replacing a directory at the same path is still matched and emitted. --- .../src/builders/library/builder-state.ts | 38 ++--- .../builders/library/builder-state_spec.ts | 33 +++- .../src/builders/library/pipeline/assets.ts | 127 ++++++++++++++-- .../builders/library/pipeline/assets_spec.ts | 141 +++++++++++++++++- 4 files changed, 304 insertions(+), 35 deletions(-) diff --git a/packages/angular/build/src/builders/library/builder-state.ts b/packages/angular/build/src/builders/library/builder-state.ts index 24257782b333..e4d1bf79ef5d 100644 --- a/packages/angular/build/src/builders/library/builder-state.ts +++ b/packages/angular/build/src/builders/library/builder-state.ts @@ -417,6 +417,21 @@ export class BuilderState { for (const emittedAsset of toEmit) { let outputs = this.#emittedAssets.get(emittedAsset.source); + if (outputs?.has(emittedAsset.path)) { + continue; + } + + if (this.#hasEmittedAssets) { + for (const [source, prevOutputs] of this.#emittedAssets) { + if (prevOutputs.delete(emittedAsset.path)) { + if (prevOutputs.size === 0) { + this.#emittedAssets.delete(source); + } + break; + } + } + } + if (!outputs) { outputs = new Set(); this.#emittedAssets.set(emittedAsset.source, outputs); @@ -424,26 +439,13 @@ export class BuilderState { outputs.add(emittedAsset.path); } - if (toDelete.length) { - const candidateDeletes = new Set(); - for (const source of toDelete) { - const outputs = this.#emittedAssets.get(source); - if (outputs) { - for (const output of outputs) { - candidateDeletes.add(output); - } - this.#emittedAssets.delete(source); - } - } - - for (const outputs of this.#emittedAssets.values()) { + for (const source of toDelete) { + const outputs = this.#emittedAssets.get(source); + if (outputs) { for (const output of outputs) { - candidateDeletes.delete(output); + filesToDelete.add(output); } - } - - for (const output of candidateDeletes) { - filesToDelete.add(output); + this.#emittedAssets.delete(source); } } diff --git a/packages/angular/build/src/builders/library/builder-state_spec.ts b/packages/angular/build/src/builders/library/builder-state_spec.ts index 312301b09df2..97dd8d96e259 100644 --- a/packages/angular/build/src/builders/library/builder-state_spec.ts +++ b/packages/angular/build/src/builders/library/builder-state_spec.ts @@ -261,24 +261,43 @@ describe('BuilderState', () => { ); }); - it('should not delete an asset output path when another source still emits to it', () => { + it('should evict overridden asset sources and not delete an output path when a fallback source is re-emitted', async () => { const state = new BuilderState(); const sourceA = '/workspace/projects/my-lib/README.md'; const sourceB = '/workspace/docs/README.md'; + // Initial build emits sourceA state.recordEmittedOutputs({ - filesToEmit: [ - createDiskOutputFile(sourceA, 'README.md'), - createDiskOutputFile(sourceB, 'README.md'), - ], + filesToEmit: [createDiskOutputFile(sourceA, 'README.md')], + filesToDelete: [], + }); + + // Subsequent watch iteration overrides 'README.md' with sourceB, evicting sourceA + state.resetExecutionResult(); + state.recordEmittedOutputs({ + filesToEmit: [createDiskOutputFile(sourceB, 'README.md')], filesToDelete: [], }); - // Delete sourceB while sourceA remains active + // Deleting the now-shadowed sourceA is a no-op + const shadowedCollected = await state.collectAssets( + { + workspaceRoot: '/workspace', + assets: [ + { glob: 'README.md', input: '/workspace/projects/my-lib', output: '.' }, + { glob: 'README.md', input: '/workspace/docs', output: '.' }, + ], + }, + new Set([sourceA]), + ); + expect(shadowedCollected.filesToEmit).toEqual([]); + expect(shadowedCollected.filesToDelete).toEqual([]); + + // Delete sourceB while fallback sourceA is re-emitted state.resetExecutionResult(); const firstDeletes = state.recordEmittedOutputs({ - filesToEmit: [], + filesToEmit: [createDiskOutputFile(sourceA, 'README.md')], filesToDelete: [sourceB], }); expect(firstDeletes.has('README.md')).toBeFalse(); diff --git a/packages/angular/build/src/builders/library/pipeline/assets.ts b/packages/angular/build/src/builders/library/pipeline/assets.ts index ae87edb8d4e3..199b2d2419b0 100644 --- a/packages/angular/build/src/builders/library/pipeline/assets.ts +++ b/packages/angular/build/src/builders/library/pipeline/assets.ts @@ -47,13 +47,38 @@ export interface AssetMatcher { */ function normalizeGlobPattern(pattern: string): string { let result = pattern.endsWith('/') ? pattern.slice(0, -1) : pattern; - if (pattern.endsWith('/') || !picomatch.scan(result).isGlob) { + if (pattern.endsWith('/') || !result.endsWith('*')) { result += '/**'; } return path.posix.normalize(result); } +/** + * Resolves the candidate POSIX source file path within `matcher` that would map to `outputPath`, + * or `undefined` if `outputPath` is outside `matcher.asset.output` or does not match `matcher`. + */ +function getCandidateSourceForOutput( + matcher: AssetMatcher, + outputPath: string, +): string | undefined { + const relative = path.posix.relative(matcher.asset.output, outputPath); + if (!relative || relative.startsWith('..') || path.posix.isAbsolute(relative)) { + return undefined; + } + + return matcher.isMatch(relative) ? `${matcher.prefix}${relative}` : undefined; +} + +/** + * Checks whether a POSIX path exists on disk as a file, respecting `followSymlinks`. + */ +function isMatchingAssetFile(file: string, followSymlinks: boolean | undefined): boolean { + return followSymlinks + ? (statSync(file, { throwIfNoEntry: false })?.isFile() ?? false) + : (lstatSync(file, { throwIfNoEntry: false })?.isFile() ?? false); +} + /** * Compiles picomatch matchers and POSIX input prefixes for each asset configuration. * @@ -62,15 +87,16 @@ function normalizeGlobPattern(pattern: string): string { */ export function createAssetMatchers(assets: NormalizedAssetsPattern[]): readonly AssetMatcher[] { return assets.map((asset) => { - const ignore = [...DEFAULT_ASSET_IGNORE, ...(asset.ignore ?? [])] + const { glob: rawGlob, ignore: rawIgnore, input } = asset; + const ignore = [...DEFAULT_ASSET_IGNORE, ...(rawIgnore ?? [])] .filter((pattern) => pattern.length > 0) .map(normalizeGlobPattern); - const isMatch = picomatch(asset.glob ? path.posix.normalize(asset.glob) : [], { + const isMatch = picomatch(rawGlob ? path.posix.normalize(rawGlob) : [], { dot: true, windows: false, ignore, }); - const prefix = asset.input.endsWith('/') ? asset.input : `${asset.input}/`; + const prefix = input.endsWith('/') ? input : `${input}/`; return { asset, prefix, isMatch }; }); @@ -89,7 +115,7 @@ export function createAssetMatchers(assets: NormalizedAssetsPattern[]): readonly export async function collectAssetsToEmit( assets: NormalizedAssetsPattern[], workspaceRoot: string, - emittedAssets: ReadonlyMap, + emittedAssets: ReadonlyMap>, modifiedFiles?: ReadonlySet, assetMatchers?: readonly AssetMatcher[], ): Promise { @@ -108,16 +134,21 @@ export async function collectAssetsToEmit( if (!emittedAssets.has(file) && emittedAssets.size > 0) { const dirPrefix = file.endsWith('/') ? file : `${file}/`; - let isDeletedDir: boolean | undefined; + let isDirectory: boolean | undefined; for (const emittedFile of emittedAssets.keys()) { if (emittedFile.startsWith(dirPrefix)) { - if (!(isDeletedDir ??= !statSync(file, { throwIfNoEntry: false })?.isDirectory())) { - break; + if (isDirectory === undefined) { + const stats = statSync(file, { throwIfNoEntry: false }); + isDirectory = stats?.isDirectory() ?? false; + isFileStat = stats?.isFile() ?? false; + if (isDirectory) { + break; + } } filesToDelete.add(emittedFile); } } - if (isDeletedDir !== undefined) { + if (isDirectory !== undefined && !isFileStat) { continue; } } @@ -154,6 +185,84 @@ export async function collectAssetsToEmit( } } + if (matchers.length > 1) { + // Ensure lower-priority modified assets do not overwrite existing higher-priority assets on disk. + for (const [outputPath, { file, matcherIndex }] of filesToEmit) { + for (let laterIndex = matchers.length - 1; laterIndex > matcherIndex; laterIndex--) { + const laterMatcher = matchers[laterIndex]; + const candidateFile = getCandidateSourceForOutput(laterMatcher, outputPath); + if (!candidateFile || candidateFile === file.source) { + continue; + } + + if (isMatchingAssetFile(candidateFile, laterMatcher.asset.followSymlinks)) { + if (emittedAssets.get(candidateFile)?.has(outputPath)) { + filesToEmit.delete(outputPath); + } else { + filesToEmit.set(outputPath, { + file: createDiskOutputFile(candidateFile, outputPath), + matcherIndex: laterIndex, + }); + } + break; + } else if (emittedAssets.get(candidateFile)?.has(outputPath)) { + filesToDelete.add(candidateFile); + } + } + } + + // Restore lower-priority fallback assets when a higher-priority overriding asset is deleted. + for (const deletedFile of filesToDelete) { + for ( + let deletedMatcherIndex = 0; + deletedMatcherIndex < matchers.length; + deletedMatcherIndex++ + ) { + const { asset, prefix, isMatch } = matchers[deletedMatcherIndex]; + if (!deletedFile.startsWith(prefix)) { + continue; + } + + const relative = deletedFile.slice(prefix.length); + if (!isMatch(relative)) { + continue; + } + + const outputPath = path.posix.join(asset.output, relative); + if (filesToEmit.has(outputPath)) { + continue; + } + + for (let candidateIndex = matchers.length - 1; candidateIndex >= 0; candidateIndex--) { + if (candidateIndex === deletedMatcherIndex) { + continue; + } + + const candidateMatcher = matchers[candidateIndex]; + const candidateFile = getCandidateSourceForOutput(candidateMatcher, outputPath); + if (!candidateFile || filesToDelete.has(candidateFile)) { + continue; + } + + if (isMatchingAssetFile(candidateFile, candidateMatcher.asset.followSymlinks)) { + if ( + candidateIndex < deletedMatcherIndex || + !emittedAssets.get(candidateFile)?.has(outputPath) + ) { + filesToEmit.set(outputPath, { + file: createDiskOutputFile(candidateFile, outputPath), + matcherIndex: candidateIndex, + }); + } + break; + } else if (emittedAssets.get(candidateFile)?.has(outputPath)) { + filesToDelete.add(candidateFile); + } + } + } + } + } + return { filesToEmit: Array.from(filesToEmit.values(), ({ file }) => file), filesToDelete: Array.from(filesToDelete), diff --git a/packages/angular/build/src/builders/library/pipeline/assets_spec.ts b/packages/angular/build/src/builders/library/pipeline/assets_spec.ts index 52a9f11dee53..610f783811ab 100644 --- a/packages/angular/build/src/builders/library/pipeline/assets_spec.ts +++ b/packages/angular/build/src/builders/library/pipeline/assets_spec.ts @@ -32,7 +32,14 @@ describe('library pipeline assets', () => { glob: './**/*.txt', input: `${tempRoot}/assets`, output: 'assets', - ignore: ['', 'ignored-dir', './ignored-dir-2/', 'specific.txt'], + ignore: [ + '', + 'ignored-dir', + './ignored-dir-2/', + '**/ignored-subdir', + '{brace-dir-a,brace-dir-b}', + 'specific.txt', + ], }, ]); @@ -46,6 +53,11 @@ describe('library pipeline assets', () => { expect(matcher.isMatch('ignored-dir/file.txt')).toBeFalse(); expect(matcher.isMatch('ignored-dir/sub/file.txt')).toBeFalse(); expect(matcher.isMatch('ignored-dir-2/file.txt')).toBeFalse(); + expect(matcher.isMatch('ignored-subdir/file.txt')).toBeFalse(); + expect(matcher.isMatch('nested/ignored-subdir/file.txt')).toBeFalse(); + expect(matcher.isMatch('nested/ignored-subdir/deep/file.txt')).toBeFalse(); + expect(matcher.isMatch('brace-dir-a/file.txt')).toBeFalse(); + expect(matcher.isMatch('brace-dir-b/sub/file.txt')).toBeFalse(); expect(matcher.isMatch('.gitkeep')).toBeFalse(); expect(matcher.isMatch('nested/.DS_Store')).toBeFalse(); }); @@ -107,6 +119,101 @@ describe('library pipeline assets', () => { ]); }); + it('should not overwrite a higher-priority emitted asset when only a lower-priority asset is modified in watch mode', async () => { + const defaultDir = `${tempRoot}/lib`; + const customDir = `${tempRoot}/docs`; + await mkdir(defaultDir, { recursive: true }); + await mkdir(customDir, { recursive: true }); + + const defaultReadme = `${defaultDir}/README.md`; + const customReadme = `${customDir}/README.md`; + await writeFile(defaultReadme, '# Default updated'); + await writeFile(customReadme, '# Custom'); + + const assets = [ + { glob: 'README.md', input: defaultDir, output: '.' }, + { glob: 'README.md', input: customDir, output: '.' }, + ]; + + // When customReadme was already emitted, modifying defaultReadme should not emit anything. + const emittedAssets = new Map>([[customReadme, new Set(['README.md'])]]); + const resultWithEmitted = await collectAssetsToEmit( + assets, + tempRoot, + emittedAssets, + new Set([defaultReadme]), + ); + expect(resultWithEmitted.filesToEmit).toEqual([]); + expect(resultWithEmitted.filesToDelete).toEqual([]); + + // When customReadme was not yet recorded in emittedAssets, modifying defaultReadme should emit customReadme. + const resultWithoutEmitted = await collectAssetsToEmit( + assets, + tempRoot, + new Map(), + new Set([defaultReadme]), + ); + expect(resultWithoutEmitted.filesToEmit).toEqual([ + { + origin: 'disk', + source: customReadme, + path: 'README.md', + }, + ]); + + // When customReadme was recorded in emittedAssets but no longer exists on disk, + // modifying defaultReadme should queue customReadme for deletion and emit defaultReadme. + await rm(customReadme); + const resultWithMissingHigherPriority = await collectAssetsToEmit( + assets, + tempRoot, + emittedAssets, + new Set([defaultReadme]), + ); + expect(resultWithMissingHigherPriority.filesToDelete).toEqual([customReadme]); + expect(resultWithMissingHigherPriority.filesToEmit).toEqual([ + { + origin: 'disk', + source: defaultReadme, + path: 'README.md', + }, + ]); + }); + + it('should restore a lower-priority fallback asset when a higher-priority overriding asset is deleted in watch mode', async () => { + const defaultDir = `${tempRoot}/lib`; + const customDir = `${tempRoot}/docs`; + await mkdir(defaultDir, { recursive: true }); + await mkdir(customDir, { recursive: true }); + + const defaultReadme = `${defaultDir}/README.md`; + const customReadme = `${customDir}/README.md`; + await writeFile(defaultReadme, '# Default'); + + const assets = [ + { glob: 'README.md', input: defaultDir, output: '.' }, + { glob: 'README.md', input: customDir, output: '.' }, + ]; + + // customReadme was previously emitted, and is now deleted from disk while defaultReadme still exists. + const emittedAssets = new Map>([[customReadme, new Set(['README.md'])]]); + const result = await collectAssetsToEmit( + assets, + tempRoot, + emittedAssets, + new Set([customReadme]), + ); + + expect(result.filesToDelete).toEqual([customReadme]); + expect(result.filesToEmit).toEqual([ + { + origin: 'disk', + source: defaultReadme, + path: 'README.md', + }, + ]); + }); + it('should respect followSymlinks in watch mode', async () => { const assetsDir = `${tempRoot}/assets`; await mkdir(assetsDir, { recursive: true }); @@ -175,5 +282,37 @@ describe('library pipeline assets', () => { expect(result.filesToEmit).toEqual([]); expect(result.filesToDelete).toEqual([nestedFileA, nestedFileB]); }); + + it('should delete nested emitted assets and emit the new file when a directory is replaced by a file at the same path', async () => { + const assetsDir = `${tempRoot}/assets`; + await mkdir(assetsDir, { recursive: true }); + + const replacedPath = `${assetsDir}/item.txt`; + const nestedFile = `${replacedPath}/nested.txt`; + // replacedPath was previously a directory containing nested.txt, and is now a regular file on disk. + await writeFile(replacedPath, 'now a file'); + + const emittedAssets = new Map>([ + [nestedFile, new Set(['out/item.txt/nested.txt'])], + ]); + + const assets = [{ glob: '**/*.txt', input: assetsDir, output: 'out' }]; + + const result = await collectAssetsToEmit( + assets, + tempRoot, + emittedAssets, + new Set([replacedPath]), + ); + + expect(result.filesToDelete).toEqual([nestedFile]); + expect(result.filesToEmit).toEqual([ + { + origin: 'disk', + source: replacedPath, + path: 'out/item.txt', + }, + ]); + }); }); });