-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Fix isTypeDerivedFrom to properly handle {}
and intersections
#51631
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@typescript-bot test this |
Heya @ahejlsberg, I've started to run the extended test suite on this PR at 8ccd6d5. You can monitor the build here. |
Heya @ahejlsberg, I've started to run the abridged perf test suite on this PR at 8ccd6d5. 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 8ccd6d5. 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 8ccd6d5. You can monitor the build here. |
Heya @ahejlsberg, I've started to run the diff-based top-repos suite on this PR at 8ccd6d5. You can monitor the build here. Update: The results are in! |
@ahejlsberg Here are the results of running the user test suite comparing Everything looks good! |
@ahejlsberg Here they are:Comparison Report - main..51631
System
Hosts
Scenarios
Developer Information: |
Heya @ahejlsberg, I've run the RWC suite on this PR - assuming you're on the TS core team, you can view the resulting diff here. |
// S is a type variable with a base constraint that is derived from T, | ||
// S is an intersection type and some constituent of S is derived from T, or | ||
// S is a type variable with a base constraint that is derived from T, or | ||
// T is {} and S is an object-like type (ensuring {} is less derived than Object), or |
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.
q: what does "less derived" mean? how something can be more or less derived?
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.
It means that any object-like type, including Object
, appears to be derived from {}
. The effect of this addition is that x instanceof Object
will narrow {}
to Object
, which in turn means that x
qualifies as the right hand operand of an in
check.
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.
I'd probably reword this like this then:
// T is {} and S is an object-like type (ensuring {} is less derived than Object), or | |
// T is {} and S is an object-like type (ensuring that object-likes, including Object, are derived from {}), or |
Maybe I'm just missing the appropriate background and lingo but this explanation is more readable to me.
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.
In the context of type theory, "less derived" is definitely a thing. e.g. Animal
is less derived than Dog
which is less derived than PitBull
, while Cat
and Dog
are equally derived, being sister types. These relationships are obvious under nominal typing, but probably get pretty hairy under structural typing.
@ahejlsberg Here are the results of running the top-repos suite comparing Everything looks good! |
Fixes #50844.
Fixes #51007.