-
Notifications
You must be signed in to change notification settings - Fork 12.8k
Instantiate this for contextually typed type parameters #9746
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
Instantiate this for contextually typed type parameters #9746
Conversation
@ahejlsberg can you look at this? I fixed this right before you got back from vacation, then went on vacation myself and forgot about it. |
The problem may actually be that thisParameter is instantiated too early compared to other parameters on the signature. |
|
||
const thisType = getThisTypeOfDeclaration(container); | ||
if (thisType) { | ||
return thisType; | ||
} | ||
const type = getContextuallyTypedThisType(container); |
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 had to save this block for contextually-typed object literals like this:
interface CI {
m: number
method(this: this, n: number);
}
let o: CI = {
m: 12,
method(n) {
return this.m + n;
}
}
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.
Based on our in-person discussion, I checked on contextual typing for parameters. It's 1 of 9 cases in getTypeForVariableLikeDeclaration
:
const type = declaration.symbol.name === "this"
? getContextuallyTypedThisType(func)
: getContextuallyTypedParameterType(<ParameterDeclaration>declaration);
The contextual typing for an undeclared this
is basically the same thing here in checkThisExpression
since there is no parameter to get the type of.
I tried moving the contextual typing out of |
@@ -9326,11 +9330,11 @@ namespace ts { | |||
} | |||
} | |||
|
|||
function getContextuallyTypedThisType(func: FunctionLikeDeclaration): Type { | |||
function getContextuallyTypedThisParameter(func: FunctionLikeDeclaration): Symbol { |
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 think a better name would be getContextualThisParameter
.
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.
done
👍 Once you make the suggested name change. |
This essentially undoes #9746, which only contextually typed object literal methods, but did so early, during `getSignatureFromDeclaration`. The compiler now contextually types everything, as it did before, but only looks for a contextual type if there is no declared `this` type. After #9746, a member list request from the language service would call `getSignatureFromDeclaration`, but it would never get a contextual this-type, because contextual typing was only enabled for object literal methods.
This essentially undoes #9746, which only contextually typed object literal methods, but did so early, during `getSignatureFromDeclaration`. The compiler now contextually types everything, as it did before, but only looks for a contextual type if there is no declared `this` type. After #9746, a member list request from the language service would call `getSignatureFromDeclaration`, but it would never get a contextual this-type, because contextual typing was only enabled for object literal methods.
Fixes #9673
Previously
this
parameters were not instantiated when they came from a contextual signature. To fix this, I instantiate the signature ingetThisTypeOfSignature
. However, the mapper for the instantiation is only available much earlier, when checking the containing function. So I save the mapper on the containing function's signature — that signature is the one that will end up contextually typingthis
later.This doesn't look right, though. The instantiation code is
signature = instantiateSignature(signature, signature.mapper);
and based on other instantiations in the code I don't think signatures with no target are even supposed to be instantiated, and not with their own mapper.@ahejlsberg or @mhegazy any ideas on how to do this right? I thought about creating a new signature with the correct target/mapper setup, but I'm not sure how to make sure it gets used in place of the original one.