8000 fix(compiler-cli): avoid duplicate diagnostics about unknown pipes (#… · angular/angular@861e4fa · GitHub
[go: up one dir, main page]

Skip to content

Commit 861e4fa

Browse files
alxhubmhevery
authored andcommitted
fix(compiler-cli): avoid duplicate diagnostics about unknown pipes (#39517)
TCB generation occasionally transforms binding expressions twice, which can result in a `BindingPipe` operation being `resolve()`'d multiple times. When the pipe does not exist, this caused multiple OOB diagnostics to be recorded about the missing pipe. This commit fixes the problem by making the OOB recorder track which pipe expressions have had diagnostics produced already, and only producing them once per expression. PR Close #39517
1 parent 1c6cf8a commit 861e4fa

File tree

2 files changed

+37
-0
lines changed

2 files changed

+37
-0
lines changed

packages/compiler-cli/src/ngtsc/typecheck/src/oob.ts

+11
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,12 @@ export interface OutOfBandDiagnosticRecorder {
7373
export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecorder {
7474
private _diagnostics: TemplateDiagnostic[] = [];
7575

76+
/**
77+
* Tracks which `BindingPipe` nodes have already been recorded as invalid, so only one diagnostic
78+
* is ever produced per node.
79+
*/
80+
private recordedPipes = new Set<BindingPipe>();
81+
7682
constructor(private resolver: TemplateSourceResolver) {}
7783

7884
get diagnostics(): ReadonlyArray<TemplateDiagnostic> {
@@ -90,6 +96,10 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
9096
}
9197

9298
missingPipe(templateId: TemplateId, ast: BindingPipe): void {
99+
if (this.recordedPipes.has(ast)) {
100+
return;
101+
}
102+
93103
const mapping = this.resolver.getSourceMapping(templateId);
94104
const errorMsg = `No pipe found with name '${ast.name}'.`;
95105

@@ -101,6 +111,7 @@ export class OutOfBandDiagnosticRecorderImpl implements OutOfBandDiagnosticRecor
101111
this._diagnostics.push(makeTemplateDiagnostic(
102112
templateId, mapping, sourceSpan, ts.DiagnosticCategory.Error,
103113
ngErrorCode(ErrorCode.MISSING_PIPE), errorMsg));
114+
this.recordedPipes.add(ast);
104115
}
105116

106117
illegalAssignmentToTemplateVar(

packages/compiler-cli/src/ngtsc/typecheck/test/diagnostics_spec.ts

+26
Original file line numberDiff line numberDiff line change
@@ -195,6 +195,32 @@ runInEachFileSystem(() => {
195195
]);
196196
});
197197

198+
it('does not repeat diagnostics for missing pipes in directive inputs', () => {
199+
// The directive here is structured so that a type constructor is used, which resuts in each
200+
// input binding being processed twice. This results in the 'uppercase' pipe being resolved
201+
// twice, and since it doesn't exist this operation will fail. The test is here to verify that
202+
// failing to resolve the pipe twice only produces a single diagnostic (no duplicates).
203+
const messages = diagnose(
204+
'<div *dir="let x of name | uppercase"></div>', `
205+
class Dir<T> {
206+
dirOf: T;
207+
}
208+
209+
class TestComponent {
210+
name: string;
211+
}`,
212+
[{
213+
type: 'directive',
214+
name: 'Dir',
215+
selector: '[dir]',
216+
inputs: {'dirOf': 'dirOf'},
217+
isGeneric: true,
218+
}]);
219+
220+
expect(messages.length).toBe(1);
221+
expect(messages[0]).toContain(`No pipe found with name 'uppercase'.`);
222+
});
223+
198224
it('does not repeat diagnostics for errors within LHS of safe-navigation operator', () => {
199225
const messages = diagnose(`{{ personn?.name }} {{ personn?.getName() }}`, `
200226
class TestComponent {

0 commit comments

Comments
 (0)
0