Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Conversation

@mheiber
Copy link
Contributor

@mheibermheiber commentedApr 9, 2019
edited
Loading

Private-Named Instance Fields

This PR implementsthe tc39 class fields proposal for TypeScript. It includes:

  • parse and check private-named fields, methods, and accessors
  • displayprivate names in the language server
  • transform private-named instance fields

PR merge checklist

  • BB: incorporate remaining feedback
  • BB: add multiple@targets to conformance tests espthis one
  • MS: reviewer 1 👍 remaining feedback
  • MS: reviewer 2 👍 remaining feedback
  • BB: squash and rebase
  • MS: run rw test suite
  • mergecompanion PR to tslib

Example:

ts

classGreeter{    #name:string;constructor(name:string){this.#name=name;}greet(){console.log(`hello${this.#name}`);}}constgreeter=newGreeter("world");console.log(greeter);// logs 'Greeter {}'console.log(Object.keys(greeter));// logs '[]'greeter.greet();// logs 'hello world'

js

var_classPrivateFieldSet=function(receiver,privateMap,value){if(!privateMap.has(receiver)){thrownewTypeError("attempted to set private field on non-instance");}privateMap.set(receiver,value);returnvalue;};var_classPrivateFieldGet=function(receiver,privateMap){if(!privateMap.has(receiver)){thrownewTypeError("attempted to get private field on non-instance");}returnprivateMap.get(receiver);};var_nameWeakMap_1;classGreeter{constructor(name){_nameWeakMap_1.set(this,void0);_classPrivateFieldSet(this,_nameWeakMap_1,name);}greet(){console.log(`hello${_classPrivateFieldGet(this,_nameWeakMap_1)}`);}}_nameWeakMap_1=newWeakMap();constgreeter=newGreeter("world");console.log(greeter);// logs 'Greeter {}'console.log(Object.keys(greeter));// logs '[]'greeter.greet();// logs 'hello world'

This PR lead to the following related work by the team:

This PR includes work by the following engineers:

trotyl, mihailik, RichiCoder1, chriskrycho, swarnimarun, saschanaz, DanielRamosAcosta, martinmcwhorter, a1essar, SalvatorePreviti, and 66 more reacted with thumbs up emojimonfa-red, dimitropoulos, diabolicle, corbane, chiqui3d, chriskuech, nulladdict, demensky, styfle, quadband, and 12 more reacted with thumbs down emojitrotyl, aduh95, 0kku, jbhoosreddy, MattiasBuelens, motss, alitaheri, dnalborczyk, styfle, tim-mc, and 2 more reacted with hooray emojitrotyl, mihailik, johnnyreilly, aduh95, SalvatorePreviti, G-Rath, 0kku, jbhoosreddy, JunichiSugiura, Kornil, and 15 more reacted with heart emojiv-checha and riyadshauk reacted with rocket emojihoufio, sod, johnnyreilly, motss, and amitbeck reacted with eyes emoji
@msftclas
Copy link

msftclas commentedApr 9, 2019
edited
Loading

CLA assistant check
All CLA requirements met.

@mheibermheiberforce-pushed theprivate-named-instance-fields branch 2 times, most recently fromb242db5 tof2c5233CompareApril 9, 2019 12:33
@mihailik
Copy link
Contributor

Amazing achievement!

Are the helpers_classPrivateFieldGet/_classPrivateFieldSet treated in the same way as__extends,__awaiter,__generator?

(that would include modifications totslib)

@mheiber
Copy link
ContributorAuthor

@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
Copy link
Member

I was hoping the syncing was semi-automated.

Sadly, they are not. A paired PR againsttslib with the finalized helpers will be needed.

@Neuroboy23
Copy link

@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.

shyce reacted with thumbs up emojiweswigham reacted with heart emoji

@G-Rath
Copy link

G-Rath commentedMay 30, 2019
edited
Loading

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 doprivate #bar = 3;?
(In case its not clear, I mean this purely as a meaningless sugar that would never make it into compiled code)

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/private #/ regex, since# is only valid for private class properties - the only issue with this I can think of would be with strings.

Also, could someone confirm for me if its planned for TS somewhere down the line to transformprivate to#? (I suspect it is, but again there's a lot of heavy comments, PRs, & issues on the matter, so it'd be nice to have a simple one-liner comment about it if possible).

Sovgut, matrunchyk, LifeIsStrange, ems-ja, lsagetlethias, dfernandeza, dlOuOlb, tommytroylin, kokushkin, and 1valdis reacted with thumbs up emojitrotyl reacted with thumbs down emoji

@ljharb
Copy link
Contributor

@G-Rath no, no chance of it being supported by JS itself (see theFAQ). I suspect/hope TS would not be likely to support non-type-related syntax that isn't standard to JS.

@G-Rath
Copy link

@ljharb fair enough - so then I assume TS is aiming to deprecate & remove theprivate keyword? (over time of course)

@robpalme
Copy link

@G-Rath Your question on the future of TypeScript's keywordprivate is one that many people will have, and may provoke more discussion. Would you like to raise it as a new top-level issue?

levenleven reacted with thumbs up emojiG-Rath and datatypevoid reacted with heart emoji

@G-Rath
Copy link

@robpalme more than happy to - done via#31670

@septs
Copy link
Contributor

"private field" decorator "private" only?

classSample{private #field:number=1;}
ExE-Boss, ems-ja, jerry-TangHao, and 17621152239 reacted with thumbs up emojijhpratt and fkajerry reacted with thumbs down emojiG-Rath, septs, RyanCavanaugh, MichalLytek, AsukaSong, and hasezoey reacted with confused emoji

@robpalme
Copy link

@rbuckton has re-landedthe refactoring of class properties. The next step is to rebase this PR.

mheiber reacted with thumbs up emoji

@Aqours
Copy link

Doesprivate name use same private fields transformation of#name?

michahell and jerry-TangHao reacted with thumbs up emoji

@DanielRosenwasser
Copy link
Member

@Aqours no,private is meant to be strictly design-time.

littledan reacted with thumbs up emoji

@mheiber
Copy link
ContributorAuthor

The rebase is in progress and we're expecting to update soon!

RobertWHurst reacted with thumbs up emoji

@mheiber
Copy link
ContributorAuthor

mheiber commentedJun 21, 2019
edited
Loading

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 typingthis.#foo

Actual behavior:

langservice

Autocomplete completes incorrectly (see screenshot). It is actually completing with secret variables we use in the transformation

Expected behavior:

Autocomplete completes as#foo.

Advice welcome on how to fix autocomplete in this PR!

gugadev reacted with thumbs up emoji

@DanielRosenwasser
Copy link
Member

That is actually really weird. Are you sure that it's not just picking up the output.js files? Are you able to reproduce in a fourslash test?

@mheibermheiberforce-pushed theprivate-named-instance-fields branch fromf2c5233 toa9cb10eCompareJune 25, 2019 08:47
@mheiber
Copy link
ContributorAuthor

Thanks for your suggestion,@DanielRosenwasser: the completionswere picking up the JS output. I added anexport to let TS know the file is a module, and no longer see weird completions.
Unfortunately, I also don't see the desired completion, which isthis.#foo:

image

DanielRosenwasser reacted with thumbs up emoji

@joeywattsjoeywattsforce-pushed theprivate-named-instance-fields branch froma9cb10e to36a1648CompareJune 26, 2019 19:31
@mheibermheiberforce-pushed theprivate-named-instance-fields branch from36a1648 tod2adc3cCompareJune 27, 2019 10:31
@joeywattsjoeywattsforce-pushed theprivate-named-instance-fields branch 2 times, most recently from71c384a toafcc88cCompareJune 27, 2019 21:23
@mheiber
Copy link
ContributorAuthor

mheiber commentedDec 31, 2019
edited
Loading

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 otherprivacy hills privacy holes, as well?

@littledan
Copy link

@mheiber What do you mean by privacy hills?

mheiber reacted with laugh emoji

@mheiber
Copy link
ContributorAuthor

mheiber commentedDec 31, 2019
edited
Loading

I meant "privacy holes." Autocorrect got me!

@littledan
Copy link

littledan commentedDec 31, 2019
edited
Loading

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
Copy link

Jamesernator commentedJan 2, 2020
edited
Loading

We have no plans to implement #-private parameter properties.

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 usingthis.prop as a parameter (or destructured parameter) e.g.:

classImage{constructor(this.data:ImageData,// Simple parameter{height:this.height,width:this.width/* in destructuring */}:{width:number,height:number},){}}
ctsstc and danbord reacted with thumbs up emoji

@avonwyss
Copy link

Given that the__classPrivateFieldGet and__classPrivateFieldSet could be customized in the way they work by implementing them differently and importing that customized tslib, wouldn't it make sense to also put the_nameWeakMap_1 = new WeakMap(); map initialization into its own tslib helper function?

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 orSymbol-based private fields (knowing that these are not private as per tc39 proposal, but may perform better).

@mbrowne
Copy link

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:
https://github.com/microsoft/TypeScript/wiki/Roadmap#38-february-2020

kohlmannj, awkaiser, and aloisbarreras reacted with thumbs up emojikokushkin reacted with thumbs down emojihybrist reacted with hooray emoji

sheetalkamat added a commit to microsoft/TypeScript-TmLanguage that referenced this pull requestJan 8, 2020
@kokushkin
Copy link

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.

kokushkin, hax, colonelchlorine, and TeoTN reacted with thumbs up emojiljharb, hybrist, pyrocat101, and ShlomiAltostra reacted with thumbs down emojiaduh95 and matrunchyk reacted with laugh emojikokushkin reacted with confused emoji

@mbrowne
Copy link

@kokushkin Obviously# should not be dropped in TypeScript 4 since it's already an ECMAScript standard (still stage 3, but advancing to stage 4 is just a formality at this point in the process). And I think droppingprivate would be premature since it would break backward compatibility and there is currently no other way to create compile-time only private properties (or at least properties that are accessible somehow outside the class, e.g. symbols). Anyway, this PR isn't the best place for these discussions...just thought I would briefly give my two cents in response to this.

hybrist reacted with thumbs up emojikokushkin reacted with thumbs down emoji

@hax
Copy link

hax commentedJan 27, 2020

since it's already an ECMAScript standard

This is not true. See myprevious comment.

@mbrowne
Copy link

@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.

matrunchyk, hybrist, and Sominemo reacted with thumbs up emoji

@hybrist
Copy link

hybrist commentedJan 27, 2020
edited
Loading

with private fields now enabled by default in Chrome, Firefox, and node.

Maybe you meants/Firefix/Edge/? Firefox has public instance fields but no real support for private fields yet. Though it looks like it's just a matter of time until they get to it:https://bugzilla.mozilla.org/show_bug.cgi?id=1562054. But >50% of web users have private fields enabled so your overall point stands I think. :)

@mbrowne
Copy link

@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:
https://github.com/tc39/proposal-class-fields#implementations

@danielearwicker
Copy link

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 supportsWeakMap to some extent. And I find that TS does actually produce what looks like a good emit private fields for ES5, but gives me an error message.

Is IE11'sWeakMap good enough for this usage? It returns undefined fromset but the TS emit here doesn't care about that.

If I could suppress the error somehow...#29950

cdata, ExE-Boss, justinfagnani, pyrocat101, and UltiRequiem reacted with thumbs up emoji

kdesysadmin pushed a commit to KDE/syntax-highlighting that referenced this pull requestFeb 29, 2020
…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
Copy link
Contributor

trusktr commentedMay 14, 2020
edited
Loading

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
Copy link
Contributor

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.

ExE-Boss reacted with thumbs up emoji

@trusktr
Copy link
Contributor

trusktr commentedMay 14, 2020
edited
Loading

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, wheren is number of class instances and1 is the constant number of defined private fields. I overlooked that.

ljharb and ExE-Boss reacted with thumbs up emoji

@mbrowne
Copy link

@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.

@dragomirtitiandragomirtitian mentioned this pull requestFeb 3, 2021
3 tasks
@microsoftmicrosoft locked asresolvedand limited conversation to collaboratorsOct 21, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.

Reviewers

@DanielRosenwasserDanielRosenwasserDanielRosenwasser requested changes

@sheetalkamatsheetalkamatsheetalkamat left review comments

@sandersnsandersnsandersn approved these changes

+4 more reviewers

@ljharbljharbljharb left review comments

@joeywattsjoeywattsjoeywatts left review comments

@rbucktonrbucktonrbuckton requested changes

@JamesernatorJamesernatorJamesernator left review comments

Reviewers whose approvals may not affect merge requirements

Labels

Update Docs on Next ReleaseIndicates that this PR affects docs

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

20 participants

@mheiber@msftclas@mihailik@weswigham@Neuroboy23@G-Rath@ljharb@robpalme@septs@Aqours@DanielRosenwasser@sandersn@rbuckton@joeywatts@aduh95@orta@typescript-bot@ExE-Boss@nicolo-ribaudo@gugadev

[8]ページ先頭

©2009-2025 Movatter.jp