GeezerVison is showing github.com — Stricter generic signature checks by ahejlsberg · Pull Request #16368 · microsoft/TypeScript
Conversation
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:
type A = <T, U>(x: T, y: U) => [T, U];
type B = <S>(x: S, y: S) => [S, S];
function f(a: A, b: B) {
a = b; // Error
b = a; // Ok
}
Previously, no errors were reported above because S, T, and U were replaced by any, causing the signatures to become identical. Now, once we unify the type parameters we can correctly determine that A is assignable to B, but not vice versa. Specifically, in the first assignment we infer T | U for S which after instantiation yields a signature that isn't assignable to A, whereas in the second assignment we infer S for each of T and U which after instantiation yields the same signature as B.
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
zpdDG4gta8XKpMCd, krryan, gcnew, alitaheri, HerringtonDarkholme, jovdb, antogyn, SlurpTheo, Igorbek, aluanhaddad, and 2 more reacted with thumbs up emoji
zpdDG4gta8XKpMCd, krryan, alitaheri, HerringtonDarkholme, Igorbek, aluanhaddad, nishanths, and gcnew reacted with hooray emoji
zpdDG4gta8XKpMCd, krryan, alitaheri, HerringtonDarkholme, nishanths, and gcnew reacted with heart emoji
All reactions
-
👍
12 reactions
-
🎉
8 reactions
-
❤️
6 reactions
zpdDG4gta8XKpMCd, krryan, alitaheri, kitsonk, Igorbek, KiaraGrouwstra, and wwwillchen reacted with thumbs up emoji
RyanCavanaugh, krryan, s-panferov, tinganho, alitaheri, HerringtonDarkholme, hastebrot, markusjohnsson, SlurpTheo, prashaantt, and 5 more reacted with laugh emoji
All reactions
-
👍
7 reactions
-
😄
15 reactions
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Anders Hejlsberg (@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
|
HerringtonDarkholme, sergeyzenchenko, hastebrot, SlurpTheo, sean-vieira, and JsonFreeman reacted with hooray emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
Great work!
One issue that still stands is #16107. As #16368 is already a breaking change, is it now the time to correct the union inference wart as well? I hope it is.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
|
}
|
|
|
}
|
|
|
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
# Conflicts:
# src/compiler/checker.ts
|
Latest commits add 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.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Aug 17, 2017
This was referenced Aug 21, 2017
Rado Kirov (rkirov)
added a commit
to rkirov/zone.js
that referenced
this pull request
Oct 11, 2017
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.
Rado Kirov (rkirov)
added a commit
to rkirov/zone.js
that referenced
this pull request
Oct 11, 2017
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.
Rado Kirov (rkirov)
added a commit
to rkirov/zone.js
that referenced
this pull request
Oct 12, 2017
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.
Reviewers
2 more reviewers
Reviewers whose approvals may not affect merge requirements
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
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.
|