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

Don't emit abstract members #40632

Closed
Qwertiy opened this issue Sep 18, 2020 · 0 comments · Fixed by #40699
Closed

Don't emit abstract members #40632

Qwertiy opened this issue Sep 18, 2020 · 0 comments · Fixed by #40699
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Help Wanted You can do this

Comments

@Qwertiy
Copy link

Qwertiy commented Sep 18, 2020

TypeScript Version: 3.7 - 4.1.0-dev.20200918

Search Terms:
abstract properties emit useDefineForClassFields

Code

Turn on useDefineForClassFields

abstract class A {
    protected abstract x: number;
}

Expected behavior:

Abstract members are hint for typescript and they don't emit any code. So expected compiled code will be:

"use strict";
class A {
}

Actual behavior:

"use strict";
class A {
    constructor() {
        Object.defineProperty(this, "x", {
            enumerable: true,
            configurable: true,
            writable: true,
            value: void 0
        });
    }
}

Playground Link:
https://www.typescriptlang.org/play?useDefineForClassFields=true&ts=4.1.0-dev.20200918#code/IYIwzgLgTsDGEAJYBthjAgggg3gKAUIQAcoB7CAU3koBMFRIZ4EAPALgQDsBXAWxCUoAbjwBfIA

Consequence of such generation:

There is a larger problem caused by compiling abstract member.

There are 2 ways of override - using property or using getter/setter and they both should be (and are according to ts) compatible with abstract member. But only one of them is fine in runtime.

Code

Turn on useDefineForClassFields

abstract class A {
    protected readonly abstract x: number;

    public doSmth() {
        console.log(this.x)
    }
}

class B extends A {
    protected x = 10
}

class C extends A {
    protected get x() { return 7 }
}

new B().doSmth()
new C().doSmth()

Run compiled code

Expected behavior:

Output:

10 
7

Compiled code:

"use strict";
class A {
    doSmth() {
        console.log(this.x);
    }
}
class B extends A {
    constructor() {
        super(...arguments);
        Object.defineProperty(this, "x", {
            enumerable: true,
            configurable: true,
            writable: true,
            value: 10
        });
    }
}
class C extends A {
    get x() { return 7; }
}
new B().doSmth();
new C().doSmth();

Actual behavior:

Ouptut:

10 
undefined

Compiled code:

"use strict";
class A {
    constructor() {
        Object.defineProperty(this, "x", {
            enumerable: true,
            configurable: true,
            writable: true,
            value: void 0
        });
    }
    doSmth() {
        console.log(this.x);
    }
}
class B extends A {
    constructor() {
        super(...arguments);
        Object.defineProperty(this, "x", {
            enumerable: true,
            configurable: true,
            writable: true,
            value: 10
        });
    }
}
class C extends A {
    get x() { return 7; }
}
new B().doSmth();
new C().doSmth();

Playground Link:
https://www.typescriptlang.org/play?useDefineForClassFields=true&ts=4.1.0-dev.20200918#code/IYIwzgLgTsDGEAJYBthjAgggg3gKAUIQAcoB7CAU3koBMEpLhayA7ZATwVEhngQAeALgSsArgFsQlKAG48BIsTEhkAS1gIWAZQkQAFgAoAlLkVEisNmDLJKAOmRkA5oYNqw9gcfMIAvngBeChoGABCCJQCVKy0GNj4FqQU1FT0AggAvAgAjAAMgQoh6AgAwpHRlLHxZknkVDT0zpSIAia4DC1iUKwIAOz+hXislADuCGEm9jp6Rj4j46VTMwYmQA

Related Issues:
#31225

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript Help Wanted You can do this labels Sep 21, 2020
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 4.2.0 milestone Sep 21, 2020
@typescript-bot typescript-bot added the Fix Available A PR has been opened for this issue label Sep 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue Help Wanted You can do this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants