fix(compiler-cli): detect uninvoked signals in bound expressions using ternary#68079
fix(compiler-cli): detect uninvoked signals in bound expressions using ternary#68079cexbrayat wants to merge 1 commit intoangular:mainfrom
Conversation
…g ternary Extend the interpolated signal extended diagnostic to inspect ternary-bound expressions and report uninvoked signal reads in bound bindings. ``` <div [style.width]="width() ? 1 : width"></div> ``` where the false branch should invoke the signal as `width()`.
| { | ||
| fileName, | ||
| templates: { | ||
| 'TestCmp': `<div [style.width]="width() ? 1 : width"></div>`, |
There was a problem hiding this comment.
Shouldn't this be picked up by the typechecker ?
There was a problem hiding this comment.
I don’t think the type-checker can catch this, because checkTypeOfDomBindings is currently always false (even with strictTemplates)
There was a problem hiding this comment.
Yeah but is it a reason to fix it here ?
This diagnostic should only address cases where the type system cannot see the problem.
There was a problem hiding this comment.
I think it is, as we don't have a compiler that checks dom bindings (and we'll maybe never have one?). Even if it was, the error message would not be as precise as the one coming from the diagnostic.
This is a real issue we encountered in a migrated app, so I thought it would be worth it to spare the pain to other developers.
| { | ||
| fileName, | ||
| templates: { | ||
| 'TestCmp': `<div [style.width]="width() ? 1 : width"></div>`, |
There was a problem hiding this comment.
Yeah but is it a reason to fix it here ?
This diagnostic should only address cases where the type system cannot see the problem.
| { | ||
| fileName, | ||
| templates: { | ||
| 'TestCmp': `<button [disabled]="!isEnabled"></button>`, |
There was a problem hiding this comment.
This one seems a good case for the diagnostic to cover !
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
The current diagnostic does not report the uninvoked signal in the following case:
where the false branch should invoke the signal as
width().What is the new behavior?
Extend the interpolated signal extended diagnostic to inspect ternary-bound expressions.
Does this PR introduce a breaking change?
Other information
Also added tests to cover already handled cases