-
Notifications
You must be signed in to change notification settings - Fork 12.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Calling abstract methods inside abstract class constructor #9209
Comments
How about a base class where it needs to configure itself based on the actual implementation of de derived/consumer class. |
@blendsdk Yes, I was trying something similar, but the compiled JS fails to run so I don't think I got that right. Besides, the output looks buggy to me. |
Here is an example: https://github.com/blendsdk/trunk/tree/master/abstract
|
the methods would work. properties would not. not sure if there is much we can do here though. |
Isn't this dangerous? Isn't |
methods go on the prototype. so does not matter when you call the method, it should be there. properties are added to the instance after the issue is we can prohibit the use of abstract properties in the constructor, and we probably should, but you can call a method from the constructor and the method tries to use a property, this we can not check. |
That is exactly what I meant. I was sort of saying that calling a derived method from a base class constructor is probably a bad idea. It is certainly discouraged in many languages. Perhaps it should be an error if a method marked as abstract is called from a constructor implementation within the same class. Edit: I'm not sure it matters that the base class is marked |
FWIW, I ran into this trying to make // abstract.ts
export abstract class AbstractClass {
constructor(s: string) {
this.abstractMethod(parseInt(s));
}
abstract abstractMethod(n: number): void;
}
// interface.ts
export interface Interface {
new (s: string): AbstractClass;
staticMethod(): void;
}
// concrete.ts
export class ConcreteClass extends AbstractClass {
abstractMethod(n: number) { }
static staticMethod() { }
}
export const concreteClass: Interface = ConcreteClass; This works great in TS, but then emits that JS above. I believe this is a valid requirement, but is there another way to achieve this? |
By the way, I'm still new to TypeScript so the other thing I'm curious about here is, why can't the Language Service not automatically infer that my derived class (not declared |
To my best knowledge these are handled by the IDE. If you use VSCode, it will complain about the abstract method not being implemented in the derived class. At the moment VSCode does not auto-generate code for the abstract function. |
Yes, that it does. But perhaps it could go a step further in suggesting the method name and signature as well inside the derived class block. |
Yes agreed. It is best to address these issues at VSCode. The typescript team cannot do much about it here I guess. |
VSCode does not do any code generation for TypeScript. It is all generated via the TypeScript compiler. Furthermore, the TypeScript language service itself is responsible for producing errors in VSCode. Ide plugins like tslint can produce warnings. Neither effect code generation. If you're using WebStorm, or Resharper in Visual Studio, JetBrains has their own language service but I would be shocked if the actual generated code was different. |
Great insight! Thanks :) |
@prashaantt can you explain a bit more about the errors and code gen you expected to see? Specifically, when you say this:
This is fine -- the compiler will stop you from instantiating |
About your other question, I believe Quick Fixes are coming to Visual Studio soon, which means that the API will be available to other editors. Quick Fixes is the VS name for things like suggesting a method implementation of an abstract method. It looks like VSCode supports quick fixes in C#, so it should be pretty easy to add them for TypeScript once they are done. |
@sandersn, the bug here, i believe, is the compiler allowing access of abstract proprieties in the base constructor. these are guaranteed to fail at runtime, no matter what. |
I think @prashaantt's question is about methods (at least principally). I created a separate bug for accessing abstract properties in the constructor: #9230. |
The reason I want to enforce the above contract (specific const ConcreteClass = require('./concrete.js');
ConcreteClass.concreteClass("1"); // TypeError: this.abstractMethod is not a function I erroneously reported earlier that the following works, but it's a completely different thing and not what the framework actually does with the const ConcreteClass = require('./concrete.js');
new ConcreteClass.concreteClass("1"); // this works I can avoid using So @sandersn at this point my problem might be very specific, but I still think the compiler should probably warn me against calling |
In fact, I've been able to get closer to my desired behaviour with the new abstract properties feature, when used a function: // abstract.ts
export abstract class AbstractClass {
constructor(s: string) {
this.abstractMethod(parseInt(s));
}
abstract abstractMethod: (n: number) => void;
}
// concrete.ts
export class ConcreteClass extends AbstractClass {
abstractMethod = (n: number) => { };
static staticMethod() { };
}
export const concreteClass: Interface = ConcreteClass; The output from // concrete.js
...
var ConcreteClass = (function (_super) {
__extends(ConcreteClass, _super);
function ConcreteClass() {
_super.apply(this, arguments);
this.abstractMethod = function (n) { };
}
ConcreteClass.staticMethod = function () { };
return ConcreteClass;
}(abstract_1.AbstractClass));
... And this would work perfectly if only |
It sounds to me that your problem is not with // abstract.ts
class Class {
constructor(s: string) {
this.method(parseInt(s));
}
method(n: number): void {
alert("don't call me!");
}
}
// concrete.ts
class ConcreteClass extends Class {
method(n: number) { alert(n) }
static staticMethod() { }
}
let call = ConcreteClass('1'); // error!
let cons = new ConcreteClass('1'); The question is why you don't get an error on export function concreteClass(n: number) {
return new ConcreteClass(n);
} If you need other functions nested on that function, then you can do: function create(n: number) {
return new ConcreteClass(n);
}
export const concreteClass = create as Interface;
concreteClass.staticMethod = function() { }; |
That's right, that's their contract. Unless I misunderstood you, the following doesn't compile up front: export const concreteClass = create as Interface;
// `Property 'staticMethod' is missing in type '(s: string) => ConcreteClass'` I removed the static check from concreteClass.staticMethod = function() { };
// `Property 'staticMethod' does not exist on type '(s: string) => ConcreteClass'` Thanks for your time everyone. I guess I'll have it give this up for now and maybe try again later. |
You have to change interface Interface {
(): AbstractClass; // no *new* here.
staticMethod(): void;
}
function create() {
// same as before ...
};
function f() { }
const concreteClass = create as Interface;
concreteClass.staticMethod = f; |
TypeScript Version:
1.8.10
Code
Output
Question
Is there a situation where calling
this.abstractMethod
inside theconstructor
makes sense? I have accidentally hit this pattern and the class compiles successfully, but the resulting JavaScript obviously fails becausethis.abstractMethod is not a function
.The text was updated successfully, but these errors were encountered: