Optional variance annotations #48240
Conversation
typescript-bot
added
Author: Team
For Uncommitted Bug
|
@typescript-bot test this |
|
Heya @ahejlsberg, I've started to run the abridged perf test suite on this PR at 3799524. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 3799524. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the extended test suite on this PR at 3799524. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the diff-based community code test suite on this PR at 3799524. You can monitor the build here. Update: The results are in! |
|
@ahejlsberg Here they are:Comparison Report - main..48240
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@ahejlsberg |
ahejlsberg
requested review from
DanielRosenwasser,
RyanCavanaugh and
weswigham
|
In here it has been mentioned that:
So from what I get - with this PR here we are going to have a somewhat unpredictable behavior by default and to make it predictable we'll be able to use variance annotations introduced by this PR. Couldn't/shouldn't the problem be flipped though? Couldn't you go with predictable and safe behavior (although slow) and propose using those variance annotations as performance optimization? I understand that some libraries out there might be hard/slow to upgrade and to start using those annotations but with the time that should become less of a concern and people will notice great slowdowns pretty soon and they should start helping library authors to migrate to this performance optimization. With I'm worried that by accepting the random behavior now the community will get stuck with it forever - while if this would be treated as a perf optimization then most people would get great safety improvements immediately and out of the box. |
|
Correct me if Im wrong but in here:
It should be “contravariant”
hm, how measuring is different from what u check and for what u raise errors described in the thread? Since reporting “correct” errors would require full structural computation - shouldnt this be described here as a limitation for raising those errors? I mean - sometimes they just wont be raised and u will trust the user input, did i get this right? |
We considered it and we don't think it's feasible. An unknown number of code bases out there would be negatively impacted, in some cases quite dramatically. To wit, compilation of Also, it really is relatively rare that the current implementation gets variance measurement wrong. It requires types to be circularly dependent and to flip variance somewhere in the circularity. That happens, but it definitely isn't common.
The scenarios in which the issue occurs are rare and well understood, and we'll continue to think about ways in which we can improve accuracy of variance measurement without adversely affecting performance. Nothing prevents us from adopting a better algorithm in the future.
Indeed. Thanks for catching that, I have fixed it.
Variance measurement is similar to variance annotation checking, but requires more checks. Specifically, we always perform at least two checks (subtype-to-supertype and supertype-to-subtype) to measure variance, and sometimes require a third check to distinguish bivariance from unwitnessed type parameters. In contrast, annotation checking requires only one check for co- or contravariance, and no check at all for invariance. |
|
We'll be putting this in Specifically, you cannot reliably create an invariant type by applying |
Yeah, variance annotations are only consulted when relating instantiations of type aliases for object, function, and constructor type literals and mapped types, so it is kind of meaningless to add variances annotations for other kinds of types. I think the best course of action is for us to error on attempts to do so. I will put up a PR to that effect. |
|
this is interesting |
|
I'm just dropping by to wonder if whoever came up with this was a C programmer back in the day who loved SAL? Because I sense a bit of SAL here 😉 |
Checking after diagnostics have been added it seems that it is still possible to define a parameter invariant even if the measured variance is either covariant or contravariant. Is this intended? It seems to me that given annotations are ignored in structural comparisons the only thing that make sense is to have them being strictly equal to the measured one |
Yes it's explicitly stated in OP
|
Yes, I am asking if it is still intended and an explanation of why that is ok |
Yes. Part of the goal here is not just correctness, but performance. For many libraries causing slowdowns during typing (as in, typing in the editor), variance measurement accounts for a very large portion of the CPU time spent, so "skipping" checks when the author tells us what the result is going to be is a big performance win. Asserting invariance specifically means there's no possible soundness gap created as a result, since only fewer relations are allowed as a result. |
Thanks! I have definitely felt the pain in some scenarios, I agree there is no soundness gap from the structural perspective, there may be a perceived soundness gap when those annotations are ignored in structural fallback if there is a mismatch between the annotated variance and the actual variance but if the library authors are aware of it perf wins |
Two things got a bit mixed together here.
One is that we want the Disabled plugins to all be defined directly in
plugin/index.ts, so that loading ApolloServerPluginInlineTraceDisabled
does not spend time loading the protobuf library.
The other thing is that we started thinking a bit more carefully about
plugin context generics. We wrote some tests to make sure that you can
use an `ApolloServerPlugin<BaseContext>` (ie, a plugin that doesn't need
any particular fields on the context) with any `ApolloServer`, but not
vice versa. As part of getting this to work, we added another
`__forceTContextToBeContravariant`. We also noticed that it made more
sense for BaseContext to be `{}` ("an object with no particular fields")
rather than `Record<string, any>` ("an object where you can do anything
with any of its fields").
We investigated whether the new contravariance annotation coming in the
next two months in TS 4.7
(microsoft/TypeScript#48240) would allow us to
get rid of the `__forceTContextToBeContravariant` hack and the answer is
yes! However, trying `4.7.0-beta` failed for two other reasons. One is
that microsoft/TypeScript#48366 required us to
add some missing `extends` clauses, which we are doing now in this PR.
The other is that `graphql-tools` needs some fixes to work with TS4.7
which we hope they can do soon
(ardatan/graphql-tools#4381).
|
Is TS going to emit variance annotations in |
|
At the moment, the intent is to emit whatever you've written. If you leave a type parameter bare, we'll continue to leave it bare, which will mean that consuming modules will continue to infer the variance. If you need to provide variance annotations for newer users, but need to provide an experience that gracefully degrades, you can use |
Can I add a compiler flag or something to let typescript emit those variance annotations in |
Do you mean emitting the computed variance annotations? |
|
The computed variances are more often than not much more complicated than the simple |
Would love to read a write-up about this! It could demystify some bits about the TS compiler for a lot of people |
|
Should using the covariance Example:
|
|
I'm pretty sure I wasn't thinking and parameters aren't allowed to be bivariant...I was experimenting with adding variance annotations to |
With this PR we introduce optional declaration site variance annotations for type parameters of classes, interfaces and type aliases. Annotations take the form of an
inand/oroutkeyword immediately preceding the type parameter name in a type parameter declaration.outannotation indicates that a type parameter is covariant.inannotation indicates that a type parameter is contravariant.in outannotation indicates that a type parameter is invariant.Generally, type parameter variance is simply a function of how a type parameter is used in its generic subject type. Indeed, when generic type instantiations are related structurally, variance annotations serve no purpose. This is why TypeScript strictly doesn't need variance annotations. However, variance annotations are useful to assert desired type relations of their subject generic types. Specifically, given a generic type
G<T>and any two type argumentsSuperandSubfor whichSubis a subtype ofSuper,Tis covariant (declared asout T),G<Sub>is a subtype ofG<Super>,Tis contravariant (declared asin T),G<Super>is a subtype ofG<Sub>, andTis invariant (declared asin out T), neitherG<Super>norG<Sub>is a subtype of the other.Intuitively, covariance restricts a type parameter to output (read) positions and contravariance restricts a type parameter to input (write) positions--hence the
inandoutmodifiers. For example:Covariance and contravariance annotations are checked by structurally relating representative instantiations of their subject generic types. For example, in the following generic type, the type parameter
Tis used in both input and output positions andTis thus invariant. Attempting to markTcovariantreports the following error on
out T:Likewise, attempting to mark
Tcontravariantreports the following error on
in T:Notice how the error elaborations reveal where and how variance is breached.
Invariance annotations (
in out T) are never checked but simply assumed to hold. Thus, it is possible to assert invariance even when the actual usage of a type parameter is co- or contravariant.When multiple interface declarations are merged, or when a class declaration and one or more interface declarations are merged, variance annotations are aggregated. In the example
the aggregate variance of
Tisin out, andTis thus assumed to be invariant.When variance annotations are present, the type checker doesn't need to measure variance. Thus, variance annotations can help improve the performance of checking complex and interdependent types. In particular, marking a type parameter invariant means that no measurement or checking is necessary for that type parameter.
In addition, variance annotation can help establish correct variance for multiple circularly dependent generic types. Specifically, when measuring variance, TypeScript limits the structural search space in order to avoid runaway recursion. In the example
the compiler measures
Tto be covariant even though it is actually invariant due to variance reversal inBarand the circular reference inBaz. The compiler could establish that by continuing to structurally relating nested circular references until some fixed point, but this gets exponentially expensive and isn't feasible in complex scenarios. Adding anin outannotation toTestablishes the correct variance and produces the expected errors.We're marking #1394 and #10717 fixed by this PR, although the feature implemented isn't exactly what is suggested in those issues.
Fixes #1394.
Fixes #10717.