Why are class decorators applied before static fields?

[Jamesernator]

In the proposal it is mentioned:

The class decorator is called only after all method and field decorators are called and applied.

Finally, static fields are executed and applied.

However there doesn't seem to be any justification for this bizarre decision despite it locking out specific decorators (such as @frozen as is mentioned in the README).

With the other decorators, the value they passed is always complete, this seems an arbitrary asymmetry with class decorators to get a partially complete object.

This is even contrary to another part of the README which suggests @defineElement could be desugared to:

class MyClass extends HTMLElement { }
MyClass = defineElement('my-class')(MyClass, {kind: "class"});

However if MyClass has static fields, then any desugaring would have to become:

@defineElement('my-class')
class MyClass extends HTMLElement {
  static foo = "bar";
}

// DESUGARS TO

class MyClass extends HTMLElement {}
MyClass = defineElement('my-class')(MyClass, {kind:"class"})
Object.defineProperty(MyClass, "foo", { value: "bar", enumerable: true, writable: true, configurable: true });

Which means any transpiler implementation of class decorators also needs to process all static fields.

I would propose that it would be simpler for class decorators to simply run last, once all static methods and fields are processed.

[Lodin]

Agree. Calling static decorators after the class one may also ruin the system of metadata if a user or a decorator system developer (as me with my framework) considers using static members as a part of the decorator system. IMO, class decorator is the final call; it should finalize all the work with decorators.

[justinfagnani]

I tend to agree that static fields would be useful from within decorators.

this bizarre decision

I don't think this language is helpful though. I'm sure there's either a reason or an oversight, and Dan has been nothing but extremely accommodating to the many competing and often conflicting in this conversation.

[Lodin]

It doesn't look like an oversight, so, probably, there is some reason we miss.

And Dan is great 😁

[senocular]

Assuming there's justification for the current ordering, would @init support for class decorators be helpful in this case? The idea here being class decorators could supply an initialize that would run after the static fields have been applied. While maybe not helpful for the transpiler case, it would give decorators the chance to access static fields if needed (e.g. @init: frozen).

[littledan]

This decision was really made long ago, in the May 2016 TC39 meeting in Munich, where the committee approved @bterlson 's and @wycats 's proposal for the ordering of execution of all class elements, including field evaluation and decorators. In this proposal, static fields can reference the class (for example, to instantiate it). This means that the class is fully executed before static fields run, in order for the constructor to no longer be in TDZ, and then the static fields are added on top. Part of executing the class is running the class decorators. Since the class decorator may replace the class, but the inner binding of the class within the class body is const, it would not be possible to delay executing class decorators until after static fields, as this would require us to mutate an immutable binding. I believe TypeScript takes this path out--of switching to a mutable binding for decorated classes (@rbuckton can confirm)--but I doubt TC39 would go for such an inconsistency unless we had a very strong reason for it. Static public fields with access to the constructor in the scope of the initializer (and immutable inner class bindings) are already shipping in several JS implentations--including multiple web browsers--so I don't really see this aspect as open for change.

[Jamesernator]

--so I don't really see this aspect as open for change.

Note that the order is only contingent on the inner declaration being the decorated class, which itself would break private static methods e.g. this would fail if Foo is the decorated class:

function dec() {
    return klass => class extends klass {}
}

@dec
class Foo {
    static #bar() { return 12 }

