-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Report error for invalid 'this' type during 'await' #48946
Conversation
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.
Looks good, although I would personally collapse the two error parameters into one since they seem to be mutually exclusive.
@@ -36256,7 +36256,7 @@ namespace ts { | |||
* @param type The type of the promise. | |||
* @remarks The "promised type" of a type is the type of the "value" parameter of the "onfulfilled" callback. | |||
*/ | |||
function getPromisedTypeOfPromise(type: Type, errorNode?: Node): Type | undefined { | |||
function getPromisedTypeOfPromise(type: Type, errorNode?: Node, thisTypeForErrorOut?: { value?: Type }): Type | undefined { |
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.
Probably should be a union since you don't want to issue the error twice, and the only place thisTypeForErrorOut
is added did not previously pass errorNode
.
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 don't want to have to test whether errorNode
is a Node
and I'd also rather not allocate a nursery object for { errorNode }
if I don't have to.
@DanielRosenwasser worth taking for 4.7? If so I can merge now. |
@typescript-bot test this |
@typescript-bot user test this inline |
Heya @DanielRosenwasser, I've started to run the diff-based community code test suite on this PR at 0b6ad03. You can monitor the build here. Update: The results are in! |
Heya @DanielRosenwasser, I've started to run the abridged perf test suite on this PR at 0b6ad03. You can monitor the build here. Update: The results are in! |
@DanielRosenwasser Here they are:Comparison Report - main..48946
System
Hosts
Scenarios
Developer Information: |
@DanielRosenwasser |
The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master. |
Fixes #47711