Bad esnext emit for static property that references another static property when the class has a decorator

[yseymour]

Bug Report

🔎 Search Terms

decorator static usedefineforclassfields

🕗 Version & Regression Information

• In the Playground, this behaviour first appears in 3.7.5.

⏯ Playground Link

Playground link with relevant code

💻 Code

function foo(ctor: Function) {}
@foo
class C {
	static X = 1;
	static Y = C.X;
}

🙁 Actual behavior

When targeting esnext, the useDefineForClassFields option is set by default. Using this combination of options, class C is emitted as follows:

let C = C_1 = class C {
    static X = 1;
    static Y = C_1.X;
};

The name mangling to C_1 causes the assignment to Y to fail at runtime.

If the decorator is removed, the emit becomes:

class C {
    static X = 1;
    static Y = C.X;
}

which works fine.

If useDefineForClassFields is disabled, the property assignment gets moved outside the class definition, where the name mangling is correct.

🙂 Expected behavior

Probably just not performing the name mangling when emitting within the class.

I appreciate the decorator functionality is unstable, so please consider this bug report purely informational.

[rbuckton]

This is fairly tricky until decorators stabilizes. The problem is that TypeScript decorators attempt to work around the fact that class declarations are "doubly bound" (the class name is bound both in the lexical environment outside the class and inside the class). For TypeScript, we made the decision to reference the outer binding for the class for cases like this:

function decorator(target) {
  return class extends target {
    x = 1;
  };
}
@decorator
class C {
  static makeC() { return new C(); } 
}

console.log(C.makeC().x);

For some users, they might expect C.makeC() to return an instance of the decorated C, because its not obvious that decorator changes the binding for C to be a new value.

The simplest approach would be to inject a comma expression in the first static initializer that is evaluated to capture the class binding:

// source
@decorator
class C {
  static X = 1;
  static Y = C.x;
  static Z = () => new C();
  static W() { return new C(); }
}

// generated
var C_1;
let C = C_1 = class C {
  static X = (C_1 = C, 1);
  static Y = C_1.X;
  static Z = () => new C_1();
  static W() { return new C_1(); }
}
C = C_1 = __decorate(...);

Above, we inject C_1 = C into the initializer for C.X, which will capture the undecorated C. C.Y will get the value 1 since it evaluates before decorators. C.Z will close over C_1, which means that if it is called while the class body is still evaluating it will return the undecorated C, but after decorators run it will return the decorated C, just like C.W.

The current decorators proposal for ECMA262 does not try to work around the double binding. References to C inside the class body will always refer to the undecorated C, meaning that users who want the behavior of makeC, above, will need to handle this themselves:

@dec
class C {
  static X = 1;
  static Y = C.X; // accesses undecorated `C`
  makeC() { return new C(); } // returns instance of undecorated `C`
}

let D = @dec class D_ {
  static X = 1;
  static Y = D_.X; // accesses undecorated `D_`
  makeD() { return new D(); } // returns instance of decorated `D`
};
// fix `name`, if its important to you
Object.defineProperty(D, "name", { ...Object.getOwnPropertyDescriptor(D, "name"), value: "D" });

// alternatively, just use `this`:
let D = @dec class {
  static X = 1;
  static Y = this.X; // accesses undecorated `D`
  makeD() { return new D(); } // returns instance of decorated `D`
};
// no need to fix `name` because it is assigned from the variable name.