microsoft / TypeScript

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

Allow static members in abstract classes to reference type parameters #34665

Open hdodov opened 5 years ago

hdodov commented 5 years ago

Search Terms

abstract generic class static type parameter

Suggestion

It has been previously concluded in #24018 that referencing class type parameters in the static side is problematic unless that class is meant to be extended. Well, abstract classes are meant to be extended, so it makes sense to allow type parameters to be used?

To quote @andy-ms in #24018:

Without inheritance that wouldn't make much sense:

class Super<T> {
    static m(x: T): void;
}
Super.m(); // What's `T`?

This is a valid point. But with a solution for abstract static members from #34516, this could be refactored like so:

abstract class Super<T> {
    abstract static m(x: T): void;
}

class A extends Super<number> {
    static m(x) {
        console.log(x * 42);
    }
}

class B extends Super<string> {
    static m(x) {
        console.log(x + ' World!');
    }
}

A.m(2);
B.m('Hello');

Use Cases

Pretty much everywhere that instance side types are related to static side types and where instance methods depend on static properties.

Examples

As an example I'll give my personal use case. I have a class with a static defaults property and instances merge it with a constructor argument of the same type but partial. Then, the resulting object is stored in an instance property:

abstract class Base<T> {
    static defaults: T
    config: T

    constructor(options: Partial<T>) {
        this.config = Object.assign({}, (this.constructor as typeof Base).defaults, options);
    }
}

interface Options {
    a: string
    b: number
}

class A extends Base<Options> {
  static defaults = {
      a: 42,    // Type '42' is not assignable to type 'string'.
      b: 'oops' // Type '"oops"' is not assignable to type 'number'.
  };
}

let inst = new A({
    a: 'bar', // OK
    b: 'baz'  // Type '"baz"' is not assignable to type 'number'.
});

inst.config.a = 12; // Type '12' is not assignable to type 'string'.

Checklist

My suggestion meets these guidelines:

j-oliveras commented 5 years ago

Duplicate of #32246, #32211, #24018 and others?

hdodov commented 5 years ago

@j-oliveras those issues are related to using type parameters in all classes (which is problematic). My suggestion is to allow them only in abstract classes, as they are meant to be inherited.

RyanCavanaugh commented 5 years ago

I think this is really putting the cart before the horse relative to #34516.

Anyway, A and B happen to be both concrete and non-generic, but they could vary on either axis without problem. Is the idea that this code would just be illegal because there's no legal parameter declaration for m ?

abstract class Super<T> {
    abstract static m(x: T): void;
}

class A<U> extends Super<U> {
    static m(x: __??__) {
        console.log(x * 42);
    }
}
hdodov commented 5 years ago

I think this is really putting the cart before the horse

That's true, but pointing out possibilities that #34516 could open will make it easier to determine it's value and whether it should be implemented, no?

Is the idea that this code would just be illegal because there's no legal parameter declaration for m?

I haven't thought about that case, but it makes sense, yes. After all, abstract classes allow for ambiguity. It's in their name. If you extend an abstract class and don't give concrete types for its parameters, there's still ambiguity left over, i.e. the resulting class should still be abstract.

So if you want to have a class with generics that extends an abstract class, it should be abstract itself. Your example should turn into:

abstract class Super<T> {
    abstract static m(x: T): void;
}

abstract class A<U> extends Super<U> {
    static m(x: U) {
        console.log(x * 42);
    }
}

...and then you use the resulting class:

class B extends A<number> {}
legowerewolf commented 4 years ago

I'd appreciate this. I'm running into this problem when I try to declare a type parameter for the shape of a config object for plugins in a plugin system I've created for a Discord bot project.

Right now, I have declared type any because different plugins (classes extending from an abstract Plugin class) use different config objects - they're not standardized because different plugins do different things, so a universal interface wouldn't be the right way to go. A type parameter would end up being used for the constructor's config argument, the non-static config member, and the static defaultConfig member (used to define the minimum working config).

It would be great to get my linter off my back the right way by providing actual types instead of shutting it up with rule-ignore comments.

RobertAKARobin commented 4 years ago

I also have a need for this.

aleksre commented 4 years ago

I would also love this feature to be added, but I don't see why it should be limited to abstract members? As long as the super class is abstract, it should be sufficient. Consumption of inherited members is allowed through the non-abstract class.

In my use case I have a generic service for creating, updating, deleting and fetching resources through REST. This service is extended with specific services that modify the path used in the request (and also add their own requests), but TypeScript is unable to correctly type the response.

abstract class ResourceService<T> {
    protected static resourcePath = 'override-me'

    static async fetch(id: string) {
        const url = `api/${this.resourcePath}/${id}`
        return getRequest<T>(url) // 'Static members cannot reference class type parameters.'
    }
}

class UserService extends ResourceService<User> {
    protected static resourcePath = 'users'

    // Additional functions specific to UserService
}

UserService.fetch('123') // GET api/users/123 -> Should be inferred as Promise<User>
antoniogamiz commented 3 years ago

I would also love this feature to be added, but I don't see why it should be limited to abstract members? As long as the super class is abstract, it should be sufficient. Consumption of inherited members is allowed through the non-abstract class.

In my use case I have a generic service for creating, updating, deleting and fetching resources through REST. This service is extended with specific services that modify the path used in the request (and also add their own requests), but TypeScript is unable to correctly type the response.

