Skip to content

Commit f0b045e

Browse files
clydinalan-agius4
clydin
authored and
alan-agius4
committed
refactor(@angular/build): isolate TypeScript diagnostics and AST caching in TypeScriptCompilation
Decouple the AngularCompilation base class from TypeScript diagnostic structures and types, and encapsulate TypeScript-specific compilation logic in an intermediate TypeScriptCompilation class. TypeScriptCompilation extends AngularCompilation to manage the ts.SourceFile AST cache, file invalidation, and TypeScript diagnostic collection and conversion. This allows AngularCompilation.diagnoseFiles to return an empty diagnostics result by default, removing collectDiagnostics, static loadTypescript, and typescript imports from AngularCompilation entirely. AotCompilation and JitCompilation now extend TypeScriptCompilation, sharing unified AST caching and file invalidation logic, while NoopCompilation and ParallelCompilation no longer require dummy collectDiagnostics stubs. In addition, diagnostics.ts is relocated from tools/esbuild/angular/ into tools/angular/compilation/ so TypeScript diagnostic formatting remains strictly encapsulated within the compilation subsystem.
1 parent 4d3b5b7 commit f0b045e

9 files changed

Lines changed: 152 additions & 83 deletions

File tree

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

Lines changed: 3 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,7 @@
88

99
import type * as ng from '@angular/compiler-cli';
1010
import type { PartialMessage } from 'esbuild';
11-
import type ts from 'typescript';
12-
import { convertTypeScriptDiagnostic } from '../../esbuild/angular/diagnostics';
13-
import { profileAsync, profileSync } from '../../esbuild/profiling';
11+
import { profileSync } from '../../esbuild/profiling';
1412
import type { AngularHostOptions } from '../angular-host';
1513

