Skip to content

Commit 4965b1d

Browse files
committed
perf(@angular/build): overlap library asset resolution and deduplicate output mkdir calls
- Start `buildState.collectAssets(options, modifiedFiles)` before `compileLibrary` so asset globbing overlaps with TypeScript/Angular compilation, Rolldown bundling, and manifest generation. - Execute `deleteFilesFromDisk` and `emitFilesToDisk` concurrently via `Promise.all` since deleted and emitted paths are disjoint. - Cache ancestor directory promises created by `mkdir(..., { recursive: true })` in `emitFilesToDisk` so parent directories (such as `outputPath`) do not incur redundant `mkdir` syscalls when subdirectories (`fesm2022`, `types`) are created first. - Return early when `result.hasErrors` is true before adding `bundleOutput.warnings` so secondary bundler warnings are not logged alongside compilation errors. - Include `options.packageJsonPath` in the error message when `verifyAllowedDependencies` fails.
1 parent d609a21 commit 4965b1d

5 files changed

Lines changed: 37 additions & 16 deletions

File tree

‎packages/angular/build/src/builders/library/pipeline/build-action.ts‎

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,13 @@ export async function buildAction(actionContext: BuildActionContext): Promise<Ex
7171
return result;
7272
}
7373

74+
const collectedAssetsPromise =
75+
actionContext.collectedAssets ?? buildState.collectAssets(options, modifiedFiles);
76+
if (collectedAssetsPromise instanceof Promise) {
77+
// Prevent unhandled promise rejections if compilation or bundling returns early.
78+
collectedAssetsPromise.catch(() => {});
79+
}
80+
7481
const filesToEmit: OutputFile[] = [];
7582

7683
if (shouldCompileEntryPoints) {
@@ -127,10 +134,10 @@ export async function buildAction(actionContext: BuildActionContext): Promise<Ex
127134
return result;
128135
}
129136

130-
result.addWarnings(bundleOutput.warnings);
131137
if (result.hasErrors) {
132138
return result;
133139
}
140+
result.addWarnings(bundleOutput.warnings);
134141

135142
filesToEmit.push(...bundleOutput.filesToEmit);
136143
buildState.recordBundleOutput(bundleOutput.bundleResults);
@@ -151,19 +158,17 @@ export async function buildAction(actionContext: BuildActionContext): Promise<Ex
151158
filesToEmit.push(...manifestFiles);
152159
}
153160

154-
const collectedAssets =
155-
actionContext.collectedAssets ?? (await buildState.collectAssets(options, modifiedFiles));
161+
const collectedAssets = await collectedAssetsPromise;
156162
filesToEmit.push(...collectedAssets.filesToEmit);
157163

158-
await emitFilesToDisk(filesToEmit, (file) => path.posix.join(options.outputPath, file.path));
159-
160164
const filesToDelete = buildState.recordEmittedOutputs(collectedAssets, manifestFiles);
161165

