Skip to content

Commit 69749ae

Browse files
committed
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 `**/<dir>` 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.
1 parent a38958c commit 69749ae

4 files changed

Lines changed: 299 additions & 35 deletions

File tree

‎packages/angular/build/src/builders/library/builder-state.ts‎

Lines changed: 15 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -416,6 +416,16 @@ export class BuilderState {
416416
const { filesToDelete: toDelete, filesToEmit: toEmit } = collectedAssets;
417417

418418
for (const emittedAsset of toEmit) {
419+
for (const [source, outputs] of this.#emittedAssets) {
420+
if (
421+
source !== emittedAsset.source &&
422+
outputs.delete(emittedAsset.path) &&
423+
outputs.size === 0
424+
) {
425+
this.#emittedAssets.delete(source);
426+
}
427+
}
428+
419429
let outputs = this.#emittedAssets.get(emittedAsset.source);
420430
if (!outputs) {
421431
outputs = new Set();
@@ -424,26 +434,13 @@ export class BuilderState {
424434
outputs.add(emittedAsset.path);
425435
}
426436

427-
if (toDelete.length) {
428-
const candidateDeletes = new Set<string>();
429-
for (const source of toDelete) {
430-
const outputs = this.#emittedAssets.get(source);
431-
if (outputs) {
432-
for (const output of outputs) {
433-
candidateDeletes.add(output);
434-
}
435-
this.#emittedAssets.delete(source);
436-
}
437-
}
438-
439-
for (const outputs of this.#emittedAssets.values()) {
437+
for (const source of toDelete) {
438+
const outputs = this.#emittedAssets.get(source);
439+
if (outputs) {
440440
for (const output of outputs) {
441-
candidateDeletes.delete(output);
441+
filesToDelete.add(output);
442442
}
443-
}
444-
445-
for (const output of candidateDeletes) {
446-
filesToDelete.add(output);
443+
this.#emittedAssets.delete(source);
447444
}
448445
}
449446

‎packages/angular/build/src/builders/library/builder-state_spec.ts‎

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -261,24 +261,43 @@ describe('BuilderState', () => {
261261
);
262262
});
263263

264-
it('should not delete an asset output path when another source still emits to it', () => {
264+
it('should evict overridden asset sources and not delete an output path when a fallback source is re-emitted', async () => {
265265
const state = new BuilderState();
266266

267267
const sourceA = '/workspace/projects/my-lib/README.md';
268268
const sourceB = '/workspace/docs/README.md';
269269

270+
// Initial build emits sourceA
270271
state.recordEmittedOutputs({
271-
filesToEmit: [
272-
createDiskOutputFile(sourceA, 'README.md'),
273-
createDiskOutputFile(sourceB, 'README.md'),
274-
],
272+
filesToEmit: [createDiskOutputFile(sourceA, 'README.md')],
273+
filesToDelete: [],
274+
});
275+
276+
// Subsequent watch iteration overrides 'README.md' with sourceB, evicting sourceA
277+
state.resetExecutionResult();
278+
state.recordEmittedOutputs({
279+
filesToEmit: [createDiskOutputFile(sourceB, 'README.md')],
275280
filesToDelete: [],
276281
});
277282

278-
// Delete sourceB while sourceA remains active
283+
// Deleting the now-shadowed sourceA is a no-op
284+
const shadowedCollected = await state.collectAssets(
285+
{
286+
workspaceRoot: '/workspace',
287+
assets: [
288+
{ glob: 'README.md', input: '/workspace/projects/my-lib', output: '.' },
289+
{ glob: 'README.md', input: '/workspace/docs', output: '.' },
290+
],
291+
},
292+
new Set([sourceA]),
293+
);
294+
expect(shadowedCollected.filesToEmit).toEqual([]);
295+
expect(shadowedCollected.filesToDelete).toEqual([]);
296+
297+
// Delete sourceB while fallback sourceA is re-emitted
279298
state.resetExecutionResult();
280299
const firstDeletes = state.recordEmittedOutputs({
281-
filesToEmit: [],
300+
filesToEmit: [createDiskOutputFile(sourceA, 'README.md')],
282301
filesToDelete: [sourceB],
283302
});
284303
expect(firstDeletes.has('README.md')).toBeFalse();

‎packages/angular/build/src/builders/library/pipeline/assets.ts‎

Lines changed: 118 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,38 @@ export interface AssetMatcher {
4747
*/
4848
function normalizeGlobPattern(pattern: string): string {
4949
let result = pattern.endsWith('/') ? pattern.slice(0, -1) : pattern;
50-
if (pattern.endsWith('/') || !picomatch.scan(result).isGlob) {
50+
if (pattern.endsWith('/') || !result.endsWith('*')) {
5151
result += '/**';
5252
}
5353

5454
return path.posix.normalize(result);
5555
}
5656

57+
/**
58+
* Resolves the candidate POSIX source file path within `matcher` that would map to `outputPath`,
59+
* or `undefined` if `outputPath` is outside `matcher.asset.output` or does not match `matcher`.
60+
*/
61+
function getCandidateSourceForOutput(
62+
matcher: AssetMatcher,
63+
outputPath: string,
64+
): string | undefined {
65+
const relative = path.posix.relative(matcher.asset.output, outputPath);
66+
if (!relative || relative.startsWith('..') || path.posix.isAbsolute(relative)) {
67+
return undefined;
68+
}
69+
70+
return matcher.isMatch(relative) ? `${matcher.prefix}${relative}` : undefined;
71+
}
72+
73+
/**
74+
* Checks whether a POSIX path exists on disk as a file, respecting `followSymlinks`.
75+
*/
76+
function isMatchingAssetFile(file: string, followSymlinks: boolean | undefined): boolean {
77+
return followSymlinks
78+
? (statSync(file, { throwIfNoEntry: false })?.isFile() ?? false)
79+
: (lstatSync(file, { throwIfNoEntry: false })?.isFile() ?? false);
80+
}
81+
5782
/**
5883
* Compiles picomatch matchers and POSIX input prefixes for each asset configuration.
5984
*
@@ -62,15 +87,16 @@ function normalizeGlobPattern(pattern: string): string {
6287
*/
6388
export function createAssetMatchers(assets: NormalizedAssetsPattern[]): readonly AssetMatcher[] {
6489
return assets.map((asset) => {
65-
const ignore = [...DEFAULT_ASSET_IGNORE, ...(asset.ignore ?? [])]
90+
const { glob: rawGlob, ignore: rawIgnore, input } = asset;
91+
const ignore = [...DEFAULT_ASSET_IGNORE, ...(rawIgnore ?? [])]
6692
.filter((pattern) => pattern.length > 0)
6793
.map(normalizeGlobPattern);
68-
const isMatch = picomatch(asset.glob ? path.posix.normalize(asset.glob) : [], {
94+
const isMatch = picomatch(rawGlob ? path.posix.normalize(rawGlob) : [], {
6995
dot: true,
7096
windows: false,
7197
ignore,
7298
});
73-
const prefix = asset.input.endsWith('/') ? asset.input : `${asset.input}/`;
99+
const prefix = input.endsWith('/') ? input : `${input}/`;
74100

75101
return { asset, prefix, isMatch };
76102
});
@@ -89,7 +115,7 @@ export function createAssetMatchers(assets: NormalizedAssetsPattern[]): readonly
89115
export async function collectAssetsToEmit(
90116
assets: NormalizedAssetsPattern[],
91117
workspaceRoot: string,
92-
emittedAssets: ReadonlyMap<string, unknown>,
118+
emittedAssets: ReadonlyMap<string, ReadonlySet<string>>,
93119
modifiedFiles?: ReadonlySet<string>,
94120
assetMatchers?: readonly AssetMatcher[],
95121
): Promise<CollectedAssets> {
@@ -108,16 +134,21 @@ export async function collectAssetsToEmit(
108134

109135
if (!emittedAssets.has(file) && emittedAssets.size > 0) {
110136
const dirPrefix = file.endsWith('/') ? file : `${file}/`;
111-
let isDeletedDir: boolean | undefined;
137+
let isDirectory: boolean | undefined;
112138
for (const emittedFile of emittedAssets.keys()) {
113139
if (emittedFile.startsWith(dirPrefix)) {
114-
if (!(isDeletedDir ??= !statSync(file, { throwIfNoEntry: false })?.isDirectory())) {
115-
break;
140+
if (isDirectory === undefined) {
141+
const stats = statSync(file, { throwIfNoEntry: false });
142+
isDirectory = stats?.isDirectory() ?? false;
143+
isFileStat = stats?.isFile() ?? false;
144+
if (isDirectory) {
145+
break;
146+
}
116147
}
117148
filesToDelete.add(emittedFile);
118149
}
119150
}
120-
if (isDeletedDir !== undefined) {
151+
if (isDirectory !== undefined && !isFileStat) {
121152
continue;
122153
}
123154
}
@@ -154,6 +185,84 @@ export async function collectAssetsToEmit(
154185
}
155186
}
156187

188+
if (matchers.length > 1) {
189+
// Ensure lower-priority modified assets do not overwrite existing higher-priority assets on disk.
190+
for (const [outputPath, { file, matcherIndex }] of filesToEmit) {
191+
for (let laterIndex = matchers.length - 1; laterIndex > matcherIndex; laterIndex--) {
192+
const laterMatcher = matchers[laterIndex];
193+
const candidateFile = getCandidateSourceForOutput(laterMatcher, outputPath);
194+
if (!candidateFile || candidateFile === file.source) {
195+
continue;
196+
}
197+
198+
if (isMatchingAssetFile(candidateFile, laterMatcher.asset.followSymlinks)) {
199+
if (emittedAssets.get(candidateFile)?.has(outputPath)) {
200+
filesToEmit.delete(outputPath);
201+
} else {
202+
filesToEmit.set(outputPath, {
203+
file: createDiskOutputFile(candidateFile, outputPath),
204+
matcherIndex: laterIndex,
205+
});
206+
}
207+
break;
208+
} else if (emittedAssets.get(candidateFile)?.has(outputPath)) {
209+
filesToDelete.add(candidateFile);
210+
}
211+
}
212+
}
213+
214+
// Restore lower-priority fallback assets when a higher-priority overriding asset is deleted.
215+
for (const deletedFile of filesToDelete) {
216+
for (
217+
let deletedMatcherIndex = 0;
218+
deletedMatcherIndex < matchers.length;
219+
deletedMatcherIndex++
220+
) {
221+
const { asset, prefix, isMatch } = matchers[deletedMatcherIndex];
222+
if (!deletedFile.startsWith(prefix)) {
223+
continue;
224+
}
225+
226+
const relative = deletedFile.slice(prefix.length);
227+
if (!isMatch(relative)) {
228+
continue;
229+
}
230+
231+
const outputPath = path.posix.join(asset.output, relative);
232+
if (filesToEmit.has(outputPath)) {
233+
continue;
234+
}
235+
236+
for (let candidateIndex = matchers.length - 1; candidateIndex >= 0; candidateIndex--) {
237+
if (candidateIndex === deletedMatcherIndex) {
238+
continue;
239+
}
240+
241+
const candidateMatcher = matchers[candidateIndex];
242+
const candidateFile = getCandidateSourceForOutput(candidateMatcher, outputPath);
243+
if (!candidateFile || filesToDelete.has(candidateFile)) {
244+
continue;
245+
}
246+
247+
if (isMatchingAssetFile(candidateFile, candidateMatcher.asset.followSymlinks)) {
248+
if (
249+
candidateIndex < deletedMatcherIndex ||
250+
!emittedAssets.get(candidateFile)?.has(outputPath)
251+
) {
252+
filesToEmit.set(outputPath, {
253+
file: createDiskOutputFile(candidateFile, outputPath),
254+
matcherIndex: candidateIndex,
255+
});
256+
}
257+
break;
258+
} else if (emittedAssets.get(candidateFile)?.has(outputPath)) {
259+
filesToDelete.add(candidateFile);
260+
}
261+
}
262+
}
263+
}
264+
}
265+
157266
return {
158267
filesToEmit: Array.from(filesToEmit.values(), ({ file }) => file),
159268
filesToDelete: Array.from(filesToDelete),

0 commit comments

Comments
 (0)