1614
export interface EmitFileResult {
@@ -51,20 +49,13 @@ export enum DiagnosticModes {
5149

5250
export abstract class AngularCompilation {
5351
static #angularCompilerCliModule?: typeof ng;
54-
static #typescriptModule?: typeof ts;
5552

5653
static async loadCompilerCli(): Promise<typeof ng> {
5754
AngularCompilation.#angularCompilerCliModule ??= await import('@angular/compiler-cli');
5855

5956
return AngularCompilation.#angularCompilerCliModule;
6057
}
6158

62-
static async loadTypescript(): Promise<typeof ts> {
63-
AngularCompilation.#typescriptModule ??= await import('typescript');
64-
65-
return AngularCompilation.#typescriptModule;
66-
}
67-
6859
protected async loadConfiguration(tsconfig: string): Promise<ng.CompilerOptions> {
6960
const { readConfiguration } = await AngularCompilation.loadCompilerCli();
7061

@@ -100,40 +91,10 @@ export abstract class AngularCompilation {
10091

10192
transformFile?(filename: string, content: string): Promise<FileTransformResult | null>;
10293

103-
protected collectDiagnostics?(
104-
modes: DiagnosticModes,
105-
): Iterable<ts.Diagnostic> | Promise<Iterable<ts.Diagnostic>>;
106-
10794
async diagnoseFiles(
108-
modes = DiagnosticModes.All,
95+
modes?: DiagnosticModes,
10996
): Promise<{ errors?: PartialMessage[]; warnings?: PartialMessage[] }> {
110-
if (!this.collectDiagnostics) {
111-
return {};
112-
}
113-
114-
const result: { errors?: PartialMessage[]; warnings?: PartialMessage[] } = {};
115-
116-
// Avoid loading typescript until actually needed.
117-
// This allows for avoiding the load of typescript in the main thread when using the parallel compilation.
118-
const typescript = await AngularCompilation.loadTypescript();
119-
120-
await profileAsync('NG_DIAGNOSTICS_TOTAL', async () => {
121-
const diagnostics = await this.collectDiagnostics?.(modes);
122-
if (!diagnostics) {
123-
return;
124-
}
125-
126-
for (const diagnostic of diagnostics) {
127-
const message = convertTypeScriptDiagnostic(typescript, diagnostic);
128-
if (diagnostic.category === typescript.DiagnosticCategory.Error) {
129-
(result.errors ??= []).push(message);
130-
} else {
131-
(result.warnings ??= []).push(message);
132-
}
133-
}
134-
});
135-
136-
return result;
97+
return {};
13798
}
13899

139100
update?(files: Set<string>): Promise<void>;

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

Lines changed: 82 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,14 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9+
import ts from 'typescript';
910
import type { AngularHostOptions } from '../angular-host';
1011
import {
1112
AngularCompilation,
1213
AngularCompilationResult,
14+
DiagnosticModes,
1315
NoopCompilation,
16+
TypeScriptCompilation,
1417
createAngularCompilation,
1518
} from './index';
1619

@@ -68,15 +71,90 @@ describe('AngularCompilation', () => {
6871
expect(result.compilerOptions['customOption']).toBe(true);
6972
});
7073

71-
it('throws when calling collectDiagnostics or emitAffectedFiles', () => {
74+
it('throws when calling emitAffectedFiles', () => {
7275
const compilation = new NoopCompilation();
73-
expect(() =>
74-
(compilation as unknown as { collectDiagnostics(): unknown }).collectDiagnostics(),
75-
).toThrowError('Not available when using noop compilation.');
7676
expect(() => compilation.emitAffectedFiles()).toThrowError(
7777
'Not available when using noop compilation.',
7878
);
7979
});
80+
81+
it('returns empty diagnostics from diagnoseFiles', async () => {
82+
const compilation = new NoopCompilation();
83+
const diagnostics = await compilation.diagnoseFiles();
84+
expect(diagnostics).toEqual({});
85+
});
86+
});
87+
88+
describe('TypeScriptCompilation', () => {
89+
class MockTypeScriptCompilation extends TypeScriptCompilation {
90+
async initialize(): Promise<AngularCompilationResult> {
91+
return { compilerOptions: {}, referencedFiles: [] };
92+
}
93+
94+
protected override *collectDiagnostics(modes: DiagnosticModes): Iterable<ts.Diagnostic> {
95+
if (modes & DiagnosticModes.Option) {
96+
yield {
97+
category: ts.DiagnosticCategory.Error,
98+
code: 1234,
99+
messageText: 'Mock option error',
100+
file: undefined,
101+
start: undefined,
102+
length: undefined,
103+
};
104+
}
105+
if (modes & DiagnosticModes.Semantic) {
106+
yield {
107+
category: ts.DiagnosticCategory.Warning,
108+
code: 5678,
109+
messageText: 'Mock semantic warning',
110+
file: undefined,
111+
start: undefined,
112+
length: undefined,
113+
};
114+
}
115+
}
116+
117+
public getCachedSourceFiles(): Map<string, ts.SourceFile> {
118+
return this.sourceFiles;
119+
}
120+
}
121+
122+
it('collects and converts diagnostics categorized by error and warning', async () => {
123+
const compilation = new MockTypeScriptCompilation();
124+
const diagnostics = await compilation.diagnoseFiles(DiagnosticModes.All);
125+
126+
expect(diagnostics.errors?.length).toBe(1);
127+
expect(diagnostics.errors?.[0].text).toContain('Mock option error');
128+
expect(diagnostics.warnings?.length).toBe(1);
129+
expect(diagnostics.warnings?.[0].text).toContain('Mock semantic warning');
130+
});
131+
132+
it('filters diagnostics according to requested DiagnosticModes', async () => {
133+
const compilation = new MockTypeScriptCompilation();
134+
const diagnostics = await compilation.diagnoseFiles(DiagnosticModes.Option);
135+
136+
expect(diagnostics.errors?.length).toBe(1);
137+
expect(diagnostics.errors?.[0].text).toContain('Mock option error');
138+
expect(diagnostics.warnings).toBeUndefined();
139+
});
140+
141+
it('returns empty diagnostics immediately when mode is DiagnosticModes.None', async () => {
142+
const compilation = new MockTypeScriptCompilation();
143+
const diagnostics = await compilation.diagnoseFiles(DiagnosticModes.None);
144+
145+
expect(diagnostics).toEqual({});
146+
});
147+
148+
it('evicts files from AST cache on update and invalidateFiles', async () => {
149+
const compilation = new MockTypeScriptCompilation();
150+
const mockSourceFile = ts.createSourceFile('test.ts', '', ts.ScriptTarget.Latest);
151+
compilation.getCachedSourceFiles().set('/src/test.ts', mockSourceFile);
152+
153+
expect(compilation.getCachedSourceFiles().has('/src/test.ts')).toBeTrue();
154+
155+
await compilation.update?.(new Set(['/src/test.ts']));
156+
expect(compilation.getCachedSourceFiles().has('/src/test.ts')).toBeFalse();
157+
});
80158
});
81159

82160
describe('createAngularCompilation', () => {

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

Lines changed: 5 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import assert from 'node:assert';
1111
import { relative } from 'node:path';
1212
import ts from 'typescript';
1313
import { useTypeChecking } from '../../../utils/environment-options';
14-
import { toPosixPath } from '../../../utils/path';
1514
import { profileAsync, profileSync } from '../../esbuild/profiling';
1615
import {
1716
AngularHostOptions,
@@ -28,6 +27,7 @@ import {
2827
EmitFileResult,
2928
} from './angular-compilation';
3029
import { collectHmrCandidates } from './hmr-candidates';
30+
import { TypeScriptCompilation } from './typescript-compilation';
3131
import { printSourceFileWithMap } from './typescript-printer';
3232

3333
/**
@@ -54,9 +54,8 @@ class AngularCompilationState {
5454
}
5555
}
5656

57-
export class AotCompilation extends AngularCompilation {
57+
export class AotCompilation extends TypeScriptCompilation {
5858
#state?: AngularCompilationState;
59-
readonly #sourceFiles = new Map<string, ts.SourceFile>();
6059

6160
constructor(private readonly browserOnlyBuild: boolean) {
6261
super();
@@ -100,9 +99,9 @@ export class AotCompilation extends AngularCompilation {
10099
let staleSourceFiles;
101100
let clearPackageJsonCache = false;
102101
if (hostOptions.modifiedFiles) {
103-
for (const modifiedFile of hostOptions.modifiedFiles) {
104-
this.#sourceFiles.delete(toPosixPath(modifiedFile));
102+
this.invalidateFiles(hostOptions.modifiedFiles);
105103

104+
for (const modifiedFile of hostOptions.modifiedFiles) {
106105
if (this.#state) {
107106
// Clear package.json cache if a node modules file was modified
108107
if (!clearPackageJsonCache && modifiedFile.includes('node_modules')) {
@@ -128,7 +127,7 @@ export class AotCompilation extends AngularCompilation {
128127
compilerOptions,
129128
hostOptions,
130129
packageJsonCache,
131-
this.#sourceFiles,
130+
this.sourceFiles,
132131
);
133132

134133
// Create the Angular specific program that contains the Angular compiler
@@ -463,12 +462,6 @@ export class AotCompilation extends AngularCompilation {
463462

464463
return emittedFiles.values();
465464
}
466-
467-
override async update(files: Set<string>): Promise<void> {
468-
for (const file of files) {
469-
this.#sourceFiles.delete(toPosixPath(file));
470-
}
471-
}
472465
}
473466

474467
function findAffectedFiles(

packages/angular/build/src/tools/esbuild/angular/diagnostics.ts renamed to packages/angular/build/src/tools/angular/compilation/diagnostics.ts

File renamed without changes.

‎packages/angular/build/src/tools/angular/compilation/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,4 @@ export {
1616
} from './angular-compilation';
1717
export { createAngularCompilation, type AngularCompilationMode } from './factory';
1818
export { NoopCompilation } from './noop-compilation';
19+
export { TypeScriptCompilation } from './typescript-compilation';

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

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import type * as ng from '@angular/compiler-cli';
1010
import assert from 'node:assert';
1111
import ts from 'typescript';
12-
import { toPosixPath } from '../../../utils/path';
1312
import { profileSync } from '../../esbuild/profiling';
1413
import { AngularHostOptions, createAngularCompilerHost } from '../angular-host';
1514
import { createJitResourceTransformer } from '../transformers/jit-resource-transformer';
@@ -21,6 +20,7 @@ import {
2120
DiagnosticModes,
2221
EmitFileResult,
2322
} from './angular-compilation';
23+
import { TypeScriptCompilation } from './typescript-compilation';
2424

2525
class JitCompilationState {
2626
constructor(
@@ -32,9 +32,8 @@ class JitCompilationState {
3232
) {}
3333
}
3434

35-
export class JitCompilation extends AngularCompilation {
35+
export class JitCompilation extends TypeScriptCompilation {
3636
#state?: JitCompilationState;
37-
readonly #sourceFiles = new Map<string, ts.SourceFile>();
3837

3938
constructor(private readonly browserOnlyBuild: boolean) {
4039
super();
@@ -59,9 +58,7 @@ export class JitCompilation extends AngularCompilation {
5958
compilerOptionsTransformer?.(originalCompilerOptions) ?? originalCompilerOptions;
6059

6160
if (hostOptions.modifiedFiles) {
62-
for (const modifiedFile of hostOptions.modifiedFiles) {
63-
this.#sourceFiles.delete(toPosixPath(modifiedFile));
64-
}
61+
this.invalidateFiles(hostOptions.modifiedFiles);
6562
}
6663

6764
// Create Angular compiler host
@@ -70,7 +67,7 @@ export class JitCompilation extends AngularCompilation {
7067
compilerOptions,
7168
hostOptions,
7269
undefined,
73-
this.#sourceFiles,
70+
this.sourceFiles,
7471
);
7572

7673
// Create the TypeScript Program
@@ -167,10 +164,4 @@ export class JitCompilation extends AngularCompilation {
167164

168165
return emittedFiles;
169166
}
170-
171-
override async update(files: Set<string>): Promise<void> {
172-
for (const file of files) {
173-
this.#sourceFiles.delete(toPosixPath(file));
174-
}
175-
}
176167
}

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

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,6 @@ export class NoopCompilation extends AngularCompilation {
2424
return { compilerOptions, referencedFiles: [] };
2525
}
2626

27-
protected override collectDiagnostics(): never {
28-
throw new Error('Not available when using noop compilation.');
29-
}
30-
3127
override emitAffectedFiles(): never {
3228
throw new Error('Not available when using noop compilation.');
3329
}

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

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,14 +133,6 @@ export class ParallelCompilation extends AngularCompilation {
133133
}
134134
}
135135

136-
/**
137-
* This is not needed with this compilation type since the worker will already send a response
138-
* with the serializable esbuild compatible diagnostics.
139-
*/
140-
protected override collectDiagnostics(): never {
141-
throw new Error('Not implemented in ParallelCompilation.');
142-
}
143-
144136
override async diagnoseFiles(
145137
modes = DiagnosticModes.All,
146138
): Promise<{ errors?: PartialMessage[]; warnings?: PartialMessage[] }> {

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.dev/license
7+
*/
8+
9+
import type { PartialMessage } from 'esbuild';
10+
import ts from 'typescript';
11+
import { toPosixPath } from '../../../utils/path';
12+
import { profileAsync } from '../../esbuild/profiling';
13+
import { AngularCompilation, DiagnosticModes } from './angular-compilation';
14+
import { convertTypeScriptDiagnostic } from './diagnostics';
15+
16+
export abstract class TypeScriptCompilation extends AngularCompilation {
17+
protected readonly sourceFiles = new Map<string, ts.SourceFile>();
18+
19+
protected invalidateFiles(files: Iterable<string>): void {
20+
for (const file of files) {
21+
this.sourceFiles.delete(toPosixPath(file));
22+
}
23+
}
24+
25+
override async update(files: Set<string>): Promise<void> {
26+
this.invalidateFiles(files);
27+
}
28+
29+
protected abstract collectDiagnostics(
30+
modes: DiagnosticModes,
31+
): Iterable<ts.Diagnostic> | Promise<Iterable<ts.Diagnostic>>;
32+
33+
override async diagnoseFiles(
34+
modes = DiagnosticModes.All,
35+
): Promise<{ errors?: PartialMessage[]; warnings?: PartialMessage[] }> {
36+
if (modes === DiagnosticModes.None) {
37+
return {};
38+
}
39+
40+
const result: { errors?: PartialMessage[]; warnings?: PartialMessage[] } = {};
41+
42+
await profileAsync('NG_DIAGNOSTICS_TOTAL', async () => {
43+
const diagnostics = await this.collectDiagnostics(modes);
44+
45+
for (const diagnostic of diagnostics) {
46+
const message = convertTypeScriptDiagnostic(ts, diagnostic);
47+
if (diagnostic.category === ts.DiagnosticCategory.Error) {
48+
(result.errors ??= []).push(message);
49+
} else {
50+
(result.warnings ??= []).push(message);
51+
}
52+
}
53+
});
54+
55+
return result;
56+
}
57+
}

0 commit comments

Comments
 (0)