microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.16k stars 12.38k forks source link

Implement private fields proposal #9950

Closed dead-claudia closed 4 years ago

dead-claudia commented 8 years ago

I think it would be nice to have the stage 1 private fields proposal implemented in TypeScript. It'll mostly supercede the current pseudo-private implementation, and a fully-correct implementation is only transpliable to ES6 using per-class WeakMaps. I'll note that the spec itself uses WeakMaps internally to model the private state, which may help in implementing the proposal.

Currently, the proposal only includes private properties, but methods are likely to follow. Also, the most important part of the proposal is that private properties are only available within the class itself.

mbrowne commented 5 years ago

I think it would be nice to have a compiler option to make private transpile to some kind of soft private that would be enforced at run-time. I don't think the use case for needing true hard private is especially common, so even aside from any dislike of the # syntax, I think most people would tend toward sticking with private. But compile-time only private is too soft IMO. Also, such an option would help reduce the number of possible variants of private...if developers are left to implement this on their own then you could have all of these:

class Demo {
  private a

  #b

  @reflect
  #c
}

So I propose that with the new option enabled, private a would transpile to @reflect #a, where @reflect is a decorator that enables reflection on the property via a community-supported API. There has been some talk of a standard decorator library, so the @reflect decorator (or whatever we'd want to call it) could be standardized in the future; I think it would be a good candidate for that.

I suppose the main downside of my proposal would be the performance impact. We can hope that one day JS might have some standard decorators that are implemented natively to optimize them, but the reality for the foreseeable future is that @reflect #x would be slower than just #x. But I'm only proposing this as an option, which should probably be disabled by default (for backward compatibility reasons as well).

mbrowne commented 5 years ago

@MichaelTheriot

Late on this, but there actually is a proposal suggesting this->var syntax.

Granted, this proposal is most likely dead ("the committee was not enthused" is the only reason I have been able to find)

The committee has given plenty of well-considered reasons for endorsing the current proposal instead of the classes 1.1 proposal, for example https://github.com/tc39/proposal-class-fields/issues/100#issuecomment-429460917

MichaelTheriot commented 5 years ago

@mbrowne

I may have missed it. The issues page of classes 1.1 has been inactive for half a year, and even the example you linked was written a month after my comment.

FYI, I mentioned this in the thread you linked several months ago. My comment here was just to address a misconception that no alternative syntax has been proposed, since I believe all parties involved should be aware that is not the case.

Edit: My comment does not suggest TypeScript do anything different. I believe it is useful to correct misinformation on referenced issues of a proposal.

kitsonk commented 5 years ago

@MichaelTheriot the parties that need to be informed of alternatives proposals are not the parties involved in this conversation. TypeScript will follow the proposal that is on track to be ratified. The fact that alternative syntaxes have been proposed and not adopted isn't really useful for this conversation in my opinion.

trusktr commented 5 years ago

Hey guys, in case it helps, I'd like to share my own implementation of runtime protected/private members:

See lowclass.

Check out the extensive test files showing all sorts of uses of runtime public/protected/private members. The implementation is not long, and some features can be omitted in order to have only the minimal protected/private functionality.

Notes:

To run tests:

npm install
npm test
trusktr commented 5 years ago

Also see https://github.com/babel/proposals/issues/12 and https://github.com/babel/babel/issues/8421 for details and issues with Babel's implementation.

trusktr commented 5 years ago

Hello you all,

please don't implement the not-yet-finalized private class fields spec. It has problems.

Please read this thread.

