-
Notifications
You must be signed in to change notification settings - Fork 12.9k
Implement(60881): Added detailed diagnostic for parent to child assignment #61145
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
base: main
Are you sure you want to change the base?
Conversation
@microsoft-github-policy-service agree |
…0881 Syncing with upstream changes
…cMessages.json and changed it's code to 2881
This newest change will fail the CI tests. However, it should correctly add the message we are looking for. I didn't accept the baseline so we can perform the difference checker in class tomorrow and ensure the new message is correct. |
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
This PR contains a bunch of unrelated formatting changes which make this PR hard to review; you should revert these. If you have auto-format enabled in your editor, make sure it's running dprint. |
@jakebailey I've removed the formatting issues. Please let us know if there are any additional changes to be made. |
This reverts commit 8933230.
@jakebailey Can you take another look at this issue? I think its ready for review. |
tests/baselines/reference/better_subType_assignable_to_superType_error_messsage.errors.txt
Outdated
Show resolved
Hide resolved
@@ -1,9 +1,9 @@ | |||
conditionalTypes2.ts(15,5): error TS2322: Type 'Covariant<A>' is not assignable to type 'Covariant<B>'. | |||
Type 'A' is not assignable to type 'B'. | |||
'B' could be instantiated with an arbitrary type which could be unrelated to 'A'. | |||
'B' is constrained to be a subtype of 'A'. |
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'm not understanding why this error message is an improvement. What's the smallest example where this error is going to be easier to understand?
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.
@RyanCavanaugh This error message is an improvement because the original error message says that the two types could be unrelated but we know they are related in a parent child relationship, you just can't assign a parent to a child. We tried to pick a new error message that was similar to the suggestion in the original issue. This is the simplest example we looked at:
function example<SuperType, SubType extends SuperType>(superType: SuperType, subType: SubType) {
subType = superType;
}
Fixes #60881