Skip to content
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

Converter for the "this" type #383

Merged
merged 3 commits into from
Apr 3, 2017

Conversation

rklfss
Copy link
Contributor

@rklfss rklfss commented Jan 14, 2017

Created a converter for the type "this" that is often used in chainable
methods.

Created a converter for the type "this" that is often used in chainable
methods.
@blakeembrey
Copy link
Member

@makana Were you still interested or needing this? Can you share more information on how it works?

@rklfss
Copy link
Contributor Author

rklfss commented Mar 23, 2017

@blakeembrey

Yeah, I'm still interested in having this.

This converter is just for chainable methods or similar that have a the "this" type as return type.

export class Test
{
  public chainableMethod(): this
  {
    // do stuff
    return this;
  }
}

The converter uses the intrinsic type model to output the word "this".
I don't think we need to work out a new type model here, since there would be no real benefit (link to something -> doesn't make sense, we are already in the class, ...).

Without this converter, having methods with the "this" type led to some null or undefined errors (I have to take a look again to specify them) because it wasn't represented by a type model instance at all.

Since my pull request was some days ago, I don't know if the this type is now supported without my changes. I will try later.

Regards,
makana

@blakeembrey
Copy link
Member

Thanks. If you're willing to update the PR and post a screenshot of what the change looks like, I'd be happy to merge 👍

@rklfss
Copy link
Contributor Author

rklfss commented Mar 26, 2017

Hi @blakeembrey ,

I updated the PR. Unfortunately the checks failed. I'm sure the test passes, but exceeded with a timeout...

Here a screenshot of how the output looks like without the converter:
without converter

Note the missing return type.

here with converter:
with converter

@blakeembrey blakeembrey merged commit e08c3b3 into TypeStrong:master Apr 3, 2017
@blakeembrey
Copy link
Member

Thanks @makana!

@rklfss rklfss deleted the this-type-converter branch April 22, 2017 06:42
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.

2 participants