- Notifications
You must be signed in to change notification settings - Fork13.2k
Private named instance fields#30829
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Private named instance fields#30829
Uh oh!
There was an error while loading.Please reload this page.
Conversation
msftclas commentedApr 9, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
b242db5 tof2c5233Comparemihailik commentedApr 15, 2019
Amazing achievement! Are the helpers (that would include modifications totslib) |
mheiber commentedApr 17, 2019
@mihailik thanks! As far as I know, the helpers work the same way. Is any additional work required to get the helpers into tslib? I was hoping the syncing was semi-automated. |
weswigham commentedApr 17, 2019
Sadly, they are not. A paired PR against |
Neuroboy23 commentedApr 17, 2019
@weswigham: I have asked our IP team to set up a fork of the tslib repo. @joeywatts and@mheiber: Will you take a look at this other repo? I think it will be an easy mod. |
G-Rath commentedMay 30, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Forgive me if this is obvious, but I've look around on the issues and couldn't see this covered anywhere: Is there any particular reason why we can't do Would there be any chance of getting that syntax supported? In my eyes it'd give us the best of both worlds, as then all class properties can maintain their indentation level, along with making refactoring easy for people like me who prefix all our private properties with I mean you could pretty much just use a global find-and-replace using Also, could someone confirm for me if its planned for TS somewhere down the line to transform |
ljharb commentedMay 30, 2019
G-Rath commentedMay 30, 2019
@ljharb fair enough - so then I assume TS is aiming to deprecate & remove the |
robpalme commentedMay 30, 2019
@G-Rath Your question on the future of TypeScript's keyword |
G-Rath commentedMay 30, 2019
septs commentedMay 31, 2019
"private field" decorator "private" only? classSample{private #field:number=1;} |
robpalme commentedJun 18, 2019
@rbuckton has re-landedthe refactoring of class properties. The next step is to rebase this PR. |
Aqours commentedJun 21, 2019
Does |
DanielRosenwasser commentedJun 21, 2019
@Aqours no, |
mheiber commentedJun 21, 2019
The rebase is in progress and we're expecting to update soon! |
mheiber commentedJun 21, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I noticed an issue with our Language Service changes. Type inference and red underlines work correctly, but autocomplete is borked. Steps to reproduce:In a class with private field #foo#, start typing Actual behavior:Autocomplete completes incorrectly (see screenshot). It is actually completing with secret variables we use in the transformation Expected behavior:Autocomplete completes as Advice welcome on how to fix autocomplete in this PR! |
DanielRosenwasser commentedJun 24, 2019
That is actually really weird. Are you sure that it's not just picking up the output |
f2c5233 toa9cb10eComparemheiber commentedJun 25, 2019
Thanks for your suggestion,@DanielRosenwasser: the completionswere picking up the JS output. I added an |
a9cb10e to36a1648Compare36a1648 tod2adc3cCompare71c384a toafcc88cComparemheiber commentedDec 31, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
fwiw, I agree that the hack is a bad idea. Wouldn't want the hack to be a secret, though, since it's good to know that the privacy of the transformed code is not absolute, barring control over the WeakMap prototype.@littledan, are there other |
littledan commentedDec 31, 2019
@mheiber What do you mean by privacy hills? |
mheiber commentedDec 31, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I meant "privacy holes." Autocorrect got me! |
littledan commentedDec 31, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I'm not aware of any privacy holes in the main design. There may be other hacks that just work on this transform, though. |
Jamesernator commentedJan 2, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
The feature is pretty popular, I imagine a lot of places won't adopt private fields primarily for this reason. Has a similar feature been considered for proposing at TC39? e.g.: classPoint{constructor(#x:number, #y:number){}} And for public fields maybe allow using classImage{constructor(this.data:ImageData,// Simple parameter{height:this.height,width:this.width/* in destructuring */}:{width:number,height:number},){}} |
avonwyss commentedJan 2, 2020
Given that the This would allow to implement a version which also works without WeakMap support (e.g. IE <= 10), or for anyone who wants to use a different mechanism, such as with non-enumerable properties or |
mbrowne commentedJan 3, 2020
In case anyone else is wondering the same thing I was, it looks like the first production release to include this will be version 3.8, scheduled for sometime in February: |
kokushkin commentedJan 23, 2020
This looks ridiculous and disrupt developer experience with the language, especially for newers) Please, don't do it. This is feature for Typescript 4, not for 3.8, where we can break backward compatibility. Or, at least either "private" or "#" must be deleted in Typescript 4. |
mbrowne commentedJan 24, 2020
@kokushkin Obviously |
hax commentedJan 27, 2020
This is not true. See myprevious comment. |
mbrowne commentedJan 27, 2020
@hax You're right, I didn't mean to mislead anyone, just should have written more carefully... I was trying to emphasize that this isn't just an early-stage proposal that TypeScript decided to implement on their own. I should have said it's a proposed standard that's very near the final stage of the standardization process, with private fields now enabled by default in Chrome, Firefox, and node. |
hybrist commentedJan 27, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Maybe you meant |
mbrowne commentedJan 27, 2020
@jkrems Ah yes, thanks for the correction. The public fields part of the proposal is enabled by default in Firefox...next step will probably be to add private fields behind a flag. Implementation status is periodically updated here: |
danielearwicker commentedFeb 20, 2020
A question about the downlevel emit - to use this feature I have to target ES2015. But that will mean classes remain as classes, which breaks IE11. And yet IE11 supports Is IE11's If I could suppress the error somehow...#29950 |
…e fixesSummary:* Add private-named instance fields. Ex: `x.#name;`. [1]* Support type-only imports and exports. [2]* Improve the detection of conditional expressions `a ? b : c`. Allow multiple lines.* Add rules of round brackets `()` to correct the highlighting of pairs of brackets. [3][1]microsoft/TypeScript#30829[2]microsoft/TypeScript#35200[3]https://unix.stackexchange.com/questions/527268/kate-18-12-3-no-longer-shows-matching-parenthesis-for-typescriptReviewers: #framework_syntax_highlighting, dhaumann, cullmannReviewed By: #framework_syntax_highlighting, cullmannSubscribers: kwrite-devel, kde-frameworks-develTags: #kate, #frameworksDifferential Revision:https://phabricator.kde.org/D27692
trusktr commentedMay 14, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
The implementation could use a single WeakMap per module, instead of one WeakMap per property. It think it may be more efficient. Maybe I missed it: is there a specific reason it needs to be one WM per property? |
ljharb commentedMay 14, 2020
At the least you’d need one for statics, and one for instances - but actually, it wouldn’t necessarily be more efficient, since you’d need a containing object to look up the individual fields in - and for functions, you’d have to bevery careful not to expose the containing object as the receiver. |
trusktr commentedMay 14, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Ah, right! I take that back, because for each instance, we'd need that accompanying object, which means O(n) instead of O(1) mem use for storage containers for the key-value pairs, where |
mbrowne commentedMay 22, 2020
@trusktr FYI, there's also this issue:https://github.com/tc39/proposal-class-fields/blob/master/PRIVATE_SYNTAX_FAQ.md#how-can-you-model-encapsulation-using-weakmaps. Babel works around this by creating a separate WeakMap for each field. |

Uh oh!
There was an error while loading.Please reload this page.
Private-Named Instance Fields
This PR implementsthe tc39 class fields proposal for TypeScript. It includes:
PR merge checklist
@targets to conformance tests espthis oneExample:
ts
js
This PR lead to the following related work by the team:
Babel issues reported:
.nameincorrect for private fields babel/babel#10175V8 issues reported:
Related TS PRs:
This PR includes work by the following engineers: