babel/babylon

Class Fields (combined) - Stage 2

hzoo opened this issue · 11 comments

hzoo commented

Updates on class-field proposals (both public and private)

Champions: @jeffmo (public class fields) @littledan (private + combined)
Spec Repo: https://github.com/littledan/proposal-class-fields/
Spec Text: https://littledan.github.io/proposal-class-fields/
Slides: https://drive.google.com/file/d/0B-TAClBGyqSxWHpyYmg2UnRHc28/view

Remember that proposals are just that; subject to change until Stage 4!

Examples

class C {
  static x = 1, #y, [a];
  z, #w = 2, [b];
  
  a() {
    this.z;
    this.#w++;
    #w; // #w is this.#w
  }
}

Parsing/ESTree

  • AST handled already in current classProperties and classPrivateProperties.
  • May need to rename though (new plugin name).
  • make sure no "private computed" parse
  • need to support comma separated properties.

Transform

  • combine the class-properties plugin + tbd private properties one

Contacts

Since the code for classBody is a bit messy, as part of the "unification" I want to clean all that up.

Moreover, for internal renaming I would love to have feedback on the following (mostly implementation details):

  1. Renaming classProperty and classPrivateProperty to: classField and classPrivateField
    • Same for the internal AST nodes (used in the plugins).
  2. Potentially unifying all to have just one plugin for both cases maybe classFields
  3. Revisit wether the property name inside is a string or an Identifier.
    • Transformer plugins look very odd since you will have to path.skip() to avoid the internal identifier, just look odd.

@hzoo Will try to work on all of this over the weekend.
//cc @loganfsmyth 3) is just for you :) here is an example on why I want to change it:

function privateStatePlugin () {
  return {
    visitor: {
      Program() {
      },
      ClassPrivateProperty(path) {
        // We want to rename this one
      },
      PrivateName(path) {
         // IMHO most of the times, you want to skip here
         // path.node.name == Identifier
         // otherwise you will get the Identifier several times, sometimes unexpectedly
         path.skip();       
      },
      Identifier(path) {
        //console.log('\n>>>>> ID ', path.node); // Nothing new here.
      }
    },
  };
}

Note that Waldemar Horwat pointed out an ambiguity in the grammar, that because we can have static fields named static, set or get, it takes some degree of lookahead to tell whether something is a field declaration or a method definition. From some simple tests, it looks like Babel already handles this sort of case correctly already; I just wanted to mention that this is an area that is likely to change.

@littledan not sure I'm following, what is likely to change?

Well, I'm not sure exactly, I have sent an email to Waldemar to clarify. Now that I look at it some more, I'm not sure what the ambiguity is.

I heard back from Waldemar; it seems the issue was just a typo in the proposal, where it rendered wrong. But declarations end in a semicolon (and ASI fixes it). This was a recently introduced error in the spec formatting, so no need to hold anything up!

tc39/proposal-class-public-fields#63

@littledan Is this the answer to above question?

and what about props and methods named like in/instanceof etc

I will try to give it a run over the weekend.

@bounceme in and instanceof are allowed as both public and private field names, as any IdentifierName can be used as a public or private field name (though if you use get, set or static, you may need to place more semicolons than you would otherwise).

@diervo Still catching up on things, but for your comment about Identifier, I wouldn't recommend ever traversing for Identifier on its own, without checking in what context it is used, so you'd either just traverse for PrivateName or traverse for Identifier and ensure that its parent node is PrivateName.

@diervo To add to @loganfsmyth's comment, you still have to be aware of context even now, because of existing MemberExpressions (where computed: false) and the like.

This issue has been moved to babel/babel#6692.