Skip to content

Commit 2db3151

Browse files
committed
fixup! perf(@angular/build): optimize incremental watch rebuilds in angular compilation
1 parent 0912b99 commit 2db3151

2 files changed

Lines changed: 201 additions & 7 deletions

File tree

‎packages/angular/build/src/builders/application/tests/behavior/rebuild-errors_spec.ts‎

Lines changed: 111 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,6 @@ import {
1515
expectNoLog,
1616
} from '../setup';
1717

18-
/**
19-
* Maximum time in milliseconds for single build/rebuild
20-
* This accounts for CI variability.
21-
*/
22-
export const BUILD_TIMEOUT = 30_000;
23-
2418
describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
2519
describe('Behavior: "Rebuild Error Detection"', () => {
2620
it('detects template errors with no AOT codegen or TS emit differences', async () => {
@@ -128,6 +122,116 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
128122
);
129123
});
130124

125+
it('detects template errors across components on watch rebuild when isolatedModules is enabled', async () => {
126+
harness.useTarget('build', {
127+
...BASE_OPTIONS,
128+
watch: true,
129+
});
130+
131+
await harness.modifyFile('tsconfig.json', (content) => {
132+
const tsconfig = JSON.parse(content);
133+
tsconfig.compilerOptions = {
134+
...tsconfig.compilerOptions,
135+
isolatedModules: true,
136+
};
137+
138+
return JSON.stringify(tsconfig);
139+
});
140+
141+
const goodChildComponentContents = `
142+
import { Component, Input } from '@angular/core';
143+
@Component({
144+
selector: 'child',
145+
standalone: true,
146+
template: '<p>{{ value }}</p>',
147+
})
148+
export class ChildComponent {
149+
@Input() value!: number;
150+
}
151+
`;
152+
153+
const typeErrorText = `Type 'number' is not assignable to type 'string'.`;
154+
155+
await harness.writeFiles({
156+
'src/app/child.component.ts': goodChildComponentContents,
157+
'src/app/app.module.ts': `
158+
import { NgModule } from '@angular/core';
159+
import { BrowserModule } from '@angular/platform-browser';
160+
import { AppComponent } from './app.component';
161+
@NgModule({
162+
imports: [
163+
BrowserModule,
164+
AppComponent,
165+
],
166+
providers: [],
167+
bootstrap: [AppComponent]
168+
})
169+
export class AppModule { }
170+
`,
171+
'src/app/app.component.ts': `
172+
import { Component } from '@angular/core';
173+
import { ChildComponent } from './child.component';
174+
@Component({
175+
selector: 'app-root',
176+
standalone: true,
177+
imports: [ChildComponent],
178+
template: '<child [value]="123" />',
179+
})
180+
export class AppComponent {}
181+
`,
182+
});
183+
184+
await harness.executeWithCases(
185+
[
186+
async ({ result }) => {
187+
expect(result?.success).toBeTrue();
188+
189+
// Update child component to change input type from number to string
190+
await harness.writeFile(
191+
'src/app/child.component.ts',
192+
`
193+
import { Component, Input } from '@angular/core';
194+
@Component({
195+
selector: 'child',
196+
standalone: true,
197+
template: '<p>{{ value }}</p>',
198+
})
199+
export class ChildComponent {
200+
@Input() value!: string;
201+
}
202+
`,
203+
);
204+
},
205+
async ({ result, logs }) => {
206+
expect(result?.success).toBeFalse();
207+
expectLog(logs, typeErrorText);
208+
209+
// Make an unrelated change to verify error persists
210+
await harness.modifyFile('src/main.ts', (content) => content + '\n');
211+
},
212+
async ({ result, logs }) => {
213+
expect(result?.success).toBeFalse();
214+
expectLog(logs, typeErrorText);
215+
216+
// Revert back to number
217+
await harness.writeFile('src/app/child.component.ts', goodChildComponentContents);
218+
},
219+
async ({ result, logs }) => {
220+
expect(result?.success).toBeTrue();
221+
expectNoLog(logs, typeErrorText);
222+
223+
// Make an unrelated change to verify error cache cleared
224+
await harness.modifyFile('src/main.ts', (content) => content + '\n');
225+
},
226+
({ result, logs }) => {
227+
expect(result?.success).toBeTrue();
228+
expectNoLog(logs, typeErrorText);
229+
},
230+
],
231+
{ outputLogsOnFailure: false },
232+
);
233+
});
234+
131235
it('detects cumulative block syntax errors', async () => {
132236
harness.useTarget('build', {
133237
...BASE_OPTIONS,
@@ -234,7 +338,7 @@ describeBuilder(buildApplication, APPLICATION_BUILDER_INFO, (harness) => {
234338
await harness.appendToFile('src/app/app.component.html', '<div>Guten Tag</div>');
235339
},
236340
({ logs }) => {
237-
expectNoLog(logs, 'invalid-css-content');
341+
expectNoLog(logs, 'Unexpected character "EOF"');
238342

239343
harness.expectFile('dist/browser/main.js').content.toContain('Hello, world!');
240344
harness.expectFile('dist/browser/main.js').content.toContain('Guten Tag');

‎packages/angular/build/src/tools/angular/compilation/aot-compilation.ts‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,10 @@ export class AotCompilation extends TypeScriptCompilation {
240240
!/\.(?:css|scss|sass|less)$/i.test(resourceDependency)
241241
) {
242242
this.#state.diagnosticCache.delete(sourceFile);
243+
const oldSourceFile = this.#state.typeScriptProgram.getSourceFile(sourceFile.fileName);
244+
if (oldSourceFile) {
245+
this.#state.diagnosticCache.delete(oldSourceFile);
246+
}
243247
// Also mark as affected in case changed template affects diagnostics
244248
affectedFiles.add(sourceFile);
245249
}
@@ -567,6 +571,88 @@ function findAffectedFiles(
567571
}
568572
}
569573

574+
// When `isolatedModules` or `verbatimModuleSyntax` is enabled, TypeScript's builder skips
575+
// walking `referencedMap` for affected files. As a result, TTC shims (`.ngtypecheck.ts`) that depend
576+
// on modified components or directives are not returned by `getSemanticDiagnosticsOfNextAffectedFile`.
577+
// Propagate affected status through `referencedMap` to identify dependent TTC shims.
578+
const compilerOptions = builder.getCompilerOptions();
579+
if (
580+
(compilerOptions.isolatedModules || compilerOptions.verbatimModuleSyntax) &&
581+
affectedFiles.size > 0
582+
) {
583+
const internalBuilder = builder as InternalBuilderProgram;
584+
const referencedMap = internalBuilder.state?.referencedMap;
585+
586+
if (referencedMap) {
587+
const queue: string[] = [];
588+
const seen = new Set<string>();
589+
590+
for (const sf of affectedFiles) {
591+
const resolvedPath =
592+
(sf as ts.SourceFile & { resolvedPath?: string }).resolvedPath ?? sf.fileName;
593+
queue.push(resolvedPath);
594+
seen.add(resolvedPath);
595+
}
596+
597+
while (queue.length > 0) {
598+
const currentPath = queue.pop();
599+
if (!currentPath) {
600+
break;
601+
}
602+
603+
const dependents = referencedMap.getKeys(currentPath);
604+
if (!dependents) {
605+
continue;
606+
}
607+
608+
for (const depPath of dependents) {
609+
if (seen.has(depPath)) {
610+
continue;
611+
}
612+
613+
seen.add(depPath);
614+
615+
if (depPath.endsWith('.ngtypecheck.ts')) {
616+
const originalFilename = depPath.slice(0, -15) + '.ts';
617+
const originalSourceFile = builder.getSourceFile(originalFilename);
618+
if (originalSourceFile) {
619+
affectedFiles.add(originalSourceFile);
620+
}
621+
} else {
622+
queue.push(depPath);
623+
}
624+
}
625+
}
626+
} else if (typeof builder.getAllDependencies === 'function') {
627+
const affectedPathSet = new Set<string>();
628+
for (const sf of affectedFiles) {
629+
affectedPathSet.add(sf.fileName);
630+
const resolvedPath = (sf as ts.SourceFile & { resolvedPath?: string }).resolvedPath;
631+
if (resolvedPath) {
632+
affectedPathSet.add(resolvedPath);
633+
}
634+
}
635+
636+
for (const sourceFile of builder.getSourceFiles()) {
637+
if (
638+
!sourceFile.fileName.endsWith('.ngtypecheck.ts') ||
639+
!ignoreForDiagnostics.has(sourceFile)
640+
) {
641+
continue;
642+
}
643+
644+
const deps = builder.getAllDependencies(sourceFile);
645+
if (deps.some((dep) => affectedPathSet.has(dep))) {
646+
const originalFilename = sourceFile.fileName.slice(0, -15) + '.ts';
647+
const originalSourceFile = builder.getSourceFile(originalFilename);
648+
if (originalSourceFile) {
649+
affectedFiles.add(originalSourceFile);
650+
}
651+
}
652+
}
653+
}
654+
}
655+
570656
return affectedFiles;
571657
}
572658

@@ -576,6 +662,10 @@ interface InternalBuilderProgram extends ts.EmitAndSemanticDiagnosticsBuilderPro
576662
changedFilesSet?: Set<string>;
577663
oldSignatures?: Map<string, string | false>;
578664
hasCalledUpdateShapeSignature?: Set<string>;
665+
referencedMap?: {
666+
getKeys(path: string): Set<string> | undefined;
667+
getValues(path: string): Set<string> | undefined;
668+
};
579669
};
580670
}
581671

0 commit comments

Comments
 (0)