microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.89k stars 12.47k forks source link

Unable to access extended properties in constructor #1617

Closed brenthoneybone closed 9 years ago

brenthoneybone commented 9 years ago

I'm not sure if this is an intention of typescript or not but acts differently to how I would've expected.

If i have a base class and extend it, overriding some base class properties, then want to work with the properties in the child class constructor I still access the parent properties.

This is shown in the below basic example.

class Base {

    public myVar:string = 'Base';

    public constructor() {
        console.log(this.myVar);
    }

}

class Child extends Base {

    public myVar:string = 'Child';

}

var base:Base = new Base(); // 'Base' - As expected
var child:Child = new Child(); // 'Base' - I would've expected this to be 'Child'

console.log(base.myVar); // 'Base' - As expected
console.log(child.myVar); // 'Child' - As expected

This happens in the compiled JS because the super is called before the child properties are set:

function Child() {
    _super.apply(this, arguments);
    this.myVar = 'Child';
}

Is this intended? An issue? Or a trade off because of JS limitations? Thanks!

Note that if I do something like this, it works as I would've expected. However it feels like a bit of a hacky work around.

class Base {

    public myVar:string = 'Base';

    public constructor() {
        this.setup();
    }

    protected setup() {
        console.log(this.myVar);
    }

}

class Child extends Base {

    public myVar:string = 'Child';

    public constructor() {
        super();
        this.setup();
    }

}

var base:Base = new Base(); // 'Base'
var child:Child = new Child(); // 'Child' - Now as expected

console.log(base.myVar); // 'Base'
console.log(child.myVar); // 'Child'
RyanCavanaugh commented 9 years ago

Edit: I've posted a longer exploration of this at http://stackoverflow.com/questions/43595943/why-are-derived-class-property-values-not-seen-in-the-base-class-constructor/43595944

This is the intended behavior.

The order of initialization is:

  1. The base class initialized properties are initialized
  2. The base class constructor runs
  3. The derived class initialized properties are initialized
  4. The derived class constructor runs

Why is this the order?

First, look closely at the JavaScript emitted for a class. There are only two places where code runs -- in the static initialization of the class (where the prototype is set up and static members are initialized), and in the constructor function for the class instance. Property initialization is injected into the first part of the constructor, which means that step 1 and step 2 are necessarily going to happen one after the other.

It's also clear that the base class constructor has to run before the derived class constructor.

This leaves us with only one alternative:

  1. The derived class initialized properties are initialized
  2. The base class initialized properties are initialized
  3. The base class constructor runs
  4. The derived class constructor runs

That ordering would result in very surprising behavior:

class Base {
    x = 'foo';
    getX() {
        return this.x;
    }
}
class Derived extends Base {
    m = this.getX(); // Would be 'undefined', not 'x'
    constructor() {
        super();
    }
}
SimonCsoma commented 8 years ago

I would expect that example to give an compile error because you would/should never use this in class variable declaration

I.E.: protected m = this.getX();

richardlawley commented 8 years ago

I think it's worth noting that the behaviour of C# is exactly as the alternative mentioned above, i.e.

  1. The derived class initialized properties are initialized
  2. The base class initialized properties are initialized
  3. The base class constructor runs
  4. The derived class constructor runs

The example given that would result in "very surprising" behaviour is prevented by the compiler:

class Base {
    public string x = "foo";
    public string getX() {
        return x;   
    }
}

class Derived : Base {
    public string m = this.getX();   // CS000027 Keyword 'this' is not available in the current context
}

I suspect this is one of the reasons people keep raising this issue - programming in the style they're used to simply doesn't work in TS, and you have to use horrible workarounds to accomplish the same thing.

However, the TS declaration style requires being able to access this:

class Test {
    FirstName: string;
    Surname: string;
    FullName = () => `${this.FirstName} ${this.Surname}`;
}

EDIT: As pointed out, the above example would still be fine as it's a function. So I still think this issue is valid.

gregtour commented 8 years ago

This behavior seems like an unusual issue to confront.

Yogarine commented 7 years ago

However, the TS declaration style requires being able to access this:

class Test {
    FirstName: string;
    Surname: string;
    FullName = () => `${this.FirstName} ${this.Surname}`;
}

Just wanted to mention this example is accessing this in the context of a function, which should still work in the alternative behaviour.

JoelLeach commented 7 years ago

I'd like to add my vote for a solution that makes more sense to OOP veterans coming from other languages, whether it is for EcmaScript or TypeScript only. I wrote an article on the subject, including suggested workarounds.

paishin commented 7 years ago

To execute the Base constructor after Derived class values have been set I am including Base constructor's operations in a setTimeout function. Everything seems to work ok but I don't know what other implications this might have. Anyone with more knowledge can comment if this is a bad practice and why?

class Base {
    x = 'foo';
    constructor(){
       setTimeout(()=>{alert(this.x)},0);
    }
}
class Derived extends Base {
    x='bar';
    constructor() {
        super();
    }
}

new Derived(); //Alerts 'bar'
vasa-c commented 7 years ago

Sorry for my question. But, still, it is a bug or a feature?

paishin commented 7 years ago

Its just the way TypeScript works

mhegazy commented 7 years ago

It is rather, how JavaScript (ES6) works.

PMXScott commented 7 years ago

This makes no sense what so ever in my view.

JS is prototypal. The object oriented community found it weird and came up with loads of hacks to simulate class based inheritance. In doing so there became lots of gotchas such as accidentally forgetting the "new" keyword, which in turn created more hacks. The JS comity finally decided to support the vast number of object oriented users of JS by adding in class support. However, they decided they didn't like being pressured into adding "normal" class based inheritance that most people would instantly understand (and assume) and so decided to follow the somewhat normal JS way of doing the occasional "wtf" implementation to trip up users ( https://martin-thoma.com/javascript-wtf/ https://www.smashingmagazine.com/2011/05/10-oddities-and-secrets-about-javascript/ http://charlieharvey.org.uk/page/javascript_the_weird_parts to name a few). Implementing sub class constructors in a way that most of the object oriented community will trip up on I guess is at least sticking with the core of the JS craziness! Shame though that TypeScript still follows that philosophy when it has so much of everything else right (in my opinion).

SimonCsoma commented 7 years ago

Adding to PMXScott their argument is that typescript usually perceived as a superset of ES6 which has a typical inheritance system.

The problem that comes forth from this inheritance that typescript implements is one where when a framework wants to have a class initialize some part of the system which also needs to be configurable on a class by class basis it will simply be ignored by the language. Thus using inheritance in these kinds of situations will demand a pretty weird structure.

I.E. Backbonejs is using an initialize function for its views. When one is extending from this view and wants to for example overwrite some auto rendering function (like implemented by the Chapin library) this will not be overwritten until after the initialization is complete.

Circumventing this, for now, is still possible by using the constructor of the class and passing them in an acrobatic way on as options to the base view. But with the support of ES6 classes will be less and less supported want IMHO will not encourage the use of typescript by instead will enforce developers to move away.

One might argue that typescript is meant for the smaller projects, but this, in my opinion, is not the case since type casing I the entire idea of the language and there for meant for larger project ergo for projects using frameworks.

These kinds of mistakes make me wonder if typescript will be a lasting language.

Edit: miss read mhegazy's comment

RyanCavanaugh commented 7 years ago

They're going to be really disappointed when ES6 class property initializers have the exact same semantics.

Yogarine commented 7 years ago

@paishin @RyanCavanaugh @mhegazy I'm not sure what you guys are talking about, because ES6 doesn't even support proper class property initialisers. It requires you to set the object instance properties in the constructor just like ES5.

So how can you say "that's just how ES6 works" or "ES6 will have the same semantics" if the whole notion of class property initialisers doesn't even exist in the latest ES8 draft?

Since TypeScript is emulating class property initialisers, it should do so in a way that we would expect ES9 to maybe implement them, or at least implement it the way it's implemented in just about any other OOP language, since those are what TypeScript is basing this language feature on in the first place. You can't use the way ES6 does object property initialisation as an excuse to just break a separate language feature.

For me it shows that whoever implemented class property initialisers in TypeScript doesn't really grasp the difference between setting properties in the object instance or having them inherited and initialised from a child class.

It's just plain incorrect from an OOP standpoint.

When setting properties from the constructor, you already have an object instance of a class. You're working with that instance and what class it was doesn't really matter anymore, because the class was only used as a template for the object instance.

When instantiating a class that extends another class (new Child()), you're expecting the object instance to be based on the complete flattened "template" of the class you're instantiating, including class property initialisers. By the time you're in the context of the object's constructor, you're already working with the object, which should be based on the class you instantiated, which was Child and not Base. So why are you getting Base's properties? It just doesn't make sense.

PMXScott commented 7 years ago

@Yogarine I added two language examples (Python & PHP) in #15247 which do things how one would expect. However, the java and .net languages tend to do the same as JS.

