You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
static property inheritance complaining when it shouldn't #4628
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.
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?
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.
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.
So... and now there a plans to change this? I mean, this is pretty old now? Plans for a survey or something?
Reacted by Lucas, Minh Quy, Shaun Cutts, Mateja Petrovic, Nicolas Froidure, kleveque, Eduard Jacko, maharjans, Mauro Mandracchia, Dan Aprahamian and 19 more
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);
}
}
Reacted by everhardt, Sayan Pal, Jim Geurts, Brian Andress, Daniel Loureiro, Utkarsh Verma, AwesomeAdventure, Logan and Phips Peter
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.
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.
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:
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):
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->Snapshotrestore::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:
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.
Reacted by Stephen Hicks, Phips Peter and Alexander G
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)
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.
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.
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.
Reacted by Stephen Hicks, Brecht and Andreas Titan
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)
Activity
mhegazy commentedon Sep 3, 2015
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?
benlesh commentedon Sep 3, 2015
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.
benlesh commentedon Sep 3, 2015
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
createfunctions.RyanCavanaugh commentedon Oct 19, 2015
Summary of discussion from the Slog today:
staticproperties) polymorphically, and some don'tstaticmembers align or notConclusion: 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
Committedtag here is tentative.[-]static property inheritance is incorrect[/-][+]static property inheritance complaining when it shouldn't[/+]SinoBoeckmann commentedon Jan 25, 2018
So... and now there a plans to change this? I mean, this is pretty old now? Plans for a survey or something?
benwerner01 commentedon Feb 7, 2020
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 (forcreate(), this would enforce the use of the same parameters). Is there an argument for maintaining this restriction?For example, consider the following:
11 remaining items
richardgirges commentedon Mar 3, 2022
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-errorwithout having to suppress errors on the entire class.benlesh commentedon Mar 4, 2022
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.
shicks commentedon Jun 6, 2022
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:
Example usage:
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'sstatic foo()callsthis.bar(42)butSubincompatibly overridesstatic 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):Playground examples
zimtsui commentedon Jul 27, 2022
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 thanf(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:
In TypeScript, you can capture like
a.capture(). But therestoreoperation's input is not A, you cannot write somthing likeXXX.restore(), so you have to write it as a static method: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.
fatcerberus commentedon Jan 25, 2023
It's perhaps worth noting that
constructoris 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 thestaticmembers of the subclass are compatible with the ones in the base class doesn't actually accomplish much.steveetm commentedon Mar 1, 2023
Is the following the same or a bug?
Strange, as if I remove
doSomethingtha 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.shicks commentedon Mar 1, 2023
Yes, this is the same issue. The declared type of
Sub.bulkCreateis not a subtype of the declared type ofBase.bulkCreate. TypeScript is expecting the static methods to follow Liskov substitution, so that you could writeBut as you've written your
Sub.bulkCreatesignature, you've constrained the input typeMto be narrower in the subclass than in the base class (it requiresthisto beSub<ExtendedEntity>, rather than aBase<BaseEntity>). This is somewhat confusing for two reasons: (1) because function parameters are contravariant, so(arg: number) => voidis actually a supertype of(arg: unknown) => void, even thoughnumberis a subtype ofunknown. 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
doSomethingis because TypeScript types classes structurally: so without that method the template parameter no longer shows up anywhere in the type, so thatSub<A> === Base<B>for allAandB, as far as the type checker is concerned (andSubdoesn't add any other properties, either). Removing the extra property fromExtendedEntitywould also fix the error, since as you've written itSub<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 Subis not assignable totypeof 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 removebulkCreatefromBasein theextendsclause, but we shouldn't have to do that.steveetm commentedon Mar 1, 2023
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.docs: mark args as any
docs: mark args as any
feat: allow adding additional members to production parse methods (#745
hazae41 commentedon Aug 27, 2023
How about a new keyword
overridein the child class, that would explicitly tell the compiler to fuck off?Gnuxie commentedon Feb 15, 2024
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 aResultstatic factory on anErrortype to wrap them in aResulttype, 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.
christianschmitz commentedon Mar 6, 2024
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)
TSExternsGenerator: subclasses can't have static members with the sam…