abstract class ResourceService<T> {
    protected static resourcePath = 'override-me'

    static async fetch(id: string) {
        const url = `api/${this.resourcePath}/${id}`
        return getRequest<T>(url) // 'Static members cannot reference class type parameters.'
    }
}

class UserService extends ResourceService<User> {
    protected static resourcePath = 'users'

    // Additional functions specific to UserService
}

UserService.fetch('123') // GET api/users/123 -> Should be inferred as Promise<User>

I have a very similar use case and I would like this to be implemented. Just commenting to see if this will move forward someday.

jeffhappily-seek commented 3 years ago

I need this as well, how can we push this forward?

frank-weindel commented 3 years ago

I would also love this feature to be added, but I don't see why it should be limited to abstract members? As long as the super class is abstract, it should be sufficient. Consumption of inherited members is allowed through the non-abstract class.

In my use case I have a generic service for creating, updating, deleting and fetching resources through REST. This service is extended with specific services that modify the path used in the request (and also add their own requests), but TypeScript is unable to correctly type the response.

I have a similar use case and was able to workaround this limitation with the mixin pattern. I've adapted your example to the pattern:

abstract class ResourceService {
    protected static resourcePath = 'override-me'
}

function withResourceService<T>() {
  abstract class ResourceServiceMixin extends ResourceService { // Incase you need to rely on `instanceof ResourceService`
    static async fetch(id: string) {
      const url = `api/${this.resourcePath}/${id}`
      return getRequest<T>(url) // 'Static members cannot reference class type parameters.'
    }
  }
  return ResourceServiceMixin;
}

class UserService extends withResourceService<User>() {
    protected static resourcePath = 'users'

    // Additional functions specific to UserService
}

UserService.fetch('123') // GET api/users/123 -> Should be inferred as Promise<User>

Warning, this apparently causes an error if compiler option "declaration": true is set. Due to #35822 😢

1mike12 commented 1 year ago

Huge need for this. I think the fact that abstract classes already are a thing makes this a no brainer.

Here's my use case if anyone's interested. Basically there's some very common patterns that this ORM I use doesn't have built in so I have to create a common static method on the BaseModel. But it would only ever be used by Child classes extending, but I want the children to return the correct arrays of themselves, not the parent. :

import { Model } from "@nozbe/watermelondb/"

export default abstract class BaseModel<T> extends Model {
  static query(...args) : Query<T> {
    return database.get<T>(this.table).query(...args) as Query<T>
  }
}

export default class EntryModel extends BaseModel<EntryModel> {
  static table = "entries"
  @text("siteId") siteId
  @text("title") title
}

// the results are automatically typed correctly as  EntryModel[]
const correctlyTypedResults = await EntryModel.query(Q.where("title", "something")).fetch()
// right now they are incorrectly typed as Model[]
reverofevil commented 4 months ago

Need this for an implementation of DI. I can't really see how type error is justified in the following example.

class Base<T> {
    static foo = (): T => ...;
}

class Child extends Base<number> {}

const x: number = Child.foo(); // T is unambiguous

The problematic code would be

Base.foo()

but that's precisely the case and location where an error message is required.

DanielOrtel commented 2 months ago

Is there anything we could potentially do to help move this along?

I see this is still awaiting feedback, but from what I can tell, there are multiple use cases already presented for why this would make sense. My use case is very similar, we're trying to migrate a REST API where we heavily rely on abstract classes to do all the heavy lifting. Those abstract classes are assigned other classes in static properties, so things like a Model could be reused and accessed anywhere from the class. Not to mention all the static properties that a Model class needs to have, like what its attributes are, what files it can upload, what kind of relationships it has, etc.

This framework was written in JS a while ago, and we've been trying to strongly type it for a while, but we're having to redefine types in different places, causing a lot of duplication and maintenance overhead, because we simply can't properly infer the types, due to TS not inferring the static properties correctly. We also have to keep a bunch of static props weakly typed, since we can't actually tell the static prop in the abstract class that it is dependent on the generic property defined in the child class.

Just ot add a different problem than what was already mentioned in this thread, weakly typed statics:

// current FieldDefinition
export type FieldDefinition = {
  [key: string]: Fields;
};

// what it should be
export type FieldDefinition<Attr extends AttributeDefinition> = {
  [key in keyof Attr]: Fields;
};

abstract class BaseResource<Attr extends AttributeDefinition> {
  static fields: FieldDefinition = { // <-- FieldDefinition should be FieldDefinition<Attr>, so we can always strongly type fields, and not end up in a situation where `name: 'write'` is defined on fields, but the type of name is forgotten about
    // id: 'read',
   // name: 'write'
  };
}

interface UserAttr {
  name: string;
  email: string;
}

class User extends BaseResource<UserAttr> {
  static fields: = {
    foo: 'write' <-- this should not be possible, but it is
    name: 'write' <-- this is okay
  } as const;
}

This leads to having to redefine the types everytime to get type inference to work, but runs the risk of forgetting, and then it defaults to any or unknown, which is not desirable and confusing in case a developer is not familiar with the myriads of workarounds we introduced to get types to work at least somewhat reasonably well. Overall, nothing we've looked at managed to solve our problems (factory pattern introduced different issues, for example), but allowing abstract class static methods to reference class type parameters would just solve the majority of our typing woes in the project.

Again, if there is anything that we can do to help move this along, I'd be happy to help.