Use identity with the permissive instantation to detect nongenric instances and disable variance probing on nongeneric instances #29981
Conversation
…stances and disable variance probing on nongeneric instances
…rgument comparisons for postive comparisons if possible
|
cc @ahejlsberg this was the PR for the thing I was just talking to you about. |
|
Note from DT: In its current state, this should fix the failure in |
|
@typescript-bot test this & @typescript-bot run dt |
|
Heya @weswigham, I've started to run the Definitely Typed test suite on this PR at cd92006. You can monitor the build here. It should now contribute to this PR's status checks. |
|
Heya @weswigham, I've started to run the extended test suite on this PR at cd92006. You can monitor the build here. It should now contribute to this PR's status checks. |
|
@typescript-bot test this |
|
Heya @weswigham, I've started to run the extended test suite on this PR at bb250df. You can monitor the build here. It should now contribute to this PR's status checks. |
|
I looked at the DT diff. Most of the ember errors are gone, so I guess that the ones remaining have strictFunctionTypes: true. Jasmine is also fixed. However, the process ran out of memory before finishing, so I'm not sure it finished yargs successfully. Worse, mithril overflows the stack now. You should probably take a look. |
|
@typescript-bot test this & @typescript-bot run dt |
|
Heya @weswigham, I've started to run the extended test suite on this PR at ba0720e. You can monitor the build here. It should now contribute to this PR's status checks. |
|
Heya @weswigham, I've started to run the Definitely Typed test suite on this PR at ba0720e. You can monitor the build here. It should now contribute to this PR's status checks. |
|
Remaining DT changes just look to be some minor changes in ember that @sandersn already has a PR up on DT to fix, so this looks good now ❤️ |
weswigham
changed the title
|
After some soul searching on how to best fix the depth crash found via specced, i swapped from using the restrictive instantiation to the permissive instantiation as an identity target - both map all type parameters, but since permissive maps type parameters into wildcard, repeated instantiations don't chain (and the wildcard type often bails out of identity checking way faster than a type parameter when it doesn't match as an added bonus). |
|
@typescript-bot test this & @typescript-bot run dt |
|
Heya @weswigham, I've started to run the extended test suite on this PR at 1565c97. You can monitor the build here. It should now contribute to this PR's status checks. |
|
Heya @weswigham, I've started to run the Definitely Typed test suite on this PR at 1565c97. You can monitor the build here. It should now contribute to this PR's status checks. |
|
The DT run is all clean except for the remaining ember failures, which I have a fix for, and another project URL change, which I just updated. |
|
Is the project url failure just only reporting one failure at a time or something? |
|
@weswigham That is unlikely because dtslint issues the error, and it's being run in parallel by dtslint-runner. It's much more likely that of the 5200+ npm packages that DT tracks, one or two per day change their urls. Edit: If that's the case, I'll look into an automated way to keep the urls up-to-date, because otherwise it'll constantly break the DT build. |
|
Update from #30133: Reverting the invariance of conditional types shows that this PR still fixes errors in
I'm not sure of the exact cause of ember's problem, but it's still going through their However, spected, mithril and yargs no longer error on master, so this PR doesn't need to address those errors. |
|
@typescript-bot run dt |
|
Heya @weswigham, I've started to run the Definitely Typed test suite on this PR at f8989af. You can monitor the build here. It should now contribute to this PR's status checks. |
Previously @ahejlsberg had mentioned disabling variance probing for generic mapped types - that's not good enough. In the examples given by, eg
varianceProblingAndZeroOrderIndexSignatureRelationsAlign(which come from user code - see #29817), the mapped types are within something else which we're still doing variance calculations for. The real "heuristic" as it were, is simply that if we know we're not relating generic types, then we should compare structurally rather than with variances (since we know the structural comparison will yield the correct result and likely be more permissive than any pessimistic higher order relationship may have been). Since we can't directly check if a type still contains generics or not, I proxy the check by seeing if the permissive instantiation of the type and the type itself are identical. If they are, then either A. we're looking at a permissive instantiation of a type (which is by definition non-generic, since all generics have been mapped out), or B. we're looking at a type which is unaffected by instantiation of all generics in it (so contains no generics).