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

ES private class elements #42458

Merged
merged 87 commits into from
Mar 25, 2021

Conversation

dragomirtitian
Copy link
Contributor

@dragomirtitian dragomirtitian commented Jan 22, 2021

Private Class Elements

This PR implements the TC39 Stage-3 Private Methods and Accessors proposal as well as TC39 Stage 3 Static Class features.

Fixes #39066, #37677, #42985

Remaining Work

  • Resolve open questions (see below)
  • PR the finalized helpers to tslib
  • Validate limitations below

Downlevel

WeakSets are used for instance private methods and accessors, in the same way that the existing Private Fields downlevel uses WeakMaps.

Instance Private Methods and Accessors

TypeScript

class Square {
    #size: number;
    constructor(size: number) {
        this.#size = size;
    }
    get #diagonal() {
        return Math.hypot(this.#size, this.#size);
    }
    area() {
        return Math.pow(this.#diagonal, 2) / 2;
    }
}
const square = new Square(7);
console.log(square.area());     // logs 49

JavaScript - ES2020 emit

var __classPrivateFieldSet = (this && this.__classPrivateFieldSet) || function (receiver, privateMap, value) { if (!privateMap.has(receiver)) { throw new TypeError("attempted to set private field on non-instance"); } privateMap.set(receiver, value); return value; };
var __classPrivateFieldGet = (this && this.__classPrivateFieldGet) || function (receiver, privateMap) { if (!privateMap.has(receiver)) { throw new TypeError("attempted to get private field on non-instance"); } return privateMap.get(receiver); };
var __classPrivateAccessorGet = (this && this.__classPrivateAccessorGet) || function (receiver, instances, fn) { if (!instances.has(receiver)) { throw new TypeError("attempted to get private accessor on non-instance"); } return fn.call(receiver); };
var _Square_size, _Square_diagonal_get, _Square_instances;
class Square {
    constructor(size) {
        _Square_instances.add(this);
        _Square_size.set(this, void 0);
        __classPrivateFieldSet(this, _Square_size, size);
    }
    area() {
        return Math.pow(__classPrivateAccessorGet(this, _Square_instances, _Square_diagonal_get), 2) / 2;
    }
}
_Square_size = new WeakMap(),
_Square_instances = new WeakSet(),
_Square_diagonal_get = function _Square_diagonal_get() {
    return Math.hypot(__classPrivateFieldGet(this, _Square_size), __classPrivateFieldGet(this, _Square_size));
};
const square = new Square(7);
console.log(square.area());     // logs 49
Static Private Fields, Methods and Accessors
class A {
    static #field = 10
    static #method() : void {} // no error

    static get #roProp() { return 0; }
    static set #woProp(value: number) { }

    static get #prop() { return 0 }
    static set #prop(value: number) { }

