Skip to content
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

Relax the rule that ‘super’ call must be the first statement #945

Closed
jbaron opened this issue Oct 23, 2014 · 10 comments
Closed

Relax the rule that ‘super’ call must be the first statement #945

jbaron opened this issue Oct 23, 2014 · 10 comments
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead

Comments

@jbaron
Copy link

jbaron commented Oct 23, 2014

Right now the compiler always checks if a super call is made, it is indeed the first statement in the constructor. This is even true if the statements before the super call don’t access anything of the object just being constructed.

It would be nice to not allow access to object variables and methods but allow for other kind of statements. For example the following gives an warning:

constructor(name) {
    var filePath = someGlobalFunction(name); 
    super(filePath);
}

while the following is ok:

constructor(name) {
    super(someGlobalFunction(name));
}

Of course the sequence of commands in the above two example is the same (and one might argue that the second example is the less maintainable/debuggable code). So would be great if the first example wouldn't throw a compiler warning.

I guess the tricky part is to determine which statements are ok to be used before the super call. But I would assume if the statements don’t include this or super keywords, they should be ok.

@jednano
Copy link
Contributor

jednano commented Oct 23, 2014

Totally 100% agree! I ran into some issues with this in my TypeScript code and I would have loved to have had this feature. This is the first language I've used where it forces you to call the super function at the beginning of the constructor. This is not always ideal.

Unless it's a code smell of which I was not aware. In which case, please provide information as to how it's a code smell or bad practice to do otherwise.

@RyanCavanaugh
Copy link
Member

C#, C++, and Java enforce this rule - it's not an uncommon one.

Basically there's nothing valid you can do that must come before the base class super call. It's sometimes annoying in the case where you'd like to use the result of a function call more than once, but it's important to not try to use the base class before it's been initialized.

@electricessence
Copy link

Actually, it doesn't force you to have it first all the time.
If you do not initialize values outside the constructor or provide private/public parameters, then it doesn't complain. There are ways around most of it, but I really think the solution is that the compiler adapt to certain situations. Like for example, I think the private/public parameters blocking you from having the super call later is pretty silly. Those values should simply be initialized before the super call and that's that. But I do agree that if you initialize a variable outside the constructor, you could be asking for trouble.


Works:
blah:number;

constructor() {
   var somethingbeforetheconstructor = 10;
   this.blah = somethingbeforetheconstructor;
   super();
}

Doesn't Work:
blah:number = 10;

constructor() {
   var somethingbeforetheconstructor = 10;
   var whatIsBlah = this.blah; // will be undefined;
   super();
}

This example it makes sense to fail because there are too many conditions to check where there might be problems since blah actually get's initialized after the constructor.


Doesn't Work:
constructor(public blah:number) {
   var somethingbeforetheconstructor = 10;
   super();
}

This sample should be able to work and the compiler should go ahead and initialize the blah value before the constructor code begins.

@danquirk
Copy link
Member

The exact rules from the spec explaining the above behavior (8.3.2):

The first statement in the body of a constructor must be a super call if both of the following are true:
• The containing class is a derived class.
• The constructor declares parameter properties or the containing class declares instance member variables with initializers.
In such a required super call, it is a compile-time error for argument expressions to reference this.
Initialization of parameter properties and instance member variables with initializers takes place immediately at the beginning of the constructor body if the class has no base class, or immediately following the super call if the class is a derived class.

As Ryan said, for the cases that are disallowed there are workarounds, because for the most part you can't do many safe things this way.

Like for example, I think the private/public parameters blocking you from having the super call later is pretty silly. Those values should simply be initialized before the super call and that's that.

You mean like this?

class Base { 
    blah = 4;
}
class Derived extends Base {
    constructor(public blah: number) {
        this.blah = blah;
        super();
    }
}

var d = new Derived(3);
console.log(d.blah);

@jbaron
Copy link
Author

jbaron commented Oct 23, 2014

Agree that most problems come when using initialized properties. If you would indeed avoid them, more things are allowed (I guess I just a fan of intializing properties early on, since it saves some typing and for feels a bit more natural).

But I would also have expected that a simple rule like no access to this before the super call would be easy to implement and understand, while provide still lots of posiibilities to do stuff before the super call.

So the following would work (since there is no reference to this before the super call):

blah:number = 10;

constructor() {
   var somethingbeforetheconstructor = 10;
   super();
}

At the end there are ways around it, so no deal breaker. Just after using TypeScript extensively noticed that the current implemented rule is more often than not a (minor) annoyance.

@danquirk
Copy link
Member

That case is the same as the parameter property one. Replace the definition of my Derived example with your code and you'll see the same bad behavior where Derived's constructor initialization work is overwritten by something in the base class constructor..

@jbaron
Copy link
Author

jbaron commented Oct 23, 2014

Not sure I get that. How can a local scoped variable (var somethingbeforetheconstructor = 10) ever be overwritten by something in the base class constructor? Am I missing something?

My typical use case would be something like this (just to demonstrate it with an example that in my endeavors with TypeScript regular occurs):

class MyWidget extends Widget {
  color = "black";

  constructor(id:String) {
       var label = I18N.translate(id);
       var icon = IconMap.get(id);
       super(label, icon);
       this.setBackground(this.color);
 }

}

Ofcourse my current workaround would be to invoke the super call as follows to avoid the compiler complaints:

super( I18N.translate(id),  IconMap.get(id) );

@RyanCavanaugh RyanCavanaugh added the By Design Deprecated - use "Working as Intended" or "Design Limitation" instead label Oct 24, 2014
@RyanCavanaugh
Copy link
Member

We'd still have to do fairly extensive analysis to determine whether or not you were indirectly using this in an arbitrary expression. Other languages do fine with this restriction and we think it's one that makes a lot of sense.

@pdfernhout
Copy link

I suggest this issue be reopened based on discussion here: microsoft/TypeScript-Handbook#214

While it is reasonable in TypeScript to not permit this to be referenced in a constructor before calling super when there are initialized properties or constructor parameter properties, broader restrictions on calling other code don't seem that helpful and can be worked around anyway, like by function calls inside a super call such as super(logThisName(name)); where the called function refers to this. As show by an example in the above linked discussion, ES6 permits other code in a constructor before a super call (although accessing this in called code would generate a runtime error before super was called), so in that sense, TypeScript is being more strict than what ES6 permits.

@pdfernhout
Copy link

As requested by @mhegazy, I have made a new issue for a fresh discussion on this topic (issue #8277).

kukoo1 pushed a commit to kukoo1/sequelize-typescript that referenced this issue Jul 31, 2017
Most cloud services offer database connection string as URI, not username and password.
This branch adds ability to initialize Sequelize using URI connection string, eg. "postgres://user:pass@host/db", by either calling Sequelize.init(URI, config) or adding "uri" property into config object in Sequelize.init(config) call.

Due to Typescript's limitation that doesn't allow passing spreaded arguments into super() nor conditional super() call, this change will break backward compatability by removing the constructor and replaced by static init() method.
(Refer to microsoft/TypeScript#4130 and microsoft/TypeScript#945)

new Sequelize(config) won't work anymore, use Sequelize.init() instead.
@microsoft microsoft locked and limited conversation to collaborators Jun 18, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead
Projects
None yet
Development

No branches or pull requests

6 participants