-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
Stricter generic signature checks #16368
Conversation
@Aleksey-Bykov Now you can retire the shirt! |
@ahejlsberg this is one of all time grand features of TS since unions, type guards and exhaustive switches, it brings point free programming style like what you can find in big boy's FP languages, thank you for your hard work and consideration |
src/compiler/checker.ts
Outdated
if (!related) { | ||
return Ternary.False; | ||
} | ||
result &= related; | ||
} | ||
} | ||
else if (sourceSignatures.length === 1 && targetSignatures.length === 1) { | ||
// For pure functions (functions with a single signature) we only erase type parameters for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe simple/just/only functions would be a better name than pure, as pure brings a different connotation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Will fix.
# Conflicts: # src/compiler/checker.ts
Latest commits add a |
Due to stricter generic type checks, signatures for ZoneAwarePromise need to be changed. See microsoft/TypeScript#16368 The signatures from lib.es5.d.ts were copied over for .then and .catch.
Due to stricter generic type checks, signatures for ZoneAwarePromise need to be changed. See microsoft/TypeScript#16368 The signatures from lib.es5.d.ts were copied over for .then and .catch.
Due to stricter generic type checks, signatures for ZoneAwarePromise need to be changed. See microsoft/TypeScript#16368 The signatures from lib.es5.d.ts were copied over for .then and .catch.
Due to stricter generic type checks, signatures for ZoneAwarePromise need to be changed. See microsoft/TypeScript#16368 The signatures from lib.es5.d.ts were copied over for .then and .catch.
This PR complements #16305 by implementing stricter checking of type relationships for generic signatures. Previously, when checking type relationships for generic signatures we would always first erase type parameters (by substituting type
any
for the type parameters). Now, we properly unify the type parameters by first inferring from the target signature to the source signature and then instantiating the source signature with the inferences. For example:Previously, no errors were reported above because
S
,T
, andU
were replaced byany
, causing the signatures to become identical. Now, once we unify the type parameters we can correctly determine thatA
is assignable toB
, but not vice versa. Specifically, in the first assignment we inferT | U
forS
which after instantiation yields a signature that isn't assignable toA
, whereas in the second assignment we inferS
for each ofT
andU
which after instantiation yields the same signature asB
.Note that we perform unification only when the source and target types each have a single signature. When either the source or target has multiple signatures (i.e. overloads) we still resort to type parameter erasure as it otherwise becomes prohibitively expensive to determine the relationships.
This PR is technically a breaking change as we now uncover errors we previously wouldn't catch. For this reason we have included a
--noStrictGenericChecks
compiler option to disable the stricter generic signature checks. Existing projects can use this as a stopgap solution until errors resulting from the stricter checking are corrected.Fixes #138, #3410, #5616.