fix(compiler-cli): infer correct generic types for directives when so…#57724
fix(compiler-cli): infer correct generic types for directives when so…#57724JoostK wants to merge 2 commits into
Conversation
JoostK
added
breaking changes
compiler: template type-checking
action: global presubmit
a45bf44 to
4b35c7e
Compare
angular-robot
Bot
removed
the
area: compiler
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? |
|
I think this approach is sound and improves type checking accuracy, potentially reporting diagnostics for code that was undesirably deemed valid before. I believe I have revised the implementation since the last TGP so let me rebase the current state and determine the impact using a new TGP. I suspect this may need to be done behind a flag to be able to land this. |
|
This draft PR is being closed because it has been stale for 28 days and has seen no activity from you. If you'd like to see this change land, you can re-open this PR. Thank you for being an Angular contributor! |
|
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. |
2 participants
…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.