162-
if (filesToDelete.size) {
163-
await deleteFilesFromDisk(
166+
await Promise.all([
167+
deleteFilesFromDisk(
164168
Array.from(filesToDelete, (file) => path.posix.join(options.outputPath, file)),
165-
);
166-
}
169+
),
170+
emitFilesToDisk(filesToEmit, (file) => path.posix.join(options.outputPath, file.path)),
171+
]);
167172
} catch (error) {
168173
assertIsError(error);
169174
result.addError(error.message);
@@ -184,7 +189,7 @@ function verifyAllowedDependencies(
184189
options: NormalizedLibraryOptions,
185190
result: ExecutionResult,
186191
): boolean {
187-
const { packageJson, allowedNonPeerDependencies } = options;
192+
const { packageJson, packageJsonPath, allowedNonPeerDependencies } = options;
188193
const dependencies = {
189194
...(packageJson.dependencies ?? {}),
190195
...(packageJson.optionalDependencies ?? {}),
@@ -195,7 +200,7 @@ function verifyAllowedDependencies(
195200
if (!allowedNonPeerDependencies.some((regex) => regex.test(dep))) {
196201
result.addError(
197202
`Dependency '${dep}' must be explicitly allowed using the 'allowedNonPeerDependencies' option, ` +
198-
`or moved to 'peerDependencies' in 'package.json'.`,
203+
`or moved to 'peerDependencies' in '${packageJsonPath}'.`,
199204
);
200205
isValid = false;
201206
}

‎packages/angular/build/src/builders/library/tests/behavior/build_spec.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ describeLibraryBuilder(executeLibraryBuilder, LIBRARY_BUILDER_INFO, (harness) =>
9292

9393
const { result, logs } = await harness.executeOnce();
9494
expect(result?.success).toBeTrue();
95-
expect(logs.some((l) => l.level === 'warn')).toBeFalse();
95+
expect(logs).not.toContain(jasmine.objectContaining({ level: 'warn' }));
9696
});
9797

9898
it('should preserve JSDoc comments in .d.ts and annotation comments in .mjs bundles', async () => {
@@ -210,10 +210,13 @@ describeLibraryBuilder(executeLibraryBuilder, LIBRARY_BUILDER_INFO, (harness) =>
210210
);
211211
});
212212

213-
it('should log failure on build error', async () => {
213+
it('should log failure on build error without secondary bundler warnings', async () => {
214214
await harness.writeFile(
215215
'projects/lib/src/public-api.ts',
216-
`export const title: number = 'invalid type';`,
216+
`
217+
export const title: number = 'invalid type';
218+
export function runDynamic(code: string) { return eval(code); }
219+
`,
217220
);
218221

219222
harness.useTarget('build', {
@@ -236,6 +239,7 @@ describeLibraryBuilder(executeLibraryBuilder, LIBRARY_BUILDER_INFO, (harness) =>
236239
),
237240
}),
238241
);
242+
expect(logs).not.toContain(jasmine.objectContaining({ level: 'warn' }));
239243
expect(logs).not.toContain(
240244
jasmine.objectContaining({
241245
message: jasmine.stringMatching(/Output location:/),

‎packages/angular/build/src/builders/library/tests/options/allowed-non-peer-dependencies_spec.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@ describeLibraryBuilder(executeLibraryBuilder, LIBRARY_BUILDER_INFO, (harness) =>
2929
expect(logs).toContain(
3030
jasmine.objectContaining({
3131
level: 'error',
32-
message: jasmine.stringMatching(/lodash-es.*allowedNonPeerDependencies/),
32+
message: jasmine.stringMatching(
33+
/lodash-es.*allowedNonPeerDependencies.*projects\/lib\/package\.json/,
34+
),
3335
}),
3436
);
3537
});

‎packages/angular/build/src/utils/disk-files.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,13 @@ export async function emitFilesToDisk<T extends EmittableFile = EmittableFile>(
5050
let mkdirPromise = directoryCreationPromises.get(fileBasePath);
5151
if (!mkdirPromise) {
5252
mkdirPromise = mkdir(fileBasePath, { recursive: true });
53-
directoryCreationPromises.set(fileBasePath, mkdirPromise);
53+
for (
54+
let currentDir = fileBasePath;
55+
!directoryCreationPromises.has(currentDir);
56+
currentDir = dirname(currentDir)
57+
) {
58+
directoryCreationPromises.set(currentDir, mkdirPromise);
59+
}
5460
}
5561
await mkdirPromise;
5662
}

‎packages/angular/build/src/utils/disk-files_spec.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ describe('disk-files', () => {
3939
contents: new TextEncoder().encode('mem-1'),
4040
},
4141
{ origin: 'memory', path: 'nested/a/mem2.txt', contents: 'mem-2' },
42+
{ origin: 'memory', path: 'nested/parent.txt', contents: 'parent-mem' },
43+
{ origin: 'memory', path: 'root.txt', contents: 'root-mem' },
4244
{ origin: 'disk', path: 'nested/b/disk1.txt', source: sourceFile },
4345
{ origin: 'disk', path: 'nested/c/disk2.txt', source: sourceFile },
4446
{ origin: 'memory', path: 'skipped/file.txt', contents: 'should-not-exist' },
@@ -51,6 +53,8 @@ describe('disk-files', () => {
5153

5254
expect(await readFile(join(outDir, 'nested/a/mem1.txt'), 'utf-8')).toBe('mem-1');
5355
expect(await readFile(join(outDir, 'nested/a/mem2.txt'), 'utf-8')).toBe('mem-2');
56+
expect(await readFile(join(outDir, 'nested/parent.txt'), 'utf-8')).toBe('parent-mem');
57+
expect(await readFile(join(outDir, 'root.txt'), 'utf-8')).toBe('root-mem');
5458
expect(await readFile(join(outDir, 'nested/b/disk1.txt'), 'utf-8')).toBe('from-disk');
5559
expect(await readFile(join(outDir, 'nested/c/disk2.txt'), 'utf-8')).toBe('from-disk');
5660
expect(existsSync(join(outDir, 'skipped/file.txt'))).toBeFalse();

0 commit comments

Comments
 (0)