Skip to content

static property inheritance complaining when it shouldn't #4628

Open

Description

@benlesh

I've seen that you've already closed many issues on this. But I'm reporting another one in hopes to wear you down ;)

It seems I can't change the signature of a static function in a derived class. The fact that the subclass is at all aware of the static functions on the super class is really strange.

I developed in C# for years, so I know what you were trying to do... but given that you don't support a new keyword like C# does, I think this behavior is really wrong. If you're looking to target being a superset of ES6/ES7, you need to correct this behavior ASAP. Generally in JavaScript, static properties are not copied to subclasses unless the copy is explicit.

I ran into this issue with RxJS Next, where I Subject must inherit from Observable, but they both need different static create() signatures. So I end up hacking around and fighting TypeScript.

Activity

  1. mhegazy commented on Sep 3, 2015

    @mhegazy
    Contributor

    The intuition we tried to match is that a specialized class can stand in for its base; which is a general intuition that an OOP developer would expect.

    can you expand more on your scenario, and assuming a base class is needed but not expected to be generic enough to handle the child?

  2. benlesh commented on Sep 3, 2015

    @benlesh
    Author

    The issue is specifically where Observable.create and Subject.create need to have different signatures.

    As you can see, I've found a workaround, but it's not ideal.

  3. benlesh commented on Sep 3, 2015

    @benlesh
    Author

    I guess ES6 does pull the static functions over. But I'm able to redefine them at will in ES6. TypeScript complains. It's problematic for functions that are idiomatic in many JavaScript libraries like static create functions.

  4. RyanCavanaugh commented on Oct 19, 2015

    @RyanCavanaugh
    Member

    Summary of discussion from the Slog today:

    • Static inheritance is part of the ES6 spec, but...
    • Some people use the static side of classes (i.e. constructor functions with static properties) polymorphically, and some don't
    • The first group wants this check as much as they want general substitutability checks for the instance side of classes
    • The second group doesn't care at all if their static members align or not
    • The first group should have their substitutability failures detected at the use sites, which is worse than at declaration sites, but still (mostly) works
    • The second group is just out of luck in our current design

    Conclusion: Try removing the check for assignability of the static side of classes and see what kind of errors go away (mostly in the baselines, since we mostly have clean code in our real-world-code baselines). Proceed depending on those results.

    The Committed tag here is tentative.

  5. changed the title [-]static property inheritance is incorrect[/-] [+]static property inheritance complaining when it shouldn't[/+] on Nov 10, 2015
  6. modified the milestone: on Sep 21, 2016
  7. SinoBoeckmann commented on Jan 25, 2018

    @SinoBoeckmann

    So... and now there a plans to change this? I mean, this is pretty old now? Plans for a survey or something?

  8. benwerner01 commented on Feb 7, 2020

    @benwerner01

    Static factory methods are a useful design paradigm, for instance when object creation relies on data that needs to be asynchronously fetched a static create() method can encapsulate the data fetching, and return the object type (as such fetches cannot be conducted directly in the constructor). This behaviour also aligns with the OOP principles in the popular object oriented languages C# and Java. Requiring that in inheritance each static method with the same name must have compatible types restricts this significantly (for create(), this would enforce the use of the same parameters). Is there an argument for maintaining this restriction?

    For example, consider the following:

    class Entity {
      id: string;
      name: string;
      constructor(id: string, name: string) {
        this.name = name;
        this.id  = id;
      }
    
      static async create = (name: string): Promise<Entity> => {
        const { id } = await db.createEntity(name);
        return new Entity(id, name);
      }
    }
    
    class Bob extends Entity {
      age: number;
      constructor(id: string, age: number) {
        super(id, 'Bob');
        this.age = age;
      }
    
      // TS Error: Types of property 'create' are incompatible
      static async create = (age: number): Promise<Bob> => {
        const { id } = await db.createBob(age);
        return new Bob(id, age);
      }
    }
    
  9. 11 remaining items

  10. richardgirges commented on Mar 3, 2022

    @richardgirges

    At the very least, is there any chance we can get the error to appear at the static method level, as opposed to the class? This would allow us to suppress it with // @ts-expect-error without having to suppress errors on the entire class.

  11. benlesh commented on Mar 4, 2022

    @benlesh
    Author

    FWIW: I'm going to stop watching this, but I'll leave it open for others. I'm okay with the current behavior because I try to avoid inheritance like the plague now. ... and static methods, TBH. haha. Partially because of this issue.

  12. shicks commented on Jun 6, 2022

    @shicks
    Contributor

    I still believe this is worth fixing (see my earlier comments), but I've come up with a free-after-typechecking workaround that at least unblocks me, so I thought I'd share it here:

    type OmitStatics<T, S extends string> =
        T extends {new(...args: infer A): infer R} ?
            {new(...args: A): R}&Omit<T, S> :
            Omit<T, S>;

    Example usage:

    class Base {}
    namespace Base {
      export enum EventType { A }
    }
    class Sub extends (Base as OmitStatics<typeof Base, 'EventType'>) {}
    namespace Sub {
      export enum EventType { B }
    }

    TypeScript fully understands the correct class relationship, but it doesn't complain about the otherwise-incompatible override anymore.

    Warning: nothing prevents you from calling an incompatibly-overridden static method unsafely (e.g. Base's static foo() calls this.bar(42) but Sub incompatibly overrides static bar(arg: string)). If you want that extra protection, you could take a broader approach by eliminating all statics from the base class (this has the added "benefit" of enforcing that statics are only called directly on the classes they're defined on, which I'd consider best practice as long as you're not relying on the class-side dynamism):

    type OmitAllStatics<T extends {new(...args: any[]): any, prototype: any}> =
        T extends {new(...args: infer A): infer R, prototype: infer P} ?
            {new(...args: A): R, prototype: P} :
            never;
    class Sub extends (Base as OmitAllStatics<typeof Base>) { /* ... */ }

    Playground examples

  13. zimtsui commented on Jul 27, 2022

    @zimtsui

    In functional type theory, a type is defined as a set of object along with a set of operation. It's analogous to an algebraic structure except it doesn't have to comply with closure axiom.

    The operations above may have the type as both its input or output.

    However, in TypeScript, method call is written as a.f() rather than f(a). This syntax cannot describe those operations which have another 3-party type as their input.

    For example, there is a type A, which have 2 operations:

    • capture an object's state as a snapshot.
    • restore as an object from a snapshot.
    capture :: A -> Snapshot
    restore :: Snapshot -> A

    In TypeScript, you can capture like a.capture(). But the restore operation's input is not A, you cannot write somthing like XXX.restore(), so you have to write it as a static method:

    class A {
    	public capture(): Snapshot;
    	public static restore(s: Snapshot): A;
    }
    const a = A.restore(snapshot);

    Therefore, the semantic meaning of static methods of a class is the algebra-like operations as well.

    This is why many non-functional OOP language all deicide to inherit static members automatically.

  14. fatcerberus commented on Jan 25, 2023

    @fatcerberus

    It's perhaps worth noting that constructor is already exempt from substitutability checks, and in JS the constructor function is the class, which means in practice there's a good chance a subclass can't stand in for its parent class anyway and at that point enforcing that all the static members of the subclass are compatible with the ones in the base class doesn't actually accomplish much.

  15. steveetm commented on Mar 1, 2023

    @steveetm

    Is the following the same or a bug?

    interface BaseEntity {
        id: number;
    }
    
    interface ExtendedEntity extends BaseEntity {
         someextendedprop: number;
    }
    
    class Base<T extends BaseEntity> {
            public static async bulkCreate<E extends BaseEntity, M extends Base<E>>(
                this: { new(): M },
            ): Promise<M[]> {
                return [new this];
            }
            doSomething():T {
                return {} as T;
            }
    };
    
    
    class Sub<T extends ExtendedEntity> extends Base<T> {
        public static async bulkCreate<E extends ExtendedEntity,M extends Sub<E>>(
            this: { new(): M },
        ): Promise<M[]> {
            return [new this];
        }
    };
    

    Strange, as if I remove doSomething tha static complain on class disappears. But I don't really understand what is the problem, the subclass returns instances which are compatible with base class instances.

    • If I have only the static declaration, everything is ok.
    • If I have only the member functions (as generic), everything is ok.
    • If I have both :
    Class static side 'typeof Sub' incorrectly extends base class static side 'typeof Base'.
      Types of property 'bulkCreate' are incompatible.
        Type '<E extends ExtendedEntity, M extends Sub<E>>(this: new () => M) => Promise<M[]>' is not assignable to type '<E extends BaseEntity, M extends Base<E>>(this: new () => M) => Promise<M[]>'.
          The 'this' types of each signature are incompatible.
            Type 'new () => M' is not assignable to type 'new () => Sub<ExtendedEntity>'.
              Type 'M' is not assignable to type 'Sub<ExtendedEntity>'.
                Type 'Base<E>' is not assignable to type 'Sub<ExtendedEntity>'.
                  The types returned by 'doSomething()' are incompatible between these types.
                    Type 'E' is not assignable to type 'ExtendedEntity'.
                      Property 'someextendedprop' is missing in type 'BaseEntity' but required in type 'ExtendedEntity'.(2417)
    
  16. shicks commented on Mar 1, 2023

    @shicks
    Contributor

    Yes, this is the same issue. The declared type of Sub.bulkCreate is not a subtype of the declared type of Base.bulkCreate. TypeScript is expecting the static methods to follow Liskov substitution, so that you could write

    Sub.bulkCreate.call(Base);
    

    But as you've written your Sub.bulkCreate signature, you've constrained the input type M to be narrower in the subclass than in the base class (it requires this to be Sub<ExtendedEntity>, rather than a Base<BaseEntity>). This is somewhat confusing for two reasons: (1) because function parameters are contravariant, so (arg: number) => void is actually a supertype of (arg: unknown) => void, even though number is a subtype of unknown. But also (2) TypeScript plays a little fast and loose with this rule by allowing methods (i.e. using method shorthand rather than a property with a colon) to break this rule, so a lot of people get away with breaking it without realizing what they're doing.

    The reason the error goes away when you remove doSomething is because TypeScript types classes structurally: so without that method the template parameter no longer shows up anywhere in the type, so that Sub<A> === Base<B> for all A and B, as far as the type checker is concerned (and Sub doesn't add any other properties, either). Removing the extra property from ExtendedEntity would also fix the error, since as you've written it Sub<A> === Base<A> already.

    Back to the original issue, there should be nothing wrong with what you've written. The type checker can see that typeof Sub is not assignable to typeof Base, and can enforce this just fine. If you wanted to make it assignable, you can, but it should let you choose to have it not be assignable (which it's not). You can use the workaround I posted earlier to remove bulkCreate from Base in the extends clause, but we shouldn't have to do that.

  17. steveetm commented on Mar 1, 2023

    @steveetm

    Stephen Hicks (@shicks) reading your workaround again after this explanation just really made it super clear, I can't upvote this enough. I switch would be nice to to disable this check(or make it appear on the function and just ignore it there), but I really think this could work well for more complex cases we use. Thanks!

    *update: Still have an issue with generic classes as now the generic properties simply grabbed from Base, not Base<T>. But trying to figure out something.

  18. added a commit that references this issue on Apr 17, 2023
  19. added a commit that references this issue on Jun 14, 2023
  20. hazae41 commented on Aug 27, 2023

    @hazae41

    How about a new keyword override in the child class, that would explicitly tell the compiler to fuck off?

  21. Gnuxie commented on Feb 15, 2024

    @Gnuxie

    So from what I can gather, the reason this issue is dead in the water is because of these concerns.

    However, the inaction has come at the cost that you cannot share the name of your static factory methods. As I understand this exact same restriction isn't applied to constructors, as the issue would appear far more frequently. Which makes sense because it would make using constructors completely intolerable. Well this is an intolerable issue and you simply cannot use static factory methods that will clash. The type gymnast workarounds also all suck, and no one should reasonably incorporate them into their code.

    The workaround I wanted to use is probably something like this: pay for the pain in verbose names, use the class name again in your static factory methods such that FooBar.factory = FooBar.FooBarFactory. However, even this sucks. If I wanted the factory to have a short name, for example having a Result static factory on an Error type to wrap them in a Result type, then you're all out of luck. There are for sure other valid examples where shorter and consistent names are desired.

    What makes it acceptable for this issue to be parked? The hope that very few users come across it without quickly finding a suitable workaround, but there does seem to be a lot of confidence in that. Meanwhile I'll compromise too by using different names.

  22. christianschmitz commented on Mar 6, 2024

    @christianschmitz

    FWIW: I'm going to stop watching this, but I'll leave it open for others. I'm okay with the current behavior because I try to avoid inheritance like the plague now. ... and static methods, TBH. haha. Partially because of this issue.

    Good point: "prefer composition over inheritance...". Hitting this issue in Typescript can help you decide in favor of composition (without necessarily completely avoiding inheritance for an entire codebase)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    CommittedThe team has roadmapped this issueSuggestionAn idea for TypeScript

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions