Type 'this' in function properties of object literals by sandersn · Pull Request #8382 · microsoft/TypeScript (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation7 Commits2 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Fixes #8110
Previously, methods of object literals would give a type to this. Now function properties of object literals also give a type to this.
In other words, there are no more any types in the below sample. Previously f: (n: number) => any because this: any inside its body:
let o = { x: 12, m(n: number) { return this.x + n }, f: function(n: number) { return this.x + n; } }
Previously, methods of object literals would give a type to 'this'. Now function properties of object literals also give a type to 'this'.
| @@ -67,7 +68,7 @@ c.explicitVoid = c.explicitThis; // error, 'void' is missing everything |
|---|
| var o = { |
| n: 101, |
| explicitThis: function (m) { |
| return m + this.n.length; // ok, this.n: any |
| return m + this.n.length; // error, 'length' does not exist on 'number' |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These comments are unhelpful to future test updates, please just remove them instead of changing them in the future.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I mean is, you run the tests, find out something is wrong, realize you have to update the test itself as well because the comment is now lying to you, and also update the baselines as well.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about the case that you run the tests, see that it changed, and have to figure out whether or not the change is an improvement? In that case the comment helps you figure out if the test should still pass, and why.
I've hit several old-ish tests that have accumulated multiple failures -- I couldn't tell if they were supposed to fail or even what the test was supposed to show.
Hi @sandersn , I trying use that feature with typescript@2.1.0-dev.20160714 but stills this:any and f: (n: number) => any.
I mean you PR gives:
this: { x: number, m(n: number) : number, f(n: number): number }
I'm right?
There is something in tsconfig.json to change?
Thanks for you PR!
Unfortunately, we rolled back this change after some Ember and Dojo folks pointed out that it breaks common construction patterns where a function extends an object literal argument with some properties. TypeScript doesn't have a way to type that pattern at all today, so we decided to go back to any until it can.
See the discussion in #8191 for details.
