Skip to content

this in object literals intersects contextual type and literal type #8356

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

Merged
merged 3 commits into from
Apr 28, 2016

Conversation

sandersn
Copy link
Member

Fixes #8191

  1. If there is a contextual type and an object literal type for an object literal, intersect them.
  2. If the contextual type of a function is any, its contextual this type is now any as well.
  3. Make Salsa detection of x.prototype.y = function () { ... } pattern override the contextual type.

@@ -8450,6 +8453,12 @@ namespace ts {
if ((isFunctionExpressionOrArrowFunction(func) || isObjectLiteralMethod(func)) &&
isContextSensitive(func) &&
func.kind !== SyntaxKind.ArrowFunction) {
const type = isObjectLiteralMethod(func)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this logic is duplicated between here and getContextualSignature. consider refactoring it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done, although it still does the work twice in the case that there is no contextualSignature.

1. Remove intersection code. The any-defaulting code means that it's never hit.
2. Remove some code duplication.
If a contextual signature is found, if its thisType is undefined, then the contextual type of `this` is now `any`.
Previously `checkThisExpression` would keep looking for a different type for `this`.

Also update tests to show this new behaviour.
@sandersn
Copy link
Member Author

OK, I changed the behaviour of contextual signatures that don't define thisType. Now an undefined thisType causes the contextual type of this to be any. Previously it would cause the contextual type to be undefined, so checkThisExpression would keep looking for a type for this.

@mhegazy I think this is a good compromise. Can you take a look?

@sandersn
Copy link
Member Author

Also npm install failed in the Travis build. I guess npm is down? Or something?

@mhegazy
Copy link
Contributor

mhegazy commented Apr 28, 2016

@yuit published then unpublished a package.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 28, 2016

👍

@mhegazy mhegazy closed this Apr 28, 2016
@mhegazy mhegazy reopened this Apr 28, 2016
@sandersn sandersn merged commit 9120121 into master Apr 28, 2016
@sandersn sandersn deleted the smarter-object-literal-this-contextual-type branch May 1, 2016 03:53
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants