Change finishers to be normal element descriptors.

[nicolo-ribaudo]

Conceptually, finishers are like initializers but they run at a different time (and they can alter the class declaration value). However, they are declared in two different ways:

function decorator(el) {
  return {
    ...el,
    finisher(Class) {
      Class.bar = 2;
      return wrap(Class);
    },
    extras: [{
      kind: "initializer",
      placement: "static",
      initializer() {
        this.foo = 2;
      }
    }]
  };
}

I propose to declare finishers like normal element descriptors:

function decorator(el) {
  return {
    ...el,
    extras: [{
      kind: "initializer",
      placement: "static",
      initializer() {
        this.foo = 2;
      }
    }, {
      kind: "finisher",
      // placement: "static" <-- Required?
      finisher(/* Class is passed as "this" */) {
        this.bar = 2;
        return wrap(this);
      }
    }]
  };
}

[littledan]

Interesting idea. This would definitely leave a clearer path to instance finishers in the future. Any thoughts from others? Cc @rbucktpm @wycats

[wycats]

Does this remove any capabilities from the existing design?

[nicolo-ribaudo]

No it doesn't. It actually adds a feature*: it makes it possible to replace a class element with a finisher.

* More than a feature, it is a side-effect of the symmetry with the other kinds.

[rbuckton]

I'm not opposed to a "finisher" being its own thing, per se. as it is a minor semantic change to support something we already discussed as being in scope for decorators.

As far as using a finisher as a way to replace a property, I'm wondering if the return value from a member decorator couldn't just be an array of descriptors (and we could get rid of the extras property). A decorator that removes a property would just return []. A decorator that returns its descriptor could return undefined, descriptor, or [desciptor], and you could add finishers or other descriptors via [descriptor, { kind: "finisher", ... }].

OT: I'm confused. When did we add "initializer" as a thing? I recall an issue thread about it though I expressed my concern for adding the feature at this stage of the proposal as it would be a net new concept and not just an adjustment. I can't seem to find the issue now, yet I now see it as a thing in TABLE.md.

[nicolo-ribaudo]

As far as using a finisher as a way to replace a property, I'm wondering if the return value from a member decorator couldn't just be an array of descriptors (and we could get rid of the extras property). A decorator that removes a property would just return []. A decorator that returns its descriptor could return undefined, descriptor, or [desciptor], and you could add finishers or other descriptors via [descriptor, { kind: "finisher", ... }].

👍👍👍👍👍👍👍👍👍👍👍 Yes please

OT: I'm confused. When did we add "initializer" as a thing? I recall an issue thread about it though I expressed my concern for adding the feature at this stage of the proposal as it would be a net new concept and not just an adjustment. I can't seem to find the issue now, yet I now see it as a thing in TABLE.md.

There were a few reasons:

1. It wasn't possible to replace a class elements with a finisher: the class element did still need to be declared, and then deleted in the finisher. This issue makes that argument invalid.
2. Using a finisher doesn't maintain the original execution order. Consider this example:

class A {
  @useSetInsteadOfDefine x = 1;
  y = this.x;
}

If useSetInsteadOfDefine uses a finisher, y will be undefined.

[littledan]

About expressivity, wouldn't using a no-op initializer get you the same thing?

@wycats told me that he previously considered returning an array of descriptors, and rejected this option for several reasons. One issue that comes to mind for me is, if there is a subsequent decorator, what do we apply it to?

[rbuckton]

One issue that comes to mind for me is, if there is a subsequent decorator, what do we apply it to?

This is one of the reasons that stage 1 decorators never gave you the option of removing a member (including changes to the key), only changes to the property descriptor.

This is also problematic if you can return a "finisher" descriptor in the member's place. Do the remaining decorators run against the "finisher"? This seems like a footgun for users who might only be expecting a class or member descriptor.

[littledan]

One option: If you return just a finisher (or initializer), throw a TypeError if there's a subsequent decorator.

The more I think about this change, the more I like it. Does anyone have strong objections to it? Would anyone be interested in writing up a PR?

[nicolo-ribaudo]

I am ok with it, but probably we should also throw with initializers?
If someone comes up with an usecase for not throwing, we can relax this restriction in the future

[littledan]

@rbuckton We discussed initializer in #44, merged it in #165, and then I presented on it in the November 2018 TC39 meeting (slides). I believe it's essential for v1 decorators to avoid the "dead private field" idiom that several people have been advocating, which is likely to cause un-optimizable space overhead in practice.

I am thinking that we should go ahead and make this change to finishers, as proposed by @nicolo-ribaudo, before moving to Stage 3. Any concerns?

[littledan]

This was done in #199 (together with name changes).