8000 fix(compiler-cli): add signal checks to handle negated calls (#59970) · angular/angular@b7ab5fa · GitHub
[go: up one dir, main page]
Skip to content

Commit b7ab5fa

Browse files
aldillekAndrewKushnir
aldillek
authored and
AndrewKushnir
committed
fix(compiler-cli): add signal checks to handle negated calls (#59970)
By adding these checks, we can find scenarios where a signal was expected to be called but wasn't. PR Close #59970
1 parent 21fc93b commit b7ab5fa

File tree

2 files changed

+30
-8
lines changed

2 files changed

+30
-8
lines changed

‎packages/compiler-cli/src/ngtsc/typecheck/extended/checks/interpolated_signal_not_invoked/index.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
ASTWithSource,
1212
BindingType,
1313
Interpolation,
14+
PrefixNot,
1415
PropertyRead,
1516
TmplAstBoundAttribute,
1617
TmplAstNode,
@@ -45,6 +46,7 @@ class InterpolatedSignalCheck extends TemplateCheckWithVisitor<ErrorCode.INTERPO
4546
// interpolations like `{{ mySignal }}`
4647
if (node instanceof Interpolation) {
4748
return node.expressions
49+
.map((item) => (item instanceof PrefixNot ? item.expression : item))
4850
.filter((item): item is PropertyRead => item instanceof PropertyRead)
4951
.flatMap((item) => buildDiagnosticForSignal(ctx, item, component));
5052
}
@@ -59,6 +61,7 @@ class InterpolatedSignalCheck extends TemplateCheckWithVisitor<ErrorCode.INTERPO
5961
return [];
6062
}
6163
// otherwise, we check if the node is
64+
const nodeAst = isPropertyReadNodeAst(node);
6265
if (
6366
// a bound property like `[prop]="mySignal"`
6467
(node.type === BindingType.Property ||
@@ -70,16 +73,28 @@ class InterpolatedSignalCheck extends TemplateCheckWithVisitor<ErrorCode.INTERPO
7073
node.type === BindingType.Attribute ||
7174
// or an animation binding like `[@myAnimation]="mySignal"`
7275
node.type === BindingType.Animation) &&
73-
node.value instanceof ASTWithSource &&
74-
node.value.ast instanceof PropertyRead
76+
nodeAst
7577
) {
76-
return buildDiagnosticForSignal(ctx, node.value.ast, component);
78+
return buildDiagnosticForSignal(ctx, nodeAst, component);
7779
}
7880
}
7981
return [];
8082
}
8183
}
8284

85+
function isPropertyReadNodeAst(node: TmplAstBoundAttribute): PropertyRead | undefined {
86+
if (node.value instanceof ASTWithSource === false) {
87+
return undefined;
88+
}
89+
if (node.value.ast instanceof PrefixNot && node.value.ast.expression instanceof PropertyRead) {
90+
return node.value.ast.expression;
91+
}
92+
if (node.value.ast instanceof PropertyRead) {
93+
return node.value.ast;
94+
}
95+
return undefined;
96+
}
97+
8398
function isFunctionInstanceProperty(name: string): boolean {
8499
return FUNCTION_INSTANCE_PROPERTIES.has(name);
85100
}

‎packages/compiler-cli/src/ngtsc/typecheck/extended/test/checks/interpolated_signal_not_invoked/interpolated_signal_not_invoked_spec.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,14 +61,15 @@ runInEachFileSystem(() => {
6161
{
6262
fileName,
6363
templates: {
64-
'TestCmp': `<div>{{ mySignal1 }} {{ mySignal2 }}</div>`,
64+
'TestCmp': `<div>{{ mySignal1 }} {{ mySignal2 }} {{ !mySignal3 }}</div>`,
6565
},
6666
source: `
6767
import {signal, Signal} from '@angular/core';
6868
6969
export class TestCmp {
7070
mySignal1 = signal<number>(0);
7171
mySignal2: Signal<number>;
72+
mySignal3 = signal<number>(0);
7273
}`,
7374
},
7475
]);
@@ -81,11 +82,12 @@ runInEachFileSystem(() => {
8182
{} /* options */,
8283
);
8384
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
84-
expect(diags.length).toBe(2);
85+
expect(diags.length).toBe(3);
8586
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
8687
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED));
8788
expect(getSourceCodeForDiagnostic(diags[0])).toBe(`mySignal1`);
8889
expect(getSourceCodeForDiagnostic(diags[1])).toBe(`mySignal2`);
90+
expect(getSourceCodeForDiagnostic(diags[2])).toBe(`mySignal3`);
8991
});
9092

9193
it('should produce a warning when a readonly signal is not invoked', () => {
@@ -638,13 +640,15 @@ runInEachFileSystem(() => {
638640
{
639641
fileName,
640642
templates: {
641-
'TestCmp': `<div [${binding}]="mySignal"></div>`,
643+
'TestCmp': `<div [${binding}]="mySignal"></div>
644+
<div [${binding}]="!negatedSignal"></div>`,
642645
},
643646
source: `
644647
import {signal} from '@angular/core';
645648
646649
export class TestCmp {
647650
mySignal = signal<number>(0);
651+
negatedSignal = signal<number>(0);
648652
}`,
649653
},
650654
]);
@@ -657,10 +661,11 @@ runInEachFileSystem(() => {
657661
{} /* options */,
658662
);
659663
const diags = extendedTemplateChecker.getDiagnosticsForComponent(component);
660-
expect(diags.length).toBe(1);
664+
expect(diags.length).toBe(2);
661665
expect(diags[0].category).toBe(ts.DiagnosticCategory.Warning);
662666
expect(diags[0].code).toBe(ngErrorCode(ErrorCode.INTERPOLATED_SIGNAL_NOT_INVOKED));
663667
expect(getSourceCodeForDiagnostic(diags[0])).toBe(`mySignal`);
668+
expect(getSourceCodeForDiagnostic(diags[1])).toBe(`negatedSignal`);
664669
});
665670

666671
it(`should not produce a warning when signal is invoked on ${name} binding`, () => {
@@ -669,13 +674,15 @@ runInEachFileSystem(() => {
669674
{
670675
fileName,
671676
templates: {
672-
'TestCmp': `<div [${binding}]="mySignal()"></div>`,
677+
'TestCmp': `<div [${binding}]="mySignal()"></div>
678+
<div [${binding}]="!negatedSignal()"></div>`,
673679
},
674680
source: `
675681
import {signal} from '@angular/core';
676682
677683
export class TestCmp {
678684
mySignal = signal<number>(0);
685+
negatedSignal = signal<number>(0);
679686
}`,
680687
},
681688
]);

0 commit comments

Comments
 (0)
0