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

Arrow function is not compatible with ordinary one #17113

Closed
xfoxfu opened this issue Jul 12, 2017 · 16 comments
Closed

Arrow function is not compatible with ordinary one #17113

xfoxfu opened this issue Jul 12, 2017 · 16 comments
Labels
Committed The team has roadmapped 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

@xfoxfu
Copy link

xfoxfu commented Jul 12, 2017

TypeScript Version: 2.4.1

Code

class Component {
  render(): boolean {
    return false;
  }
}
class App extends Component {
  render = () => false;
}

Expected behavior:

Compiles correctly.

Actual behavior:

The compiler reports an error:

test.ts(7,3): error TS2424: Class 'Component' defines instance member function 'render', but extended class 'App' defines it as instance member property.

I know this may be designed, a feature rather than a bug, but such behavior is quite weird.

@RyanCavanaugh RyanCavanaugh added In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Jul 12, 2017
@RyanCavanaugh
Copy link
Member

There are good reasons not to allow the reverse case, but I can't remember why we disallow this. I think it's come up before but will take it up as a suggestion

@xtuc
Copy link

xtuc commented Jul 12, 2017

An arrow fonction has no prototype or even a name. This is a good reason why they shouldn't have the same type.

@kitsonk
Copy link
Contributor

kitsonk commented Jul 12, 2017

Also, in the base class, the method is part of the classes prototype. You are overwriting it with a property initialiser that gets desugared in the constructor and isn't actually part of the prototype chain. You end up with differences in own properties at run time, so they are effectively, subtly different, though the shape is the same. At runtime, the shape of the Class.prototype is actually different, though I believe TypeScript includes the shape of properties in the type of the prototype, even though they aren't really there.

I thought you might also get into trouble with super but actually that seems fine.

@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this Good First Issue Well scoped, documented and has the green light Committed The team has roadmapped this issue and removed In Discussion Not yet reached consensus labels Oct 10, 2017
@RyanCavanaugh RyanCavanaugh added this to the Community milestone Oct 10, 2017
@RyanCavanaugh
Copy link
Member

Easy PR if someone wants to submit one.

@Kingwl
Copy link
Contributor

Kingwl commented Oct 11, 2017

I would like to work on this one.😅

@Kingwl
Copy link
Contributor

Kingwl commented Oct 12, 2017

@RyanCavanaugh @kitsonk i have some question

class Component {
  render(): boolean {
    return false;
  }
}
class App extends Component {
  get render () {
    return () => true
  }
}
  1. if i can override method with property, could i override method with accessor?
  2. could i override property defined in base with method?

@RyanCavanaugh
Copy link
Member

@Kingwl

  1. Not sure - does it work at runtime?
  2. No

@Kingwl
Copy link
Contributor

Kingwl commented Nov 16, 2017

@RyanCavanaugh

  1. yes

@cshaa
Copy link

cshaa commented Mar 8, 2018

@Kingwl
I don't think it makes sense to “override” a method – which in general can have any signature – with an accessor, which can be thought of as a group of methods with two overloads, one get: () => T and one set: (T) => void.

However overriding a method with an equivalent arrow function is an useful shorthand and seems like a good idea to me. I think the right approach would be to convert the arrow function to the “full form” and treat it as a normal method (i.e. add it to the prototype, let it have a name, …).

The only problem I can think of is that we could decide to implement a C#-like syntax for method shorthands. The new syntax would be preferable to this one and we would have to deprecate it.

Here's a comparsion of the current and possible syntaxes:

// Current full-body syntax
class A {
    function foo() { return 42; }
}

// Current lambda syntax
class B {
    foo = () => 42;
}

// @coderfox's Arrow extension proposal
// compiles to the full-body function
class C extends A {
    foo = () => 1/0;
}

// C#-like expression-bodied method
// compiles to the same code as A
// is more elegant and clearer than B
class D {
    foo() => 42;
}

// Extending with an exp.-bodied method
// no problem, uncontroversial
// works automatically as long as D works
class E extends A {
    foo() => 1/0;
}

@RyanCavanaugh @xtuc @kitsonk What are your opinions on this? Is the possibility of implementing expression-bodied functions enough to pass over @coderfox's syntax?

@RyanCavanaugh
Copy link
Member

@m93a Class properties and arrow functions are both TC39-controlled entities now; proposals about new syntax that combine the two belong on ES Discuss or other forums.

@BlueSialia
Copy link

Overriding a method (instance member function) with an arrow function (instance member property)... Will the context, this, be different depending on what the actual object is?

Also, arrow functions can't be called with super.foo(). In the example of @coderfox, what would happen if I extend the class App and execute super.render()?

@DanielRosenwasser
Copy link
Member

I have still not seen an overriding method that needed to be written that way. You end up with something less efficient by doing so. I think that might've been the original motivation.

@denis-sokolov
Copy link

This error TS2424 happens for me with TypeScript 2.9.2, but not with TypeScript 3.0.1.

@RyanCavanaugh
Copy link
Member

Can't find the PR for this but it does appear to have been fixed.

@ackvf
Copy link

ackvf commented Oct 11, 2019

I get this error now with 3.7.0-dev.20191011.
Not getting this error with 3.6.2.

Did something change?

@anuja8275
Copy link

TypeScript Version: 2.4.1

Code

class Component {
  render(): boolean {
    return false;
  }
}
class App extends Component {
  render = () => false;
}

Expected behavior:

Compiles correctly.

Actual behavior:

The compiler reports an error:

test.ts(7,3): error TS2424: Class 'Component' defines instance member function 'render', but extended class 'App' defines it as instance member property.

I know this may be designed, a feature rather than a bug, but such behavior is quite weird.

Do we have solution for this m also stuck in such issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Committed The team has roadmapped 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