I would encourage anyone who reads this to please take a look at some other syntax ideas (just peruse around, there's more), and voice your opinions.

f.e. here's another thread (with better syntax ideas than the current # IMHO).


If there's any JavaScript community already using public/private/protected widely, it's the TypeScript community which has a big chance to help shape the future of JS private/protected.

andria-dev commented 5 years ago

So, private class fields are being shipped in V8 v7.2 and Chrome 72. Are there any plans to begin implementing the private fields proposal?

jhpratt commented 5 years ago

@ChrisBrownie55 Public fields are shipping. Private are not.

andria-dev commented 5 years ago

@jhpratt that's my bad, but it would seem that the Chrome team is planning on shipping private in the near future

dead-claudia commented 5 years ago

For reference: https://v8.dev/blog/v8-release-72#public-class-fields

It documents they're shipping public class fields as of V8 7.2, but there's this regarding private fields (emphasis mine):

Support for private class fields is planned for a future V8 release.

andria-dev commented 5 years ago

@isiahmeadows I was actually referring to a different post on Google's developer blog.

Screenshot of Google Developers page

Conclusion

Public class fields are shipping in V8 v7.2 and Chrome 72. We plan on shipping private class fields soon.

Link to article on Google's developer blog

littledan commented 5 years ago

These articles were written by @mathiasbynens and Andreas Haas, who you can ask for clarification if needed (though I think each of those posts were pretty clear). cc @gsathya and @joyeecheung who are working on the finishing touches of the implementation in V8.

Anyway, the January 2019 TC39 meeting capped off a year and a half of rethinking alternatives to the private fields and methods proposals with yet another reaffirmation of the consensus on the proposals. I think we can consider the current version stable and ready to go for TypeScript.

andria-dev commented 5 years ago

Chrome just released private class fields 🎉 They're only in Chrome 74 currently released in the canary build.

screenshot of tweet

I just shipped private class fields in Chrome 74! Try it out in Chrome canary today! — Sathya Gunasekaran via Twtiter

Does anyone know where we're at here on choosing a specific implementation (or building one) for private class fields? I saw that @trusktr recommended lowclass but I don't know if "Private Inheritance" is a part of the spec.

ljharb commented 5 years ago

It decidedly is not; private fields do not walk the prototype chain (because then other objects would be able to see them, and thus they wouldn't be private)

mbrowne commented 5 years ago

I would assume that private fields will transpile to WeakMaps (one WeakMap per field, the same way Babel does it). There should also be an option to keep the # syntax in the final output, for those who are exclusively targeting browsers that support it (this will become more relevant over time of course).

mbrowne commented 5 years ago

For performance reasons, it might also be worth considering an option that would allow developers to use # syntax for future compatibility (when more browsers support it), but actually transpile to public properties with some special prefix, e.g. __ts#. WeakMaps should be benchmarked to see if this is actually necessary, but I suspect that the performance difference between public properties and WeakMaps could be significant. If such an option were added, it should be off by default of course, since developers using # syntax would normally expect it to be both compile-time and run-time private. But for those who want to maximize run-time performance and are satisfied with just compile-time private in the meantime, it could be a nice way to start using the new syntax without potentially causing a performance degradation.

jhpratt commented 5 years ago

Any updates on implementation progress? Perhaps a PR that we can build to test out and provide feedback?

mheiber commented 5 years ago

@jhpratt sure! If you check out the branch in this PR, you'll be able to test out private-named instance fields. Methods and accessors are in progress.

We're hoping to upstream this work. We can P.R. the changes against this repo (Microsoft/TypeScript) as soon as this prerequisite PR is merged: https://github.com/Microsoft/TypeScript/pull/30467.

Feedback is most welcome.

trusktr commented 5 years ago

I don't know if "Private Inheritance" is a part of the spec.

Sidenote, I'm planning to drop that feature in lowclass, to enable 100% true privacy. I haven't really needed it in practice, it was more of an experiment.

trusktr commented 5 years ago

By the way, I feel like posting here, in case any theory on "private inheritance" may possibly be of any use (maybe just interesting, or perhaps helping to spark any other ideas):

in case you're interested: "Private inheritance" (as seen in the current version of lowclass) means that a subclass can use private methods inherited from a superclass, but the method applies changes _**only**_ to private properties of the instance _within the scope of that subclass_, as if the subclass had those methods defined as private methods in its own definition, but the subclass can not modify private properties of the instance in a super- or sibling- class scope. In comparison, in "protected inheritance" a subclass _can_ modify properties of it's own instance or superclass instance, but still can not modify properties of a sibling class. In protected inheritance a subclass modifies properties that all super classes are able to see (super classes all read/write to those same properties). In other words, in the "private inheritance" concept each class has a private scope, and any inherited private methods only operate on the local class scope, in contrast to protected where protected methods operate on a single scope for the entire class hierarchy.
trusktr commented 5 years ago

Not sure if that made much sense, so here's simple code examples to explain what "private inheritance" would be similar to, if we wrote it using the current of ecma proposal-class-fields private field features:

example code is easier to understand: There's no "private inheritance" in proposal-class-fields, so the following won't work (and I agree this is desirable, for maintaining true privacy): ```js class Foo { test() { this.#privateMethod() } #foo = 'foo' #privateMethod() { console.log(this.#foo) } } class Bar extends Foo { test() { // This does not work, no private inheritance: this.#privateMethod() } // #foo is private only inside Bar code, it is NOT the same #foo as in Foo // scope. #foo = 'bar' } ``` In lowclass, "private inheritance" would be equivalent to writing the following non-DRY code to emulate the same thing, using some copy/paste in your editor: ```js class Foo { test() { this.#privateMethod() } #foo = 'foo' #privateMethod() { console.log(this.#foo) } } class Bar extends Foo { test() { // This does not work, no private inheritance: this.#privateMethod() } // #foo is private only inside Bar code, it is NOT the same #foo as in Foo // scope. #foo = 'bar' // copy the method over by hand (because there's no private inheritance): #privateMethod() { console.log(this.#foo) } } ``` In both examples, the `#foo` variable is a specific variable to each class. There's two `#foo` variables, not one; each one readable and writable only by code in the same class definition. In lowclass, private inheritance allows us to re-use private superclass methods, **_but_** the method operates in the scope where the method is being used. So it ends up being as if we copied and pasted the superclass method into the subclass, just like in the above example, but there still are two separate `#foo` variables for each class. In lowclass, if `#someMethod` is used in the super class, it operates on the `#foo` of the superclass. If `#someMethod` is used in the scope of the subclass, it operates on the `#foo` of the subclass, not the `#foo` of the superclass! I hope that explains the concept, and even if it won't be used, I hope it may be interesting or useful in sparking ideas of any sort.
richiksc commented 5 years ago

It's in Chrome stable channel now, so developers can use it natively. I'm mostly interested in this issue because of VSCode - when writing vanilla .js javascript, VSCode marks it as an error. Is there a way to add support for private class fields in Vanilla JS in VSCode separate from the debate on whether to add them to Typescript?

dead-claudia commented 5 years ago

Personally, I don't feel TS should add support for them before it hits stage 4. TS already has compile-time private that works well enough for now. After it hits stage 4, then I would agree it makes sense. (There's also been occasional bouts of flux in the discussion, so I still wouldn't consider it ready for prime time.)

richiksc commented 5 years ago

Update: It's also supported in Node 12 stable. Also, Babel has a plugin to transform this syntax. Developers often use features before they become stage 4 proposals. I would like to see support added for this syntax in Vanilla JS (non-ts) or a way to enable it through jsconfig.json so VSCode doesn't show an error when using them. The VSCode repo says that their JS support is backed by TypeScript and referred to this issue.

andria-dev commented 5 years ago

I would have to say that it doesn't make sense for TypeScript to not support the feature at all since it already exists in production in a major browser and Node. This isn't to say that TypeScript has to completely solidify everything right now and release it as an on-by-default feature.

I would suggest that TypeScript introduces private fields under a flag that indicates that this feature is experimental and is subject to change between stage 3 and stage 4.

saschanaz commented 5 years ago

Maybe we could implement the syntax support first without semantics?

trotyl commented 5 years ago

Maybe we could implement the syntax support first without semantics?

For type-checking/intellisense, I think there must be some kind of semantics, but can have no downlevel transformation support like BigInt (can only be used with esnext).

ljharb commented 5 years ago

fwiw, it’s only subject to change in stage 3 due to implementation feedback. Given that chrome is shipping it, any change whatsoever is highly unlikely.

Sceat commented 5 years ago

@ChrisBrownie55 i'd say it doesn't make any sense that this feature is not yet implemented

mbrowne commented 5 years ago

Looks like this is in progress: https://github.com/microsoft/TypeScript/pull/30829 (which is apparently waiting on https://github.com/microsoft/TypeScript/pull/30467)

mheiber commented 5 years ago

Right, #30829 is about ready for merge after rebase, modulo some feature-gating we may want to do so it blows up on private-named methods.

trusktr commented 5 years ago

Guys, just because a major browser implemented the feature (to see how it feels, play with it, etc) doesn't mean everyone should.

There's also been occasional bouts of flux in the discussion, so I still wouldn't consider it ready for prime time

As @isiahmeadows mentioned, the class fields proposal is full of controversy and many people dislike it. Here are a couple issues showing community dislike of the current private fields:

If I were a language implementer, I would be concerned about this, and I would not want to be responsible for a much-disliked feature being released (set in stone) because I decided to allow thousands of (unknowing) developers start using a problematic feature in production code.

Private fields is perhaps the most controversial of all features introduced since ES6. I think it would be wise for a language implementer to take the dislike into consideration before helping set the feature in stone.

richiksc commented 5 years ago

@trusktr It may be a controversial feature, but whether developers use it should be a decision left up to the developer, not the tools. A major browser has shipped it not to "play with it" but to ship it for use in production. It has also shipped in Node, the most popular server-side JS runtime. Again, I'm less concerned on whether the syntax makes it to TypeScript or not, but more concerned on whether the syntax is supported by the JavaScript language server on VSCode (which is backed by TypeScript).

trusktr commented 5 years ago

It has also shipped in Node

Only as a side-effect of the Chrome release, which maintains Node's JS engine.

It may be a controversial feature, but whether developers use it should be a decision left up to the developer, not the tools

That may be the case for developers like you (maybe you just need to release a product, and you will work with whatever tools you have, which is in fact what you should do in that regard, and what many employed developers do).

There are also many developers that may not know about the foot guns associated with class-fields, while just trying to use tools given to them.

And there are also developers who care about the language itself, and the future of maintainable code, and would like to see it evolve correctly, before certain mistakes are too late to be undone.

The good thing is, because the new private fields use # syntax, there's still room to fix it using the private keyword as an alternative.

TeoTN commented 5 years ago

Does TS really have to follow the standard 100%? We already know that the committee is going against community with thisGlobal or #privateLol, so maybe it's time to say that Google has no monopoly over JavaScript before it's too late?

jhpratt commented 5 years ago

@TeoTN Private fields are not a Google thing. It has proceeded through TC39, as has every other recent standard.

RyanCavanaugh commented 5 years ago

A few points to help provide clarity:

kkimdev commented 5 years ago

@RyanCavanaugh Thanks for the clarification and also for all the awesome work! A small suggestion: ES are evolving continuously and there are many features being worked on, I think it's good to have a rule that at what stage - Stage 4? Stage 3? or somewhere inbetween? - ES features are eligible for inclusion. Otherwise, this kind of discussion could repeat again and again in the future for other features.

slikts commented 5 years ago

Private fields can be reasonably said to be fully baked.

@kkimdev Stage 4 is as far as it goes; it means having become part of the standard.

trusktr commented 5 years ago

ES features that aren't fully baked

Curious, what are fully baked ES features? What's "fully baked" mean in this example?

trusktr commented 5 years ago

implementing ratified and shipping ES features is not optional

That's totally reasonable considering TypeScript's goal to be simply types over vanilla JS (so things like namespace were a bad choice with respect to current goals, and enum is debatable too).

TypeScript has some power to help (or not help) the community in swaying language specs, by waiting (or not waiting) to ship a feature until all major engines have unanimously shipped the given feature.

ljharb commented 5 years ago

Seems inefficient, since TS team members sit on TC39, so they’ve already been part of consensus for a feature being stage 3 before anyone has shipped it in the first place - this one included.

littledan commented 5 years ago

I really value TypeScript's design decisions to be conservative in what they ship; in many cases, it makes sense to wait a bit longer in order to give guaranteed stability to developers. I've been very happy with the policy decisions with respect to private fields, and in frequent contact with @DanielRosenwasser about them. The TypeScript team has given a lot of useful feedback to TC39 over the years, which helped shape the design of many features.

I think the best place to discuss these language design decisions would be in the proposal repositories, in TC39 meetings, and in other contexts dedicated to this work. We've discussed the private fields and methods proposals extensively with @trusktr there; discussion here seems off-topic.

Ultimately, I think TC39 is the place where we should continue to do the language design, and the TypeScript repository could be a place to discuss other aspects, such as the priority and implementation details for a feature, and the judgement of when a feature is stable enough to ship.

TomasHubelbauer commented 5 years ago

Since TypeScript Intellisense powers JavaScript syntax highlighting in VS Code and the two syntax highlighing issues I've found (Microsoft/vscode#72867 and Microsoft/vscode#39703) in the VS Code repository redirect here - where implementation of this into TypeScript is discussed - let me contribute by saying that it would be cool for this to not be an error in JavaScript files.

Perfectly valid JavaScript gets marked as erroneous in VS Code because a compiler for a different language is in the process of decising whether the syntax will be supported or not. :-)

I don't have a problem with TypeScript taking its time before making a decision on this (in fact I support it!), but this affects JavaScript syntax highlighting in a way which cannot be easily fixed or worker around even locally (as far as I know) by patching the syntax highlighter grammar file, because there is none to patch (AFAIK).

mbrowne commented 5 years ago

While I'm not on the TypeScript team, they're actively working on implementing this and I don't think there's much doubt at this point that private fields will be supported. See https://github.com/microsoft/TypeScript/pull/30829.

mheiber commented 5 years ago

I can confirm that it's being worked on- we're collaborating with the TS team and are currently wrapping up a rebase. Excited for private-named fields in TS (and, by extension, Language Server).

DanielRosenwasser commented 4 years ago

Big thanks to our collaborators at Bloomberg for implementing this in https://github.com/microsoft/TypeScript/pull/30829!

wessberg commented 4 years ago

Hey there. If you'd like me to create a separate issue for this, please say so. I'm interested in learning what the rationale is for emitting the special #private PropertyDeclaration that led to this issue.

For example, the following declarations will be generated:

// index.d.ts
declare class Foo {
    #private;
    // ...
}

...Given the input:

// index.ts
class Foo {
    #name: string;
}

I'm trying to understand first why it needs to be there, but also if it can be left out, given that I'm building tooling around generation of declarations.

weswigham commented 4 years ago

I'm trying to understand first why it needs to be there, but also if it can be left out, given that I'm building tooling around generation of declarations.

Presence of a private make the class nominalish in our analysis, so preserving that in a declaration file is important. You could maybe use a post-emit declaration transformer tool like dts-downlevel to downlevel the private declaration for older TSes in some way? (I don't know if it has a downlevel for #privates yet)