Implement private variables, getter/setters and methods using Symbols

[Arnavion]

Example implementation for #684

Not for merging, only discussion.

TODO:

• Implement compiler flag. Done.
• Use compiler flag to selectively turn on new codegen. Done.
• Only generate __symbol declaration when the current sourceFile has any classes with private members. The current code always generates it for every file regardless of whether it has classes or not.
• Tests.
• Handle static and instance members with the same name. Done.

[sophiajt]

First off - cool idea. Automatically using symbols for privates, rather than forcing users to generate symbols themselves, sounds like it'd be helpful.

It raises a few questions that'd be good to hash out:

1. Should codegen emit a check that looks for Symbols and falls back if they're not there? This means that the semantics would be subtly different from browser to browser depending on what ES6 level they support.
2. Does it make sense to polyfill down to ES3/ES5? The browser needs to support at least some ES6. The rewriting to use symbols is useful for users working with ES6 and emitting ES6, also. Should that be the focus?
3. From a design perspective - do we want multiple ways of outputting privates? Will this lead to splintering the community, or is this a feature that doesn't have compatibility impact? Something to talk about.

[Arnavion]

Did you look at #684 ? The answers are there. In short:

1 and 2: __symbol falls back to a function that returns the parameter if global.Symbol is not found. It's not a polyfill (a full polyfill for Symbol is much more complicated) but it atleast retains the current semantics of TypeScript (pre-symbols).

3: As pointed out in #684 this is definitely not something that should be enabled by default - the perf impact is enormous in all browsers I tested even when not actually using Symbols (i.e., using the fallback). That leads to three situations:

• A hypothetical future where the Symbols+fallback codegen is as performant as TS's current codegen. In this future, there is no need to give the user a choice.
• A situation where the user cares more about hiding privates than performance. Do such users exist?
• The current situation: Symbols+fallback is way slower than the current codegen. Users would want a choice.

Re: splintering the community with choice - as mentioned in #684, since this only affects the internals of a class and classes can't be split across files (as of now), it's a file-level decision whether to compile with symbols or not. It doesn't have a compatibility impact in that regard.

Open classes will be a problem if they're implemented in a way that allows cross-file private member access - then all files that contain the class must be compiled with the same value of the switch. Protected will be a problem if it's implemented using symbols - the symbols must be made accessible from the base class closure to the derived class closure (and preferably not more?), not collide with user-defined members, etc.

But yes, all the discussion is in #684

[CyrusNajmabadi]

I don't see any test changes as part of tihs. I'd like to see how this ends up affecting the emitted code. Thanks!

[CyrusNajmabadi]

please use arrow notation. i.e. d => d.flags...

also, you just need:

symbol.declarations.some(d => d.flags & SymbolFlags.Private);

[Arnavion]

I've changed it to arrow notation, but I can't remove the equality check because the result of bit-wise and is a number and some wants a callback that returns a bool.

[Arnavion]

I just discovered that ts.forEach is actually the same as Array.prototype.some and not Array.prototype.forEach, so I'll just use that...

[Arnavion]

@CyrusNajmabadi

I'd like to see how this ends up affecting the emitted code.

Until I add tests, see #684 for the new codegen sample.

[Arnavion]

Rebased on latest master.

I noticed getPropertyAccessSubstitution is gone now, so I replaced it with two methods that just return true if the property access is for a private or static property respectively.

Also reordered the commits so that the codegen is always under the compiler options switch (previously the compiler options switch was added after the new codegen).

[sophiajt]

We talked about this at the most recent TypeScript design meeting. Your proposed patch covers a few design points.

There was general hesitation around codegen that would do feature detection and then act differently, as there is no precedence for this in TypeScript. Instead, the precedence is around using flags (like outputting to ES5 rather than ES3) that would allow certain patterns through. If we supported symbols as a private implementation, we'd prefer it to not be under feature detection. Instead, let the developer opt in using commandline flags, rather through an ES6 mode or an additional flag.

On that note, we debated the pros and cons of using symbols for privates in general as part of an ES6 mode. There's quite a bit we just don't know, yet. Are privates going to be implemented using symbols in ES6? Or will symbols generally not be used for this pattern and largely be used as the known keys, eg [Symbol.iterator]. We don't have enough information to know if this pattern will be common. Until this pattern starts to emerge, we don't have enough to go on to be prescriptive about privates+symbols.

The general consensus is to wait and see how classes and symbols get used together. Because of the current concern with perf, it appears that engines need time to catch up, and with that developer practice.

It's an interesting idea, and one we should revisit in the future as patterns emerge.

[Arnavion]

There was general hesitation around codegen that would do feature detection and then act differently, as there is no precedence for this in TypeScript.

Agree.

If we supported symbols as a private implementation, we'd prefer it to not be under feature detection. Instead, let the developer opt in using commandline flags, rather through an ES6 mode or an additional flag.

That would just mean changing the __symbol function to use global.Symbol unconditionally and remove the fallback function that returns the original string. But anyway...

We don't have enough information to know if this pattern will be common. Until this pattern starts to emerge, we don't have enough to go on to be prescriptive about privates+symbols.

Definitely a valid point. User code not using symbols and browsers not optimizing symbols is somewhat of a vicious circle, but I agree that TS doesn't need to take a stance in this. I'm sure Symbols for privates is a quite a ways out before being used in actual code.

Are privates going to be implemented using symbols in ES6?

Last I checked that got backed out for ES7.

The general consensus is to wait and see how classes and symbols get used together.

I agree. I primarily did this proposal not because it would be useful, but as an experiment to see what the codegen would look like. I'm myself not convinced that symbols for privates is a good idea either, seeing as it's harder to optimize (although const will make that easier since the VM can then recognize single-assignment keyed accessors more easily) and it's of limited value when TS already has a type-checker that forbids private access.

[sophiajt]

Great, sounds like a plan. Closing this pull request. We can revisit later.

Thanks for the brainstorming!