Suggestion: Allow member decorators to add both static and instance *extra* initializers.

[rbuckton]

The current proposal exposes a single addInitializer function on the context passed to a decorator. The callback provided is then invoked either as a staticExtraInitializer or an instanceExtraInitializer depending on whether the decorator is applied to a static or non-static member of a class (or a classExtraInitializer for a class decorator).

For example:

function decorator(value, context) {
  context.addInstanceInitializer(function () { console.log("instance", context.name); }); // instance initializer
  context.addStaticInitializer(function () { console.log("static", context.name); }); // static initializer

  // or, possibly:
  // context.addInitializer(function () { console.log("instance"); }, "instance"); // instance initializer
  // context.addInitializer(function () { console.log("static"); }, "static"); // static initializer
}

class C {
  @decorator
  static x() {}

  @decorator;
  y() {}
}
// prints:
//   static x
//   static y

new C();
// prints:
//   instance x
//   instance y

One benefit to this approach is that it further reduces the need for a spec-defined behavior for metadata. A method or field decorator could register a callback to run when staticExtraInitializers are run, providing access to the constructor/prototype to use as a key in a WeakMap.

While this runs fairly close to the possible prototype-manipulation shenanigans that were frequently seen in implementations of stage-1/TS/Babel decorators (and frowned upon by engine implementers), I'd argue that its much more limited:

• You cannot directly manipulate a descriptor during decorator evaluation.
• The worst offenders were those that converted a field into an accessor. However, in the current proposal fields will still be defined over accessors, making it highly unlikely this would be regularly abused for defineProperty.
• accessor provides a more convenient and better optimized approach to wrapping a "field" with additional functionality.

cc: @pzuraq, @syg, @codehag

[Jamesernator]

• making it highly unlikely this would be regularly abused for defineProperty.

I'm not so sure, I for one would eagerly be writing a exposeReadonly decorator as such:

function exposeReadonly(_privateThing, ctx) {
    assert(ctx.isPrivate, "That thing is already public");
    
    ctx.addStaticInitializer(function() {
        const target = ctx.isStatic ? this : this.prototype;
        Object.defineProperty(target, {
            enumerable: false,
            configurable: true,
            get() {
                return ctx.access.get();
            },
        });
    });
    
    return _privateThing;
}

class Lock {
    // Exposed publically as readonly isLocked
    @exposeReadonly #isLocked = false;
    
    #unlock() {
        // ...unlock steps
    }
    
    acquire() {
        if (!this.#isLocked) {
            // Internally the field is still writable
            this.#isLocked = true;
            return once(() => this.#unlock());
        }
        // add to queue for unlocking
    }
}

const lock = new Lock();

lock.isLocked === false; // publically readable
lock.isLocked = true; // ERROR, can't write

Although even if addStaticInitializer isn't added, I'll probably do as such anyway by just pairing with a class decorator.

[rbuckton]

• making it highly unlikely this would be regularly abused for defineProperty.

I'm not so sure, I for one would eagerly be writing a exposeReadonly decorator as such: [...]

That's a scenario that https://github.com/tc39/proposal-grouped-and-auto-accessors (which builds on the accessor keyword) intends to solve via a native syntactic transformation:

class Lock {
  accessor isLocked { get; #set; } = false;
  acquire() {
    if (!this.isLocked) {
      this.#isLocked = true;
      ...
    }
  }
}

[Jamesernator]

That's a scenario that https://github.com/tc39/proposal-grouped-and-auto-accessors (which builds on the accessor keyword) intends to solve via a native syntactic transformation:

The thing with this is it half defeats the point of having #isLocked private, as now acquire() is reading the public this.isLocked rather than this.#isLocked (which is a pretty good example of fragile base class).

It'd be more useful in that proposal if this worked:

accessor isLocked { get; #get; #set }

so that one could do:

class Lock {
  accessor isLocked { get; #get; #set }
  acquire() {
    // still using private getter, so this method is still
    // guaranteed to be consistent, rather than delegating to
    // own-properties/subclasses for reading but not writing #isLocked
    if (!this.#isLocked) { 
      this.#isLocked = true;
      // ...
    }
  }
}

[trusktr]

Although even if addStaticInitializer isn't added, I'll probably do as such anyway by just pairing with a class decorator.

This feature would indeed simplify things by not having us do the pair-with-class-decorator dance. I'm using that dance with current decorators to achieve something like the following without using accessor:

@reactive
class Foo {
  @signal foo = 123 // reactive "signal" using getter/setter without `accessor` feature
}

The question is: should we just make this easier, since we can do it anyway? Maybe so! In either case, as Ron mentioned, fields are still applied over getters/setters (ugh, saying "accessors" is now confusing because it has double meaning, so I'm using "getters/setters" instead), so the end result is the same.

[EisenbergEffect]

I just wanted to chime in here and see where this is at. I was reviewing the latest TS 5.0 work and lamenting that I can't actually use the new decorators until I have some way to associate metadata with a class from an accessor. This proposal interests me because it not only gives me a way to handle metadata, but it also seems to fill a whole in the spec given the existence of static initialization blocks. Is there any chance of us amending the decorators spec to add this?

[justinfagnani]

@EisenbergEffect see https://github.com/tc39/proposal-decorator-metadata/ for the metadata proposal, which gates my team's usage of them as well.

[rbuckton]

This capability may be even more relevant now that addInitializer for fields now occurs in field order after all extra initializers from methods have been invoked. While I support the change to field order, it does mean that extra initializer order is observably different for the following two declarations:

class C1 {
  @((t, c) => c.addInitializer(() => console.log("A"))
  static a() {}
  @((t, c) => c.addInitializer(() => console.log("B"))
  static b() {}
  @((t, c) => c.addInitializer(() => console.log("C"))
  static c() {}
}

class C2 {
  @((t, c) => c.addInitializer(() => console.log("A"))
  static a() {}
  @((t, c) => c.addInitializer(() => console.log("B"))
  static b = function() {};
  @((t, c) => c.addInitializer(() => console.log("C"))
  static c() {}
}

Before pzuraq/ecma262#12, this printed A B C A B C. Now it prints A B C A C B.

The prior order matched developer intuition for decorators that aren't specific to the kind of element they decorate, but didn't match developer intuition for decorators that are specific to fields. The new order merely swaps which developer intuitions are and aren't met.

While we could change "extra" initializer order to be based entirely on lexical document order (within a given placement), it may just be worthwhile to make this option completely configurable:

type AddInitializerBucket =
  | "class"             // default for class decorators
  | "static"            // default for static method decorators
  | "static-inline"     // default for static field decorators
  | "non-static"        // default for non-static method decorators
  | "non-static-inline" // default for non-static field decorators
  ;

interface DecoratorContext {
  ...
  addInitializer(callback: Function, bucket?: AddInitializerBucket): void;
}