-
Notifications
You must be signed in to change notification settings - Fork 26.2k
feat(compiler-cli): enable narrowing of signal reads in templates #55456
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signal reads in Angular templates don't currently benefit from TypeScript's narrowing capabilities, since function calls are not eligible for type narrowing. This is unfortunate since Angular templates should not be updating signal values, such that narrowing would be a safe operation. Ideally there would be a way for TypeScript to enable type-narrowing of signal reads, but this is unfortunately not possible today. Various potential designs have been explored to support type-narrowing of signal reads in the compiler's Type-Check Block (TCB): 1. Assign call expressions to a local variable in the TCB, rewriting further usages of the same signal to the local variable instead. This way, TypeScript is able to apply narrowing of this local variable that all subsequent reads are then able to leverage. 2. Augment the `Function` declaration with a getter field corresponding with the function's return type, then emitting call expression as property reads using this augmented getter. Although approach 2 simplifies the TCB emit compared to 1, it could not be made to work in practice as the `Function` declaration itself doesn't have any generic type to extract the return type from. Instead, this commit implements approach 1. The primary known design limitation of this approach arises with optional chaining, where the call expressions end up with different identities to prevent reusing an earlier variable declaration for the call expression. For example, consider the following template: ``` @if (person()?.address()) { <app-address [address]="person().address()"> } ``` Normally in TypeScript this would be accepted (provided that the call expressions are just property reads), but our referencing approach does not consider the signal reads of `address()` to have the same receiver. You may think this limitation can be overcome by always identifying property reads as equivalent to their safe counterpart (i.e. always treating `.` as `?.`) but doing so would introduce false negatives in the negated case: ``` @if (!person()?.address()) { <app-address [address]="person().address()"> } ``` If the `person().address()` was to correspond with `person()?.address()` to make it align with the condition, then no error would be reported that `person()` may be `null`. To avoid such false negatives in template diagnostics this commit is being conservative with optional chains, where narrowing is not achieved as would be the case with property reads. Perhaps there may be ways of improving this in the future. Another side effect of this change is that diagnostics won't be reported within expressions that have been substituted with a reference. Consider: ``` @if (person().nam()) { {{ person().nam() }} } ``` Since the secondary occurrence of `person().nam()` is being substituted for the local variable that is assigned within the `@if`-condition, the typo in `nam()` is not reported for the expression in the block.
Is this still planned? Currently that's the only reason signals and control flow are still clunky |
There's an issue with the current implementation that is yet to be resolved. It's not a dead end (yet) but no ETA right now. |
What is the recommended workaround? I have a discriminated union signal I want to switch case on its type. I thought |
Use |
Doing some cleanup, I'll allow myself to close the PR since we're not going in this direction. |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Signal reads in Angular templates don't currently benefit from TypeScript's narrowing capabilities, since function calls are not eligle for type narrowing. This is unfortunate since Angular templates should not be updating signal values, such that narrowing would be a safe operation.
Ideally there would be a way for TypeScript to enable type-narrowing of signal reads, but this is unfortunately not possible today.
Various potential designs have been explored to support type-narrowing of signal reads in the compiler's Type-Check Block (TCB):
Assign call expressions to a local variable in the TCB, rewriting further usages of the same signal to the local variable instead. This way, TypeScript is able to apply narrowing of this local variable that all subsequent reads are then able to leverage.
Augment the
Function
declaration with a getter field corresponding with the function's return type, then emitting call expression as property reads using this augmented getter.Although approach 2 simplifies the TCB emit compared to 1, it could not be made to work in practice as the
Function
declaration itself doesn't have any generic type to extract the return type from.Instead, this commit implements approach 1. The primary known design limitation of this approach arrises with optional chaining, where the call expressions end up with different identities to prevent reusing an earlier variable declaration for the call expression. For example, consider the following template:
Normally in TypeScript this would be accepted (provided that the call expressions are just property reads), but our referencing approach does not consider the signal reads of
address()
to have the same receiver. You may think this limitation can be overcome by always identifying property reads as equivalent to their safe counterpart (i.e. always treating.
as?.
) but doing so would introduce false negatives in the negated case:If the
person().address()
was to correspond withperson()?.address()
to make it align with the condition, then no error would be reported thatperson()
may benull
. To avoid such false negatives in template diagnostics this commit is being conservative with optional chains, where narrowing is not achieved as would be the case with property reads. Perhaps there may be ways of improving this in the future.Another side effect of this change is that diagnostics won't be reported within expressions that have been substituted with a reference. Consider:
Since the secondary occurrence of
person().nam()
is being substituted for the local variable that is assigned within the@if
-condition, the typo innam()
is not reported for the expression in the block.This is still a draft, the code needs more tests and we have to determine how to roll this out (i.e. opt-in vs opt-out vs always on).