Transform private named instance fields #15

joeywatts · 2018-11-21T01:54:46Z

Downlevel transform for ESNext private named instance fields. Uses WeakMaps to store the value for each private field, and helper functions to access and mutate the field.

Design Notes (and Insights for Implementation of Private Named Methods, Accessors, etc)

A "private name environment" stack is maintained in the ESNext transformer which keeps track of all the private names. This is a stack because private names are lexically scoped and you can nest classes.
When the ESNext transformer visits a class, it loops over all the class members and populates the private name environment (effectively a map from the private names to an object storing the necessary information for each private name) for that class.
At the usage sites of a private name, we look up the private name (using the private name environment stack). Based on the type of the private name, we can transform the usage sites differently.
Where possible, the transformation was designed to be agnostic of the type of private name. For example, unary modification operators (++, --) piggy-back off of the instance field access and assignment transformations by first transforming to an intermediate representation and then transforming that. this.#x++ will first transform to this.#x = this.#x + 1 and then will finally transform to _classPrivateFieldSet(this, _x, _classPrivateFieldGet(this, _x) + 1). So for private named get/set accessors, if the access and assignment transformations are implemented, most things should just work! Another example of this is the transformation of call expressions on private names, this.#myFunc() will first transform to this.#myFunc.call(this, /*...args*/) then it will rely on the private name property access transformation to perform the rest of the work. If property accesses are implemented for private named methods, then calling them should just work!

mheiber · 2018-12-07T16:47:34Z

tests/baselines/reference/api/tsserverlibrary.d.ts

@@ -621,6 +621,9 @@ declare namespace ts {
        type?: TypeNode;
        initializer?: Expression;
    }
+    interface PrivatePropertyDeclaration extends PropertyDeclaration {


since this is public API:

maybe we don't need another interface for this

if we keep it, PrivateNamedPropertyDeclaration is probably more accurate, since there are already private properties

I don't have a strong opinion on the interface, but I'm with Max on the name. "private-named" is our official shortest adjective for this.

mheiber · 2018-12-07T16:47:52Z

tests/baselines/reference/api/typescript.d.ts

@@ -621,6 +621,9 @@ declare namespace ts {
        type?: TypeNode;
        initializer?: Expression;
    }
+    interface PrivatePropertyDeclaration extends PropertyDeclaration {


see comment https://github.com/bloomberg/TypeScript/pull/15/files#r239873848

mheiber · 2018-12-10T14:51:07Z

src/compiler/binder.ts

@@ -3637,6 +3637,10 @@ namespace ts {
        let excludeFlags = TransformFlags.NodeExcludes;


@joeywatts I think we can merge this soon. Would help if you could:

add a little bit more to the PR description:

what were some judgment calls or design decisions made in this PR?

are there outstanding issues with private-named fields not covered in this PR?

please note that this PR builds on Keep class properties in ESNext target #10

rebase this down to a few smaller commits

mheiber · 2018-12-10T14:55:04Z

src/compiler/transformers/esnext.ts

+    /**
+     * A mapping of private names to information needed for transformation.
+     */
+    type PrivateNameEnvironment = UnderscoreEscapedMap<PrivateNamedInstanceField>;


as discussed, we will fiddle with this structure more in subsequent PRs (so that methods + static methods + getters can share the same WeakSet of instances)

Neuroboy23

Looks good.

Neuroboy23 · 2018-12-17T17:25:55Z

tests/baselines/reference/api/tsserverlibrary.d.ts

@@ -621,6 +621,9 @@ declare namespace ts {
        type?: TypeNode;
        initializer?: Expression;
    }
+    interface PrivatePropertyDeclaration extends PropertyDeclaration {


I don't have a strong opinion on the interface, but I'm with Max on the name. "private-named" is our official shortest adjective for this.

src/compiler/binder.ts

mheiber

psyched about this

Signed-off-by: Joseph Watts <jwatts43@bloomberg.net>

mheiber reviewed Dec 7, 2018

View reviewed changes

mheiber reviewed Dec 10, 2018

View reviewed changes

Neuroboy23 requested changes Dec 17, 2018

View reviewed changes

mheiber reviewed Dec 17, 2018

View reviewed changes

src/compiler/binder.ts Show resolved Hide resolved

joeywatts force-pushed the esnext-private-instance-fields branch from b416c58 to 4f59597 Compare December 17, 2018 22:09

mheiber approved these changes Dec 17, 2018

View reviewed changes

joeywatts force-pushed the esnext-private-instance-fields branch from 4f59597 to 2641392 Compare December 18, 2018 00:09

mheiber closed this Dec 18, 2018

mheiber reopened this Dec 18, 2018

Joseph Watts added 4 commits December 18, 2018 15:20

Downlevel private named field initializers

b7ea7f4

Signed-off-by: Joseph Watts <jwatts43@bloomberg.net>

Transform property access for private named instance fields

229f3ae

Signed-off-by: Joseph Watts <jwatts43@bloomberg.net>

Transform private name bindings in destructuring assignments

7bf2171

Signed-off-by: Joseph Watts <jwatts43@bloomberg.net>

Transform call expressions on private names to properly bind this

fa17cdf

Signed-off-by: Joseph Watts <jwatts43@bloomberg.net>

joeywatts force-pushed the esnext-private-instance-fields branch from 2641392 to fa17cdf Compare December 18, 2018 20:20

Neuroboy23 approved these changes Dec 18, 2018

View reviewed changes

Neuroboy23 merged commit 8bfc540 into bloomberg:es-private-fields Dec 18, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Transform private named instance fields #15

Transform private named instance fields #15

joeywatts commented Nov 21, 2018 •

edited

Loading

mheiber Dec 7, 2018

Neuroboy23 Dec 17, 2018

mheiber Dec 7, 2018 •

edited

Loading

mheiber Dec 10, 2018 •

edited

Loading

mheiber Dec 17, 2018

mheiber Dec 10, 2018

Neuroboy23 left a comment

Neuroboy23 Dec 17, 2018

mheiber left a comment

		@@ -3637,6 +3637,10 @@ namespace ts {
		let excludeFlags = TransformFlags.NodeExcludes;

Transform private named instance fields #15

Transform private named instance fields #15

Conversation

joeywatts commented Nov 21, 2018 • edited Loading

Design Notes (and Insights for Implementation of Private Named Methods, Accessors, etc)

mheiber Dec 7, 2018

Choose a reason for hiding this comment

Neuroboy23 Dec 17, 2018

Choose a reason for hiding this comment

mheiber Dec 7, 2018 • edited Loading

Choose a reason for hiding this comment

mheiber Dec 10, 2018 • edited Loading

Choose a reason for hiding this comment

mheiber Dec 17, 2018

Choose a reason for hiding this comment

mheiber Dec 10, 2018

Choose a reason for hiding this comment

Neuroboy23 left a comment

Choose a reason for hiding this comment

Neuroboy23 Dec 17, 2018

Choose a reason for hiding this comment

mheiber left a comment

Choose a reason for hiding this comment

joeywatts commented Nov 21, 2018 •

edited

Loading

mheiber Dec 7, 2018 •

edited

Loading

mheiber Dec 10, 2018 •

edited

Loading