(Update by @RyanCavanuagh)
Please see this comment before asking for "any updates", "please add this now", etc.. Comments not meaningfully adding to the discussion will be removed to keep the thread length somewhat reasonable.
(NOTE, this is _not_ a duplicate of Issue #1524. The proposal here is more along the lines of the C++ override specifier, which makes much more sense for typescript)
An override keyword would be immensely useful in typescript. It would be an optional keyword on any method that overrides a super class method, and similar to the override specifier in C++ would indicate an intent that "_the name+signature of this method should always match the name+signature of a super class method_". This catches a whole range of issues in larger code bases that can otherwise be easily missed.
Again similar to C++, _it is not an error to omit the override keyword from an overridden method_. In this case the compiler just acts exactly as it currently does, and skips the extra compile time checks associated with the override keyword. This allows for the more complex untyped javascript scenarios where the derived class override does not exactly match the signature of the base class.
class Animal {
move(meters:number):void {
}
}
class Snake extends Animal {
override move(meters:number):void {
}
}
// Add an additional param to move, unaware that the intent was
// to override a specific signature in the base class
class Snake extends Animal {
override move(meters:number, height:number):void {
}
}
// COMPILE ERROR: Snake super does not define move(meters:number,height:number):void
// Rename the function in the base class, unaware that a derived class
// existed that was overriding the same method and hence it needs renaming as well
class Animal {
megamove(meters:number):void {
}
}
// COMPILE ERROR: Snake super does not define move(meters:number):void
// Require the function to now return a bool, unaware that a derived class
// existed that was still using void, and hence it needs updating
class Animal {
move(meters:number):bool {
}
}
// COMPILE ERROR: Snake super does not define move(meters:number):void
As well as additional compile time validation, the override keyword provides a mechanism for typescript intellisense to easily display and select available super methods, where the intent is to specifically override one of them in a derived class. Currently this is very clunky and involves browsing through the super class chain, finding the method you want to override, and then copy pasting it in to the derived class to guarantee the signatures match.
Within a class declaration:
Indeed a good proposal.
However what about the following examples, which are valid overrides to me:
class Snake extends Animal {
override move(meters:number, height=-1):void {
}
}
class A {...}
class Animal {
setA(a: A): void {...}
getA(): A {...}
}
class B extends A {...}
class Snake extends Animal {
override setA(a: B): void {...}
override getA(): B {...}
}
Additionally I would add a compiler flag to force the override keyword to be present (or reported as a warning).
The reason is to catch when renaming a method in a base class that inherited classes already implement (but not supposed to be an override).
Ah nice examples. Generally speaking I would expect the use of the override keyword to enforce _exact_ matching of signatures, as the goal of using it is to maintain a strict typed class hierarchy. So to address your examples:
class C extends A {...}
var animal : Animal = new Snake();
animal.setA(new C());
// This will have undefined run-time behavior, as C will be interpreted as type B in Snake.setA
So example (2.) is actually a great demo of how an override keyword can catch subtle edge cases at compile time that would otherwise be missed! :)
And I would again stress that both examples may be valid in specific controlled/advanced javascript scenarios that may be required... in this case users can just choose to omit the override keyword.
This will be useful. We currently work around this by including a dummy reference to the super method:
class Snake extends Animal {
move(meters:number, height?:number):void {
super.move; // override fix
}
}
But this only guards against the second case: super methods being renamed. Changes in signature do not trigger a compilation error. Furthermore, this is clearly a hack.
I also don't think that default and optional parameters in the derived class method's signature should trigger a compilation error. That may be correct but goes against the inherent flexibility of JavaScript.
@rwyborn
It seems we don't expect the same behaviour.
You would use this override keyword to ensure a same signature, whereas I would use it more as a readibiliy option (so my request to add a compiler option to force its usage).
In fact what I would really expect is that TS detects invalid overriding methods (even without the use of override).
Typically:
class Snake extends Animal {
move(meters:number, height:number):void {}
}
should raise an error, because it's really an override of Animal.move() (JS behaviour), but an incompatible one (because height is not supposed to be optional, whereas it will be undefined if called from an Animal "reference").
In fact using override would only confirm (by the compiler) that this method really exists in the base class (and so with a compliant signature, but due to the previous point, not due to the override keyword).
@stephanedr , speaking as a single user I actually agree with you that the compiler should just always confirm the signature, as I personally like to enforce strict typing within my class hierarchies (even if javascript doesn't!!).
However in proposing that this behavior is optional via the override keyword I am trying to keep in mind that ultimately javascript is untyped, and hence enforcing the strict signature matching by default would result in some javascript design patterns no longer being expressible in Typescript.
@rwyborn I'm glad you mentioned the C++ implementation because that's exactly how I imagined it should work before I got here - optionally. Although, a compiler flag that forced the use of the override keyword would go down well in my book.
The keyword would allow compile time errors for a developers clumsy typing, which is what worries me most about overrides in their current form.
class Base {
protected commitState() : void {
}
}
class Implementation extends Base {
override protected comitState() : void { /// error - 'comitState' doesn't exist on base type
}
}
Currently (as of 1.4) the Implementation
class above would just declare a new method and the developer would be none the wiser until they notice their code isn't working.
Discussed at suggestion review.
We definitely understand the use cases here. The problem is that adding it at this stage in the language adds more confusion than it removes. A class with 5 methods, of which 3 are marked override
, wouldn't imply that the other 2 _aren't_ overrides. To justify its existence, the modifier would really need to divide the world more cleanly than that.
Please excuse the whining, but honestly, while your argument does apply to the public
keyword in a language where everything is public by default, quite world dividing indeed, having things like abstract
and an optional override
keyword will simply help developers feel safer, make less mistakes and waste less time.
Overriding is one of the few remaining highly-typo-sensitive aspects of the language, because a mistyped overriding method name is not an obvious compile time problem. The benefit of override
is obvious, as it you allows you to state your intention to override - if the base method doesn't exist its a compile time error. All hail the type system. Why would anyone not want this?
I concur 100% with @hdachev , the small inconsistency referred too by @RyanCavanaugh is easily out weighed by the benefits of the keyword in bringing compile time checks to method overrides. I would again point out that C++ uses an optional override keyword successfully in exactly the same way as suggested for typescript.
I can not emphasize enough how much of a difference override checking makes in a large scale code base with complex OO trees.
Finally I would add that if the inconsistency of an optional keyword really is a concern, then the C# approach could be used, that is the mandatory use of either the "new" or "override" keywords:
class Dervied extends Base {
new FuncA(newParam) {} // "new" says that I am implementing a new version of FuncA() with a different signature to the base class version
override FuncB() {} // "override" says that I am implementing exactly the same signature as the base class version
FuncC() {} // If FuncC exists in the base class then this is a compile error. I must either use the override keyword (I am matching the signature) or the new keyword (I am changing the signature)
}
This isn't analogous to public
because a property without an access modifier is known to be public; a method without override
is _not_ known to be non-override.
Here's a runtime-checked solution using decorators (coming in TS1.5) that produces good error messages with very little overhead:
/* Put this in a helper library somewhere */
function override(container, key, other1) {
var baseType = Object.getPrototypeOf(container);
if(typeof baseType[key] !== 'function') {
throw new Error('Method ' + key + ' of ' + container.constructor.name + ' does not override any base class method');
}
}
/* User code */
class Base {
public baseMethod() {
console.log('Base says hello');
}
}
class Derived extends Base {
// Works
@override
public baseMethod() {
console.log('Derived says hello');
}
// Causes exception
@override
public notAnOverride() {
console.log('hello world');
}
}
Running this code produces an error:
Error: Method notAnOverride of Derived does not override any base class method
Since this code runs at class initialization time, you don't even need unit tests specific to the methods in question; the error will happen as soon as your code loads. You can also sub in a "fast" version of override
that doesn't do any checking for production deployments.
@RyanCavanaugh So we are at Typescript 1.6 and decorators are still an experimental feature, not something I want to deploy in a large scale production codebase as a hack to get override working.
To try yet another angle, every popular typed language out there supports the "override" keyword; Swift, ActionScript, C#, C++ and F# to name a few. All these languages share the minor issues you have expressed in this thread about override, but clearly there is a large group out there who see that the benefits of override far out weigh these minor issues.
Are your objections purely based on cost/benefit? If I was to actually go ahead and implement this in a PR would it be accepted?
It's not just a cost/benefit issue. As Ryan explained, the issue is that marking a method as an override doesn't imply that another method _isn't_ an override. The only way that it would make sense is if all overrides needed to be marked with an override
keyword (which if we mandated, would be a breaking change).
@DanielRosenwasser As outlined above, in C++ the override keyword is optional (exactly as proposed for Typescript) yet everyone uses it no problem and it is hugely useful on large code bases. Furthermore in Typescript is actually makes a lot of sense for it to be optional because of javascript function overloading.
class Base {
method(param: number): void { }
}
class DerivedA extends Base {
// I want to *exactly* implement method with the same signature
override method(param: number): void {}
}
class DerivedB extends Base {
// I want to implement method, but support an extended signature
method(param: number, extraParam: any): void {}
}
As for the whole "doesn't imply that another method isn't an override" argument, It is exactly analogous to "private". You can write an entire codebase without ever using the private keyword. Some of the variables in that codebase will only ever be accessed privately and everything will compile and work fine. However "private" is some extra syntactic sugar you can use to tell the compiler "No really, compile error if someone tries to access this". In the same way "overload" is extra syntactic sugar to tell the compiler "I want this to exactly match the base class declaration. If it doesn't compile error".
You know what, I think you guys are fixated on the literal interpretation of "override". Really what it is marking up is "exactly_match_signature_of_superclass_method", but thats not quite as readable :)
class DerivedA extends Base {
exactly_match_signature_of_superclass_method method(param: number): void {}
}
I too would like to have the override keyword available, and have the compiler generate an error if the method marked for override doesn't exist, or has a different signature, in the base class. It would help readability and for refactoring
+1, also the tooling will get much better. My use case is using react. I've to check the definiton every time I'm using the ComponentLifecycle
methods:
``` C#
interface ComponentLifecycle
{
componentWillMount?(): void;
componentDidMount?(): void;
componentWillReceiveProps?(nextProps: P, nextContext: any): void;
shouldComponentUpdate?(nextProps: P, nextState: S, nextContext: any): boolean;
componentWillUpdate?(nextProps: P, nextState: S, nextContext: any): void;
componentDidUpdate?(prevProps: P, prevState: S, prevContext: any): void;
componentWillUnmount?(): void;
}
With override, or other equivalent solution,you'll get a nice auto-completion.
One problem however is that I will need to override interface methods...
``` C#
export default class MyControlextends React.Component<{},[}> {
override /*I want intellisense here*/ componentWillUpdate(nextProps, nextState, nextContext): void {
}
}
@olmobrutall it seems like your use case is better solved by the language service offering refactorings like "implement interface" or offering better completion, not by adding a new keyword to the language.
Lets not get distracted thou :) Language service features are only a small part of maintaining interface hierarchies in a large code base. By far and away the biggest win is actually getting compile time errors when a class way down in your hierarchy somewhere does not conform. This is why C++ added an optional override keyword (non-breaking change). This is why Typescript should do the same.
Microsoft's documentation for C++ override sums things up nicely, https://msdn.microsoft.com/en-us/library/jj678987.aspx
Use override to help prevent inadvertent inheritance behavior in your code. The following example shows where, without using override, the member function behavior of the derived class may not have been intended. The compiler doesn't emit any errors for this code.
...
When you use override, the compiler generates errors instead of silently creating new member functions.
By far and away the biggest win is actually getting compile time errors when a class way down in your hierarchy somewhere does not conform.
I have to agree. One pitfall that crops up in our teams is people thinking they've overridden methods when in fact they've slightly mis-typed or extended the wrong class.
Interface changes in a base library when working across many projects is harder than it should be in a language with an agenda like TypeScript.
We have so much great stuff, but then there are oddities like this and no class-level const properties.
@RyanCavanaugh true, but the language service could be triggered after writing override, as many languages do, without the keyword is much harder to figure out when is the right moment.
About implement interface, note that most of the methods in the interface are optional and you're meant to override only the few ones you need, not the whole package. You could open a dialog with check boxes but still...
And while my current pain point is to find the name of the method, in the future will be great to get notified with compile-time errors if someone renames or changes the signature of a base method.
Couldn't the inconsitency be solve just by adding a warning when you don't write override? I think typescript is doing the right thing adding small reasonable breaking changes instead of preserving bad decisions for the end of time.
Also abstract is already there, they will make an awesome couple :)
I felt the need for an 'override' specifier, too. In medium to large projects, this feature becomes essential and with all due respect I hope the Typescript team will reconsider the decision to decline this suggestion.
For anyone interested we've written a custom tslint rule that provides the same functionality as the override keyword by using a decorator, similar to Ryan's suggestion above but checked at compile-time. We'll be open-sourcing it soon, I'll post back when it's available.
I strongly felt necessity of 'override' keyword, too.
In my case, I changed some of method name in a base class and I forgot to rename some of names of overriding methods. Of course, this leads some of bugs.
But, If there was such a feature, we can find these class methods easily.
As @RyanCavanaugh mentioned, if this keyword is just a optional keyword, this feature makes confusion. So, how about to make something flag in tsc to enable this feature?
Please reconsider this feature again....
To me, if the override keyword is to be useful, it needs to be enforced, like it is in C#. If you specify a method in C# that overrides the signature of a base method, you _must_ label it either override or new.
C++ is annoying and inferior compared to C# in some places because it makes too many keywords optional, and therefore precludes _consistency_. For example, if you overload a virtual method, the overloaded method can be marked virtual or not - either way it will be virtual. I prefer the latter because it aids other developers who read the code, but I can't get the compiler to enforce it being put in, which means our code-base will undoubtedly have missing virtual keywords where they should really be. The override keyword is similarly optional. Both are crap in my opinion. What is overlooked here is that code can serve as documentation and improve maintainability by enforcing the need for keywords rather than a "take it or leave it" approach. The "throw" keyword in C++ is similar.
To achieve the above objective in TypeScript, the compiler needs a flag to "enable" this stringent behaviour or not.
Insofar as the function signatures of the base and the override are concerned, they should be identical. But it would be desirable to allow covariance in the specification of the return type to enforce a compile-time check.
I'm coming to TS from AS3 so of course, I'm going to throw my vote here for an override
keyword as well. To a developer who's new to any given codebase, seeing an override
is a huge clue as to what might be going on in a (child) class. I think such a keyword hugely adds value. I would opt to make it mandatory, but I can see how that would be a breaking change and so should be optional. I really don't see a problem with the ambiguity that imposes, though I am sensitive to the difference between an optional override
and the default public
keyword.
For all the +1ers -- can you talk about what's not sufficient about the decorator solution shown above?
To me it feels like an artificial construct, not a property of the language itself. And I guess that's by design, because that's what it is. Which I guess sends developer (well, me anyway) the message that it's transient, not a best practice.
obviously every language has its own paradigm and, being new to TypeScript I'm slow with the paradigm shift. I have to say though that override
DOES feel like a best practice to me for a number of reasons. I'm making the switch to TypeScript because I have fully swallowed the strongly-typed koolaid, and believe that the cost (in terms of keystrokes and learning curve) is heavily outweighed by the benefits both to error-free code and code comprehension. override
is a very important piece of that puzzle, and communicates some very important information about the role of the overriding method.
For me it's less about IDE convenience, though that is undeniably awesome when properly supported, and more about fulfilling what I believe to be the principles upon which you've built this language.
@RyanCavanaugh some issues that I see:
The compiler is already checking the argument list and return type, though. And I think even if override
did exist as a first-class keyword, we wouldn't enforce some new rule about signature identicalness
And I think even if override did exist as a first-class keyword, we wouldn't enforce some new rule about signature identicalness.
@RyanCavanaugh then I think you might be on a different page about the intent of the keyword. The whole point of it is for cases where you _want_ to enforce signature identicalness. This is for the design pattern where you have a deep complex hierarchy sitting on a base class that defines a contract of interface methods, and all classes in the hierarchy must _exactly_ match those signatures. By adding the override keyword on these methods in all the derived classes, you are alerted to any cases where the signature diverges from the contract set out in the base class.
As I keep saying this is not some esoteric use case. When working on a large code bases (with say hundreds or thousands of classes) this is an _every day_ occurrence, ie someone needs to change the signature of the base class (modify the contract), and you want the compiler to alert you to any cases throughout the hierarchy that no longer match.
The compiler will already warn of illegal overrides. The issue with the
current implementation is the lack of intent when declaring an overridden
method.
The decorator mentioned before just seems to go against what the language
is trying to achieve. There shouldn't be a runtime cost incurred for
something that can be dealt with at compile time, however small the cost.
We want to catch as much stuff going wrong as possible, without needing to
run the code to find out.
It's possible to achieve using the decorator and custom a custom tslint
rule, but would be better for the tooling ecosystem and the community for
the language to have an official keyword.
I think being optional is one approach, but as with with some C++ compilers
there are flags you can set which enforce its use (e.g. suggest-override,
inconsistent-missing-override). This would seem the best approach to avoid
breaking changes and seems consistent with other new features being added
recently such as nullable types and decorators.
On Wed, 23 Mar 2016 at 21:31, Rowan Wyborn [email protected] wrote:
And I think even if override did exist as a first-class keyword, we
wouldn't enforce some new rule about signature identicalness.@RyanCavanaugh https://github.com/RyanCavanaugh then I think you might
be on a different page about the intent of the keyword. The whole point of
it is for cases where you _want_ to enforce signature identicalness. This
is for the design pattern where you have a deep complex hierarchy sitting
on a base class that defines a contract of interface methods, and all
classes in the hierarchy must _exactly_ match those signatures. By adding
the override keyword on these methods in all the derived classes, you are
alerted to any cases where the signature diverges from the contract set out
in the base class.As I keep saying this is not some esoteric use case. When working on a
large code bases (with say hundreds or thousands of classes) this is an _every
day_ occurrence, ie someone needs to change the signature of the base
class (modify the contract), and you want the compiler to alert you to any
cases throughout the hierarchy that no longer match.—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
https://github.com/Microsoft/TypeScript/issues/2000#issuecomment-200551774
@kungfusheep fwiw the compiler only catches a certain class of illegal overrides, that is where there is a direct clash between declared params. It does _not_ catch the addition or removal of params, nor does it catch return type changes. These extra checks are what any override keyword would switch on.
The compiler correctly warns you if you _add_ a parameter to an overriding function.
Removing a parameter, though, is _totally valid_:
class BaseEventHandler {
handleEvent(e: EventArgs, timestamp: number) { }
}
class DerivedEventHandler extends BaseEventHandler {
handleEvent(e: EventArgs) {
// I don't need timestamp, it's OK
}
}
Changing the return type is also _totally valid_:
class Base {
specialClone(): Base { ... }
}
class Derived extends Base {
specialClone(): Derived { ... }
}
@RyanCavanaugh yes they are valid from a strict language perspective, but this is why I started using the term "contract" above. If a base class lays out a specific signature, when I use override I am stating that I want my derived class to strictly follow that signature. If the base class adds additional params or changes return type, I want to know every point in my code base that no longer matches, as the contract they were originally written against has now changed in some way.
The idea of having a big class hierarchy each with its own slightly different permutations of the base methods (even if they are valid from a language perspective) is nightmare inducing and takes me back to the bad old days of javascript before typescript came along :)
If the optionality is the main problem with the keyword, then why not make it mandatory when the base class method is defined as abstract
? This way, if you wanted to enforce the pattern strictly you could simply add an abstract base class. To avoid breaking code a compiler switch could disable the check.
We seem to be talking about two different sets of expectations now. There's the missing-override/illegal override situation and then there's the explicit signature one. Can we all agree the former is the absolute bare-minimum we'd expect from this keyword?
I only say this because there are other ways of enforcing explicit method signatures currently, such as interfaces, but there is currently no way of explicitly stating the intent to override.
Ok, there is no way of enforcing explicit signatures of overridden methods but given that the compiler enforces that any signature changes are at least 'safe' then it seems like there is a separate conversation to be had around the solution to that problem.
Yeah agreed. If I had to choose, the missing-override/illegal override situation is the more important problem to solve.
I'm a bit late to the party... I think the whole point of Typescript is to enforce rules at compile-time, not at runtime, otherwise we'd all be using plain Javascript. Also, it's a bit weird to use hacks/kludges to do things that are standard in so many languages.
Should there be an override
keyword in Typescript? I certainly believe so. Should it be mandatory? For compatibility reasons I'd say its behavior could be specified with a compiler argument. Should it enforce the exact signature? I think this should be a separate discussion, but I haven't had any problem with the current behavior so far.
The original reason for closing this seems to be that we cannot introduce the breaking change that all overridden methods needs the override
specifier, which would make it confusing since the methods not marked with override
could also in fact be overrides.
Why not enforce it, either with a compiler option, and/or if there is _at least one_ method marked override
in the class, then all methods that are overrides must be marked as such, otherwise it is an error?
Is it worth reopening this whilst it's up in the air?
On Fri, 8 Apr 2016 at 14:38, Peter Palotas [email protected] wrote:
The original reason for closing this seems to be that we cannot introduce
the breaking change that all overridden methods needs the override
specifier, which would make it confusing since the methods not marked with
'override' could also in fact be overrides.Why not enforce it, either with a compiler option, and/or if there is at
least 'one' method marked override in the class, then all methods that are
overrides must be marked as such, otherwise it is an error?—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/Microsoft/TypeScript/issues/2000#issuecomment-207434898
I say please reopen! I'd be happy with a compiler option.
yes - please reopen
Reopen this!
Doesnt ES7 require this overriding/overloading multiple methods having same
name?
On Apr 8, 2016 10:56 AM, "Aram Taieb" [email protected] wrote:
Yes, please reopen this!
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
https://github.com/Microsoft/TypeScript/issues/2000#issuecomment-207466464
+1 - for me this is the largest gap in type safety that I have in day-to-day TypeScript development.
Every time I implement a React lifecycle method like "componentDidMount" I search for the relevant react documentation page and copy/paste the method name, to ensure that I don't have a typo. I do this because it takes 20 seconds whereas the bugs from a typo are indirect and subtle, and can take much longer to track down.
class with 5 methods, of which 3 are marked override, wouldn't imply that the other 2 aren't overrides. To justify its existence, the modifier would really need to divide the world more cleanly than that.
If this is the major concern, call the keyword check_override
to make clear that you're opting in to override checking, and by implication the others methods are _not checked_ rather than _not overridden_.
What about using implements.
? Being something like this:
class MyComponent extends React.Component<MyComponentProps, void>{
implements.componentWillMount(){
//do my stuff
}
}
This syntax has some advantages:
implements.
the IDE has an excelent oportunity to show an autocompletion popup. class MyComponent<MyComponentProps, MyComponentState> {
implements.state = {/*auto-completion for MyComponentState here*/};
implements.componentWillMount(){
//do my stuff
}
}
Note: Alternatively we could use base.
, it's sorter and more intuitive, but maybe more confusing (defining or calling?) and the meaning is not so compatible with interface implementation. Example:
class MyComponent<MyComponentProps, MyComponentState> {
base.state = {/*auto-completion for MyComponentState here*/};
base.componentWillMount(){ //DEFINING
//do my stuff
base.componentWillMount(); //CALLING
//do other stuff
}
}
I don't think implements
sufficiently covers all of the use cases
mentioned previously & it's a little ambiguous. It doesn't give any more
information to an IDE that override
couldn't either, so it would seem to
make sense to stick with the terminology many other languages have used to
achieve the same thing.
On Wed, 13 Apr 2016 at 19:06, Olmo [email protected] wrote:
What about using implements.? Being something like this:
class MyComponent extends React.Component
{
implements.componentWillMount(){
//do my stuff
}
}This syntax has some advantages:
- It uses an already existing keyword.
- After writing implements. the IDE has an excelent oportunity to show
an autocompletion method.- the keyword clarifies that can be used to force checking on abstract
methods of base classes but also implemented interfaces.- the syntax is ambiguous enough to be usable also for fields, like
this:class MyComponent
{
implements.state = {/_auto-completion for MyComponentState here_/};implements.componentWillMount(){ //do my stuff }
}
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/Microsoft/TypeScript/issues/2000#issuecomment-209571753
The problem with override is that once you use the same keyword you have the same expectations:
abstract
. virtual
keyword to annotate methods that are meant to be overridden but have default behavior. I think the TS team doesn't want to add so much OO baggage to the language, and I think is a good idea.
By using implements.
you have a lightweight syntax for getting the main benefits: auto-complete and compile-time checked name, without inventing new keywords or incrementing the concept count.
Also has the benefit of working for classes and interfaces, and for methods (in the prototype) or direct fields.
Ways of making override
mandatory have already been discussed in the thread and the solutions aren't different to other features implemented by the compiler.
The virtual
keyword doesn't really make sense in the context of the language and neither is it named in a way that is intuitive for people who haven't used languages like C++ before. Perhaps a better solution to provide this type of guard, if needed, would be the final
keyword.
I agree language features should not be added hastily that could create 'baggage', but override
plugs a legitimate hole in the inheritance system that many other languages have deemed necessary. It's functionality is best achieved with a new keyword, certainly when alternative approaches are to suggest fundamental syntax changes.
What about setting inherited fields vs redefining new ones? React state
is a good example.
Or implementing optional interface methods?
override could potentially be used on fields, if they're being re-declared
in the subclass body. Optional interface methods aren't overrides so are
outside of the scope of what we're talking about here.
On Thu, 14 Apr 2016 at 11:58, Olmo [email protected] wrote:
What about setting inherited fields vs redefining new ones? React state
is a good example.Or implementing optional interface methods?
—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/Microsoft/TypeScript/issues/2000#issuecomment-209879217
Optional interface methods aren't overrides so are outside of the scope of what we're talking about here.
Optional interface methods are a concept fairly unique to TypeScript as far as I know, and I think a TypeScript implementation of "overrides" should apply to them to.
In the case of React lifecycle methods like componentDidMount, these are optional interface methods that have no implementation in the superclass.
In the case of React lifecycle methods like componentDidMount, these are optional interface methods that have no implementation in the superclass.
Exactly, so they don't override anything. I think we're getting confused about what the override
keyword is intended to provide here, because if you're just looking for a way of getting better code hinting/intellisense there are other ways that can be achieved without adding new keywords to the language.
Guys, with all due respect, can we please stay focused. I think that discussing alternative keywords is counterproductive, especially since the Issue was started with a specific request.
Can we all either agree that we need override
and ask nicely the Typescript team to add it or drop the request and let the Issue closed?
I think it's always useful to pose the request in the context of what the problem is, there's always different ways to solve the problem. The sticking point with override was around it being mandatory or not (some making the point that it isn't mandatory in C++). The problem a lot of people have referenced is getting the name right for overridable functions (which may or may not be implemented in the super class) - React's lifecycle functions being the main problem.
If override doesn't work, then maybe we should close this issue and open a more general point about this problem. The general problem here is that the interface contract with a superclass is untyped and unchecked, and that's tripping up developers and it would be great if TS or the tooling can help.
@armandn I don't think our lack of closure or the niceness of our suggestion is what's keeping the request rejected, but the differences between C# and TS semantics:
C# base method override:
override
keywordC# interface implementation:
So the behavior in C# are quite different depending if you ask about classes or interfaces, but in any case you get the main three benefits:
With slightly different semantics, we already have 1) in TS, unfortunately 2) and 3) are missing. The reason override
was rejected, in my opinion, is that a similar syntax assumes a similar behavior, and this not desirable because:
A class with 5 methods, of which 3 are marked override, wouldn't imply that the other 2 aren't overrides.
It happens that we already have 1) 2) and 3) when writing _object literals_, but not when writing class members that extend other classes or implement interfaces.
With this in mind I think we can all agree on this semantics:
check
or super.
, or MyInterface.
)abstract/virtual
but checks for member existence in the base class / implemented interfaces. (Benefit 2)Additionally, I think this two are necessary for completeness of the solution*:
This two points are useful in the very-real use case of React component implementation:
``` C#
class MyComponent
implements.state = {/auto-completion for MyComponentState here/};
implements.componentWillMount(){
//do my stuff
}
}
```
The Annotation-based solution from @RyanCavanaugh is not enough because:
@override
. Given the semantics, it's just about choosing the right syntax. Here some alternatives:
override componentWillMount()
: Intuitive but misleading.check componentWillMount()
: Explicit but keyword-consuming. super componentWillMount()
: implements componentWillMount()
: super.componentWillMount()
: implements.componentWillMount()
:this.componentWillMount()
: ReactComponent.componentWillMount()
:Opinions?
@olmobrutall good summary. Couple of points:
One keyword option (taken from C#) would be new - indicating a new slot:
class MyComp extends React.Component<IProps,IState> {
...
new componentWillMount() { ... }
componentWillMount() { ...} // would compile, maybe unless strict mode is enabled
new componentwillmount() { ... } <-- error
My other point is the issue with mandatory nature of using the above. As a contract between the parent super class and the derived class it makes sense to specify what are interface points where the above syntax would be valid. These are really internal extensions points for the super class, so something like:
class Component<P,S> {
extendable componentWillMount() {...}
}
The same would apply to the interface too.
Thanks :)
What about just writing this
?
class MyComponent<MyComponentProps, MyComponentState> {
this.state = {/*auto-completion for MyComponentState here*/};
this.componentWillMount(){
//do my stuff
}
}
About extendable
, there's already abstract
in TS 1.6, and adding virtual will maybe create again the split world problem?
Right, I thought about abstract but there may be an implementation in the super class, so it doesn't really make sense. Likewise with virtual, as that would imply non-virtual members are not virtual - which is misleading.
this.
works, I guess you could even have (as a long form):
this.componentWillMount = () => { }
The only issue with this is that it should only be scoped to earmarked extension points, not all members of the base class.
what is going on...
TypeScript isn't and never has been the javascript version of C#. So the reason isn't because the suggested functionality differs from the semantics of C#. The reasons for closure as stated by Ryan and then clarified by Daniel R were
As Ryan explained, the issue is that marking a method as an override doesn't imply that another method isn't an override. The only way that it would make sense is if all overrides needed to be marked with an override keyword (which if we mandated, would be a breaking change).
Yet you're still persisting with your problems around autocomplete. Autocomplete doesn't need a new keyword in the language to provide you with improved suggestions.
The reason this thread exists was to get compiler errors when a function is illegally overridden or when a method was declared to be an override but didn't actually override a function in a base class. It's a simple and well defined concept across many languages that also support most if not more language features typescript has to offer. It doesn't need to solve all the worlds problems, it just needs to be the override keyword, for overrides.
More people have since shown interest in the original proposal, so lets stick to the topic the issue was raised for and raise new issues for new ideas.
TypeScript isn't and never has been the javascript version of C#.
I'm comparing it with C# as a reference language, since I assume this is the behavior you guys are assuming.
Autocomplete doesn't need a new keyword in the language to provide you with improved suggestions.
How you suggest it to be triggered then? If you show a autocomplete combobox when writing a random name in a class context will be very annoying when we just want to declare a new field or method.
The reason this thread exists was to get compiler errors when a function is illegally overridden or when a method was declared to be an override but didn't actually override a function in a base class.
I absolutely included this use case, under Benefit 2.
It doesn't need to solve all the worlds problems, it just needs to be the override keyword, for overrides.
So your suggestion is to fix _one step at a time_ instead of moving one step back and look at similar/related problems?. That maybe a good idea for your Scrum Board but not for designing languages.
The reason they are conservative about adding the keyword is that they can not remove features from a language.
Some design mistakes in C# because of lack of completion:
var
only working for variable types, not for automatic generic parameters or return types. Mayble auto
will be a better keyword like in C++?Just try to use React for a second and you'll see the other side of the picture.
Overriding a method that's already been implemented in a base class and implementing an interface method are _two completely different things_. So yes, what's being suggested is to fix one of those scenarios with a dedicated keyword instead of trying to come up with some Swiss-Army keyword.
What good is a keyword that can mean any one of 3 different things to developers reading some code for the first time? It's ambiguous and confusing, especially if you're talking about using a keyword like this
which already does other (totally unrelated!) things in the language - it couldn't be more generic, it's almost useless.
If your main concern is auto-complete, editors have enough information _now_ to be able to suggest methods from base classes and implemented interfaces 'as you type'.
Overriding a method that's already been implemented in a base class and implementing an interface method are two completely different things.
In the general case yes, but we're not talking about implementing any interface method. We're talking about an optional interface method _where the parent class implements the interface_. In this case you can say that 1) interface permits the method to be implemented as undefined
, 2) the parent class has an undefined implementation, and 3) the child class is overriding the undefined implementation with a method implementation.
@olmobrutall I think your comment about designing languages and how it is not a scrum board is a little self-serving. I have seen about four updates to TS in the space of under a year.
If the language design was as well considered as you imply it should be, then there would already be a language spec document telling us exactly how overrides should work, and we mightn't even be having this conversation.
I don't make this comment with any denigration of the TS developers/designers, because TS is already excellent and I would dread having to use standard JS instead.
Yes TS is not C# and it's not C++. But a number of languages have chosen the override keyword to meet the objectives discussed here so it seems counterproductive to suggest totally alien syntax.
The chief issue seems to be not wanting to introduce a breaking change. The simple answer is a compiler flag, end of story. For some people like me, an optional override keyword is useless. For others they want to embellish their code incrementally. Compiler flag solves the dilemma.
Signature differences are a different conversation. The new keyword appears unnecessary because JS can't support multiple methods of the same name (unless TS creates signature-derived mangled names a'la C++, which is highly unlikely).
I have seen about four updates to TS in the space of under a year.
I don't mean you can not be fast and iterate quickly. I'm as happy as anybody that ES6 and TS are evolving fast. What I mean is that you have to try to predict the future, to avoid putting the language in a dead end.
I could agree on using override
keyword. With proper arguments even keeping fields and interfaces out of the scope, but I can not agree with the '_let's stay focused and solve just this particular problem the way other languages do it without thinking too much_' argument.
But a number of languages have chosen the override keyword to meet the objectives discussed here so it seems counterproductive to suggest totally alien syntax.
None of this languages have prototypal inheritance or optional method (methods that are neither abstract or virtual, they just could _not exist_ at runtime), and this are related problems that have to be discussed (and maybe discarded) before making a commitment.
Put another way: let's say we do as you seem to suggest and we implement override without thinking to much. Then me, or anybody else using TSX, adds an issue with why override
doesn't work with React components. Whats your plan?
In the general case yes, but we're not talking about implementing any interface method. We're talking about an optional interface method where the parent class implements the interface.
It doesn't matter where the interface was set up, the fact is they are not the same thing and so shouldn't share a keyword because the _intent_ of the program is not clear.
You could, for example, be overriding a method which had been implemented for interface compliance in a base class; If we were to put all our eggs in one keyword for these two different things, how would anyone then know if this was the initial declaration of that function or an override to one previously defined in a base class? You wouldn't, and it wouldn't be possible to know without further inspection of the base class - which may even be in a 3rd party .d.ts
file & thus making it an absolute nightmare to find, depending how deep in the inheritance chain the function was originally implemented.
Put another way: let's say we do as you seem to suggest and we implement override without thinking to much. Then me, or anybody else using TSX, adds an issue with why override doesn't work with React components. Whats your plan?
Why does this need to fix React? If React has a different problem to what this is trying to solve then I can't for the life of me fathom why override
needs to fix it? Have you tried opening another issue to suggest something be done about interface implementation?
I wouldn't agree that enough thought hasn't been put in to this. We're suggesting a tried and tested technique that's been successful in every other language I can think of that's implemented it.
the fact is they are not the same thing and so shouldn't share a keyword because the intent of the program is not clear.
Are not? Look at this two alternative definitions of BaseClass
class BaseClass {
abstract myMethod();
}
interface ISomeInterface {
myMethod?();
}
class BaseClass extends ISomeInterface {
}
And then in your code you do:
``` C#
class ConcreteClass {
override myMethod() {
// Do stuff
}
}
You think it should work in just one case and not in the other? The effect is going to be 100% identical in Javascript (creating a new method in ConcreteClass prototype), from the external interface and from the tooling perspective.
Even more, maybe you want to capture `this` inside of the method, implementing it with a lambda (useful for React event handling). In this case you'll write something like this:
``` C#
class ConcreteClass {
override myMethod = () => {
// Do stuff
}
}
The behavior will be again identical if the method is abstract or comes from the interface: add a field in the class with a lambda implementing it. But it looks a little bit strange to override
a field, since you're just assigning a value.
Not let's see it using super.
(my favorite syntax right now, but I'm open to alternatives).
``` C#
class ConcreteClass {
super.myMethod() { //method in prototype
// Do stuff
}
super.myMethod = () => { //method in lambda
// Do stuff
}
}
```
Now the concept is conceptually simpler: My super class says that there is a method or field and the ConcreteClass can define it in the prototype / assign it / read it / call it.
Why does this need to fix React?
Is not just react, look at angular:
Of course most of the interfaces are not meant to be implemented, neither all the classes to be overriden, but one thing is clear: in Typescript interfaces are more important than classes.
Have you tried opening another issue to suggest something be done about interface implementation?
How should I call it? override
for interface and fields?
We're suggesting a tried and tested technique that's been successful in every other language I can think of.
The languages you have in mind are quite different. They have inheritance based in a static VTable.
In Typescript a class is just an interface + automatic prototype inheritance of methods. And methods are just fields with a function inside.
In order to adapt the override
feature to TS this two fundamental differences have to be considered.
Please @kungfusheep make the effort to think about a solution to my problem. If you want to add different keywords and implement them in a second stage is O.K., but take a second to imagine how it should be.
I can't think of another way to say what I've already said. They are not the same. They are similar, but not the same. Please see this comment by one of the TS devs RE: the readonly keyword - https://github.com/Microsoft/TypeScript/pull/6532#issuecomment-179563753 - which strengthens what I'm saying.
I agree with the general idea, but let's see in practice:
class MyComponent extends React.Component<{ prop : number }, { value: string; }> {
//assign a field defined in the base class without re-defining it (you want type-checking)
assign state = { value : number};
//optional method defined in an interface implemented by the base class
implement componentDidMount(){
}
//abstract method defined in the base class
override render(){
}
}
This looks like VB or Cobol, doesn't it?
It looks like it makes sense, at least.
Consider this example, were there only the override (or just one) keyword.
interface IDo {
do?() : void;
}
class Component implements IDo {
protected commitState() : void {
/// do something
}
override public do() : void {
/// base implements 'do' in this case
}
}
Now lets implement our own component using what we just wrote.
class MyComponent extends Component {
override protected commitState(){
/// do our own thing here
super.commitState();
}
override do() : void {
/// this is ambiguous. Am I implementing this from an interface or overriding a base method? I have no way of knowing.
}
}
One way of knowing would be the type of super
:
override do() : void {
super.do(); // this compiles, if it was an interface then super wouldn't support `do`
}
exactly! which means the design is wrong. there shouldn't be an investigation involved with skimming code, it should just make sense when you read it.
this is ambiguous. Am I implementing this from an interface or overriding a base method? I have no way of knowing.
What's the difference in practice? The object has just one space of names and you can only place one lambda in the do
field. There's no way of explicitly implementing an interface anyway. The implementation is going to be:
MyComponent.prototype.do = function (){
//your stuff
}
independently of what you write.
It doesn't matter what the output is. You could be either intentionally or unintentionally overriding some functionality in a base class, but there is no intent in a keyword that is ambiguous.
What error or unexpected behavior will be solved by having two keywords?
Come on now mate. You're obviously a clever guy. I've just said "unintentionally overriding some functionality in a base class"; can we not infer from this any unexpected behaviour that could have potentially just occurred?
To be clear, I'm not proposing turning this issue into a proposal for two keywords. This issue is for the override keyword - anything else will need a new proposal and its own discussion on semantics.
To be clear, I'm not proposing turning this issue into a proposal for two keywords. This issue is for the override keyword - anything else will need a new proposal and its own discussion on semantics.
It doesn't really matter in how many issues it should be discussed, or from whom the idea comes. You propose to split two very related thinks and don't even consider a consistent syntax.
The arguments for whether we need 1, 2 or 3 keywords belong to this thread and is not finished (...but becoming repetitive). Then maybe we can discuss the syntax in another thread (because the semantics are going to be identical anyway :P).
In my example:
class MyComponent extends React.Component<{ prop : number }, { value: string; }> {
//assign a field defined in the base class without re-defining it (you want type-checking)
assign state = { value : number};
//optional method defined in an interface implemented by the base class
implement componentDidMount(){
}
//abstract method defined in the base class
override render(){
}
}
Don't assign
, implement
and override
do exactly the same thing: Check that the name exist (in the base class, the implemented interfaces, the interfaces implemented by the base class, etc...).
If there's a name conflict between base classes and some implemented interfaces you'll get an compile-time error whether with 1, 2 or no keyword at all.
Also think about the object literal:
var mc = new MyComponent();
mc.state = null;
mc.componentDidMount =null;
mc.render = null;
With exactly the same syntax I can re-assign fields or method independently if they come from the base class, direct interface implementations or interfaces implemented in the base class.
Don't assign, implement and override do exactly the same thing: Check that the name exist (in the base class, the implemented interfaces, the interfaces implemented by the base class, etc...).
You've just described 3 different scenarios there, so obviously they're not the same. I have a feeling I could describe why they're different to you all day and you'd still be sat arguing that they're not, so I'm going to bow out of this particular line of discussion for now. There's nothing to say the TS guys are still considering this at the moment anyway.
With the closure of #6118 I think there's reason to discuss if the problems there and the problems here can be addressed simultaneously.
I was not aware of https://github.com/Microsoft/TypeScript/pull/6118. The idea looks like a possible alternative to adding override
.
If I understood the problem properly, the issue is that you can have more than one base classes / interfaces with compatible but not identical member declarations, and they have to be unified when being initialized in the child class without a type.
Without having a good idea of the consequences, I would be happy if:
More important IMO would be to have some way of triggering auto-completion when writing class members (Ctrl+Space). When the cursor is directly inside of a class, you could be defining new members of re-defining inherited ones, so the auto-completion can not be too aggressive, but with manual triggering the behavior should be OK.
Regarding @RyanCavanaugh comments:
We definitely understand the use cases here. The problem is that adding it at this stage in the language adds more confusion than it removes. A class with 5 methods, of which 3 are marked override, wouldn't imply that the other 2 aren't overrides. To justify its existence, the modifier would really need to divide the world more cleanly than that.
Typing a variable as any
doesn't imply that some other variable is or isn't also any
. But, there is a compiler flat --no-implicit-any
to enforce that we explicitly declare it. We could equally have a --no-implicit-override
, which I for one would turn on if it were available.
Having an override
keyword that is used gives developers an enormous amount of insight when reading code that they are unfamiliar with, and the ability to enforce it would give some additional compile time control.
For all the +1ers -- can you talk about what's not sufficient about the decorator solution shown above?
Is there any way in which a decorator a better solution than an override keyword? There are many reasons why it is worse: 1) It adds runtime overhead however small; 2) It's not a compile time check; 3) I have to include this code to every one of my libraries; 4) There is not way for this to catch functions that are missing the override keyword.
Let's give an example. I have a library with three classes ChildA
, ChildB
, Base
. I've added some method doSomething()
to ChildA
and ChildB
. After some refactoring, I've added some additional logic, optimized, and moved doSomething()
to the Base
class. Meanwhile, I have another project which depends on my library. There I have ChildC
with doSomething()
. There is no way when I update my dependencies to find out that ChildC
is now implicitly overriding doSomething()
, but in an unoptimized way that is also missing some checks. That is why an @overrides
decorator will never be sufficient.
What we need is an override
keyword, and a --no-implicit-override
compiler flag.
An override
keyword would help me a lot since I use in my project a simple hierarchy of base classes to create all my components. My problem lies in the fact that these components may need to declare a method to be used somewhere else, and this method may or may not be defined in a parent class and may or may not already do stuff that I need.
For example, let's say that a function validate
takes a class with a getValue()
method as parameter. To build this class, I can inherit another class that could already define this getValue()
method, but I can't really know this unless I look at its source code. What I'd instinctively do is implement this method in my own class, and as long as the signature is correct no one will tell me anything.
But maybe I shouldn't have done that. The following possibilities all suppose that I did an implicit override:
super
in my override, but no one suggested me to do so. Having a mandatory override keyword would have told me "hey, you're overriding, so maybe you should check what the original method looks like before doing your stuff". That would greatly improve my experience with inheritance.
Of course, as suggested, it should be placed under a --no-implicit-override
flag since it would be a breaking change and that most people do not care that much about all this.
I like the comparaison @eggers made with any
and --no-implicit-any
because it's the same kind of annotation and it would work in the exact same way.
@olmobrutall I was looking at some of your talk about overriding abstract methods & interface methods.
If my opinion, override
implies the existence of a super.
Neither abstract methods nor methods defined in an interface can be called via a super.
call, and so should not be overrideable (there is nothing to override). Instead if we do something more explicit for those cases, it should be an implements
keyword. But, that would be a separate feature discussion.
Here are the problems as I see them, ranked most important to least. Did I miss anything?
It's easy to _think_ you're overriding a base class method when you're not:
class Base {
hasFilename(f: string) { return true; }
}
class Derived extends Base {
// oops
hasFileName(f: string) { return false; }
}
This is probably the biggest problem.
This is very closely related, especially when the implemented interface has optional properties:
interface NeatMethods {
hasFilename?(f: string): boolean;
}
class Mine implements NeatMethods {
// oops
hasFileName(f: string) { return false; }
}
This is a less important problem than _failure to override_, but it's still bad.
It's possible to override a base class method without realizing it
class Base {
hasFilename(f: string) { return true; }
}
class Derived extends Base {
// I didn't know there was a base method with this name, so oops?
hasFilename(f: string) { return true; }
}
This should be relatively rare just because the space of possible method names is very large, and the odds that you'd also write a compatible signature _and_ not mean to have been doing this in the first place are low.
any
sIt's easy to think that override methods will get the parameter types of their base:
class Base {
hasFilename(f: string) { return true; }
}
class Derived extends Base {
// oops
hasFilename(f) { return f.lentgh > 0; }
}
We tried to type these parameters automatically but ran into problems. If hasFilename
were explicitly marked override
we might be able to solve those problems more easily.
It's not immediately clear when a method is overriding a base class method and when it's not. This seems like a smaller problem because there are reasonable workarounds available today (comments, decorators).
You have to manually type out base method names when writing derived class override methods, which is annoying. We could easily fix this by always providing those names in the completion list (I think we actually have a bug on this already), so we don't really need a language feature to fix this problem.
Listing these out as things that either belong in a separate issue or simply aren't going to happen:
override
, and it really does have undesirable semantics in many good scenariosimplements.foo = ...
). No need to bikeshed here@RyanCavanaugh I agree with all of the above in about that order as well. Just as a comment, I don't think that the override
keyword should be solving the "failure to implement" problem. As I said above, I think that problem should be solved by a different keyword (e.g. implements
) as part of a different ticket. (override
should imply a super.
method)
@RyanCavanaugh I agree with all the points but I don't see any reference to the also very related problem of initialized fields declared in a parent type without overriding the type (and then loosing type checking). I think the feature should not be confined to method declarations in a language where methods are just functions in object fields.
@eggers even if at the end the end two keywords are needed, the semantics will be very similar and I think the features should be discussed together.
override should imply a super. method
In C# override can (and must) be used also for abstract methods (with no .super). In Java @override is an attribute. Of course they are different language but with similar keywords you expect similar behaviors.
I agree with @RyanCavanaugh 's points, though I'd say the "accidental override" problem might be a little more common than he believes (especially when dealing with extending a class that already extends something else, like a React component that would already define a componentWillMount
method in the first extension).
I believe though, like @eggers said, that the "failure to implement" issue is something pretty different, and it would feel weird to use an override
keyword for a method that doesn't exist in a parent class. I know these are different languages with different problematics, but in C# override
isn't used for interfaces. I would suggest, if we could get a --no-implicit-override
flag, that interface methods would have to be prefixed with the interface name (which would look a lot like C# and thus feel more natural).
Something like
interface IBase {
method1?(): void
}
class Base {
method2() { return true; }
}
class Test extends Base implements IBase {
IBase.method1() { }
override method2() { return true; }
}
I think @RyanCavanaugh lists the fundamental problems. I would complement that with "What are the disagreements":
override
? For example: interface IDrawable
{
draw?( centerPoint: Point ): void;
}
class Square implements IDrawable
{
draw( centerPoint: Point ): void; // is this an override?
}
override
? interface IPoint2
{
x?: number;
y?: number;
}
class Circle implements IPoint2
{
x: number; // is this an override?
y: number; // is this an override?
radius: number;
}
implements
keyword, to handle the above cases instead of an override
keyword, and the form it would take.@kevmeister68 thanks for explicitly stating the disagreements.
One thing: you have to make the interface members optional to really show the problem, like this the typescript compiler will complain when a field or method is not implemented, solving "Failure to implement".
@olmobrutall Thanks for that. I've never declared a method optional - can it be (I come from a C# background and there is no such concept)? I've updated the examples I listed above to change the member variables to optional - is that better?
@kevmeister68 also the draw
method in IDrawable
:)
@RyanCavanaugh
Insufficiently exotic syntax (e.g. implements.foo = ...). No need to bikeshed here
Thanks for teaching me a new expression. I don't see a lot of problems in the semantics of the feature:
Most of the problems rely on the syntax and the behaviours that they impliy:
Let's compare the tree more promising syntaxes:
class Person{
dateOfBirth: Date;
abstract talk();
walk(){ //...}
}
interface ICanFly{
fly?();
altitude?: number;
}
class SuperMan extends Person implements ICanFly {
dateOfBirth = new Date(); //what goes here?
override talk(){/*...*/}
walk = () => {/* force 'this' to be captured*/} //what goes here
implements fly() {/*...*/}
altitude = 1000; //what goes here?
}
Advantages:
extends
/ implements
. Disadvantages:
override
or implements
feels right. Maybe super.
, but then what we do with interfaces?function()
the same problems happens.InterfaceName.
class Person{
dateOfBirth: Date;
abstract talk();
walk(){ //...}
}
interface ICanFly{
fly?();
altitude?: number;
}
class SuperMan extends Person implements ICanFly {
dateOfBirth = new Date(); //what goes here?
override talk(){/*...*/}
walk = () => {/* force 'this' to be captured*/} //what goes here
ICanFly.fly() {/*...*/}
ICanFly.altitude = 1000; //what goes here?
}
Advantages:
super.
for them. Disadvantages:
this.
class Person{
dateOfBirth: Date;
abstract talk();
walk(){ //...}
}
interface ICanFly{
fly?();
altitude?: number;
}
class SuperMan extends Person implements ICanFly {
this.dateOfBirth = new Date();
this.talk(){/*...*/}
this.walk = () => {/* force 'this' to be captured*/}
this.fly() {/*...*/}
this.altitude = 1000;
}
Advantages:
this.
to trigger auto-completion feels natural. Disadvantages:
Whilst failure to implement a non-optional interface method will cause a compiler error, if the interface changes at some point in the future (lets say a method is removed) there will be no warning for all classes that implemented that method. This might not be as big an issue as optional's but I think it's fair to say the scope of this goes beyond them.
I do however, still believe override
is a distinctly different thing and potentially having two keywords rather than one would better convey the intent of the program.
For example, consider a scenario where a developer _believes_ they are implementing an interface method when in-fact they're overriding the method which has already been declared in a base class. With two keywords the compiler can prevent this kind of mistake and throw an error to the tone of method already implemented in a base class
.
interface IDelegate {
execute?() : void;
}
class Base implements IDelegate {
implement public execute() : void { /// fine, this is correctly implementing execute
}
}
class Derived extends Base {
implement public execute() : void {
/// ERROR: `method "execute():void" already implemented in a base class`
}
}
I don't know if this is relevant in JS, but class fields are supposed to be private in OO, so I don't think we need to be overriding fields explicitly since the fact that they're private already prevent us to override altogether.
Instance methods are fields though, and from what I gather a lot of people use them instead of prototype methods. About that,
class Person{
walk(){ //...}
}
class SuperMan extends Person {
walk = () => {/* force 'this' to be captured*/}
}
is a compiler error, because walk
is a prototype method in Person
and an instance method in SuperMan
.
Anyway, i'm not sure override
would be a fit here, because well, we don't override fields. Again, it would look like C#, but I'd rather use the new
keyword instead of override
here. Because it's not the same thing as a method override (I can call super.myMethod
in an override, not here).
My preferred solution would then be something like (assuming we're in strict override mode):
class Person{
dateOfBirth: Date;
talk() { }
walk = () => { }
}
interface ICanFly {
fly?();
altitude?: number;
}
class SuperMan extends Person implements ICanFly {
new dateOfBirth = new Date();
override talk() { }
new walk = () => { }
implements fly() {/*...*/}
implements altitude = 1000;
}
My main concern is about interfaces though. We shouldn't have to write implements
for non optional interface members, because we'll be already caught by the compiler. Then, if we do so, there'll be a confusion between what is from an interface and what isn't, since not all interfaces members would be prefixed by implements
. And that word is a mouthful. I'm not ready to write something like this:
class C extends React.Component {
implements componentWillMount() { }
implements componentDidMount() { }
implements componentWillReceiveProps(props) { }
/// ... and the list goes on
}
I proposed to prefix with the interface name earlier but @olmobrutall showed me that it was a worse idea.
Anyway, I'm pretty convinced that we'll need 3 different new keywords to properly tackle this.
I also don't think that it's necessary to implicitly type the overrides since the compiler will already prevent us to write something that's not compatible, especially because it is apparently difficult to do right.
Isn't new
, override
and implement
too much keyword overhead for essentially the same JS semantics? Even if in C# they are different things there is virtual no difference in JS/TS.
It's also quite ambiguous:
new
for classes but implements
for interfaces? override
, implements
, any of the two or both at the same time? Also think about this:
class Animal {
}
class Human extends Animal {
}
class Habitat {
owner: Animal;
}
class House extends Habitat {
owner = new Human();
}
var house = new House();
house.owner = new Dog(); //Should this be allowed??
The question is:
Does owner = new Human();
re-define the field with a new (but compatible) type, or just assigns a value.
I think in general you re-defined the field except if you using the _magic keyword_. My preference is for this.
right now.
class House extends Habitat {
this.owner = new Human(); //just setting a value, the type is still Animal
}
@olmobrutall That might indeed be a bit much keyword overhead, but all 3 are indeed _different_ things.
override
would apply to methods (defined on the prototype), meaning that it does _not_ erase the original method, which is still accessible behind super
.new
would apply to fields, and mean that the original field has been erased by the new one.implements
would apply to anything from an interface, and mean that it does not erase or replace anything that already existed in a parent class, because an interface "doesn't exist".If you're changing a method from a base class that was implemented with an interface what should you use? I can see four possibilities: override , implements , any of the two or both at the same time?
It seems pretty logical to me that it would be override
since that what you're effectively doing here. implements
would mean that you're implementing something from the interface that doesn't exist otherwise. In a sense, override
and new
would have priority over implements
.
And about your field override exemple, nothing should change about how it works today. I'm not sure what happens here, but whatever it is it shouldn't change (or maybe it should, but that's not what we're discussing here).
I feel that override
would be suitable for both base methods and properties, including abstract ones (explained below).
new
is an interesting idea in concept, but this particular keyword may be confused with class instantiation, so it may give the false impression it would only work for classes, despite the fact that properties can have primitive, interface, union or even function types. Perhaps a different keyword like reassign
could work better there, but it has the problem that it may give a false idea in the case the value is only re-declared but not actually assigned with anything in the derived class (new
may have that problem as well). I think redefine
is also interesting, but could lead to the mistaken expectation that the 'redefined' property could also have a different type than the base one, so I'm not sure.. (_edit: actually I checked and it could have a different type as long as the new one is a subtype of the base one, so this may not be that bad_).
implement
(I prefer this particular verb form for consistency with override
) seems to work for interfaces. I believe it could also technically work for abstract base methods as well, but using override
could feel more consistent, despite the slightly different semantics. Another reason is that it would make it a bit inconvenient to change a method from abstract to non-abstract, as one would need and go to all the derived classes and change override
to implement
.
There could be better ideas, but that's all I have at this point..
I proposed the keyword new
because it's the one C# uses for this exact feature, but I agree this isn't the best one. implement
is indeed a better choice over implements
. I don't think we should use it for abstract methods since they're part of a base class and not an interface. override
+ new
-> base class, implement
-> interface. That seems clearer.
@JabX
I've checked and you're right:
class Person{
walk(){ //...}
}
class SuperMan extends Person {
walk = () => {/* force 'this' to be captured*/}
}
Fails with Compiler error: Class 'Person' defines instance member function 'walk', but extended class 'SuperMan' defines it as instance member property.
I don't really see the reason to fail in this case, since the parent type contract is fulfilled and you can write this anyway:
var p = new SuperMan();
p.walk = () => { };
Or even
class SuperMan extends Person {
constructor() {
super();
this.walk = () => { };
}
}
override
override would apply to methods (defined on the prototype), meaning that it does not erase the original method, which is still accessible behind super.
That's actually an argument. There is least _some_ observable difference between override
and implements
... but not quite.
Both override
and implements
are implemented in the prototype
, being able to use super
is independent, because you call super.walk()
not just super()
, and is available in every method ( overrides, implements, news and normal definitions).
class SuperMan extends Person implements ICanFly {
new dateOfBirth = new Date();
override talk() { } //goes to prototype
new walk = () => { }
implements fly() {/*...*/} //also goes to the prototype
implements altitude = 1000;
}
And being able to use super.walk()
also works if you assign to the instance
class SuperMan extends Person {
constructor() {
super();
this.walk = () => { super.walk(); };
}
//or with the this. syntax
this.walk = () => { super.walk(); };
}
There's already a clear way to differentiate prototype
fields from instance fields and is the =
token.
class SuperMan extends Person implements ICanFly {
this.dateOfBirth = new Date(); //instance
this.talk() { } //prototype
this.walk = () => { } //instance
this.fly() {/*...*/} //prototype
this.altitude = 1000; //instance
}
With the this.
syntax the problem is solved more elegantly:
this.
means check my type (base classes and implemented interfaces).=
means assign to the instance instead of the prototype. New
new
would apply to fields, and mean that the original field has been erased by the new one.
Take into account that you can not change the field or method with a different type because you will break the type system. In C# or Java when you do new
the new method will be used only on statically dispatched calls to the new type. In Javascript all the calls will be dynamic and if you change the type the code will likely break at run-time (Coercing the type could be allowed for practical purposes however, but not widening).
class Person {
walk() { }
run() {
this.walk();
this.walk();
this.walk();
}
}
class SuperMan extends Person {
new walk(destination: string) { } //Even if you write `new` this code will break
}
So more than new
the keyword should be assign
, because this is the only type-safe thing you can do:
class Person{
dateOfBirth: Date;
}
class SuperMan extends Person implements ICanFly {
assign dateOfBirth = new Date();
}
Note that reassign
doesn't make sense, because Person only declares the field but doesn't set any value on it.
Writing assign
seems redundant however, it's clear that we're assigning because we have a =
at the other side.
Again the this.
syntax solves the problem gracefully:
class Person{
dateOfBirth: Date;
}
class SuperMan extends Person implements ICanFly {
this.dateOfBirth = new Date();
}
I'm not sure how you apply this to fields in a way that makes sense.
We're talking about adding a keyword that is only applicable in the class body, yet fields can be reassigned at _any point_ during the instances lifetime.
Typescript already allows you to initialize fields in the class body:
class Person {
dateOfBirth : new Date();
}
It also allows you to re-initialize fields declared in the parent class, but the situation bad. Of course you could write the name wrongly (similar problem to override
) but the type is also erased. This means that:
class Person {
fullName: { firstName: string; lastName?: string };
}
class SuperMan extends Person {
fullName = { firstName: "Clark" };
bla() {
this.fullName.lastName; //Error
}
}
This code fails with: Property 'lastName' does not exist on type '{ firstName: string; }'. because the original type has been erased!.
We're talking about adding a keyword that is only applicable in the class body, yet fields can be reassigned at any point during the instances lifetime.
Also methods:
class Person {
talk() { }
}
class SuperMan extends Person {
talk() { }
changeMe(){
this.talk = () => { };
}
changeMyPrototype() {
SuperMan.prototype.talk = () => { };
}
}
@kungfusheep I agree, fields can be reassigned, but so do methods on the prototype.
The main reason why we would need a keyword for "overriding" fields is because instance methods _are_ fields and they're widely used instead of proper prototype methods because they're lambdas and thus autobind the context.
@olmobrutall
It's forbidden and with good reason I believe (it's not the same thing so it shouldn't be compatible), but you're right that it's possible to do so when directly manipulating the instance of the class. I don't know exactly why it is allowed, but we're not here to change how the language works so I believe we shouldn't interfere here.
Class interfaces describe the instance side of a class, meaning fields as well as methods from the prototype. I don't even think there's a difference between instance and prototype methods in an interface, I believe you can describe a method as one or the other, and implement it as one or the other. So implement
could (and should) also apply to fields.
new
should have the same semantics as override
does, so obviously you can't change the type to something that isn't compatible. I mean, again, we're not changing how the language works. I just wanted a separate keyword because it's not the same kind of redefinition. But, I agree that, as you showed me, we can already tell fields and methods apart by the use of =
, so maybe the same keyword/syntax could be used were without ambiguity.
However, the unified field/method override syntax shouldn't be this.method()
or this.field
because it looks a lot like something that could be valid Javascript, while it's not. Adding a keyword before the member would be clearer towards the fact that is indeed a Typescript thing.
this
is a fine idea though, so maybe we could drop the dot and write something less ambiguous like
class Superman {
this walk() { }
}
But still, it looks a but weird. And mixing it with implement
would look a bit out of place (because we're okay with the fact that this this
syntax wouldn't be used with interface?), and I like the implement
/override
duo. An override
modifier on fields still irks me, but maybe it is better that having a third ambiguous new
keyword. Since the assignation (=
) makes the difference between a method and a field clear, that would be fine by me now (as opposed to what I was saying a few hours ago).
I agree, fields can be reassigned, but so do methods on the prototype.
Yeah, modifying the prototype directly, in TypeScript, is very much in the realms off messing around under the hood though. It's nowhere near as common as simply re-assigning a property on an instance of something. We very rarely use fields as functions, over a 150k+ line codebase, so I think to say it's widely used is maybe subjective.
I agree with most of your other points, but I'm not sold (and by the sound of it, neither are you...) about the use of the new
keyword in this context.
We very rarely use fields as functions, over a 150k+ line codebase, so I think to say it's widely used is maybe subjective.
I don't either, but I have to resort to an @autobind
decorator to properly bind this
in my methods, which is more cumbersome than simply writing a lambda instead. And as long as you don't use inheritance, using a field works like you expect. I was under the impression that, at least in the JS/React world, most people using ES6 classes were using arrow functions as methods.
And interface methods can be implemented either with a proper method or with an instance method, which does not help with clarifying the difference.
I believe fields have the same problems as methods as far as overrides are concerned.
class A {
firstName: string;
get name() {
return this.firstName;
}
}
class B extends A {
firstname = "Joe" // oops
}
A possible workaround here is to assign the field in the constructor
class B extends A {
constructor() {
this.firstName = "Joe"; // can't go wrong
}
}
but that's not applicable for interfaces. That why I (and others here) believe we need something to check the pre-existence of a field when declaring it directly in the class body. And it's not an override
. Now that I put some more thoughts here, I believe the issue with fields is exactly the same as the issue with interface members. I'd say we need an override
keyword for methods that already have an implementation in a parent class (prototype methods, and maybe instance methods too), and another for things that are defined but without implementation (non-function fields included).
My new proposition would be, and using a tentative member
keyword (probably not the best choice):
interface IBase {
interfaceField?: string;
interfaceMethod(): void
}
abstract class Base {
baseField: number;
baseMethod() { }
baseLambda: () => { };
abstract baseAbstractMethod();
}
class Derived extends Base implements IBase {
member interfaceField = "Hello";
member interfaceMethod() { }
member baseField = 2;
override baseMethod() { }
override baseLambda = () => { };
member baseAbstractMethod() { }
}
Note that I would choose the member
keyword for an abstract method since I said override
would be for things with an implementation, indicating that we're replacing an existing behavior. Instance methods would need to use override
by virtue of being a special kind of field (which is already recognized as such by the compiler since method signatures can be implemented with it).
I think that Typescript should be a compile-time checked version of JavaScript, and by learning TS you should be learning JS as well, just as by learning C# you get a good intuition of the CLR.
Prototypal inheritance is a cool concept of JS, and a quite central one, and TS should not try to hide it with concepts from other languages that do not make too much sense here.
Prototypal inheritance doesn't make any difference if you're inheriting methods or fields.
For example:
class Rectangle {
x: number;
y: number;
color: string;
}
Rectangle.prototype.color = "black";
Here we are setting a simple field in the prototype object, so all the rectangles will be black by default without having to have a instance field for it.
class BoundingBox {
override color = "transparent"; // or should be member?
}
Also the member
keyword makes the other members in the class jealous.
What we need is a syntax that will allow in a class declaration context the same kind of behavior (compile time checked/auto-complete/renames) that we already have in object literal or member expressions.
Maybe a better alternative to this.
is just .
:
class Derived {
.interfaceField = "hello";
.interfaceMethod() {}
.baseField = 2;
.baseMethod() {}
.baseLambda = () => {};
.baseAbstractMethod(){};
someNewMethod(){}
someNewField = 3;
}
In conclussion, I don't think the the Typesystem shouod track which values come from the prototype and which from the instance, because this is a hard problem once you can access the prototype imperatively, and because is not going to fix any problem while will limit the expressivity of JS.
So there is no difference between a function field, and arrow function field and a method, nor in the type system, nor in the generated code (if this
is not used).
Hey guys, this is interesting stuff, but it's pretty out on a tangent in terms of override
specifically. I'd be happy to have some new Discussion issue for this kind of thing to go in unless we can link these comments back to the original suggestion more concretely.
A lot of what you're saying here is really not specific to TypeScript, either, so starting an ESDiscuss thread may also be appropriate. Certainly they've thought about this sort of thing just as much (in terms of what goes on the prototype vs the instance).
@olmobrutall
ES6 classes are already hiding the prototype stuff, and like @kungfusheep said, messing directly with it isn't exactly a normal thing to do.
So there is no difference between a function field, and arrow function field and a method, nor in the type system, nor in the generated code (if this is not used).
Well, in the generated code, class methods are put in the prototype and everything else (not prefixed with static
) in put in the instance, which does make a difference with inheritance.
Anyway, I now agree that we don't have to have a separate syntax for both kinds of methods, but if we were to use the override
keyword, it should be limited to methods and we'd have to find something else for the rest. Having a unique keyword for everything could be nice but it should be very clear about its meaning. override
is clear, but only for methods. You dot syntax, this like this.
is still too close to existing JS to my taste.
@RyanCavanaugh
It's preatty out of tangent in terms of
override
My issues with override is that is not general enough for interface methods and fields. I see three options:
override
just for base class methods. override
for interface methods and fields as well member
, implement
, this.
, .
, etc...)I think this decision should be taken here.
A lot of what you're saying is really not specific to Typescript
Absolutely! we are all happy about the current state for transpiling, but not on the type-checking / tooling.
Whatever the keyword is, it will be Typescript only (just as abstract)
@JabX
You right about the generated code, of course. My point was that you can write something like this:
class Person {
name: string = "John";
saySomething() {
return "Hi " + this.name;
}
}
We declare a class, name
will go to the instance while saySomething
to the prototype. Still I can write this:
Person.prototype.name = "Unknown";
Because the type of Person.prototype
is the whole person. Typescript doesn't keep track of what goes where on his type system for simplicity.
Having a unique keyword for everything could be nice but it should be very clear about its meaning.
What I think is more important and we all could agree is the semantics:
Modified XXX
checks that the member is already declared in the base class or implemented interfaces, typically used to override functions from the base class.
Since .
or this.
look too alien, and member
is redundant, I think probably the best choice is to abuse override
for all the cases. Setting fields or implementing interface methods is anyway a side-feature.
There are many precedents:
static class
is not really a _class of objects_ anymore.static
fields.virtual
methods are quite _concrete_ (VB uses Overrideable
).It will look like this:
class Person{
dateOfBirth: Date;
abstract talk();
walk(){ //...}
}
interface ICanFly{
fly?();
altitude?: number;
}
class SuperMan extends Person implements ICanFly {
override dateOfBirth = new Date();
override talk(){/*...*/}
override walk = () => {/* force 'this' to be captured*/}
override fly() {/*...*/}
override altitude = 1000;
}
Can we settle on that?
I'd rather see a separate implement
keyword for anything that comes from interfaces (with priority to override
if it's in both), because it's what it is: an implementation, not an override.
Otherwise, I agree it might be best to abuse override
for everything from the parent class.
But there's no semantic difference between implement
and override
.
Both are going to have similar auto-complete / error messages / transpilation to JS... it's just a philosophical difference.
It's really worth to explain two keywords and that a pedanting compiler tells you: Error you should use 'override' instead of 'implement'
.
What about a this case:
interface IComparable {
compare(): number;
}
class BaseClass implements IComparable {
implement compare();
}
class ChildClass extends BaseClass implements IComparable { //again
override compare(); // or implements...
}
The questions is... who cares?.
Also there is the implement
/implements
issue.
Let's just abuse override
. One concept one keyword, this is the important thing.
Well, I already proposed that override
should have priority over implement
, but I probably wasn't clear enough.
I still don't believe this is the same thing. Another example: for a mandatory interface member, a failure to implement is a compiler error whereas a failure to override is not an issue (unless the base member is abstract that is...).
I just don't think that override
on something that isn't declared on a parent class would make any sense. But maybe I'm the only one who wants the distinction. In any case, whatever keyword we end up using for this, I just hope we can settle on something and provide a strict mode to enforce its use.
Well, I already proposed that override should have priority over implement, but I probably wasn't clear enough.
Sure, I just wanted to say that there are four cases. Base Class, Interface, Interface in Base Class, Re-implement Interface in BaseClass.
@JabX
I just don't think that override on something that isn't declared on a parent class would make any sense. But maybe I'm the only one who wants the distinction.
You're not. They are two fundamentally different things and there is benefit in having that separation at language level.
I think if interfaces are to be supported in this new functionality then it cant be some half-baked solution that's been bolted onto override
. For the same reason extends
exists as a separate entity to implements
at class level, override
needs to be paired with something along the lines of implement
in order to fix this problem. It's _replacing functionality_ vs _defining functionality_.
@olmobrutall
It's really worth to explain two keywords and that a pedanting compiler tells you: Error you should use 'override' instead of 'implement'.
I feel like I've made this distinction about 4 times now, but this isn't pedantic; it's really important information!
Consider your own example
interface IComparable {
compare(): number;
}
class BaseClass implements IComparable {
implement compare();
}
class ChildClass extends BaseClass implements IComparable { //again
override compare(); // or implements...
}
'or implements...' is a totally incorrect assumption to make in this case. Just because you've told the compiler you want to implement the same interface as the base class you're extending doesn't change the fact that you've already implemented compare
.
If you were to write implement
in ChildClass instead of override
then you should be thankful that the compiler would then tell you of your incorrect assumption, because it's a big deal that you were about to unknowingly wipe out a previously implemented method!
In the codebases that I'm responsible for this would be a big problem without a doubt; so I welcome any compiler feature that can prevent any such developer mistakes!
@kungfusheep
If you were to write implement in ChildClass instead of override then you should be thankful that the compiler would then tell you of your incorrect assumption, because it's a big deal that you were about to unknowingly wipe out a previously implemented method!
If overriding an already implemented method is such an important decision then implement
should be used for abstract methods as well. If you change a method from abstract
to virtual
or the other way around you should check (and change) all the implemented versions to consider calling super
or just removing the method.
In practice this has never been an issue in C#.
I agree, override
for implemented methods, and implements
for unimplemented methods (abstract / interface).
It could be used for abstract, yes.
In C# there wasn't the 'optional' interface scenario, so perhaps wasn't deemed as much of an issue.
But my point is that in C# we override
implemented and unimplemented methods and I've never heard of someone complaining.
I don't think is really an issue that deserves making explaining the feature at least two times harder.
Well, the only reason why we need an implement
keyword is because interface members can be optional, whereas it's not possible in C# (and probably in most OO languages too). There is no keyword for that there because you cannot _not_ implement an interface completely, so no issue. Don't base your argument on "it's the same in C#" too much, because we have different problematics here.
override
is used for _base class_ methods, whether they're abstract or not. I suppose we should to the same (override
instead of implement
for abstract methods) here because I believe the distinction should be on the origin of the method (class or interface) instead of on the existence of the implementation. And in the future, if you decide to give a default implementation to your abstract method, you won't have to go through your code (or worse, to the code of others using your class) to replace the keywords.
My main question now is, should we have a strict override
/implement
flag (I totally want this), should we force the implement
keyword on mandatory interface members? Because it doesn't help a lot (you can't fail to implement these because otherwise it won't compile) and could lead to a lot of unnecessary verbosity. But on the other hand, it might be deceiving to have the implement
on some interface members but not on all of them.
@JabX I would personally want a warning if a base class added an implementation of an abstract method.
However, I am not 100% sold on the idea of an implements keyword in the first place. The compiler will already warn you if you didn't implement something. The only place that it is really helpful is for optional methods.
And anyway, it has nothing to do with why I am in this thread. I was just looking if there was a way to specify a function as an override of a parent class.
My main question now is, should we have a strict override/implement flag (I totally want this), should we force the implement keyword on mandatory interface members?
I think not writing it should be a warning.
The only place that it is really helpful is for optional methods.
Yeah, that's kind of the whole point here.
Without this it's very common error to typo method that you would like to override but in fact you are crating new method. Introduction of such keyword is only way to point out intention of overriding function.
That could be warning or whatever but currently it's very painful.
I'm adding an override annotation to the UML class view in alm.tools :rose:
I've also added a gutter indicator for class members that override a base class member in alm.
Accepting PRs for a scoped solution we think achieves the most value with the least complexity:
override
is valid on class method and property declarations (including get/set)override
if one doesget
and set
must be marked override
if either isoverride
if there's no base class property/method with this name--noImplicitOverride
(feel free bikeshed the name here) makes override
mandatory for things that are overridesdeclare class
or .d.ts files)Out of scope at this time:
@RyanCavanaugh
I'm currently trying to implement this (just started, and I'd welcome any help on this, especially for testing), but I'm not sure I understand what's behind
All signatures (including implementation) must have
override
if one does
Since you can't have multiple signatures of a method in a class, are you talking about the inheritance context? Do you mean that (without the flag that enforces the use of override
, otherwise it's obviously an error)
class A {
method() {}
}
class B extends A {
override method() {}
}
class C extends B {
method() {}
}
should output an error in class C because I didn't write override
before method
?
If that the case, I'm not sure why we'd want to enforce that. And should it also work the other way around? Output an error in class B if I specified the override
in C and not in B?
Yes, I think it applies to the inheritance tree signatures.
And I don't think it will work the other way around, it should start enforcing the override rule once it's used at a level in the hierarchy. In some cases the decision to apply override may be made on a class derived from a class not owned by the developer. So, once it's used it should apply to all derived classes only.
I have another question about
Both
get
andset
must be markedoverride
if either is
What happens there if my parent class only defines a getter and I want to define in my derived class the setter? If I follow this rule, that means that my setter would have to be defined as an override, but there is no setter in the parent class, and it should be an error to override something that doesn't exist. Well, in practice (in my current naive implementation), there is no contradiction since the getter and the setter are for the same property.
I could even write something like:
class A {
get value() { return 1; }
}
class B extends 1 {
override set value(v: number) {}
}
Which looks plain wrong to me.
Since you can't have multiple signatures of a method in a class
You can have multiple overload signatures for the same method:
class Base { bar(): { } }
class Foo extends Base {
// Must write 'override' on each signature
override bar(s: string): void;
override bar(s?: number): void;
override bar(s: string|number) { }
}
I think your intuition about the other cases is correct.
Regarding getters and setters, I think override
just applies to the property slot. Writing an overriding setter without a corresponding getter is clearly extremely suspicious, but it's no more or less suspicious in the presence of override
. Since we don't always know how the base class was implemented, I think the rule is simply that an override
getter or setter must have _some_ corresponding base class property, and that if you say override get {
, any set
declaration must also have override
(and vice versa)
Oh ok I didn't know we could write method signatures in classes like that, that's pretty neat.
I think virtual
would make sense too.
All keywords: virtual
, override
, final
actually would make much difference especially when refactoring classes.
I'm using heavily inheritance, it's so easy now to override method "by mistake".
virtual
would also improve intellisense, as you could propose only abstract/virtual methods after override keyword.
Please, please put priority on these. Thanks!
I agree with @pankleks - I'm using inheritance a lot, and it just feels wrong to override a function without specifying anything directly.
"virtual", "override" and "final" would be perfect.
This is an important issue for me.
@RyanCavanaugh
Contextual typing of parameters or initializers (tried this earlier and it was a disaster)
Can you elaborate on this? I actually found this issue while trying to find out why TS does not infer the type of my parameters on method overrides, which came as surprise to me. I don't see when a non-identical parameter list would be correct when overriding, or a return type which is not compatible.
@pankleks
I think
virtual
would make sense too.
By its nature everything is "virtual" in JS. Support for final
would suffice IMHO: if you have a method (or property) which must not be overridden you could mark it as such to trigger a compiler error when this was violated. No need for virtual
then, right? But this is tracked in issue #1534.
@avonwyss there are sort of two issues here.
When we tried contextually typing _property initializers_, we ran into some problems described at https://github.com/Microsoft/TypeScript/pull/6118#issuecomment-216595207 . We think we have a new more successful approach at #10570
Method declarations are a different ball of wax and there we have some other things to figure out, like what should happen here:
declare class Base {
method(x: string): string[];
method(x: number, count: number): number[];
}
class Derived extends Base {
method(x) { // x: ???
return x;
}
}
We may just decide that only single-signature methods get parameter types from the base. But this isn't going to make everyone happy and doesn't really solve the frequent problem of "I want my derived class to method have the same _signatures_ but just provide a different _implementation_".
Well, x
's type would be string | number
in that case, but I understand that it might be pretty hard to find out consistently.
But you probably wouldn't want the _externally-seen_ type of x
to be string | number
- assuming Derived
has the same semantics as Base
, it wouldn't be legal to invoke with (3)
or ('foo', 3)
Hum yeah, I missed that.
I see you suggested to restrict it to single-signature methods, but I believe it could be "easily" expanded to "matching-signature" methods, like in your exemple:
declare class Base {
method(x: string): string[];
method(x: number, count: number): number[];
}
class Derived extends Base {
method(x, count) { // x: number, count: number
return [];
}
}
because there is only on signature on the base class that matches.
That would be way better than what we have (nothing), and I don't think it would be _that_ hard to do? (still way out of scope for this issue and probably more work)
@RyanCavanaugh Thanks for the reply. As noted my hope for override
is that it would sort the parameter (and return type) issue out. But I don't see your example as problematic, since overloads always have to have one single "private" implementation which is compatible with all overloads (which is why the suggestion from @JabX would not work conceptually).
So, we'd have something like this:
declare class Base {
method(x: string): string[];
method(x: number, count: number): number[];
// implies: method(x: string|number, count?: number): string[]|number[]
// or fancier: method<T extends string|number>(x: T, count?: number): T[]
}
class Derived extends Base {
override method(x, count?) { // may only be called like the method on Base
return [x];
}
}
This logic already exists and the override would obviously only be available for the "private" implementation method, not the distinct overrides (just as you cannot explicitly implement a single overload anyways). So I do not see a problem here - or did I miss something?
Methods designed to be overridden usually don't have overloads anyways, so even taking the simple way out and not infer the parameter and return types if overloads exist would be perfectly fine and useful. So even if the behavior was as-is when overriding overloaded methods, you'd get an error when running in --no-implicit-any
mode and you'd have to specify compatible types, that would seem like a perfectly fine approach.
I'm probably missing something, but in Javascript (and thus Typescript) you cannot have 2 methods with the same name - even if they have different signatures?
In C#, you could have
public string method(string blah) {};
public int method(int blah) {};
But here you can never have 2 functions with the same name, no matter what you do with the signatures... so none of your examples would ever be possible anyway, right? Unless we're talking about multiple extends on the same class... but that is not what your examples are showing, so probably not? Is this a non-issue?
Right now I'm using inheritance, and it is very frustrating... I'm having to put comments about functions to indicate that they're virtual (ie. I'm overriding them somewhere) or override (ie. that this function is overriding an underlying function). Super annoying and messy! And unsafe!
@sam-s4s Yes it is possible to declare multiple logical overload signatures for a method, but they all end up on the same single implementation (which then has to figure out by the passed parameters which "overload" is being called). This is heavily used by many JS frameworks, for instance jQuery where $(function)
adds a ready function, but $(string)
searches the DOM by selector.
See here for the docs: https://www.typescriptlang.org/docs/handbook/functions.html#overloads
@avonwyss Ah yes, declarations, but not implementations, that makes sense :) Thank you
This has gone on waaay too long, and i find it annoying that I have to read this entire bloated thread to even begin to figure out why TS still does not have this basic compile-time assertion.
Where are we on either (a) an override
keyword or (b) an @override
jsdoc annotation that informs the compiler "a method having the same name and type signature MUST exist on a superclass."?
More specifically, does the comment in https://github.com/Microsoft/TypeScript/issues/2000#issuecomment-224776519 (Jun 8 2016) by @RyanCavanaugh suggest that the next step is a (community) PR?
class Bar extends Foo {
/**
* @override
*/
public toString(): string {
// ...
}
override public toString(): string {
// ...
}
}
More specifically, does the comment in #2000 (comment) (Jun 8 2016) by @RyanCavanaugh suggest that the next step is a (community) PR?
@pcj Correct
I've started a PR on this at #13217. Anyone else interested in this functionality is most welcome to participate and/or provide suggestions. Thanks!
I would prefer the same as for java with a decorator
@Override
public toString(): string {
// ...
}
@Chris2011 Agree this looks familiar, but this would imply as a decorator that @Override
would be evaluated at runtime, not compile-time. I am planning javadoc /** @override */
in a separate PR once #13217 override keyword (public override toString()
) moves forward in review.
Trying to follow the conversation of this thread. Does this include static functions? I don't see any reason why we couldn't also allow overriding static functions in derived classes with completely different signatures. This is supported in ES6. If had class A { } ; A.create = function (){}
then class B extends A { } ; B.create = function (x,y) { }
, calling A.create()
is not going to call B.create()
and cause issues. For this reason (and to create the ability to use the same function name on types as factory functions) you should also allow override the signature of base static functions. It's doesn't break anything, and adds the ability to do some neat stuff (especially for game engine frameworks, where 'new' anything is really an evil if used all the time without pulling from a cache of objects to reduce GC freezes). Since callable constructors are not supported under ES6, creating a common naming convention for methods on types is the only other option; however, doing so currently requires the derived static function to show an overload of the base static function signature along with it's own, which is not useful for a factory function on a type that only deals with THAT type. :/ In the mean time the only option I have is to force users of my framework to duplicate all base hierarchy static function signatures (such as SomeType.create()
) on their derived types, and so on, which just gets really silly.
Here is an example about the "silliness" I'm talking about (which works, but not something I'd be proud of in an extensible framework):
class A {
static create(s: string) {
var inst: A;
/* new or from cache */
inst.init(s);
}
protected init(s: string) { }
}
class B extends A {
static create(s: string);
static create(n: number);
static create(n:any) {
var inst: B;
/* new or from cache */
inst.init(n);
}
protected init(s: string);
protected init(n: number);
protected init(n: any) {
super.init(n.toString());
}
}
class C extends B {
static create(s: string)
static create(n: number)
static create(b: boolean)
static create(b: any) {
var inst: C;
/* new or from cache */
inst.init(b);
}
protected init(s: string);
protected init(n: number);
protected init(b: boolean);
protected init(b: any) {
super.init(b ? 0 : 1);
}
}
(https://goo.gl/G01Aku)
This would have been much nicer:
class A {
static create(s: string) {
var inst: A;
/* new or from cache */
inst.init(s);
}
protected init(s: string) { }
}
class B extends A {
new static create(n:number) {
var inst: B;
/* new or from cache */
inst.init(n);
}
new protected init(n: number) {
super.init(n.toString());
}
}
class C extends B {
new static create(b: boolean) {
var inst: C;
/* new or from cache */
inst.init(b);
}
new protected init(b: boolean) {
super.init(b ? 0 : 1);
}
}
@rjamesnw you may be interested in Polymorphic "this" for static members having a contract for factory functions is discussed as a primary use case throughout the comments.
so, again restating a feeling that @pcj expressed (comment) over a year ago, it's confusing to have to read so many PRs and comments here and elsewhere to determine where this feature request is at.
it seems like #13217 was so close, then shot down again by @DanielRosenwasser and company as a feature that may not fit the language, seemingly re-entering the circular conversation here on this issue about whether or not it should be done. Maybe this 'design-time' decorators (#2900) thing will solve it, maybe not? It'd be nice to know.
Here's a few more shortcomings of the runtime decorator approach posted a few years ago that I didn't see mentioned:
If the only caveat was that the checks occurred upon class initialisation then I could probably accept it as a temporary measure, but as is there are just too many limitations.
Frankly I cannot believe this feature is still so controversial over 3 years since I first logged the issue. Every "serious" object oriented language supports this keyword, C#, F#, C++... you name it.
You can argue hypotheticals all day long about why javascript is different and needs a different approach. But from a practical perspective working daily in a very large Typescript code base, I can tell you that override would make a huge difference to code readability and maintenance. It would also eliminate an entire class of bugs due to derived classes accidentally overriding base class methods with compatible but subtly different signatures.
I would really love to see a proper implementation of virtual / override / final. I'd be using it all the time, and it would make code so much more readable and less error-prone. I feel like this is an important feature.
Agreed. It's frustrating to see how relatively obscure / edge-case features are being added, while something so... fundamental is being rejected. Is there anything we can do to push for this?
Come on people! If there is this many comments and over three years, why isn't it implemented already!
Come on :joy_cat:
Please be constructive and specific; we do not need dozens of PLEASE DO IT ALREADY comments
But guys, please be specific also on your side.
Obviously community is very interested in that feature, and TS team does not give us any specifics about future of this one.
I personally completely agree with @armandn , recent releases of TS brings relatively rarely used features while something like this is being held.
If you don't plan to do it, just tell us. Else please let us know how community can help.
Just a timeline here since there are more comments than GitHub is willing to display on load:
It's not like we're not taking consideration here. This is as close as features get to going in, and we've rejected our own feature ideas on similar grounds - see #24423 for a recent example.
We really want to grow the language intentionally. This takes time, and you should expect to have to be patient. By comparison, C++ is older than a lot of people in this thread; TypeScript is not yet old enough to be at home without adult supervision. Once we add something to the language, we can't take it out again, so every addition has to be carefully weighed against its pros and cons. I speak from experience that features people were SCREAMING for (extension methods, I'm looking at you) would have destroyed our ability to continue evolving the language (no intersection types, no union types, no conditional types) had we implemented them just because there were lots of GitHub comments. I'm not saying override
is a dangerous thing to add, just that we are always approaching this with the absolute maximum of caution.
If I had to summarize the core problems with override
right now:
override
in other languages (increases cognitive load)strict
but realistically couldn't because it'd be too big of a breaking change (decreases delivered value). And anything that implies a new commandline flag is another doubling of the configuration space that we need to weigh seriously, because you can only double something so many times before it consumes your entire mental budget when making future decisionsoverride
can apply to implementing an interface member (decreases perceived value because expectations won't match reality for some proportion of people)The total upside here is that you can a) indicate that you're overriding something (which you could do today with a comment), b) catch a misspelling that autocomplete should have helped you with anyway, and c) with a new flag, catch places where you "forgot" to put a keyword and now need to (a plain mechanical task that is unlikely to find real bugs). I get that b) is extremely frustrating but again, we need to meet the complexity bar here.
At the end of the day if you think JS classes would be enormously helped by an override
keyword, then championing a TC39 proposal to add it to the core runtime would be a good place to start.
It doesn't let you do anything that you can't do today with a decorator (so it's "not new" in terms of "things you can accomplish")
Maybe I'm misunderstanding this, but there are pretty important things that a decorator can't do which the keyword could. I mentioned some in https://github.com/Microsoft/TypeScript/issues/2000#issuecomment-389393397. Definitely correct me if those things are possible, because I would like to use the decorator approach but non-abstract methods are only a small part of my use case for this feature.
Another upside not mentioned is that an override keyword would make changing something in a base class a lot more feasible. Changing a name or splitting something into two parts etc. is difficult knowing that any deriving classes will compile just fine without being updated to match but will probably fail at runtime. Also, considering that there's no final/sealed/internal keywords available, just about any exported class might be derived somewhere making changing just about anything non-private a lot riskier than it would be with override available.
My impression is that a TSLint rule would be by far the smoothest path forward here. @kungfusheep briefly mentioned the idea in https://github.com/Microsoft/TypeScript/issues/2000#issuecomment-192502734 , but I don't see any follow-up. Is anyone else aware of a TSLint implementation of this feature? If not, I'll probably start hacking on one when I get a chance. 😄
My current thought is it'll just be a comment, // @override
, just like // @ts-ignore
and various other comment-based directives I've seen. I'd personally prefer to shy away from decorators since (in their current form) they have runtime semantics and since they're still stage 2 and disabled by default.
With any luck, a custom TSLint rule would be a 90% solution, only really lacking in aesthetics, and of course it could move forward without having to commit to any details of the language. With type-aware rules, tslint-language-service, and auto-fixing, there's really not much difference between a TSLint error and a built-in TS error from the developer's perspective. My hope is that a TSLint plugin (or multiple competing plugins) would give the community some experience and a chance to come to more of an agreement on the best semantics. Then maybe it could get added as a core TSLint rule, and maybe that would provide enough clarity and motivation to justify the aesthetic benefit of an override
keyword in the core language.
Just thinking outside the box here. We have two different scenarios here. C# (just as an example) uses virtual
and override
because methods are NOT virtual by default. In JavaScript all functions on a class are virtual
by default (by nature). Does it not make more sense then to invert the process and have a nooverride
type modifier instead? Of course people could still force it, but at least it’s there to help push a convention that some functions in base classes should not be touched. Again, just thinking outside the norm here. ;) It may also be less of a breaking change.
Does it not make more sense then to invert the process and have a
nooverride
type modifier instead?
I like your way of thinking, and I think what you are looking for is final.
What about readonly
though? I believe overriding a method in JS actually means replacing the parent's with the child's during instantiation (when the prototype chain gets applied). In that case, having a readonly
method to mean "It's there for everyone to see, but I don't want anyone to change it, whether by inheritance or dynamically after instantiation" makes a lot of sense. It's already implemented for members, why not do it for methods as well?
Is there already a proposal for it? If not it could be worth investigating as an alternative to override...
_edit:_ turns out you can override a readonly member, so this whole argument collapses.
Perhaps allowing private
class functions is another option?
Edit: I was thinking "instead of marking it final" but I must have been half asleep (obviously "final" means public but cannot override), LOL; nevermind.
@rjamesnw you can already define class functions as public, protected, private.
I don't think that only having "final" would be a solution. The problem is that people can accidentally create a new function in a class that inherits from a base class with that name already in use, and then you'll be breaking things silently without knowing why. It's happened to me a few times, very frustrating, as you often don't get errors, and strange things just happen (or don't)...
So I think, really, we're looking at a new tsconfig.json entry that will force the compiler to throw an error when something is overridden without it being marked as virtual (and the overriding one being marked as override or final).
I think that without override
, TypeScript is disappointing its users on its [perceived] promise of compile-time safety and type-safety.
The "use an IDE command to override method" argument breaks down as the code is evolving (as other people have pointed out).
Also looks like all major comparable languages have added override
in some form.
To not make it a breaking change, it could be a tslint rule (similarly as in Java).
Btw, why not allow breaking changes between major versions language e.g. 2.x -> 3.x. We don't want to get stuck, do we?
If it turns out something is wrong with certain details of override
, and it requires some breaking-change-tweaking in 3.x, so be it. I think most people will understand and appreciate the tradeoff of evolution speed vs compatibility. Really without override
I cannot recommend TS as something more practical than Java or C#...
If people want to be strict, there should be some way of mandatory override
(could be an optional tslint rule). The value of mandatory override
though is much lower, because chance of accidentally overriding a method from superclass seems much much lower than accidentally not-overriding.
Really this issue being open since 2015 is a bucket of cold water on my TypeScript enthusiasm and evangelism...
This does not handle overriding methods from grand*-parents:
/* Put this in a helper library somewhere */
function override(container, key, other1) {
var baseType = Object.getPrototypeOf(container);
if(typeof baseType[key] !== 'function') {
throw new Error('Method ' + key + ' of ' + container.constructor.name + ' does not override any base class method');
}
}
It is also sad and disappointing that No one assigned
on GH here on this issue. Maybe time for a TypeScript fork? ;) CompileTimeSafeScript...
This whole thread looks like a big "Paralysis by analysis" anti-pattern. Why not just do the "80% cases value with 20% of work" and, try it in practice, and, if necessary, tweak it in 3.x ?
The simple, frequent and very valuable use cases are held "hostage" by some corner cases that most people will never really need to worry about.
So, this is possible to type-check at compile time via decorators, albeit not without a little bit of help:
export const override = <P extends Function>() => <K extends keyof P["prototype"]>(
target: Object,
methodName: K,
descriptor: TypedPropertyDescriptor<P["prototype"][K]>
) => {
// this is a no-op. The checking is all performed at compile-time, so runtime checks are not needed.
}
class Bar {
biz (): boolean {
return true;
}
qux (): string {
return "hi";
}
}
class Foo extends Bar {
// this is fine
@override<typeof Bar>()
biz (): boolean {
return false;
}
// error: type '() => number' is not assignable to type '() => boolean'
@override<typeof Bar>()
biz (): number {
return 5;
}
// error: argument of type '"baz"' is not assignable to parameter of type '"biz" | "qux"'
@override<typeof Bar>()
baz (): boolean {
return false;
}
}
I'm not sure if it's possible to get a reference to the super type without passing it in explicitly. This incurs a tiny runtime cost, but the checking is all compile time.
@alangpierce
My impression is that a TSLint rule would be by far the smoothest path forward here.
[...]
Then maybe it could get added as a core TSLint rule, and maybe that would provide enough clarity and motivation to justify the aesthetic benefit of an override keyword in the core language.
100% agreed. Let's get started already!
Hi - a little later than planned but here is our tslint rule implemented using a decorator.
https://github.com/bet365/override-linting-rule
We have been using this across our ~1mloc TypeScript codebase for a couple of years and have recently updated it to use the language service, making it much faster to execute and more feasible to open source (the previous iteration required a full pass of the project to gather the heritage information).
For our use-case it has been invaluable - though we're still of the opinion that the responsibility should ultimately lie with the compiler and as such the linting rule should be seen as a stop-gap.
Thanks
I agree. I also believe that a solution involving the TS compiler would be much better than decorators and lint rules.
What is the status of this feature? Has it been revisited as a compiler-feature yet?
So, this is possible to type-check at compile time via decorators, albeit not without a little bit of help:
Some improvements:
function override< Sup >( sup : { prototype : Sup } ) {
return <
Field extends keyof Sup ,
Proto extends { [ key in Field ] : Sup[ Field ] } ,
>(
proto : Proto ,
field : Field ,
descr : TypedPropertyDescriptor< Sup[ Field ] > ,
)=> {}
}
class Foo {
bar( a : number ) {
return a
}
bar2( a : number , b : number ) {
return a
}
}
class Foo2 {
@override( Foo )
bar( a : number ) {
return 1
}
@override( Foo )
bar2( a : number , b : number ) {
return 1
}
xxx() { return '777' }
}
class Foo3 extends Foo2 {
@override( Foo ) // OK
bar( a : number ) { return 5 }
@override( Foo ) // Error: less args than should
bar2( a : number ) { return 5 }
@override( Foo ) // Error: accidental override Foo2
xxx() { return '666' }
@override( Foo ) // Error: override of absent method
yyy() { return 0 }
}
Ok, found out override solution that prevents compilation error.
Example with Node Readable stream:
// Interface so you will keep typings for all Readable methods/properties that are not overriden:
// Fileds that are `Omit`-ed should be overriden (with any signature you want, it do not have to be compatible with parent class)
interface ReadableObjStream<T> extends Omit<stream.Readable, 'push' | 'read'> {}
// Use extends (TYPE as any) to avoid compilation errors and override `Omit`-ted methods
class ReadableObjStream<T> extends (stream.Readable as any) {
constructor() {
super({objectMode: true}); // force object mode. You can merge it with original options
}
// Override `Omit`-ed methods with YOUR CUSTOM SIGNATURE (can be non-comatible with parent):
push(myOwnNonCompatibleSignature: T): string { /* implementation*/ };
read(options_nonCompatibleSignature: {opts: keyof T} ): string { /* implementation*/ }
}
let typedReadable = new ReadableObjMode<{myData: string}>();
typedReadable.push({something: 'else'}); // will throw compilation error as expected
typedReadable.pipe(...) // non overloaded methods typings supported as expected
The only disadvantage of this solution is lack of typings when calling super.parentMethod
(but thanks to the interface
ReadableObjStream you have all the typings when using instance of ReadableObjStream.
@nin-jin @bioball Thanks for the contributions with the decorator compile-time checking.
Unfortunately it doesn't seem to work with protected
members, only with public
.
(example of error in my fork of nin-jin's playground
The override specifier was a killer feature in c++ 11.
It so much helps and protects refactoring code.
I definitely would like to have this in TS base support without any hurdles (VOTE!)
We've definitely shown wild support for this feature, but it seems they still don't plan on adding this feature anytime soon.
Another idea:
class Obj {
static override<
This extends typeof Obj,
Over extends keyof InstanceType<This> = never,
>(this: This, ...overs: Over[]) {
return this as This & (
new(...a:any[])=> InstanceType<This> & Protect< Omit<InstanceType<This> , Over > >
)
}
}
class Foo extends Obj {
bar(a: number) {
return 0
}
bar2(a: number) {
return 0
}
foo = 1
}
class Foo2 extends Foo.override('bar') {
foo = 2
bar( a : number ) {
return 1
}
// Error: Class 'Foo & Protect<Pick<Foo, "bar2" | "foo">>'
// defines instance member property 'bar2',
// but extended class 'Foo2' defines it as instance member function.
bar2( a : number ) {
return 1
}
bar3( a : number ) {
return 1
}
}
declare const Protected: unique symbol
type Protect<Obj> = {
[Field in keyof Obj]:
Object extends () => any
? Obj[Field] & { [Protected]: true }
: Obj[Field]
}
Adding my two cents here.
We have a typical angular component, that handles forms and needs to unsubscribe from valueChanges and such. We didn't want to repeat the code all over the place (kinda current state), so I've made a "TypicalFormBaseComponent" that (among other things) implements angulars OnInit and OnDestroy methods. The issue is that now if you actually use it and add your own OnDestroy method (which is pretty standard thing to do), you hide the original one and break the system. calling super.OnInit fixes it, but there is currently no mechanism for me to force the children to do that.
If you do it with the constructor, it forces you to call super()... I was looking for something similar, found this thread, and honestly, I am kinda bummed.
Implementing "new" and "override" in the ts might be a breaking change, but fixing it would be incredibly simple in basically any codebase (just adding 'override' everywhere where it screams). It might also be just a warning.
Anyways, is there some other way for me to force the super call in the children? TSLint rule that prevents hiding, or something like that?
PS: I don't agree with the "no comment" policy. It prevents people from throwing examples here. Maybe you won't implement 'override', but you'll implement something else, that takes care of their problems... that is, if you can actually read them.
@GonziHere In other languages such as C# or Java, overriding does not imply the requirement to call the super method - it often is not the case in fact. Constructors are special and not normal "virtual methods".
The override keyword would be used to specify that the method must already be defined with a compatible signature on the base class, and the compiler could assert this.
Yes, but the need for override forces you to notice that the method exists.
@GonziHere Look like what you may really need is to create abstract base classes with abstract functions. Perhaps you can create private _onInit
and _onDestroy
methods you can more rely on, then create protected onInit
and onDestroy
abstract functions (or regular functions if they are not requried). The private functions will call the others, then complete as normal.
@GonziHere @rjamesnw The safe thing to do is actually to somehow enforce that onInit
and onDestroy
are _final_, and define empty protected abstract template methods that the final methods call. This guarantees that nobody can accidentally override the methods, and trying to do so (assuming there were a way to enforce final methods) would immediately point users to the right way to implement what they want.
@shicks, @rjamesnw, I'm in the same situation as @GonziHere. I don't want the base class to have abstract methods because there is functionality that I want executed for each of those lifecycle hooks. The problem is that I don't want that base functionality to be accidentally replaced when someone adds ngOnInit or ngOnDestroy on the child class without calling super()
. Requiring override
would bring to the attention of the dev that those methods exist in the base class and that they should choose if they need to call super()
;
From my experience writing Java, @Override
is so common that it becomes noise. In many cases, the overridden method is abstract or empty (where super()
should _not_ be called), making the presence of override
not a particularly clear signal that super()
is needed.
From my experience writing Java,
@Override
is so common that it becomes noise. In many cases, the overridden method is abstract or empty (wheresuper()
should _not_ be called), making the presence ofoverride
not a particularly clear signal thatsuper()
is needed.
That has nothing to do with the subject at hand. You might just as well said to use composition over inheritance to avoid the problem.
For me, the most useful part of override
is to get an error while refactoring.
Right now it is all too easy to rename some method in a base class and forget to rename the ones that override it.
Remembering to call super
is not that important. It is even correct to not call it in some cases.
I think that there is a misunderstanding on why the _override_ would be important. Not to force someone to call the _super()_ method, but to make them aware that one exists and allow them to conscientiously decide whether they want to suppress its behavior or extend it.
The pattern I use to ensure correctness is to use Parameters
and ReturnType
while referring very explicitly to the base class, something like this:
class Base {
public methodName(arg1: string, arg2: number): boolean {
return false; // base behaviour, may be stub.
}
}
class Derived extends Base {
public methodName(...args: Parameters<Base["methodName"]>): ReturnType<Base["methodName"]> {
const [meaningful, variableNames] = args;
return true; // implemented behaviour here.
}
}
This kind of pattern is the basis for my suggestion to add an inherit
keyword.. This ensures that any updates to the base class automatically propagate and if invalid in the derived class then an error is given, or of the name changes of the base class then an error is also given, also with the inherit
idea you aren't constrained to only offer identical signature to the base class but can intuitively extend it.
I also think there's a misunderstanding about why override
would be important, but I don't really understand all these issues with "whether super()
is needed".
To echo what @lorenzodallavecchia and the original author of the issue have said, marking a function as override
is a mechanism to avoid screw-ups that happen when the superclass is refactored, specifically when the name and/or signature of the overridden function changes or the function is removed.
Keep in mind that the person changing the signature of the overridden function may not be aware that overrides exist. If (e.g. in C++) the overrides are not explicitly marked as overrides, then changing the name/signature of the overridden function does not introduce a compilation error, it simply causes those overrides to no longer be overrides. (and.. probably no longer be useful, no longer be called by code that used to call them, and introduce a bunch of new bugs because things that are supposed to call the overrides are now calling the base implementation)
The override keyword, as implemented in C++, saves the person changing the base implementation from these problems, because immediately after their refactoring, compilation errors will indicate to them that a bunch of overrides exist (that override a now-nonexistent base implementation) and clue them in to the fact that they probably need to refactor the overrides as well.
There are secondary IDE usability benefits to having the override
modifier as well (also mentioned by original author), namely that upon typing the word override
the IDE can helpfully show you a bunch of possibilities of what can be overridden.
Together with the override
keyword it would be good to also introduce a flag that, when enabled, requires that all methods that override another uses the override
keyword, so as to avoid cases where you create a method in a subclass and in the future the base class (that could be in a 3rd party library) creates a method with the same name as the method you created in the subclass (which could cause bugs now, because the subclass had overridden that method).
Getting a compilation error in this case, saying that you need to add the override
keyword in this method (even tough you may not actually override the method, just change its name so as to not override the newly created method in the base class), would be much better and avoid possible runtime bugs.
From my experience writing Java, @Override is so common that it becomes noise.
After many Java-years I strongly disagree with above statement. Override is a way of communication, like checked exceptions. It's not about like or dislike, but about quality and expectations.
And therefore a big +1 to @lucasbasquerotto , but from another point of view: if I introduce a method in a subclass I want to have a clear semantic of overriding or not overriding. For example if I want to override a method, then I want to have a way to tell that explicitly. If something wrong with my implementation, for example a typo or wrong copy&paste, then I want to get feedback about it. Or other way: if I don't expect to override, I want feedback too, if overriding occasionally happens.
Feedback from another Java developer... @override is the only feature I'm really missing from Typescript.
I have ~15 of Java experience and 1-2 years or Typescript so pretty comfortable with both.
The zen of @override is you can remove a method from an interface and the compilation will break.
It's like the reverse of tight binding for new methods. It allows you to remove old ones.
If you implement an interface and ADD a method the compilation will fail but there isn't an inverse of this.
If you remove a method you'll just end up with dead code.
(though maybe that can be fixed by a linter)
To be clear, when I said that @Override
was noise, I was specifically referring to the comment about it being a signal that you need to call super
. The checking it provides is valuable, but for any given overridden method, it's a complete toss-up whether it's required or pointless to call super
. Since such a large fraction of overridden methods _shouldn't_, it's ineffective for that purpose.
My entire point is that if you want to ensure subclasses call back into your overrideable methods, the only effective way to do that is to make the method final
(see #33446, among others) and have it call into a _differently-named_ empty template method that can be safely overridden _without_ the super
call. There's simply no other reasonable way to get that invariant. I'm completely in favor of override
, but as someone who's been burned by users incorrectly subclassing my APIs (and I'm on the hook not to break them), I believe it's worth shifting attention to final
as the correct solution to this issue, rather than override
as was suggested upthread.
I'm a little confused as to why people keeps suggesting one of final
or override
over the other. Surely we want both, as they solve different problems.
Override
Prevents people who are extending a class from accidentally overwriting a function with their own, breaking things in the process without even knowing. Having an override keyword lets the developer know that they are in fact overriding an existing function, and then they can choose to either rename their function or use the override (and then they can find out if they need to call super).
Final
Prevent people who are extending a class from overwriting a function completely, so that the original creator of the class can guarantee complete control.
@sam-s4s We want the define too :-)
Example of issue..
class Base {}
class Entity extends Base {
id() {
return 'BUG-123' // busisess entity id
}
}
class Base {
id() {
return '84256635572' // storage object id
}
}
class Entity extends Base {
id() {
return '12' // busisess entity id
}
}
We got accidental overload here.
define
keywordclass Base {
define id() {
return '84256635572' // storage object id
}
}
class Entity extends Base {
define id() {
return '12' // busisess entity id
}
}
Shold be error: Accidental redefine. Use override
keyword to override base class method.
@nin-jin isn't define
the same as _not_ using override
?
@sam-s4s We want the define too :-)
Oh, I've not seen anyone here mention a keyword define
- but from what you describe, I assume you mean like the word virtual
in C# ?
(also I found you example a little confusing, as your classes Base
and Entity
are not related. Did you mean for Entity
to extend Base
?)
I guess there are 2 scenarios...
a) you write your base class, assuming everything without final
can be overridden.
b) you write your base class assuming everything without virtual
cannot be overridden.
@sam-s4s Entity extends Base, of course. :-) I have fixed my message. virtual
is about something else.
@lorenzodallavecchia not using override
is define | override
for compiler. Using define
is only define
and compiler can strict check this.
@nin-jin In your model, this means not using the override
keyword is still legal, right? For instance:
class Base {
myMethod () { ... }
}
class Overridden extends Base {
// this would not fail because this is interpreted as define | override.
myMethod () { ... }
}
Ideally, the above example produces an error unless you use the override
keyword. Albeit, I imagine that there would be a compiler flag to opt-out of override checking, where opting out is the same thing as override | define
for all methods
@bioball yes, it's required for compatibility with a lot of existed code.
@sam-s4s Entity extends Base, of course. :-) I have fixed my message.
virtual
is about something else.
I'm still not sure what you mean by define... this is the definition of virtual
in C#:
The virtual keyword is used to modify a method, property, indexer, or event declaration and allow for it to be overridden in a derived class.
Maybe you mean the opposite, where, instead of needing to mark the function as virtual
in order to override, you can mark the function as define
to then mean you have to use the override
keyword to override it?
(why define
? I have not seen that keyword in another language)
I skimmed through this thead in a couple minutes, but I haven't seen anyone propose using override as a way to avoid duplicate specification of parameters and return values. For example:
class Animal {
move(meters:number):void {
}
}
class Snake extends Animal {
override move(meters) {
}
}
In the above example, move
would have the same required return type (void
) and meters
would also have the same type, but specifying it wouldn't be necessary. The large company I work for is trying to migrate all our javascript to typescript, away from Closure compiler typing. However, while in Closure we can just use @override
and all the types will be inferred from the superclass. This is quite useful in reducing the possibility for mismatch and reducing duplication. One could even imagine implementing this such that extension of the method with additional parameters is still possible, even while not specifying types for the parameters that are specified in the superclass. For example:
class Animal {
move(meters:number):number {
}
}
class Snake extends Animal {
override move(meters, time:number) {
}
}
The motivation for me coming here and writing a comment is that our company is now requiring that we specify all the types even for overridden methods, because typescript doesn't have this functionality (and we're in a long migration). Its quite annoying.
FWIW, while it's easier to write, omitting parameter types (and other instances of cross-file type inference) hinders readability, since it requires someone unfamiliar with the code to dig around to find where the superclass is defined in order to know what type meters
is (assuming you're reading it outside an IDE, which is quite common for unfamiliar projects where you don't have an IDE set up yet). And code is read much more often than it's written.
@shicks You could say that about literally any imported variable or class. Duplicating all the information you might need onto a single file defeats the purpose of abstraction and modularity. Forcing types to be duplicated violates the DRY principle.
Most helpful comment
Please excuse the whining, but honestly, while your argument does apply to the
public
keyword in a language where everything is public by default, quite world dividing indeed, having things likeabstract
and an optionaloverride
keyword will simply help developers feel safer, make less mistakes and waste less time.Overriding is one of the few remaining highly-typo-sensitive aspects of the language, because a mistyped overriding method name is not an obvious compile time problem. The benefit of
override
is obvious, as it you allows you to state your intention to override - if the base method doesn't exist its a compile time error. All hail the type system. Why would anyone not want this?