In typeof x === object, type parameters extending unknown narrow like unknown itself #49091
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 pack this |
|
Heya @weswigham, I've started to run the extended test suite on this PR at dc319e7. You can monitor the build here. |
|
Heya @weswigham, I've started to run the parallelized Definitely Typed test suite on this PR at dc319e7. You can monitor the build here. |
|
Heya @weswigham, I've started to run the perf test suite on this PR at dc319e7. You can monitor the build here. Update: The results are in! |
|
Heya @weswigham, I've started to run the tarball bundle task on this PR at dc319e7. You can monitor the build here. |
|
Heya @weswigham, I've started to run the diff-based community code test suite on this PR at dc319e7. You can monitor the build here. Update: The results are in! |
weswigham
commented
May 12, 2022
| // then it doesn't affect the `keyof` query, and we can unwrap the conditional and relate the unwrapped source and target. | ||
| // There may be multiple stacked conditionals, such as `T extends null ? never : T extends undefined ? never : T : T`, so | ||
| // we need to repeat the unwrapping process. | ||
| while (isConditionalFilteringKeylessTypes(sourceType)) { |
There was a problem hiding this comment.
These keyof relationship improvements aren't strictly speaking tied to the typeof x === "object" narrowing change, but they do improve common uses of it (eg, narrowing within a loop and doing an index operation), so I think make sense to pair with it.
weswigham
commented
May 12, 2022
| // Returns the String, Number, Boolean, StringLiteral, NumberLiteral, BooleanLiteral, Void, Undefined, or Null | ||
| // flags for the string, number, boolean, "", 0, false, void, undefined, or null types respectively. Returns | ||
| // no flags for all other types (including non-falsy literal types). | ||
| function getFalsyFlags(type: Type): TypeFlags { | ||
| return type.flags & TypeFlags.Union ? getFalsyFlagsOfTypes((type as UnionType).types) : | ||
| type.flags & TypeFlags.Intersection ? reduceLeft(getApparentIntersectionTypes(type as IntersectionType), (memo, type) => memo | getFalsyFlags(type), 0 as TypeFacts) : |
There was a problem hiding this comment.
Small change to falsy flag calculation to pick up T & undefined as falsy when it doesn't reduce to never.
|
@weswigham |
|
Hey @weswigham, 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 |
|
@weswigham Here they are:Comparison Report - main..49091
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||
|
|
||
| // #48468 but with an explicit constraint so as to not trigger the `{}` and unconstrained type parameter bug | ||
| function deepEquals<T extends unknown>(a: T, b: T) { |
There was a problem hiding this comment.
Can you have a test for both the explicit and non-explicit constraint?
FWIW I have tests at #48576.
|
DT looks clean (some unrelated intl change is making it fail but otherwise clean), RWC has removed an error on an |
|
This mostly got folded into #49119. I'll extract the improvements to |
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.
In addition, improves relating
keyof Tandkeyof NonNullable<T>-like types (to allow the relation) so that the impacts of, eg, chaining atypeof x === "object"and a truthiness check still produce compatible types.Should fix #48468 even after the
{}and unconstrained type parameter change is back in.