fix(compiler-cli): infer correct generic types for directives when so… #57724
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
a45bf44 to
4b35c7e
Compare
4b35c7e to
7c4614d
Compare
…me inputs are omitted The template type-checker could inadvertently end up inferring `any` for a directive's generic type argument, effectively disabling all type checking of values of that generic type. This happened when the directive reflects the generic type in multiple inputs, some of which were not bound from the parent template. Since omitted inputs were passed into the directive's type constructor using the `0 as any` expression, TypeScript sees the input of type `any` as inference source of the generic type and locks that in. The initial remediation idea was to remove omitted inputs from the type-constructor entirely, changing its type from `Pick<Dir, 'a'|'...'>` to `Partial<Pick<Dir, 'a'|'...'>>` to make all input bindings optional. This introduces issues where the `undefined` type that is introduced affects type inference, causing test failures. Instead, omitted inputs are now passed into the type constructor as `never` type, which is TypeScript's bottom type such that no inference candidates are introduced. Fixes angular#57644
…when some inputs are omitted
7c4614d to
9f60e5b
Compare
|
@JoostK Are you still working on this, or can we close it? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…me inputs are omitted
The template type-checker could inadvertently end up inferring
anyfor a directive's generic type argument, effectively disabling all type checking of values of that generic type. This happened when the directive reflects the generic type in multiple inputs, some of which were not bound from the parent template. Since omitted inputs were passed into the directive's type constructor using the0 as anyexpression, TypeScript sees the input of typeanyas inference source of the generic type and locks that in.The initial remediation idea was to remove omitted inputs from the type-constructor entirely, changing its type from
Pick<Dir, 'a'|'...'>toPartial<Pick<Dir, 'a'|'...'>>to make all input bindings optional. This introduces issues where theundefinedtype that is introduced affects type inference, causing test failures.Instead, omitted inputs are now passed into the type constructor as
nevertype, which is TypeScript's bottom type such that no inference candidates are introduced.Fixes #57644
This change is likely (too) breaking, as the inference of the
anytype used to effectively disable type-checking of any (pun not intended) expressions that reflect a generic type of a directive with missing inputs.MatCalendaris one such directive (the one reported in #57644) that has many inputs that reflect the generic type, where it is typical that not all those inputs are actually bound.A g3 global presubmit should help determine the impact of the breakage, which may require us to introduce this behind a compiler option.