Improve constraints of conditional types applied to constrained type variables #56004
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
typescript-bot
added
Author: Team
For Uncommitted Bug
|
@typescript-bot test top100 |
|
Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at 2f5d9e9. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the diff-based user code test suite on this PR at 2f5d9e9. You can monitor the build here. |
|
Heya @ahejlsberg, I've started to run the tsc-only perf test suite on this PR at 2f5d9e9. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the diff-based top-repos suite on this PR at 2f5d9e9. You can monitor the build here. Update: The results are in! |
|
@ahejlsberg Here they are:
CompilerComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@ahejlsberg Here are the results of running the top-repos suite comparing Something interesting changed - please have a look. Details
|
|
Hey @ahejlsberg, the results of running the DT tests are ready. Branch only errors:Package: reflexbox Package: knex-cleaner Package: mui-datatables Package: knex-db-manager |
|
@typescript-bot pack this |
|
Hey @gabritto, I've packed this into an installable tgz. You can install it for testing by referencing it in your and then running There is also a playground for this build and an npm module you can use via |
|
@typescript-bot test top100 |
|
Heya @ahejlsberg, I've started to run the diff-based user code test suite on this PR at c7613d6. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the tsc-only perf test suite on this PR at c7613d6. 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 c7613d6. You can monitor the build here. Update: The results are in! |
|
Heya @ahejlsberg, I've started to run the diff-based top-repos suite on this PR at c7613d6. You can monitor the build here. Update: The results are in! |
|
@ahejlsberg Here they are:
CompilerComparison Report - baseline..pr
System info unknown
Hosts
Scenarios
Developer Information: |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@ahejlsberg Here are the results of running the user test suite comparing There were infrastructure failures potentially unrelated to your change:
Otherwise... Everything looks good! |
|
Latest commit changes checking for the "possibly extends" case to use a reverse assignability check. This check is not quite as permissible as a comparability check, meaning we'll less often hit the "possibly extends" case where the constraint is a union. Still, since constraint types are typically close-to-top types (like |
This was referenced Nov 21, 2023
|
As an FYI in case anyone follows this issue, it turned out that this caused a regression in |
voxpelli
added a commit
to sindresorhus/type-fest
that referenced
this pull request
Jan 29, 2024
After the change in microsoft/TypeScript#56004 the `PickRestType` was causing some types to be considered theoretically infinite in their recursion. I can't wrap my mind around exactly why right now, but I can see how `PickRestType` on a crazy long tuple will cause a very deep recursion, so I decided to simply cap its recursion and return `unknown[]` if it goes past that point and that solved the errors. Side effects: - In practice: rarely any I believe - In theory: some merged types will regress to `unknown[]` in TypeScript versions older than 5.4 Considerations: Is the `DepthTracker extends Array<true> = []` + `PickRestTypeMaxDepth extends DepthTracker['length']` a good way to track depth? I know I have done a depth check previous times but can't remember what solution I picked then. Fixes #784
1 task
4 tasks
Haroenv
added a commit
to algolia/instantsearch
that referenced
this pull request
Jun 20, 2024
I can't manage to reproduce this locally, even if i update everything to 5.4.5, but in a fresh Next.js example (like https://codesandbox.io/s/47fy7c) `hitsPerPage` isn't accepted to Configure (because PlainSearchParameters is any) This is I believe linked to microsoft/TypeScript#56004, maybe also to sindresorhus/type-fest#846 (although the case seems different). Essentially before this change the types like `ClientLiteV5` were evaluated to `any` instead of `unknown`, poisoning the entire type and turning everything into any. The solution is to first check if `typeof AlgoliaSearchLite` is any, and if it is any we don't even evaluate the rest of teh type. Of course if it isn't any it works correctly for v5 still. Again, unfortunately this isn't reproducible in this repo, even when every typescript version is updated to 5.4.5, but at least the error will be fixed fixes #5989
Haroenv
added a commit
to algolia/instantsearch
that referenced
this pull request
Jun 20, 2024
) I can't manage to reproduce this locally, even if i update everything to 5.4.5, but in a fresh Next.js example (like https://codesandbox.io/s/47fy7c) `hitsPerPage` isn't accepted to Configure (because PlainSearchParameters is any) This is I believe linked to microsoft/TypeScript#56004, maybe also to sindresorhus/type-fest#846 (although the case seems different). Essentially before this change the types like `ClientLiteV5` were evaluated to `any` instead of `unknown`, poisoning the entire type and turning everything into any. The solution is to first check if `typeof AlgoliaSearchLite` is any, and if it is any we don't even evaluate the rest of teh type. Of course if it isn't any it works correctly for v5 still. Again, unfortunately this isn't reproducible in this repo, even when every typescript version is updated to 5.4.5, but at least the error will be fixed fixes #5989
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
With this PR we compute more accurate constraints for distributive conditional types applied to constrained type variables. For example:
Previously, we'd obtain the constraint of
IsArray<U>by applying the distributive conditional type to the constraint ofU, which resolves tofalse(becauseobjectdoesn't extendany[]). But really, it ought to beboolean(i.e.true | false) since clearly something constrained toobjectboth could or could not be an array.A type variable represents any possible type within its constraint. So, given a type parameter
Twith the constraintC, the constraint ofT extends X ? A : BisAwhenCis known to always extendX,BwhenCis known to never extendX, orA | BwhenCpossibly extendsX.We previously didn't consider the third possibility. With this PR, we choose the third outcome when
Cis not assignable toX, butX(or, ifXis a union type, some constituent ofX) is assignable toC.Fixes #30152.