microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.01k stars 12.48k forks source link

Allow to specify return type of constructor #27594

Open saintcrawler opened 6 years ago

saintcrawler commented 6 years ago

Search Terms

constructor return type

Suggestion

Add an ability to specify return type of a class constructor. Right now the compiler produces the following error: "Type annotation cannot appear on a constructor declaration."

Use Cases

Consider this example:

interface MyInterface {
  readonly data: ReadonlyArray<string>

  add(item: string): void
  remove(item: string): void

  // ...more
}

class MyClass implements MyInterface {
  data = [] as string[]

  constructor(): MyInterface {} // right now it's an error

  add(item: string) {
    this.data.push(item)
  }

  // ...more
}

Current behavior:

const c = new MyClass() // typeof c == MyClass
c.data = [] // oops! no error from the compiler

Suggested behavior:

const c = new MyClass() // typeof c == MyInterface
c.data = [] // the compiler generates an error here, correct behavior

Of course, there are several ways to achieve this result (like using private constructor with factory function, or using a type converter like this: type AsInterface<T, C> = C extends new (...args: infer A) => T ? new (...args: A) => T : never But I think that the proposed feature is more concise

Checklist

My suggestion meets these guidelines:

andrerpena commented 6 years ago

Can't you just const c = new MyClass() as MyInterface?

saintcrawler commented 6 years ago

And what if I "forgot" to do that? This behavior should be enforced by creator of the class, not by consumer

MartinJohns commented 6 years ago

If you really want to force the user to use the interface (even when they're actually creating an instance of the class), then make the constructor private and add a static create-method.

saintcrawler commented 6 years ago

Yes, I've already mentioned it. But it requires writing some boilerplate code, and my solution is simple and DRY (because for a static create method you should re-type all arguments from constructor). And moreover, I don't see any reason why this can not be allowed. It's not a breaking change. Of course, you can not return a string type, for example. If a sole purpose of a class is implementing an interface, then, I think, it seems logical to specify return type of the constructor as that inferface, and not as the type of the class.

yortus commented 6 years ago

See also #27465 and #10860.

demurgos commented 5 years ago

I'm reading the linked issues. Here is one of the reasons that was advanced against this:

This is definitely technically true, but it's really hard to imagine why you'd bother writing an ES6 class if you were going to return something else out of the constructor.

The source-map library returns a Promise<this> for its SourceMapConsumer constructor to handle Wasm loading.

https://github.com/mozilla/source-map/blob/master/lib/source-map-consumer.js#L18-L27

I am also supporting this kind of feature: I want to define constructor overloads to get stronger type inference when relating constructor arguments to the properties in my class.

conartist6 commented 5 years ago

My issue was marked as a duplicate of this one and closed, so I'm going to chime in here.

What I care about is whether TS allows you to set the type parameters for your own type using the constructor return syntax. To understand why I need this, take a look at http://burningpotato.com/sequins-docs/#/Map. This class type is impossible to represent at the moment without using two (constructor, instance) interfaces to pretend to be a class. There's nothing abnormal going on at runtime though. My problem is not being able to express the type at all, as clearly I've already done that, my beef is that since there's nothing deviant going on, it should be trivial to express such a type to consumers, which clearly it is not.

Also to clarify I propose that there are two terms in play here:

I actually think it's beneficial for a class type to only describe a class, not any newable. Interfaces with new keywords seem like an OK way to describe what's going on for non-class newables. It certainly weakens the readability of the language to weaken the definition of a class type to fit any newable. At that point seeing that something is a class doesn't necessarily tell you anything at all.

What I could go either way on is allowing a constructor return to return a subtype of the class. This does fulfill the class contract regarding instanceof, and it certainly could be useful in code. I don't need it personally, so I'm not worried.

saintcrawler commented 5 years ago

I want to add more examples.

Let's say you have a class holding an array of items. You want both an array and items in it to be readonly for class consumers. But at the same time inside the class those should be mutable, so that your class methods can operate on them.

type Item = {/*some fields*/}

// Wrong! Consumers can do new Api().items = [] by mistake
class Api {
  items: ReadonlyArray<Item>
}

// Wrong! Consumers can not mutate `items`, but
// now it's assignable only in constructor
// and that's pretty useless. What if we need to alter the array in .someMethod()?
class Api2 {
  readonly items: ReadonlyArray<Item>
}

// Wrong! We can alter the array inside our class, but
// consumers can do new Api3().items.push('whatever')
class Api3 {
  readonly items: Array<Item>
}

// OK! But why type the same twice?
// Also, if `_items` is going to be reassigned, we should write a getter for `items`, see:
class Api4 {
  _items: Array<Item>
  readonly items: ReadonlyArray<Item> = this._items

  _items2: Array<Item>
  readonly items2: ReadonlyArray<Item> = this._items2

  someMethod() {
    this._items2 = this.processNewItems()

    // Oops, we forgot to update this.items2, now it points to an older version of `_items2`
  }

  // Possible solutions:
  // 1) declare items2 as:
  get items2() { return this._items2 }

  // 2) use a helper method:
  private setItems2(items: ReadonlyArray<Item>) {
    this._items2 = items
    this.items2 = items
  }

  // Now the question is: are you ready to write boilerplate code?
}

// Use a private ctor with static factory?
// OK, but you need to re-type all arguments from ctor
class Api5 {
  private constructor(
    private readonly a: SomeLongNameYouHaveToCopyTwice,
    private readonly b: AnotherThing,
    c: AndAnother
  ) {}

  // here the `fun` begins:
  static create(
    a: SomeLongNameYouHaveToCopyTwice,
    b: AnotherThing,
    c: AndAnother
  ) {
    // all of above just to
    return new Api5(a, b, c) as IApi
  }
}

// Another attempt

interface IApi {
  readonly items: ReadonlyArray<Item>
}

const api = new Api() as IApi
// OK, but this is an optional solution:
// a consumer may write a type cast or may not.
// It's better to enforce this.

And now let me introduce suggested solution:

// 1) create an interface (IApi) from above
interface IApi {
  readonly items: ReadonlyArray<Item>
}

// 2)
class Api {
  items: Array<Items> // do whatever you want with items inside a class...

  constructor(): IApi // ...but outside it will be readonly
  {}
}

const api = new Api() // infers as IApi

So, all what I suggest is actually making the following line valid: constructor(): IApi {}

And it is as simple as this:

type Interface<T, C> = C extends new (...args: infer A) => T ? new (...args: A) => T : never

Right now I use it like:

class SomeClass implements ISomeInterface {
  // implementation...
}

export default SomeClass as Interface<ISomeInterface, typeof SomeClass>

It works fine, but little too verbose.

SkeLLLa commented 5 years ago

Here's another example that woks in javascript and there are a lot of use-cases where it can be used.

class Bar {
  constructor(): Promise<this> {
    // do some stuff
    return new Promise((resolve) => {
      return this;
    });
  }

  foo(): string {
    return 'foo';
  }
}

(async () => {
  const a = await new Bar();
  a.foo()
})()

So specifying constructor type is must have feature for typescript. Yes someone thinks that it's a bad practice https://github.com/Microsoft/TypeScript/issues/11588#issuecomment-253602163. However decision what's bad or not should be made by developers of app (probably with help of linters). And language should provide tools. And in case of typescript, as it is superset of javascript it should provide the same tools that are available in javascript. And if it doesn't, it couldn't be called "superset", it will be reduced set.

conartist6 commented 5 years ago

@SkeLLLa Nothing is preventing you from doing

class Bar {
   static async create() {
     //...
   }
}

In particular if you want to strip the new keyword of its implicit meaning (result is an instance of new's class target), it's not clear why you wouldn't just use a constructor function.

conartist6 commented 5 years ago

@saintcrawler Wouldn't another potential solution be defining a get method? I only mention it because this seems to be the solution that the core javascript language has embraced (think es6 Map), and in this case it would give you the runtime guarantee that in your example you seek to gain solely though the use of types.

SkeLLLa commented 5 years ago

@conartist6 why should I have additional static method, if I class already has a constructor? As a workaround it's okay, but not for usage on regular basis.

In js new and class is just syntax sugar and meaning of new is that a new object is created and referenced by Class's this reference. And it will be an instance if what we're returning in constructor. Keyword new just letting js engine know that we want to create a new class instance, but nothing more.

And for "implicit meanings", as I've wrote before it's said on typescript main page it is written that "TypeScript is a typed superset of JavaScript that compiles to plain JavaScript". So by the implicit meaning it stands for that typescript should cover all valid cases and constructions that I could have in javascript. And the case we're talking about is not covered.

saintcrawler commented 5 years ago

@conartist6 javascript uses get because it lacks types. And typescript has a beautiful readonly modifier.

If I understand you correctly, you suggest something like this?

class MyClass {
  private name: string
  private address: {country: string, city: string, zip: number}

  public get(key: string) {
    return this[key]
  }
}

That case works only for primitive types, i.e. MyClass.address still can be mutated. And also it will be a nigthmare to type get method correctly so that it won't return any type.

Or do you suggest something like this?

class MyClass {
  private name: string
  getName() { return this.name }

  // or with property getter
  private _name: string
  get name() { return this._name }

  private address: {country: string, city: string, zip: number}
  getAddress() { return this.address } // you need to use Readonly<T> here
}

For me it's just a useless boilerplate code. It may be helpful in plain JS, but in TS it's much easier to use a type system to restrict access to data.

As for your example with static create function, here is, again, an excerpt from my big example from previous post:

class Api5 {
  private constructor(
    private readonly a: SomeLongNameYouHaveToCopyTwice,
    private readonly b: AnotherThing,
    c: AndAnother
  ) {}

  static create(
    a: SomeLongNameYouHaveToCopyTwice,
    b: AnotherThing,
    c: AndAnother
  ) {    
    return new Api5(a, b, c)
  }
}

When a constructor does not use any arguments it's more or less fine (but still a boilerplate), But when you need to pass several arguments (e.g., for Dependency Injection) it's a completely nightmare.

And, also as @SkeLLLa noted, it's a valid code from JS perspective.

So, I don't see any good arguments againts including this feature. Moreover, it's not that hard to implement for TS-team and it won't be a breaking feature.

conartist6 commented 5 years ago

Don't get me wrong, I want the feature too. It's just want a less permissive version of it, because that's what I need.

What I'm really suggesting is that if I read the thread above, each of you has your own separate concept of what it means to be a class, and what the new keyword implies. As someone reading code (including yours), I will never want there to be more (and varied) definitions for what it means when I see the new keyword. When I read that I need to be certain that simple class instantiation is occurring, otherwise every time I see new I'm going to have to remember to check on the constructor, just to be sure.

conartist6 commented 5 years ago

@saintcrawler Why not make your addresses fully immutable? Addresses don't change too often, do they? With dependency injection I certainly wouldn't want code inside or outside the class to be able to rewire fields set up by the dependency injector.

Sorry, I'm not trying to be obstructionist, I just want to understand what you need to do and whether there is really no other way.

saintcrawler commented 5 years ago

@conartist6 Imagine the following interface:

interface IFooLoader {
  readonly isLoading: boolean
  readonly foos: ReadonlyArray<IFoo>

  //...loadFoos(), other members, etc...
}

How would you implement that interface with respect to readonly semantics?

A:

class FooLoader implements IFooLoader {
  get isLoading() { return this._isLoading }
  private _isLoading = false

  get foos() { return this._foos }
  private _foos = []

  //etc...
}

B:

class FooLoader implements IFooLoader {
  isLoading = false
  foos = []  

  private constructor() {}

  static create() {
    return new FooLoaderB() as IFooLoader
  }

  //etc...
}

C:

class FooLoader implements IFooLoader {
  isLoading = false
  foos = []  

  constructor() : IFooLoader
  {}

  //etc...
}

Which of these classes is easier to write/maintain?

RReverser commented 4 years ago

Another usecase of this feature that didn't seem to be mentioned above is specialisation. E.g. I want to be able to write this in TS defintions:

declare class C<R> {
    constructor(source: number): C<Uint8Array>;
    constructor(source: R);
    ...
}

and have it behave like normal function overloads.

For now the only way to achieve this is via var+interface declarations, which is less pleasant / obvious than a direct syntax would be:

interface C<R> {
    ...
}

declare const C: {
    new(source: number): C<Uint8Array>;
    new<R>(source: R): C<R>;
};
conartist6 commented 4 years ago

@VladRose I would not approve that code in code review. See: https://reactjs.org/docs/composition-vs-inheritance.html.

But also your example doesn't make sense to me, unless there's something I'm not getting. Why are you accessing the return value of super? It is legal for javascript classes to have constructors that return something other than this (part of the discussion here), but that's definitely not the case for React components. And because class PageLayer extends UserLayer<PageLayerProps, PageLayerState> the type of this will always be PageLayer and thus UserLayer<PageLayerProps, PageLayerState>, which is what you cast to anyway. So antipattern or not it seems to me like you've made a boondoggle to solve a problem you don't have.

ghost commented 4 years ago
  • newable: something which is not a class but can be invoked with the new keyword.

I just found that out the hard way:

new Proxy( {}, {} ) instanceof Proxy;

Hopefully, this proceeds and makes it into TS though,

conartist6 commented 4 years ago

@00ff0000red I don't think you're commenting in the right place. Proxy is defined as a newable and thus is allowed to specify a return type, and indeed it does.

Here is what I came up with for the problem you're describing. Feel free to paste that into a new issue.

ExE-Boss commented 4 years ago

Note that TypeScript also allows instanceof Function.prototype, even though it fails with a TypeError like instanceof Proxy.

This is because TypeScript makes all callables and newables have an implicit prototype property with a type of any: https://github.com/microsoft/TypeScript/blob/5d6cce5ca7b15bec206f47c6e210d2460c47f6ba/lib/lib.es5.d.ts#L298

greaterking commented 3 years ago

Can't you just const c = new MyClass() as MyInterface?

...just saying this completely saved my life. Not sure if it's lack of coffee or sleep but was way over thinking it...type assertion makes sense for the use case.

mariusmarais commented 3 years ago

There's a lot of "code smell" type comments here, advocating good over bad style, which is great when coaching newcomers.

But Typescript is supposed to allow typing Javascript "as she is spoke", i.e. as the JS runs today, so TS should be able to express the types, regardless of whether there are better or worse JS ways of doing it.

As far as my limited understanding of the TS project goals go, being able to express the types of JS in the wild is pretty high on the list. So we should talk about how to express this in TS code, not say "my code doesn't do that so you don't need to have TS type it correctly". Right?

For my own use case I want to use a class returning a proxy to generically wrap a database entity schema object, where the return type gets its keys from the schema keys, but with different return signatures, trapped by the proxy. While this absolutely works 100%, TS cannot express this without a factory function or the weird type assertion workarounds above, which is just silly.

My consumers should be able to go const thing = new Wrapped(Entity) and not something else just because not many people have this advanced need of classes and proxies.

MartinJohns commented 3 years ago

As far as my limited understanding of the TS project goals go, being able to express the types of JS in the wild is pretty high on the list. So we should talk about how to express this in TS code, not say "my code doesn't do that so you don't need to have TS type it correctly". Right?

I would agree in general. But you need to remember that the resources of the TypeScript team are finite. How common this practice is and whether there are alternative more suitable approaches must be considered when evaluating the priority and effort needed for this functionality.

conartist6 commented 3 years ago

I think the community could write a pull request, but we never got a thumbs up that what we're proposing is acceptable.

MilesBHuff commented 3 years ago

In current TypeScript, there is an implicit type of (typeof this) | void for constructors. Both of the following are valid:

(Just wanted to add the above in case anyone coming from Google is wondering whether they can return the class in the constructor, ie for chaining.)

sergeyampo commented 2 years ago

I write a js code with .d.ts types and have a Redis client wrapped with Proxy. @RReverser helped a lot with that ts trick.

DesignByOnyx commented 1 year ago

I just ran into this issue where I need to proxy my class instances like this:

interface ProxyFoo {
    bar: string;
}
class Foo {
    constructor() {
        return new Proxy(this, ...) as Foo & ProxyFoo;
    }
}

const foo = new Foo();
foo.bar; // Error: property "bar" does not exist on type "Foo"

The "bar" property is definitely available via my Proxy. Typescript should acknowledge whatever is returned from the constructor. This is mostly an advanced feature to be used by library maintainers and is not very likely to trip up your average developer.

kravetsone commented 3 months ago

+1

i really need it for constructor overload

export class Some<isTrue> {
    constructor(value: boolean): Some<true>
    constructor(): Some<false>
    constructor(value?: boolean) {}
}

https://www.typescriptlang.org/play/?#code/KYDwDg9gTgLgBAYwDYEMDOa4GUIFtgA8AlmgCpQCuwAfHAN4BQcziEAdmjJQjNABQA3FEioAuOACMIEJMBRsAlOJz4CXKtSYsE7Tt15Q+S7HkIAzYWhpbmOjup78hI4AH5xUmXMX0Avg18gA