@RyanCavanaugh did make a good point with this crazy code being possible if JS followed the path of Python and PHP.

class Base {
    x = 'foo';
    getX() {
        return this.x;
    }
}
class Derived extends Base {
    m = this.getX(); // Would be 'undefined', not 'x'
    constructor() {
        super();
    }
}

However, given the example above. That's not a style of code I've come across and languages I've used simply wouldn't allow you to define a property the return value of a function like that. All method calls would be done in constructors and other methods. So while yes, it creates crazy code so does the alternative! My opinion is that the example used to justify the decision that was taken is far more of an edge case as opposed to how often people will be stung by the case in this and #15247 bug reports.

So, I felt that was more a mistake of JS and not one that TypeScript necessarily had to make. But, this is largely opinionated since I can understand why TypeScript would attempt to follow in the same footsteps as JS. (Though I'm saying that without being overly familiar with the ES6+ specifications so I'm unsure about that path).

The current implementation I still believe does not make any logical sense (especially around abstracts), already tripped us up, people creating workarounds and custom rules (such as new Abc().init() allowing init() to be forgotten and another source of frustration), and typescript doesn't warn us about illogical code that will always result in a bug such as that mentioned in #15247 so I believe there is still room for improvement on the current implementation even if I don't fully agree with it.

Personally, I believe the other route should have been taken (as suggested in this issue) and restrictions applied within typescript to stop crazy code such as that mentioned by Ryan. After all, isn't typescript about making JS safer? With compilation, more warnings, static type checks, etc so to me I don't believe this expectation would be unreasonable.

But I understand this is just an opinion and like @RyanCavanaugh said, they had to go with one of them. If they had gone with the suggestion in here, this thread would likely be about how they didn't follow the likes of Java or .net and how confusing TS would be for them people lol. So they couldn't win either way.

Anyway, that's it from me in here as I feel this topic is becoming a battle of perseverance in voicing opinions than being productive. People have already made up their mind.

Great discussion though 👍

Yogarine commented 7 years ago

@PMXScott

However, the java and .net languages tend to do the same as JS.

In what way? Java and .net do class property initialisers the same as PHP, C++ and any other OOP language I know of.

That's not a style of code I've come across and languages I've used simply wouldn't allow you to define a property the return value of a function like that.

That is because normally when class properties are defined this is done when defining the class and not the object, so you don't have an object instance (this) yet. The reason your example works in TypeScript is because, like I said, the person implementing this didn't understand the difference between class definitions and object (or JS "function") definitions.

In purely prototype-based languages like JavaScript/ES5 the only things you can work with are objects, and inheritance is done using prototypes. However now that classes get introduced to these languages JavaScript developers that are used to treating everything as objects for the first time have to deal with structures that aren't first-class citizens. That's probably why ES6 doesn't support class property initialisers yet, and why the TypeScript developers got so confused as how to implement classes in TypeScript.

private m = this.getX(); in the class context works in TypeScript because of the way the class is translated to JavaScript. A non-first-class citizen (the class) is translated almost 1:1 to a JavaScript function (which is a first class citizen). If classes as an OOP concept would've been implemented properly in TypeScript, extra checks should've been added to prevent code in the class context to access the the object instance context.

PMXScott commented 7 years ago

@Yogarine

In what way? Java and .net do class property initialisers the same as PHP, C++ and any other OOP language I know of.

I'm not a Java or .Net developer. But I wrote out the same examples I did in PHP and Python in Java (pretty sure I did Java, did a few languages) & .Net (specifically C#) using online executors to find they both gave the same outcome as JS (though is possible those tests could have been flawed).

It's as if the JS community are so used to the prototypal paradigm that they are trying to preserve it when moving to the class based OO paradigm and creating a strange mash up of both. Instead of fully embracing OO.

Yogarine commented 7 years ago

I wrote out the same examples I did in PHP and Python in Java (pretty sure I did Java, did a few languages) & .Net (specifically C#) using online executors to find they both gave the same outcome as JS

I just double checked and you're right. You can use the this context in Java and C#. It's weird, (at least for me since I'm primarily a C++ and PHP developer,) but it works as expected because they initialise the properties before they call the constructor, which is what this whole issue is all about.

RyanCavanaugh commented 7 years ago

Class property initializers are coming to ES6 classes at some point, and the current proposal has the same behavior as TypeScript's. This is covered at https://github.com/tc39/proposal-class-public-fields/issues/54 and https://github.com/tc39/proposal-class-public-fields/issues/15

You guys keep talking about this like we had three options: 1) Do the right thing 2) Do a weird thing where derived properties are set before base constructors run 2) Do a weird thing where derived properties are set after base constructors run