    static bar() { return Foo.#bar() }
}

Foo.bar();

It'd be safest if the inner binding were simply the undecorated class.

[littledan]

You're right that making the inner binding be the undecorated class could solve this problem in a different way while maintaining consistency with static field initializer and immutable inner class binding semantics, but I think it would create many others. For example, say the decorator returns a subclass of the declared class which adds some behavior. Now, this behavior will not be accessible from the inner binding, contradicting expectations. I believe that most JS developers don't even know that there are these multiple bindings, so it could be especially confusing to debug.

EDIT: See @wycats 's much more readable example of this issue below

[Jamesernator]

For example, say the decorator returns a subclass of the declared class which adds some behavior. Now, this behavior will not be accessible from the inner binding, contradicting expectations.

Both ways around have behaviour contradicting expections. Like the example I gave above will not work as Foo does not have the original class's #bar private field.

Alternatively if all static private methods/fields are installed on the new decorated class then this means all private methods/fields must be in TDZ during decoration so a simple refactoring that adds a private field will break if the decorator tries to read it.

For example suppose we have a class like this:

// Adds attributes onto the class
function addReflectedAttributes() {
    return klass => {
        for (const attribute of klass.reflectedAttributes ?? []) {
            Object.defineProperty(klass.prototype, attribute.name, {
                set(value) { this.setAttribute(attribute, attribute.serialize(value)); }
                get() { return attribute.deserialize(this.getAttribute(attribute.name)) },
            });
        }
        return klass;
    }
}

@addReflectedAttributes
class MyElement extends HTMLElement {
    static get reflectedAttributes() {
        return [
            { name: 'my-attribute', serialize: String, deserialize: Number },
        ];
    }
}

Then addReflectedAttributes works as expected and adds the getters/setters for attribute reflection.

BUT if we change it slightly to use a private field:

@addReflectedAttributes
class MyElement extends HTMLElement {
    static #deserialize(value) {
        if (value.match(/^[0-9]+$/)) return Number(value);
        else if (value === 'never') return -Infinity;
        else return undefined;
    }

    static get reflectedAttributes() {
        return [
            { name: 'my-attribute', serialize: String, deserialize: MyElement.#deserialize },
        ];
    }
}

Then it breaks, because MyElement.#deserialize has to be in TDZ because the class has not been fully evaluated.

Personally I think an evaluation model where the ability to refactor to using private fields is broken by the presence of a decorator means the decorator model is very unexpected and effectively broken. Again the solution is just to completely evaluate the class and pass that to the decorator.

[Jamesernator]

Regardless of the evaluation model, I think it would be important to expose both the original class and decorated class to the body somehow. Probably as metaproperties:

function decorate() {
    return klass => class extends klass { static boz() { return 12; } }
}

@decorate
class Foo {
    static #baz() {
        return 12;
    }
    
    static bar() {
        // class.original is the undecorated class
        return class.original.#baz();
    }

    static qux() {
        // class.decorated is the decorated class
        return class.decorated.boz();
    }
}

[jridgewell]

For example, say the decorator returns a subclass of the declared class which adds some behavior. Now, this behavior will not be accessible from the inner binding, contradicting expectations.

This seems like that class wasn't actually written to be paired with the decorator, then. If the intention was to actually add additional behavior via a subclass, then I shouldn't be directly referring to my original class binding at all. Base classes that are written to allow subclassing behavior usually use this indirection.

class Base {
  static subclassOverridable() {
    console.log('Base');
  }

  test() {
    // This is broken. I directly referred to Base, not the current subclass.
    Base.subclassOverridable();

    // Instead, indirect though `this`
    this.constructor.subclassOverridable();
    // Or Class Access Expression
    class.subclassOverridable();
  }
}

class Sub extends Base {
  static subclassOverridable() {
    console.log('Sub');
  }
}

I don't get why a class decorator would have super powers to change the lexical binding. The Base inside Base.test() should be the original Base class. Allowing a decorator to change this when there's no obvious assignment seems magical.

And I really don't understand why we'd allow a static field's Base reference to change. If you want subclasses to be able to change the behavior of a prop-like value, you should be using a getter accessor so that it can be late evaluated. Static fields initializers should be running directly after the closing } in the class, not after a class decorator has changed the binding.

[hax]

It seems follow-on function decorators also have similar issue:

@deco function f() {
  f // <-- original f or decorated f?
}

[littledan]

I have stated reasons for disagreeing with the value judgements expressed above by @jridgewell and @Jamesernator , but I believe this subject deserves further discussion. I think this is a question that we can iterate on within Stage 2. Maybe we can discuss it at some of the biweekly calls. We have articulated two alternatives here (let the inner binding and outer binding disagree and run class decorators after static field initializers, or the current proposal where class decorators run before static fields are added). I agree with @hax that the choice here has implications for function decorators about what the inner binding points to and whether it is in TDZ while the function decorator is executing.

There is a third alternative, of letting class decorators only perform side effects, and not replace the class, but I suspect that this would be too limiting. Or even a fourth--remove the inner class binding if the class is decorated--but I also see this as too limiting (but maybe consistent with @jridgewell 's analysis).

[littledan]

And a fifth alternative is to punt on class decorators and leave them for a follow-on proposal, focusing on just field and method decorators initially.

[jsg2021]

If decorators are just functions, let's treat them like functions ... they run when the thing they decorate is evaluated? So

@deco class {}

is (effectively, for simply making the point)

deco(class {})

I'd expect functions/all to follow similarly. They apply just as functions would when called.

[littledan]

@jsg2021 Well, if @deco class C { } behaved exactly like deco(class C { }), then it would no longer define the binding C in the enclosing lexical scope :) I think this desugaring is to be understood at a slightly higher level.

[wycats]

When I originally proposed the current ordering in 2016, I was fairly motivated by examples like this one.

@someDec
class DateTime {
  static EPOCH = new DateTime();
}

DateTime.EPOCH instanceof DateTime; // the current status quo makes this true
DateTime.EPOCH.constructor === DateTime; // the current status quo makes this true

If we made the inner binding and the outer binding different, it would break these expected equivalences not only in theory but also pretty often in practice.

[littledan]

To clear up one piece of confusion: in @Jamesernator 's example:

function dec() {
    return klass => class extends klass {}
}

@dec
class Foo {
    static #bar() { return 12 }

    static bar() { return Foo.#bar() }
}

Foo.bar();

Here, it's important to note that #bar would be added to the return value of dec, not to the original underlying Foo. This is much like how private instance methods are added to whatever super() returns, in the case of a subclass. So, this code would Just Work (TM) even with the inner binding referring to the return value of the decorator.

[Jamesernator]

What happens with the inner binding if methods are called before returning from the decorator e.g.:

function dec() {
    return klass => {
        klass.bar();
        return klass;
    }
}

@dec()
class Foo {
    static bar() {
        return Foo;
    }
}

e.g. What does Foo.bar() return/throw? A TDZ error? The original class?

---

I still have signficant concerns about evaluation order, for example the @defineElement does not simply allow instances of customElements.define to be replaced with @defineElement:

At current this works:

class MyElement extends HTMLElement {
    static observedAttributes = ['my-attribute'];

    attributeChanngedCallback() {
        // will be called whenever my-attribute changes
    }
}

customElements.define('my-element', MyElement);

However if we use the @defineElement decorator as suggested in the README for this proposal then with the proposed semantics this will fail:

@defineElement('my-element')
class MyElement extends HTMLElement {
    // This attribute is only available *after* the class decorator has run
    static observedAttributes = ['my-attribute'];
    
    attributeChangedCallback() {
        // OOPS this is never called now
    }
}

This seems like a surprising difference given @defineElement seems like it should not be significantly different from defineElement('my-element')(MyElement).

---

I honestly don't think that either evaluation model will meet most developers expectations, I imagine a lot would expect @defineElement to just work as they would probably be unaware decorators even run before field initializers. At the same time a lot of developers would probably expect the inner binding to refer to the decorated class (although some developers would probably still expect it to refer to the inner class, as I would have).

I think at the very least, it'll be necessary to have a form similar to @init: bound for the class decoration, e.g. for @defineElement:

function defineElement(name, options) {
    return klass => {
        customElements.define(klass, name, options);
    }
}

@init: defineElement('my-element')
class MyElement extends HTMLElement {
    static observedAttributes = ['my-attribute'];
    attributeChangedCallback() { /* works */ }
}

[littledan]

Ultimately, decorator authors are engaging in metaprogramming and will see some interesting edge cases of the language if they use the feature in certain ways. I think it's just inherent that, if class decorators affect the inner class binding, then the decorator will be executed before the whole class definition has completed. TC39 will never meet all of the developer intuitions about what features could be, since they are just contradictory, but we make tradeoffs anyway.

We could add these @init: class decorators to run side-effect-only code after the class definition is complete, but I don't see it as necessary for the MVP. My understanding is that custom elements, for one, take effect once we return to the event loop, so there's no need to use @init:. (see explanation)

[justinfagnani]

My understanding is that custom elements, for one, take effect once we return to the event loop, so there's no need to use @init:.

The definition of the element: it's lifecycle callbacks and static properties like observedAttribute, are read off at definition time. This does not work:

class MyElement extends HTMLElement {
  attributeChangedCallback(name, oldVal, newVal) {
    console.log('attributeChangedCallback', name);
  }
}
customElements.define('my-element', MyElement);
MyElement.observedAttributes = ['foo'];

[hax]

If we change static field to static getter, could it work?

@defineElement('my-class')
class MyElement extends HTMLElement {
    static get observedAttributes() { return ['my-attribute'] }
    attributeChangedCallback() { ... }
}

[justinfagnani]

No. The value is read off at definition time.

[littledan]

Could we put the customElements.define in a queueMicrotask? :)

I'd be curious to understand why the static getter doesn't work, but it does seem like it's important to find a good solution for static fields.

[wycats]

@justinfagnani I'm trying to understand why observedAttributes needs to be a static property at all.

It seems to me that it's effectively used as a parameter to the decorator, so why not:

@defineElement('my-element', { observedAttributes: ['my-attribute'] })
class MyElement extends HTMLElement {
    attributeChangedCallback() {
        // called now!
    }
}

[hax]

@wycats seems work, though we lost some declarative (for example can't decorate observedAttributes :)

[justinfagnani]

@wycats the current customElement directive we use works with any element definition, since it just calls customElements.define(name, cls). A different API wouldn't necessarily do that.

[wycats]

@wycats seems work, though we lost some declarative (for example can't decorate observedAttributes :)

@hax can you explain what you'd hope to achieve by decorating observedAttributes?

@wycats the current customElement directive we use works with any element definition, since it just calls customElements.define(name, cls). A different API wouldn't necessarily do that.

@justinfagnani can you show me an example of this use-case with the current decorators?

[justinfagnani]

@wycats the LitElement @customElement decorator is here: https://github.com/Polymer/lit-element/blob/master/src/lib/decorators.ts#L49

It works with any custom element class.

[littledan]

I'm not sure if we should make this change, but how would people feel if class decorators:

• Were always applied at the end, after static fields
• Always had to return undefined, not a new class, so they could only be used for metadata and side effects

That's one way to meet these goals:

• avoid a mismatch between inner and outer bindings
• maintain the inner class binding available in static field initializers
• ensure that the static fields are available to the class decorator

What sorts of use cases would these semantics fail to meet? How do we want to prioritize these use cases?

[robbiespeed]

I'm in favour of restricting class decorators to metadata and side effects.

One of my main concerns with this proposal has been that it would introduce many decorators like this:

const wrap = (klass) => class extends klass {}

Which when used in combination would produce long prototype chains, that users of the decorators may not be aware of, where as with other patterns like mixins it's much more obvious how much the prototype chain is growing.

---

Another issue is knowing the shape is consistent. It seems beneficial to avoid confusing cases where using a wrapping decorator might cause an error:

const wrapWithFunc = (klass) => () => new klass();

@wrapWithFunc
@wrap // Uncaught TypeError: klass is not a constructor 
class A {}

---

Wrapping can still be achieved by wrapping classes in function calls.

const A = wrapWithFunc(class {})

The issue being that the name property of "A" cannot not be inferred.

---

If const decorators get a follow-up proposal those could even be used in situations where wrapping is desired.

If the decorators where to also supply the variable name to the context object, then inferring the wrapped values name would also be possible.

const wrapWithFunc = (klass, { name }) => {
  return { [name]: () => new klass()  }[name]; // alternatively you could use Object.defineProperty to change the name.
}

const @wrapWithFunc A = class {} // const decorator

[ljharb]

I would assume that wrapping a class must produce something with [[Construct]], so your “returns an instance” decorator would be invalid.

[robbiespeed]

@ljharb that seems like a good restriction. Just took a look in the proposal again and saw that return value for class decorators as it's written now expects a "new" class, which I assume is a mistake since that eliminates the possibility of metadata / side effect only decorators.

That does mean the current proposal isn't compatible with decorators that @jsg2021 linked above.

[littledan]

We're at a bit of an impasse here, since there's no way to meet all of the goals stated in this thread at once.

There is no way that class decorators could replace the class, and make that available to the static field initializers, while also making the static fields already initialized and visible to the decorator before it runs.

Here's an example that illustrates the contradiction:

let props, a, b;
function dec(k) {
  props = Object.keys(k);
  a = k.m();
  b = k.f;
  return class D extends k { static x = 1; };
}
@dec class C {
  static m = () => C;
  static f = C;
}
assert([...props].toString() === "m,f");  // Both of the static fields are added before the decorator runs
assert(C.m().x === 1) // The binding inside the arrow function is to D, the result of dec running
assert(C.f.x === 1) // The binding of f is also to the result of dec
// Now, we bend space-time! These didn't get reset from when they were originally created, right?
assert(a === C.m());
assert(b === C.f);
// But how can that work, given that b *is the result of* dec??

This example shows that we have to decide between three imperfect options:

1. Replace the class before running field initializers: The proposal in README.md, which would make the inner binding always be the post-decorator value, but would leave the props array empty, and the calculation of a would be a TypeError since C.m would be undefined.
2. Replace the class after running field initializers: TypeScript partially solves this by making the binding of C inside of itself mutable, so that C.m() returns the superclass inside the decorator, and the subclass after the decorator is applied. However, this leaves us with the problem that C.f points to the wrong thing, which @wycats argued above is undesirable.
3. Prohibit replacing the class: This would rule out some important use cases, but the above shows that there is just no way to meet all the goals that have been articulated about replacing the class, so we need to choose. One thing that could partially bridge the gap is to allow decorating constructor behavior, described below. We'd still leave it prohibited to replace the class identity, however (since that leads to this completely unsolvable ordering problem, even though it's a desirable feature).

---

To allow replacing the constructor behavior (but not class identity), how about decorators for the constructor? It could look like this:

@classDecorator class C {
  @constructorDecorator constructor() { 
  }
}

classDecorator would be expected to return undefined, and throw an exception otherwise.

constructorDecorator would be called with a constructor (which is a different identity from C) whose [[Construct]] behavior would match that of the constructor (e.g., creating the public fields), but which would not have access to C to be the right new.target. constructorDecorator would be required to return a value which has a [[Construct]] trap, and may or may not have a [[Call]] trap.

Because super() calls may be made by the inner constructor which is passed to the decorator, the "home object" is already available. The home object is the class itself, but it is only used for its (mutable) [[Prototype]]--the identity of the class does not have to be exposed this way, only its prototype. @rbuckton raised concerns about the complexity of super() calls here, but I don't really understand the issue.

Constructor decorators could be used for:

• Checking/casting arguments and return values
• Logging/tracing calls to the constructor
• Call constructor behavior (! finally!) which is actually more faithful than replacing the whole class, since it just replaces this behavior

These patterns do not include replacing the class with a Proxy, as mentioned above, since that feature runs into the impossibility of meeting the various constraints that have been asserted about how class decorators that replace the class interact with static field initializers.

For replacing the constructor identity, I'd suggest continuing with the pattern of placing an assignment after the class declaration. This will make it more clear that any calculation to reset the class will affect usages outside the class, but not reset static field declarations inside the class. People often cite React-Redux @connect, but official documentation has long discouraged its use as a decorator anyway, and instead encouraged this pattern.

[robbiespeed]

@littledan I really like the idea of trap style constructorDecorator, although I think the decorator should be supplied both the trap constructor function (different identity from C), and a target which has the identity of C. That way you could do a frozen style decorator:

const freezableCtors = new WeakSet();

const frozen = ({ constructor, target }) => {
  freezableCtors.add(target);

  return {
    constructor () {
      if (!freezableCtors.has(new.target)) {
        throw new Error(`Subclass ${new.target.name} must be also be @frozen`);
      }
      constructor.apply(this, arguments);

      if (new.target === target) {
        Object.freeze(this);
      }
    }
  };
}

Modified example from @rbuckton's comment

[jsg2021]

React-Redux @connect, but official documentation has long discouraged its use as a decorator anyway, and instead encouraged this pattern.

I thought the discouragement was more because of the uncertainty of decorators. I admit, with the direction away from classes, decorators in React do not feel like a highly sought feature.

I still think that decorators should still have the same timing/order as a regular function. ie: in @foo class {}, the class is fully evaluated before given to the decorator. Doesn't that simplify things?

[littledan]

@jsg2021

I thought the discouragement was more because of the uncertainty of decorators. I admit, with the direction away from classes, decorators in React do not feel like a highly sought feature.

I don't disagree, but I'm arguing that the experience shows, it's viable to ask developers to change the class binding in a line after the class declaration.

I still think that decorators should still have the same timing/order as a regular function. ie: in @foo class {}, the class is fully evaluated before given to the decorator. Doesn't that simplify things?

This would mean that the inner bindings inside the class point to the class before it is decorated. In #329 (comment) , @wycats gave an example of the unintuitive results this would cause. I think these results are more unintuitive when decorator syntax is used than when it's an assignment after the class declaration.

[littledan]

@robbiespeed I guess this decorator would work if you made sure to decorate each subclass. Two errors:

• Use Reflect.construct instead of Function.prototype.apply.
• Return a function or class, instead of an object with a non-constructable property.

[robbiespeed]

@littledan that makes sense, I wasn't familiar with Reflect.construct.
So this would be a proper example then?

const freezableCtors = new WeakSet();

const frozen = ({ constructor, target }) => {
  freezableCtors.add(target);

  return function FrozenTrap () {
    if (!freezableCtors.has(new.target)) {
      throw new Error(`Subclass ${new.target.name} must be also be @frozen`);
    }

    const instance = Reflect.construct(constructor, arguments, new.target);

    if (new.target === target) {
      Object.freeze(instance);
    }

    return instance;
  }
};

const addY = ({ constructor }) => class AddZTrap extends constructor {
  constructor () {
    super(arguments);
    this.y = 2
  }
};

class A {
  @addY
  @frozen
  constructor () {
    this.x = 1;
  }
  static foo () {
    return 'foo';
  }
}

Which would roughly desugar the class too:

class __A {
  constructor () {
    this.x = 1;
  }
}

function A () {
  if (!new.target) {
    throw new TypeError(`Class constructor ${A.name} cannot be invoked without 'new'`)
  }
  return Reflect.construct(__A_frozen, arguments, new.target);
}
Object.defineProperty(A, 'prototype', Object.getOwnPropertyDescriptor(__A, 'prototype'));
A.prototype.constructor = A;
Object.assign(A, {
  foo () {
    return 'foo';
  }
});

const __A_addY = addY({ constructor: __A, target: A }, { kind: 'constructor' });
const __A_frozen = frozen({ constructor: __A_addY, target: A }, { kind: 'constructor' });

You mentioned having a optional call trap as well to allow for call constructor behaviour, how would you supply that if the return of the decorator is a function or class?

[bergus]

To allow replacing the constructor behavior (but not class identity), how about decorators for the constructor?

Was it forbidden to place decorators on constructor in the current proposal? It's not mentioned in the readme, and I can't see anything that would prohibit this.
Btw I guess #48 is no longer relevant?

[wycats]

@bergus it's come up before and I really like the idea of using constructor decorators to sidestep a number of issues that have come up about class decorators.

[Jamesernator]

I really like the idea of constructor decorators to be able to wrap constructor behaviour without replacing the whole binding.

I wonder if for symmetry purposes it would be a good idea to instead of allowing decorating the class (and requiring the decorator to return undefined), that instead we allow decorating the static block (assuming that proposal succeeds).

Decorating the class binding could still come later if there's determined to be a strong need even with static block decorators.

[Jamesernator]

@littledan Are constructor decorators still gonna be added to the proposal?

The new addition of class init decorators alleviates some of the cases (e.g. customElements.define), but the other use cases you mentioned (Checking/casting arguments and return values, Logging/tracing calls to the constructor, call constructor behaviour) are still awkward with class decorators as not only does it add a new subclass into the chain per decorator, but you still need to modify otherwise good function-type decorators (e.g. ones that do simple method decoration should probably still work when applied to a constructor, i.e. @logged, @cached, @beforeCall, @afterCall, @freezeReturn, etc) to actually do that subclassing thing.

[pzuraq]

The current thinking was that allowing the @init: syntax to work with classes would address some of the more common use cases that exist currently, while allowing standard class decorators to maintain behavior that is similar to what exists in the ecosystem today. This also still leaves the possibility of adding constructor decorators in the future, but at the moment we're trying not to let the scope creep to additional decorator types.

[pzuraq]

This is addressed by the ability to addInitializer in the latest proposal, so I'm going to close this issue.