    static test() {
        console.log(A.#field);
        A.#field = 10;
        A.#method();
        console.log(A.#roProp);
        A.#roProp = 10
        
        console.log(A.#woProp);
        A.#woProp = 10

        console.log(A.#prop);
        A.#prop = 10
    }
}

JavaScript - ES2020 emit

var __classStaticPrivateFieldGet = (this && this.__classStaticPrivateFieldGet) || function (receiver, classConstructor, propertyDescriptor) {
    if (receiver !== classConstructor) {
        throw new TypeError("Private static access of wrong provenance");
    }
    if (propertyDescriptor === undefined) {
        throw new TypeError("Private static field was accessed before its declaration.");
    }
    return propertyDescriptor.value;
};
var __classStaticPrivateFieldSet = (this && this.__classStaticPrivateFieldSet) || function (receiver, classConstructor, propertyDescriptor, value) {
    if (receiver !== classConstructor) {
        throw new TypeError("Private static access of wrong provenance");
    }
    if (propertyDescriptor === undefined) {
        throw new TypeError("Private static field was accessed before its declaration.");
    }
    propertyDescriptor.value = value;
    return value;
};
var __classStaticPrivateMethodGet = (this && this.__classStaticPrivateMethodGet) || function (receiver, classConstructor, fn) {
    if (receiver !== classConstructor) {
        throw new TypeError("Private static access of wrong provenance");
    }
    return fn;
};
var __classStaticPrivateAccessorGet = (this && this.__classStaticPrivateAccessorGet) || function (receiver, classConstructor, fn) {
    if (receiver !== classConstructor) {
        throw new TypeError("Private static access of wrong provenance");
    }
    return fn.call(receiver);
};
var __classStaticPrivateReadonly = (this && this.__classStaticPrivateReadonly) || function () {
    throw new TypeError("Private static element is not writable");
};
var __classStaticPrivateWriteonly = (this && this.__classStaticPrivateWriteonly) || function () {
    throw new TypeError("Private static element is not readable");
};
var __classStaticPrivateAccessorSet = (this && this.__classStaticPrivateAccessorSet) || function (receiver, classConstructor, fn, value) {
    if (receiver !== classConstructor) {
        throw new TypeError("Private static access of wrong provenance");
    }
    fn.call(receiver, value);
    return value;
};

var _A_field, _A_method, _A_roProp_get, _A_woProp_set, _A_prop_get, _A_prop_set;
class A {
    static test() {
        console.log(__classStaticPrivateFieldGet(A, A, _A_field));
        __classStaticPrivateFieldSet(A, A, _A_field, 10);
        __classStaticPrivateMethodGet(A, A, _A_method).call(A);
        console.log(__classStaticPrivateAccessorGet(A, A, _A_roProp_get));
        __classStaticPrivateReadonly(A, 10);
        console.log(__classStaticPrivateWriteonly(A));
        __classStaticPrivateAccessorSet(A, A, _A_woProp_set, 10);
        console.log(__classStaticPrivateAccessorGet(A, A, _A_prop_get));
        __classStaticPrivateAccessorSet(A, A, _A_prop_set, 10);
    }
}
_A_method = function _A_method() { }, _A_roProp_get = function _A_roProp_get() { return 0; }, _A_woProp_set = function _A_woProp_set(value) { }, _A_prop_get = function _A_prop_get() { return 0; }, _A_prop_set = function _A_prop_set(value) { };
_A_field = { value: 10 };

References

Wider Contributions

Development of this PR led to these bug discoveries and fixes in other projects:

Credits

This PR includes contributions from the following Bloomberg engineers:

Design Limitations & Open Questions

  1. The pre-existing class fields transform can produce valid JavaScript from a syntactically invalid input. One example of this is duplicate private names. A similar issue will be visible in #constructor test since we now transform private methods. What is the desired TypeScript behavior in this case?

  2. This implementation does not work with the experimental Decorators in TypeScript. There is no spec for the interaction between these two features and implementing something non-standard that is likely to break in the future does not seem useful. Therefore we issue an error if a Decorators is used on a class containing a static private field/method/accessor.

Example
function dec() {
    return function (cls: new (...a: any) => any) {
        return class extends cls {
            static someField = 11;
            constructor(...a: any) {
                super(...a);
            }
        }
    }
}

@dec()
class Foo {
    static someField = 10;
    static #somePrivateField = 10;
    static m() {
        // displays 11 because decorator redefined it
        // and all references to Foo were rewritten to point to 
        // whatever the decorator returned
        Foo.someField 

        // Right now this would always fail, since Foo will be rewritten
        // to point to whatever dec returned, so it will not point to the 
        // class that actually defined `#somePrivateField`
        Foo.#somePrivateField 
    }
}
  1. Initializers are not allowed if target: esnext AND useDefineForClassFields: false. This is due to the fact that initializing private fields outside of a class is a non-trivial transform (a possible solution is described here - a modified version of it could be applied if there is desire for this combination to allow it) and keeping the init in the class would change runtime ordering of initializers.

  2. this is not allow in static initialization expressions. The restriction is a pre-existing issue and so is considered out-of-scope of this PR.

  3. Unlike instance #private class elements, static #private class elements do not depend on WeakMap or WeakSet. This means that technically we could transpile static #private class elements for es5 or even es3. However having different requirements for instance vs static #private class elements would probably cause more confusion than benefit. Therefore we retain the existing minimum target of es2015 for using static #private class elements and will error otherwise.

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Jan 22, 2021
@mkubilayk mkubilayk force-pushed the es-private-methods-and-accessors branch from eff2eda to fb73ccd Compare February 3, 2021 11:00
dragomirtitian and others added 28 commits February 3, 2021 18:44
…ing except classes.

Changes objects literal checking to not bail on first private name found in object literal.
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Private methods inside class expressions should not error.

Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
…y assignment

Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
Signed-off-by: Kubilay Kahveci <kahvecikubilay@gmail.com>
@mkubilayk
Copy link
Contributor

Created microsoft/tslib#146 for tslib updates.

@rbuckton thanks for the review again. I think this is ready for another round.

Copy link
Member

@rbuckton rbuckton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The proposed changes (as well as relevant changes in createPrivateIdentifierAssignment) should address the comment issue.

src/compiler/transformers/classFields.ts Outdated Show resolved Hide resolved
src/compiler/transformers/classFields.ts Outdated Show resolved Hide resolved
@mkubilayk
Copy link
Contributor

@rbuckton Thanks a lot for taking the time to debug this. I made the changes you suggested.


// leave invalid code untransformed
const info = accessPrivateIdentifier(node.name);
Debug.assert(info, "Undeclared private name for property declaration.");
Copy link
Member

@rbuckton rbuckton Mar 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can a user write source text that would fail this assertion?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. That would indicate a bug in addPrivateIdentifierToEnvironment.

PR Backlog automation moved this from Waiting on author to Needs merge Mar 25, 2021
@rbuckton rbuckton merged commit e638af7 into microsoft:master Mar 25, 2021
PR Backlog automation moved this from Needs merge to Done Mar 25, 2021
@rbuckton
Copy link
Member

This has been a tremendous effort. I'm happy to see these changes go in and look forward to the community having a chance to test them in nightlies starting tomorrow!

@Kingwl
Copy link
Contributor

Kingwl commented Mar 25, 2021

Congrats! Great work!

@orta
Copy link
Contributor

orta commented Mar 25, 2021

The nightly playground build just shipped for folks interested in playing with it. Also congrats, serious work this.

@danieltroger
Copy link

Hello, why is vs code still saying "A method cannot be named with a private identifier."? Shouldn't it stop since this PR has been merged since quite some time now?

@acutmore
Copy link
Contributor

acutmore commented May 4, 2021

Hello, why is vs code still saying "A method cannot be named with a private identifier."? Shouldn't it stop since this PR has been merged since quite some time now?

Hi @danieltroger, private methods will be part of TypeScript 4.3 which is still in Beta.

@danieltroger
Copy link

@acutmore I see, thank you for the swift reply. Any idea when it will be released? I feel like it will, even after the release, still take a long time until it lands in editors :(

Is it easy to switch to beta? I'm mainly using atom and https://github.com/TypeStrong/atom-typescript

@acutmore
Copy link
Contributor

acutmore commented May 4, 2021

@acutmore I see, thank you for the swift reply. Any idea when it will be released? I feel like it will, even after the release, still take a long time until it lands in editors :(

4.3 is likely to be released at the end of May #42762
https://devblogs.microsoft.com/typescript/announcing-typescript-4-3-beta/#whats-next

Is it easy to switch to beta? I'm mainly using atom and https://github.com/TypeStrong/atom-typescript

The atom-typescript docs say you can use a specific version of TypeScript by installing it.
https://github.com/TypeStrong/atom-typescript/blob/master/docs/faq.md#which-version-of-typescript-does-atom-typescript-use

npm install typescript@4.3.0-beta
@danieltroger
Copy link

danieltroger commented May 4, 2021

Yo it worked, finally no red, thanks a lot! I got into npm conflict hell tho

@mahnunchik
Copy link

When is ETA? It is horrible microsoft/vscode#106351

@acutmore acutmore mentioned this pull request Jun 18, 2021
1 task
@dragomirtitian dragomirtitian deleted the es-private-methods-and-accessors branch November 17, 2023 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
10 participants