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

Remove restriction of properties overriding prototype methods #6154

Closed
RyanCavanaugh opened this issue Dec 18, 2015 · 5 comments
Closed

Remove restriction of properties overriding prototype methods #6154

RyanCavanaugh opened this issue Dec 18, 2015 · 5 comments
Labels
Fixed A PR has been merged for this issue Good First Issue Well scoped, documented and has the green light Help Wanted You can do this Suggestion An idea for TypeScript

Comments

@RyanCavanaugh
Copy link
Member

We currently enforce this rule:

class Base {
  foo() {
  }
}

class Derived extends Base {
  // Error, cannot override method with property
  foo: () => { };
}

This breaks two real scenarios:

  • You can't declare 'foo' as a property and then mix it in later (Mixins use problem with React #6147)
  • You need to implement foo as an arrow function because you have some unbound callers that your base class didn't

The reverse case is important to continue to enforce, but no one can remember why we have this rule.

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Dec 18, 2015
@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this and removed In Discussion Not yet reached consensus labels Jan 5, 2016
@RyanCavanaugh RyanCavanaugh added this to the Community milestone Jan 5, 2016
@RyanCavanaugh
Copy link
Member Author

Pinging @bterlson to make sure we're not forgetting some weird edge case before simply removing this error

@RyanCavanaugh RyanCavanaugh added the Good First Issue Well scoped, documented and has the green light label Jan 5, 2016
@bterlson
Copy link
Member

bterlson commented Jan 6, 2016

I can't think of why this should be an error (it isn't with ES classes + class property declarations proposal, either).

@DickvdBrink
Copy link
Contributor

@RyanCavanaugh, maybe the reason was because it is a bit dangerous to use them in the constructor itself? If they where both methods it worked as expected but with the base a method and the derived a property the results are different.
Note that the code you gave (which I slightly modified) does transpile but the result is Constructor -> Base instead of Constructor -> Derived. Looking at the transpiled code it is obvious but from the TS code perspective it doesn't (might be just me).
Just guessing here though ;)

class Base {
    constructor() {
        alert("constructor")
        this.foo();
    }
  foo () {
      alert("BASE")
  }
}

class Derived extends Base {
  // Error, cannot override method with property
  foo =  () => { alert("Derived"); };
}

new Derived();

@RyanCavanaugh
Copy link
Member Author

I'd call it mildly surprising, relative to what people's random expectations of initialization order seem to be 😉

@cameron-martin
Copy link

I can have a look at this one, as my first issue :)

elizabethdinella added a commit that referenced this issue May 24, 2018
…ed class (#24343)

* Fix to issue 6154 - Overriding a method with a property in the derived class should not cause a compiler error

* new baselines

* fixed deleted baselines
@RyanCavanaugh RyanCavanaugh added the Fixed A PR has been merged for this issue label May 24, 2018
@mhegazy mhegazy modified the milestones: Community, TypeScript 3.0 May 24, 2018
@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Fixed A PR has been merged for this issue Good First Issue Well scoped, documented and has the green light Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants