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

TS1093: Can't specify return types on constructors #11588

Closed
pimterry opened this issue Oct 13, 2016 · 6 comments
Closed

TS1093: Can't specify return types on constructors #11588

pimterry opened this issue Oct 13, 2016 · 6 comments
Labels
Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds

Comments

@pimterry
Copy link
Contributor

pimterry commented Oct 13, 2016

TypeScript Version: nightly (2.1.0-dev.20160922)

Code

I have an example for my case where being able to return something else from an ES6 constructor would be useful, works nicely, and is currently blocked unnecessarily by TS1093: async construction. Might be contentious, but I think there's cases where it comes out far nicer and clearer than the alternatives. By far the simplest way of representing what I want to do is something like the below:

class CommentThread {
  private comments: any;
  constructor(private url: string): Promise<CommentThread> {
    return loadComments().then((comments) => {
      this.comments = comments;
      return this;
    });
  }
  // ...
}

// Later:
var comments = await new CommentThread("http://....");

This fails to compile with: error TS1093: Type annotation cannot appear on a constructor declaration.

This is related to #10860, but separate I think, as that issue is focused on type parameters on constructors and the challenges of those, only implicitly requiring constructor return types. Here I'm only interested in return types on constructors and why we can't have those.

I'm not sure how contentious people think this pattern is (there's probably other good examples too though). Other than the surprise factor of getting a promise-wrapped instance not a plain instance, which is nicely mitigated by static typing, the whole thing works very nicely and easily and clearly, imo.

Implementation alternatives are async factory functions that do all the loading and pass the resulting data into the class (for my case that takes from a class with a single argument to 6+, and is decidedly less readable or coherent), a class where you have to remember to call a setup() method after initialization, and not use any of its other methods before you do so, or a class where every method that depends on this setup is async, and waits for the initial loading itself. All very messy, and this is a pattern that can be expressed very nicely in JavaScript + async/await as-is.

The only blocker for this pattern is that you can't specify the return type in TypeScript. The best alternative I can come up with is:

class CommentThread {
  private comments: any;
  constructor(private url: string) {
    // Any to get around constructor return type check here, but same implementation
    return <any> loadComments().then((comments) => {
      this.comments = comments;
      return this;
    });
  }
  // ...
}

// Define a different type for the class, so we can change the constructor
let AsyncCommentThread: { new (url: string): Promise<CommentThread> } = <any> CommentThread;

// Later, correctly use the new type:
var comments = await new AsyncCommentThread("http://....");

The implementation is the same, but type wrangling makes this obviously much less nice, isn't that easy to discover, and feels very unnecessary.

I'd love to have return types on constructors, so we could express this better.

@pimterry pimterry changed the title TS1093 TS1093: Can't specify return types on constructors Oct 13, 2016
@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Oct 13, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Oct 13, 2016

It is worth nothing that this pattern specifically was discussed by TC39 as part of the async function proposal. having an async constructor was rejected as it was considered a bad practice, that should not be blessed with an official syntax. The main points were that subclassing semantics become strange, and the fact that new C() instanceof C === false which is frankly confusing.

It is also worth noting that ES6 class semantics allow returning any type, as long as it is not a primitive, though that is not something that the committee encouraged or thought of as a good practice, it is mainly a mechanism to allow exotic objects and their users to use classes, and new.target was offered as a way to allow these classes to correctly set up the prototype chain.

@yortus
Copy link
Contributor

yortus commented Oct 14, 2016

I'm not keen on the OP example because it breaks semantic expectations of new, but there are other cases for returning something else from a class constructor that don't break expectations.

One example is described in #10860 (comment). This example is for constructing callable objects, and AFAIK is it a legitimate case of 'returning something else' in conjunction with ES6's Symbol.hasInstance.

I'm not sure how useful Symbol.hasInstance is if constructors aren't allowed to return other objects when the need dictates.

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Oct 14, 2016

Implementation alternatives are async factory functions that do all the loading and pass the resulting data into the class (for my case that takes from a class with a single argument to 6+, and is decidedly less readable or coherent), a class where you have to remember to call a setup() method after initialization, and not use any of its other methods before you do so, or a class where every method that depends on this setup is async, and waits for the initial loading itself. All very messy, and this is a pattern that can be expressed very nicely in JavaScript + async/await as-is.

When consuming an API, a factory function is ideal. It is straightforward, elegant, and compositional.

There are lots of solutions to the problem of having too many constructor arguments. These include options bags, abstract class implementation, lambdas, and local sub class closures.

I agree .setup methods are awful. There are better alternatives. Why require that your consumers use opaque, misleading syntax to invoke a glorified factory masquerading as a class?

@pimterry
Copy link
Contributor Author

Interesting, ok, thanks! In my case I still quite like it. It makes the return types potentially surprising, yes, but with static typing it's hard to get caught out by that, and once you know the pattern it's extremely effective and easy to use, especially with await. It certainly makes my code dramatically simpler, even with the extra type wrangling. I dislike the idea that you have to quite substantially change the whole structure of how you build your objects depending only on whether the operations involved are sync or async, and this is a solution that solves that. Notably it's also a pattern that does come up elsewhere occasionally too.

Anyway, I can see there's downsides, and you certainly aren't going to want to do this all the time, that's fine. And for what it's worth, I think async constructor is a bad idea too - at least here in the implementation it's quite explicit about what's happening.

Regardless though, with both this and the cases in #10860 there are developers writing code (for better or worse) that explicitly returns things from constructors. This is something that works generally as a pattern in JS, even with the new class syntax.

As far as I know there's no technical reason TypeScript needs to forbid this, and it feels like a somewhat arbitrary limitation that actively stops TypeScript being able to describe code that you can easily write otherwise. Is that right? Are there any big downsides to just disabling this check, and allowing devs to specify return types on constructors?

@RyanCavanaugh RyanCavanaugh added Too Complex An issue which adding support for may be too complex for the value it adds and removed In Discussion Not yet reached consensus labels Oct 31, 2016
@RyanCavanaugh
Copy link
Member

Declined for reasons listed above #11588 (comment)

pjustino pushed a commit to pjustino/typescript-book that referenced this issue Aug 21, 2017
I notice that the interface crazy could not be implemented in a class (please check here: microsoft/TypeScript#8917) and I made a fix with some text to explain better the pattern.
I also fixed the constructor to not return any value as it seems not to be possible. Please check here: microsoft/TypeScript#11588
@BlackGlory
Copy link

You can use my module async-constructor to create an asynchronous constructor without a return value. The principle is very simple.

@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds
Projects
None yet
Development

No branches or pull requests

6 participants