-
Notifications
You must be signed in to change notification settings - Fork 12.6k
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
ES private class elements #42458
Conversation
eff2eda
to
fb73ccd
Compare
…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>
produce an error if private field helpers are not up to date
Created microsoft/tslib#146 for @rbuckton thanks for the review again. I think this is ready for another round. |
There was a problem hiding this 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.
@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."); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
.
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! |
Congrats! Great work! |
The nightly playground build just shipped for folks interested in playing with it. Also congrats, serious work this. |
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. |
@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 |
4.3 is likely to be released at the end of May #42762
The atom-typescript docs say you can use a specific version of TypeScript by installing it. npm install typescript@4.3.0-beta |
Yo it worked, finally no red, thanks a lot! I got into npm conflict hell tho |
When is ETA? It is horrible microsoft/vscode#106351 |
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
tslib
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
JavaScript - ES2020 emit
Static Private Fields, Methods and Accessors
JavaScript - ES2020 emit
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
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?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
Initializers are not allowed if
target: esnext
ANDuseDefineForClassFields: 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.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.Unlike instance #private class elements, static #private class elements do not depend on
WeakMap
orWeakSet
. This means that technically we could transpile static #private class elements fores5
or evenes3
. 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 ofes2015
for using static #private class elements and will error otherwise.