Skip to content

Add noImplicitThis in default tsconfig #84

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 1 commit into from
Nov 23, 2017

Conversation

inkless
Copy link
Contributor

@inkless inkless commented Nov 23, 2017

This is related to microsoft/TypeScript#14518
When noImplicitThis is not on, this is not ThisType<T>, so we should turn on noImplicitThis by default

@chriskrycho
Copy link
Member

chriskrycho commented Nov 23, 2017

Ohhh, interesting. I'll see how this behaves in my app, and will merge after confirming. Edit: I should add that I think this fix one major annoyance I've had for quite some time!

Thank you for the contribution!

@chriskrycho
Copy link
Member

Curiously, this fixes it nicely for the EmberObject.extend({ ... }) case but doesn't help at all with ES6 classes. It's a nice win for the non-class variant, though, so I'll take it gladly. Thanks again!

@chriskrycho chriskrycho merged commit 622ec5f into typed-ember:master Nov 23, 2017
@dwickern
Copy link
Contributor

What's wrong with ES6 classes?

@chriskrycho
Copy link
Member

@dwickern without using a this binding, you end up with errors like this:

error

The only way around it I've found so far is explicitly naming this:

import Component from '@ember/component';

export default class MyFoo extends Component {
  thingToSay = 'Hello';

  sayIt(this: MyFoo, toSomeone: string) {
    const thingToSay = this.get('thingToSay'); // errors without `this: MyFoo` above
    alert(`${thingToSay} ${toSomeone}`);
  }
}  

@dwickern
Copy link
Contributor

Are you using strictFunctionTypes?

@chriskrycho
Copy link
Member

I had no been, but turning it on has no effect on this error, either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants