tc39/proposal-decorators

accessor keyword vs. { get; set; }

Closed this issue ยท 21 comments

(I feel like I brought this up before but I can't find it... please feel free to dupe if so.)

@rbuckton has his own auto-accessors proposal in https://github.com/rbuckton/proposal-grouped-and-auto-accessors . It uses the syntax x { get; set; }.

This syntax has a few advantages IMO over accessor x:

  • It allows only a getter (or only a setter, if you want)
  • It allows a private setter/public getter (or the reverse, I guess) via x { get; #set; }
  • It reuses existing syntax semi-keywords instead of introducing a new one
  • It allows decorating either the accessor as a whole (@dec x { get; set; }) or only one of the accessor functions (x { get; @instrumented set; })

I'd encourage this proposal to build on that one if possible!

IIRC, Waldemar raised a concern with my auto-accessors proposal indicating that it needed a syntactic opt-in, otherwise we would be carving out an entire syntactic space (i.e., Identifier {...}) that could prevent future exploration of syntax similar to static {}. We've mostly agreed that the keyword used here will carry over to my proposal:

accessor x;

accessor x { get; set; } // same as above

accessor x {
  get() {...}
  set(v) {...}
}

With auto-accessors like accessor x { get; set; } giving you multiple decoration targets (the entire declaration, the getter, and the setter).

I much preferred prop over accessor, but there are concerns over confusing terminology (i.e., data properties vs. accessor properties, getOwnPropertyNames, etc.).

@rbuckton It's odd to me as a long-time mostly silent auditor of tc39 that an argument be made for this "syntactic opt-in" based on the idea of some potential future language feature no one can practically imagine. Static blocks are keyword prefixed not identifier prefixed. Personally I would love to see your initial proposal but perhaps it's more than just accessor information, could it not also include any field configuration?

class {
  x {
    enumerable = ...
    get() {...}
    set(v) {...}
  }
}

@matthewrobb I think it's reasonable, we can't really predict the future and it would be unfortunate to be limited by auto accessors. Static blocks are a great example, I would not have predicted they would be necessary prior to the proposal, but they're quite useful (in fact, it would have been impossible to polyfill this proposal in Babel without them). While I agree that the shorter syntax would be nicer, in practice it's something I and others have quickly gotten used to, especially with IDE support.

I think the enumerable syntax is compelling, this is something that decorators no longer solve for. If generalized, how would this look for methods, async methods, generator methods, and fields? I think it would only make sense to remove the accessor keyword if it could apply to all types of class elements and not just accessors.

@pzuraq

I think it would only make sense to remove the accessor keyword if it could apply to all types of class elements and not just accessors.

Yeah why not?

class {
  x {
    value = 1
    writable = false
  }
  
  y {
    value() {
    
    }
  }
}

@pzuraq um, are you saying decorators now can't be transpiled unless the target engine has native static blocks?

@ljharb no, Babel also transpiles static blocks, it's just that the timing semantics would have required us to modify the class fields transform to get everything to work. It would still have been possible, just significantly more annoying.

@matthewrobb my concern would be how this collides with auto-accessors. For instance, the following would be a valid combination:

class {
  x {
    value = 1;
    get;
    set;
  }
}

But would this also be valid?

class {
  x {
    value() {}
    get;
    set;
  }
}

The syntax seems to imply these would be composable in any way, but certain combinations don't really make sense.

Also, adding the accessor keyword does not prevent us from exploring this direction in the future. That syntax space would still be open, and accessor x; could become sugar for x { get; set; }

@pzuraq

The syntax seems to imply these would be composable in any way, but certain combinations don't really make sense.

It might not be useful but it would work exactly as you'd expect would it not?

Also, adding the accessor keyword does not prevent us from exploring this direction in the future.

I can get behind this point more than the others thus far for sure.

It might not be useful but it would work exactly as you'd expect would it not?

I'm not sure exactly how it would work though? Would value as a method be overwritten by get/set? Would it be order dependent? Would we just pick an arbitrary ordering, e.g. get and set always win?

I think it's extremely fair to say that the presence of get or set in a property descriptor always supersedes value semantics. Currently you can't defineProperty with both anyway. Thinking about how it desugars is the exact semantics I would expect and endorse. The syntactic form of the value in the block determines the Class element kind of the property. The presence of a non-method definition value field always implies instance own defined properties.

class {
  a { value = 1; } // instance defined
  b { value() {} } // proto defined
  c { get() {} } // proto defined
  d { value = 1; get; set; } // instance defined
}

hmm, fair. To be clear, the last one would be both proto and instance, auto-accessors are defined on the proto and the value itself is defined on the instance.

I could imagine actually pulling the assignment for an auto-accessor out as well, similar to the original proposal. Then we could have the exact same constraints as defineProperty, you can only provide value or get/set within the block itself:

class {
  a { value = 1;  } // valid
  b { value() {} } // valid
  c { get() {} } // valid
  d { get; set; } = 123; // valid

  b { value() {}; get; } // invalid
  d { value = 123; get; set; } // invalid
}

@pzuraq I think it would be BRILLIANT if the semantics of those blocks matched defineProperty exactly!

@pzuraq In fact, with your last examples we could do this!

class {
  x { enumerable = false  } = 1
}

I think this is worth considering and will raise it with the decorators group, thank you for raising the issues @matthewrobb. I'm still planning on proposing decorators in its current form with the accessor keyword for stage 3 at the end of this month, given that including accessor won't block this syntax in the future, and decorators are blocked on having the functionality that accessor provides. Ideally, we would move forward with decorators in it's current form, and continue iterating on grouped and auto-accessors afterward, but it's possible that ordering will be reversed in committee.

@pzuraq By all means do whatever is the shortest path to Stage 3 ๐Ÿ™ Thanks for your hard work!

Another thought, this could also potentially be done with another new keyword, like define or something like that:

class {
  define x { enumerable = false  } = 1;
}

I would be less happy about this syntax, but if there were pushback for the same reason as there was around auto-accessors (e.g. taking up the whole syntax space) it might be a way to compromise to get the functionality.

Another thought I am having is how the static analysis enabled by this would allow typescript to infer things it currently cannot:

class T {
  x { writable = false  }: number = 1;
}

const t = new T()
t.x = 2 // x is readonly
nmn commented

FWIW, I want to chime in with my vote against the โ€œaccessorโ€ keyword. Itโ€™s a confusing keyword that adds unnecessary new syntax. Even if itโ€™s sugar for {get; set}, Iโ€™m against the idea of adding the sugar and making the language needlessly complex.

Without accessor decorating fields is needlessly verbose.

The proposal is also already stage 3, and is unlikely to change.

I'm going to close this issue as the discussion has moved over to the grouped-and-auto-accessors proposal. As noted, the accessor keyword is going to remain part of this proposal and is unlikely to change.