The first option does not exist. The first option does not exist.

the person implementing this didn't understand the difference between class definitions and object (or JS "function") definitions.

For me it shows that whoever implemented class property initialisers in TypeScript doesn't really grasp the difference between setting properties in the object instance or having them inherited and initialised from a child class.

Seriously?

Yogarine commented 7 years ago

You guys keep talking about this like we had three options:

Not me. Or rather, for me the right thing is just obviously point 2, since that's what's expected considering how OOP works.

From Wikipedia:

In object-oriented programming, a class is an extensible program-code-template for creating objects, providing initial values for state (member variables) and implementations of behavior (member functions or methods)

A class should be a template for the object I'm instantiating. When using inheritance, the Child class can and will override properties of the parent.

So when I instantiate a Child, when I'm in the context of the constructor, I expect to see a Child, not a Base. I expect to Child's default values for the properties.

It's so obvious it hurts.

PMXScott commented 7 years ago

Seriously?

I don't think he quite meant it like that @RyanCavanaugh . It's clear that you and the rest of the TypeScript team do understand it very well (seriously, you guys have made a compiler!) and a decision had to be made.

It's easy for someone who has a strong belief to develop tunnel vision on a problem and forget that the problem is in fact quite small given the bigger picture. This problem is hardly a show stopper and once people understand it, they can work with it (though you wouldn't believe it given how much effort we have all contributed here).

We are an inquisitive bunch and like to know why :D You won't ever please everyone and you won't necessarily convince everyone. Just how it is. Don't take any of this personally :)

RyanCavanaugh commented 7 years ago

I actually misspoke earlier, because in ES6 it's illegal for the derived class to do anything before calling super. So there is literally no other option than the one we chose.

Any further discussion should be at https://github.com/tc39/proposal-class-public-fields/issues/54 where the official ES spec for this behavior is being worked out.

Yogarine commented 7 years ago

@RyanCavanaugh

Class property initializers are coming to ES6 classes at some point, and the current proposal has the same behavior as TypeScript's. This is covered at tc39/proposal-class-public-fields#54 and tc39/proposal-class-public-fields#15

Thanks for linking those, I wasn't aware of those proposals or these comments.

I assume you mean "come to ES8 or ES9 classes", since the ES6/ES2015 spec was finished and published in 2015. Is it commonplace to just refer to ES6 as ES6+ or is there some spec extension thing going on that I don't know about? I might have been confused since you specifically mentioned ES6 before.

I have now read those comments and the spec proposal, and I do understand the need to be forward-compatible with any future ECMAScript standards, so in that case sticking to the spec proposals is a good Idea.

Sorry if I sounded overly harsh. I wish I had seen those links you posted sooner. From what I've read in these comments in relation to why they propose this order of how properties are set, it seems like they have a valid reason. (Also, I didn't realise classes are also first-class citizens in ES6).

I'll just rest my case and accept the response that "a future ECMAScript edition will probably work like that, so that's how we're doing it". At any rate I understand now this is not the place to fight this battle.

RyanCavanaugh commented 7 years ago

I just refer to non-downleveled classes (i.e. classes that are run natively) as "ES6 classes" because ES6 is the spec version that added them.

btraut commented 7 years ago

FWIW: I really like the approach Swift takes to this. Unlike ES/TS, where a call to super is required as the first thing a constructor does, you're actually required to initialize all of your own uninitialized private variables first. A child class cannot call super or any of its own methods before each of its member variables are set (either in the constructor or using the values provided when defining the parameters). This fixes a number of issues in many parts of the language, this one included, and has been the most intuitive to me.

That said, I respect TS's stance to follow ES proposals as much as possible. It appears the best place to voice an opinion is with Ecma International.

CSMastermind commented 7 years ago

I think this example illustrates why this behavior is confusing (at least to me) better than the one above:

class Base {

    public constructor() {
        this.setup();
    }

    protected setup() {
    }
}

class Child extends Base {

    public myVar: string[] = [];

    public constructor() {
        super();
    }

    protected setup() {
        this.myVar.push('one'); // throws TypeError: Cannot read property 'push' of undefined
    }
}

var child: Child = new Child();
RyanCavanaugh commented 7 years ago

Tracking at https://stackoverflow.com/questions/43595943/why-are-derived-class-property-values-not-seen-in-the-base-class-constructor now