Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 20 additions & 18 deletions packages/angular/build/src/builders/library/builder-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -417,33 +417,35 @@ 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);
}
outputs.add(emittedAsset.path);
}

if (toDelete.length) {
const candidateDeletes = new Set<string>();
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);
}
}
Comment on lines 418 to 450

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of looping over this.#emittedAssets for every item in toEmit, we can keep a Map<string, string> from output path to source (for example this.#emittedAssetSources) to look up and remove the previous source directly:

    for (const emittedAsset of toEmit) {
      let outputs = this.#emittedAssets.get(emittedAsset.source);
      if (outputs?.has(emittedAsset.path)) {
        continue;
      }

      const prevSource = this.#emittedAssetSources.get(emittedAsset.path);
      if (prevSource !== undefined) {
        const prevOutputs = this.#emittedAssets.get(prevSource);
        if (prevOutputs) {
          prevOutputs.delete(emittedAsset.path);
          if (prevOutputs.size === 0) {
            this.#emittedAssets.delete(prevSource);
          }
        }
      }

      if (!outputs) {
        outputs = new Set();
        this.#emittedAssets.set(emittedAsset.source, outputs);
      }
      outputs.add(emittedAsset.path);
      this.#emittedAssetSources.set(emittedAsset.path, emittedAsset.source);
    }

    for (const source of toDelete) {
      const outputs = this.#emittedAssets.get(source);
      if (outputs) {
        for (const output of outputs) {
          this.#emittedAssetSources.delete(output);
          filesToDelete.add(output);
        }
        this.#emittedAssets.delete(source);
      }
    }


Expand Down
33 changes: 26 additions & 7 deletions packages/angular/build/src/builders/library/builder-state_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
127 changes: 118 additions & 9 deletions packages/angular/build/src/builders/library/pipeline/assets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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('*')) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can drop pattern.endsWith('/') || here so that patterns ending with */ (like foo/*/) do not become foo/*/**:

Suggested change
if (pattern.endsWith('/') || !result.endsWith('*')) {
if (!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.
*
Expand All @@ -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 };
});
Expand All @@ -89,7 +115,7 @@ export function createAssetMatchers(assets: NormalizedAssetsPattern[]): readonly
export async function collectAssetsToEmit(
assets: NormalizedAssetsPattern[],
workspaceRoot: string,
emittedAssets: ReadonlyMap<string, unknown>,
emittedAssets: ReadonlyMap<string, ReadonlySet<string>>,
modifiedFiles?: ReadonlySet<string>,
assetMatchers?: readonly AssetMatcher[],
): Promise<CollectedAssets> {
Expand All @@ -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;
}
}
Expand Down Expand Up @@ -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);
}
}
}
}
}
Comment on lines +188 to +264

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be simplified quite a bit. In watch mode, emittedAssets is already populated from the initial build, and any created or deleted files are already in modifiedFiles.

In the first loop, we only need to check emittedAssets and filesToDelete in memory instead of checking the disk with isMatchingAssetFile.

In the second loop, emittedAssets.get(deletedFile) already has the output paths for deletedFile, so we can loop over those directly instead of matching deletedFile against matchers again.

Suggested change
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);
}
}
}
}
}
if (matchers.length > 1) {
// Ensure lower priority modified assets do not overwrite existing higher priority assets.
for (const [outputPath, { matcherIndex }] of filesToEmit) {
for (let laterIndex = matchers.length - 1; laterIndex > matcherIndex; laterIndex--) {
const candidateFile = getCandidateSourceForOutput(matchers[laterIndex], outputPath);
if (
candidateFile &&
!filesToDelete.has(candidateFile) &&
emittedAssets.get(candidateFile)?.has(outputPath)
) {
filesToEmit.delete(outputPath);
break;
}
}
}
// Restore lower priority fallback assets when a higher priority overriding asset is deleted.
for (const deletedFile of filesToDelete) {
const deletedOutputs = emittedAssets.get(deletedFile);
if (!deletedOutputs) {
continue;
}
for (const outputPath of deletedOutputs) {
if (filesToEmit.has(outputPath)) {
continue;
}
for (let candidateIndex = matchers.length - 1; candidateIndex >= 0; candidateIndex--) {
const candidateMatcher = matchers[candidateIndex];
const candidateFile = getCandidateSourceForOutput(candidateMatcher, outputPath);
if (!candidateFile || filesToDelete.has(candidateFile)) {
continue;
}
if (isMatchingAssetFile(candidateFile, candidateMatcher.asset.followSymlinks)) {
filesToEmit.set(outputPath, {
file: createDiskOutputFile(candidateFile, outputPath),
matcherIndex: candidateIndex,
});
break;
}
}
}
}
}


return {
filesToEmit: Array.from(filesToEmit.values(), ({ file }) => file),
filesToDelete: Array.from(filesToDelete),
Expand Down
Loading
Loading