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

Super call of non-method incorrectly allowed if target >= ES6 #32121

Open
mjbvz opened this issue Jun 27, 2019 · 6 comments
Open

Super call of non-method incorrectly allowed if target >= ES6 #32121

mjbvz opened this issue Jun 27, 2019 · 6 comments
Labels
Bug A bug in TypeScript
Milestone

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Jun 27, 2019

From https://stackoverflow.com/questions/56775734/error-when-calling-super-method-in-a-typescript-create-react-app-project

TypeScript Version: 3.6.0-dev.20190624

Search Terms:

  • es5
  • super
  • arrow method

Code

class Foo {
    public bar = () => { }
}

class SubFoo extends Foo {
    public bar = () => { super.bar(); }
}

With target: "es5"

Expected behavior:
No errors. Call to super is allowed.

Actual behavior:
See error on super.bar()

Only public and protected methods of the base class are accessible via the 'super' keyword.

Everything works fine using normal (non-arrow) methods. It also works with "target": "es6"

Playground Link:

Related Issues:

@ajafff
Copy link
Contributor

ajafff commented Jun 27, 2019

What's the expected outcome at runtime? Since the property is not on the prototype there's no way to access its value in the superclass after it's overridden.

@fatcerberus
Copy link

I don’t understand how this can work anyway, since arrow functions don’t have their own super binding - they close over the one from outside. Since the arrow function doesn’t occur inside a method body, super shouldn’t even exist in that scope?

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript and removed Bug A bug in TypeScript labels Jun 27, 2019
@RyanCavanaugh RyanCavanaugh added this to the Backlog milestone Jun 27, 2019
@RyanCavanaugh RyanCavanaugh added the Bug A bug in TypeScript label Jun 27, 2019
@RyanCavanaugh
Copy link
Member

The bug is that it's allowed, not that it's disallowed! The code fails at runtime

image

@RyanCavanaugh RyanCavanaugh changed the title super call not allowed for arrow method with target:es5 Super call of non-method incorrectly allowed if target >= ES6 Jun 27, 2019
@mathieucaroff
Copy link

mathieucaroff commented Nov 20, 2019

The mismatch between the title of the issue and the Expected / Actual behaviors it describes is confusing.

The case needs to be (detected and) handled. Now there's a choice to make between:

A. Making super.bar() an error - what @RyanCavanaugh recommands
B. Supporting the expected behavior - what OP @mjbvz asked for


As long as SubFoo.bar is an arrow function,

Supporting the behavior could be done by:

  1. Saving the old method to a variable in the SubFoo constructor, after the parent constructor was called, but before the method was redefined.
  2. Then calling that saved method rather than super.bar()

N.B: if SubFoo.bar is an arrow function, Typescript could transpile super.bar() by saving it in a local varible in the constructor of SubFoo (see (B1)). However, if SubFoo.bar is a classical ES6 method, Typescript'll have to copy super.bar to an instance variable (i.e. a property) (see (C) for inspiration). The first case is slightly dirty, the second is more dirty.

Here's what the transpiled code would like -- (B1):

"use strict";
class Foo {
    constructor() {
        this.bar = () => { };
    }
}
class SubFoo extends Foo {
    constructor() {
        super(...arguments);
        const _ts_bar = this.bar //
        this.bar = () => { _ts_bar() } // this.bar = () => { super.bar(); };
    }
}

new SubFoo().bar();

Note: the comments show how things currently are.


Given that the above solution requires an extra variable (extra code :-S) (and that I haven't thought about a way to make it work if SubFoo.bar is a classical ES6 method), I think choice (A) is the better one. If this solution is acceptable, adding note to the error message to suggest this solution as an escape hatch might be good.

Here's what the solution might look like in TypeScript -- (C):

class Foo {
    public bar = () => { }
}

class SubFoo extends Foo {
    private __super_bar = this.bar
    public bar = () => { this.__super_bar(); }
}

new SubFoo().bar()

-> In the TypeScript Playgrond solution

Note though that it is extra dirty because what was a local variable in (B.) is now a property on the instance. Typescript places the declarations of the body of the class at the beginning of the constructor, so the code can't be moved to the constructor -- the method is already redefined.


Here is a Typescript Playground link for the issue by the way:

TypeScript Playground issue

@MicahZoltu
Copy link
Contributor

Interestingly, this is only a problem with arrow function derived and arrow function base.

class Fruit {
    public eat() { console.log('nom nom') }
    public toss = () => { console.log('nyah!') }
    public drop() { console.log('huh?') }
    public digest = () => { console.log('burp!') }
}
class Apple extends Fruit {
    public eat() { super.eat() } // ok
    public toss = () => { super.toss() } // runtime error
    public drop = () => { super.drop() } // ok
    // public digest() { super.burp() } // compile error
}

new Apple().eat() // ok
new Apple().drop() // ok
new Apple().toss() // super.toss is not a function

@bradzacher
Copy link
Contributor

Interestingly, this is only a problem with arrow function derived and arrow function base.

To correct the knowledge model - it is class properties in general that are the problem, not just arrow functions.
foo = () => {} is just a class property that has an arrow function as its value! In essence it's semantically no different to foo = 1.


Method definitions are declared on the prototype. I.e. in your example eat and drop on class Fruit are declared on the prototype and can be accessed via Fruit.prototype.eat.

However properties are declared on the instance, not the prototype. This is because class properties are essentially specced to be syntactic sugar:

class Foo { bar = 1 }
// is exactly the same as
class Foo { constructor() { this.bar = 1 } }

In essence, when you use super. you are just accessing the prototype chain.
super.eat is roughly equivalent to Object.getPrototypeOf(Object.getPrototypeOf(this))).eat.
Or in terms of your example:
Object.getPrototypeOf(Object.getPrototypeOf(this))) === Fruit.prototype === super


I came here as I discovered the same bug in flow and was curious if TS had the same limitation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript
Projects
None yet
Development

No branches or pull requests

7 participants