Skip to content

Commit 016a7f1

Browse files
authored
perf(@angular/build): overlap library asset resolution with bundling and typechecking
- Run `buildState.collectAssets(options, modifiedFiles)` inside `Promise.all` alongside `bundleEntryPoints` and `diagnosePromise` so filesystem asset resolution overlaps with Rolldown bundling and worker-thread typecheck diagnostics. - Delete stale files with `deleteFilesFromDisk` before emitting updated files with `emitFilesToDisk` so case-only renames on case-insensitive filesystems do not remove newly emitted files. - Check `result.hasErrors` before adding `bundleOutput.warnings` so secondary bundler warnings are not logged when compilation fails. - Include `options.packageJsonPath` in the disallowed dependency error message in `verifyAllowedDependencies`.
1 parent dd09534 commit 016a7f1

3 files changed

Lines changed: 18 additions & 12 deletions

File tree

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

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

74+
let collectedAssets = actionContext.collectedAssets;
7475
const filesToEmit: OutputFile[] = [];
7576

7677
if (shouldCompileEntryPoints) {
@@ -102,7 +103,7 @@ export async function buildAction(actionContext: BuildActionContext): Promise<Ex
102103
const itemsToBundle = collectEntryPointsToBundle(options, buildState);
103104
let bundleOutput: BundleEntryPointsOutput;
104105
try {
105-
[bundleOutput] = await Promise.all([
106+
[bundleOutput, , collectedAssets] = await Promise.all([
106107
bundleEntryPoints(
107108
itemsToBundle,
108109
esmFiles,
@@ -115,6 +116,7 @@ export async function buildAction(actionContext: BuildActionContext): Promise<Ex
115116
buildState.previousBundleResults,
116117
),
117118
diagnosePromise,
119+
collectedAssets ?? buildState.collectAssets(options, modifiedFiles),
118120
]);
119121
} catch (error) {
120122
assertIsError(error);
@@ -151,19 +153,17 @@ export async function buildAction(actionContext: BuildActionContext): Promise<Ex
151153
filesToEmit.push(...manifestFiles);
152154
}
153155

154-
const collectedAssets =
155-
actionContext.collectedAssets ?? (await buildState.collectAssets(options, modifiedFiles));
156+
collectedAssets ??= await buildState.collectAssets(options, modifiedFiles);
156157
filesToEmit.push(...collectedAssets.filesToEmit);
157158

158-
await emitFilesToDisk(filesToEmit, (file) => path.posix.join(options.outputPath, file.path));
159-
160159
const filesToDelete = buildState.recordEmittedOutputs(collectedAssets, manifestFiles);
161-
162160
if (filesToDelete.size) {
163161
await deleteFilesFromDisk(
164162
Array.from(filesToDelete, (file) => path.posix.join(options.outputPath, file)),
165163
);
166164
}
165+
166+
await emitFilesToDisk(filesToEmit, (file) => path.posix.join(options.outputPath, file.path));
167167
} catch (error) {
168168
assertIsError(error);
169169
result.addError(error.message);
@@ -184,7 +184,7 @@ function verifyAllowedDependencies(
184184
options: NormalizedLibraryOptions,
185185
result: ExecutionResult,
186186
): boolean {
187-
const { packageJson, allowedNonPeerDependencies } = options;
187+
const { packageJson, packageJsonPath, allowedNonPeerDependencies } = options;
188188
const dependencies = {
189189
...(packageJson.dependencies ?? {}),
190190
...(packageJson.optionalDependencies ?? {}),
@@ -195,7 +195,7 @@ function verifyAllowedDependencies(
195195
if (!allowedNonPeerDependencies.some((regex) => regex.test(dep))) {
196196
result.addError(
197197
`Dependency '${dep}' must be explicitly allowed using the 'allowedNonPeerDependencies' option, ` +
198-
`or moved to 'peerDependencies' in 'package.json'.`,
198+
`or moved to 'peerDependencies' in '${packageJsonPath}'.`,
199199
);
200200
isValid = false;
201201
}

‎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 with 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).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
});

0 commit comments

Comments
 (0)