Skip to content

Wrong this.propertyName reference inside abstract method realization #7724

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

Closed
sanex3339 opened this issue Mar 29, 2016 · 4 comments
Closed
Assignees
Labels
By Design Deprecated - use "Working as Intended" or "Design Limitation" instead

Comments

@sanex3339
Copy link

TypeScript Version:

1.8.9 / nightly
Code

abstract class Abstract {
  protected name: string;

  constructor () {
    this.abstractMethod();
  }

  protected abstract abstractMethod (): void;
}

class Concrete extends Abstract {
  protected name: string = 'Concrete';

  protected abstractMethod () {
    console.log(this, this.name); // Concrete, undefined
  }
}

new Concrete();

Playground: link

Expected behavior:
this.name inside abstractMethod() realization must be pointed at name property of Concrete class.

Actual behavior:
this.name inside abstractMethod() realization is pointed at name property of Abstract class.

@sanex3339 sanex3339 reopened this Mar 29, 2016
@mhegazy mhegazy added Bug A bug in TypeScript Breaking Change Would introduce errors in existing code labels Mar 29, 2016
@mhegazy mhegazy added this to the TypeScript 2.0 milestone Mar 29, 2016
@jeffreymorlan
Copy link
Contributor

The property = value syntax is shorthand for setting the property after the (possibly implicit) super() call. If you write Concrete's constructor out the long way, the problem is more apparent:

class Concrete extends Abstract {
  protected name: string;
  constructor() {
    super(); // Indirectly calls abstractMethod(). this.name is not set yet
    this.name = 'Concrete'; // Too late
  }
  protected abstractMethod() {
    console.log(this, this.name); // Concrete, undefined
  }
}

You might think that property = value should instead set the property before super(), like it does in C#. But this would be impossible for TypeScript because it must support targeting ES6 classes, in which this doesn't exist until super() returns.

In general it is a bad idea to call overridable methods from a constructor, since subclasses have not had a chance to do their initialization yet.

@sandersn sandersn added By Design Deprecated - use "Working as Intended" or "Design Limitation" instead and removed Breaking Change Would introduce errors in existing code Bug A bug in TypeScript labels Mar 29, 2016
@sandersn
Copy link
Member

Thanks @jeffreymorlan, that's a great explanation.

@RyanCavanaugh
Copy link
Member

See also the many Stack Overflow questions about why calling virtual (let alone abstract!) methods from constructors is a bad thing to do.

@sanex3339
Copy link
Author

ok, tnx

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

5 participants