Experimental decorators do not get access to the initial value when using auto-accessors

🔎 Search Terms

In order to give a good out-of-the-box experience with our library that vends decorators, we are trying to make a single set of decorators that works under the following configurations:

Experimental decorators on class fields (with useDefineForClassFields: false)
Standard decorators on auto-accessors
Experimental decorators on auto-accessors

Case (3) may seem unusual at first, but it's very important for incremental migration from experimental decorators to standard decorators. The goal is to enable experimental decorators to be callsite compatible with standard decorators so that callsites and files can be upgraded one-at-a-time without changing the tsconfig, and after the callsites are updated experimentalDecorators can be turned off.

This mostly works except that in case (3) the decorators cannot get access to the initial field value because it's set directly to the auto-accessor backing store and skips the setter. In case (1) the initial value goes through the setter, in case (2) it goes through the init() callback, but in case (3) we can't see it at all.

Case (3) is actually important for multiple reasons:

Incremental migration as mentioned
It enables one version of decorator docs (aside from talking about configs) and decorator-using code samples through our docs and example repositories.
It enables projects to start with or migrate to standard decorators and change back to experimental decorators in case the size penalty of standard decorators is too much (we're seeing a ~50% code size penalty on porting a library to standard decorators above the size with experimental decorators) by just changing the tsconfig.

🕗 Version & Regression Information

This is the behavior in every version I tried, and I reviewed the FAQ for entries about decorators

⏯ Playground Link

https://www.typescriptlang.org/play?experimentalDecorators=true&target=9#code/MYewdgzgLgBFCm0YF4YAoBQMYAcBOIUIA8ngCKLB4CWOReAXDMQEYBW8wUANFjGAEMAtvCYAFAjnh4oATwDS8Wb2wATSjTohGMCSCkzZFCFVr0MASiYCwslAD4YAbz554UAK54wzvtgDm7mgWvthhMKCQIAA28AB00SD+aADkgVApFgDcfmFunt4w6iaa9HHpAIRxwALR0WhQABbUENm5AL4qYRBBAG5MHmAA1mAgAO5gIS7h2JEQMfGJySk9GdwwvW0zRRpm2nGrVTV1Dc0Q65s54e18NzcYwNECEBAwAIKhMAACCEgCwMBEPM8DAAGYgEAoGApFgCPApHL3AD0SJgAFEAB5SLjwVQRcDzWJMABEq2J62JsLwxIeBNgAihYHgY3ewRyGBRMCWEBJZIpsIAXjSBHFwZDUDCBAKERggA

💻 Code

const test = (
  protoOrDescriptor: Object,
  name: PropertyKey,
  descriptor: PropertyDescriptor
): any => {
  return {
    get() {
      console.log('get');
      return descriptor.get!.call(this);
    },
    set(v: unknown) {
      console.log('set', v);
      descriptor.set!.call(this, v);
    }
  }
}

class A {
  @test accessor foo = 'bar';
}

// Expected console: "set", "bar"
const a = new A();

// logs: "set", "baz"
a.foo = 'baz';

🙁 Actual behavior

The setter isn't called, since the output initializes the private storage directly:

class A {
    #foo_accessor_storage = 'bar';
    get foo() { return this.#foo_accessor_storage; }
    set foo(value) { this.#foo_accessor_storage = value; }
}

🙂 Expected behavior

The setter is called. Initializers for decorated accessors under experimental decorators are called as statements in the constructor:

class A {
    #foo_accessor_storage;
    get foo() { return this.#foo_accessor_storage; }
    set foo(value) { this.#foo_accessor_storage = value; }
    constructor() {
      this.foo = 'bar';
    }
}

Additional information about the issue

No response

@rbuckton can you comment on the expected behavior for this?

If the new decorators are being designed to be callsite-compatible, why is there a need to support both at the same time? Can't users just update a version number (if not also an import path) and have it just work?

I think had Lit chosen useDefineForClassFields: true from the beginning, it would be in the best shape for a version update to simply work.

Elements already have @element on them, so you can drop accessor and do this (which I've been doing and have not had performance issues):

In other words, if you aim to make it work like the following on Legacy Decorators without accessing the prototype in the decorators and with useDefineForClassFields: true, like this,

@element('my-el')
class MyEl extends LitElement {
  @property() foo = 123
  @property() bar = 123
  @property() baz = 123
  @property() lorem = 123
  @property() ipsum = 123
}

instead of this,

@element('my-el')
class MyEl extends LitElement {
  @property() accessor foo = 123
  @property() accessor bar = 123
  @property() accessor baz = 123
  @property() accessor lorem = 123
  @property() accessor ipsum = 123
}

then when Lit updates to Standard Decorators, Lit users will be able to updgrade without any problems at all (after they fix issues they might unfortunately encounter due to useDefineForClassFields: false), and they'll enjoy writing less-verbose properties without accessor.

Without accessor, the @element decorator would need to convert from own property to getter/setter. Some ways to do it:

subclass in @element, swap descriptors in constructor
place something in the lexical scope of the @property initializers for when they run later.
decorators metada proposal will make this easier later

The tradeoff is it won't work without @element, but I haven't worked in a Lit code base that didn't use both @property and @element.

If the new decorators are being designed to be callsite-compatible, why is there a need to support both at the same time? Can't users just update a version number (if not also an import path) and have it just work?

No, because the abilities are different. Experimental decorators on auto-accessors can't see the initial value and so can't run them through the logic that standard decorators on accessors can and experimental decorators on "fields" can.

I think had Lit chosen useDefineForClassFields: true from the beginning, it would be in the best shape for a version update to simply work.

Not only did useDefineForClassFields: true not exist when Lit was first built, it's not possible to make decorators to work with spec-compliant fields in an acceptable way - this is the whole reason for auto-accessors to exist in the first place.

I don't think it's useful or relevant to litigate Lit's designs decisions that were made before the design of standard decorators because the issue I raised here applies to decorator usage today without any prior decisions.

I know you don't like accessor, but I'm not interested in the performance penalty of deleting instance properties of classes to keep class fields from shadowing accessors. You can advocate that approach to those who will listen here without bringing it into unlreated issues. accessor is the right way to use decorators and accessors, full stop.

I'm just trying to be constructive by providing suggestions (as a Lit user), and I think this is relevant.

I'm not interested in the performance penalty of deleting instance properties of classes to keep class fields from shadowing accessors

My suggestion was slightly different: replacing own properties with own getters/setters.

accessor is the right way to use decorators and accessors, full stop.

I accept your opinion. Some people will like to avoid accessor everywhere, and will take the negligible cost.

It's like comparing the speed of Lit vs Solid/React/Vue/etc: the difference in practice won't matter much.

I am 100% confident shipping own getters/setters will have no impact on rocket UI functionilty and performance. I've used this approach for 1.5 years already, and it's been great.

microsoft / TypeScript