GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Type 'this' in function properties of object literals by sandersn · Pull Request #8382 · microsoft/TypeScript

Conversation

@sandersn

@sandersn Nathan Shively-Sanders (sandersn) commented Apr 29, 2016

Copy link
Copy Markdown
Member

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; }
}

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@sandersn
Type 'this' in object literal function properties
c12cb83
Previously, methods of object literals would give a type to 'this'.
Now function properties of object literals also give a type to 'this'.
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-not-required label Apr 29, 2016
@sandersn
Update looseThisTypeInFunctions baselines
755ba73
@mhegazy

Mohamed Hegazy (mhegazy) commented Apr 29, 2016

Copy link
Copy Markdown
Contributor

👍

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

DanielRosenwasser
DanielRosenwasser reviewed Apr 29, 2016
View reviewed changes
Comment thread tests/baselines/reference/looseThisTypeInFunctions.js
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'

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Apr 29, 2016

Copy link
Copy Markdown
Member

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.

These comments are unhelpful to future test updates, please just remove them instead of changing them in the future.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Apr 29, 2016

Copy link
Copy Markdown
Member

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@sandersn Nathan Shively-Sanders (sandersn) Apr 29, 2016

Copy link
Copy Markdown
Member Author

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.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@DanielRosenwasser

Daniel Rosenwasser (DanielRosenwasser) commented Apr 29, 2016

Copy link
Copy Markdown
Member

👍

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sandersn
Nathan Shively-Sanders (sandersn) merged commit 655e24c into master Apr 29, 2016
@sandersn
Nathan Shively-Sanders (sandersn) deleted the this-type-for-object-literal-function-properties branch April 29, 2016 20:17
@mhegazy Mohamed Hegazy (mhegazy) mentioned this pull request Apr 29, 2016
Incorrect/overly strong this inference for POJOs #8191
Closed
@teintinu

José Fernando Tolentino (teintinu) commented Jul 14, 2016

Copy link
Copy Markdown

Hi Nathan Shively-Sanders (@sandersn) , I trying use that feature with [email protected] 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!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sandersn

Nathan Shively-Sanders (sandersn) commented Jul 14, 2016

Copy link
Copy Markdown
Member Author

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@yortus Troy Gerwien (yortus) mentioned this pull request Sep 10, 2016
can’t use 'this' in object literals with --noImplicitThis #10835
Closed
@sandersn Nathan Shively-Sanders (sandersn) mentioned this pull request Sep 23, 2016
In JS, this in object literals is untyped #11072
Closed
@mjbvz Matt Bierner (mjbvz) mentioned this pull request Nov 18, 2016
Object methods are missing IntelliSense microsoft/vscode#298
Closed
@FranklinWhale Franklin Tse (FranklinWhale) mentioned this pull request Dec 9, 2016
Option to make this in functions inside object literals typed #12785
Closed
@ahejlsberg Anders Hejlsberg (ahejlsberg) mentioned this pull request Feb 17, 2017
Typed 'this' in object literal methods #14141
Merged
@microsoft 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.

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@sandersn @mhegazy @DanielRosenwasser @teintinu @msftclas
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.