Side effect of decorator

[Edgar-Abgaryan]

🔎 Search Terms

"decorator" "field decorator" "side effect" "decorator side effect"

🕗 Version & Regression Information

• This changed between versions 5.0 and 5.2
• 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?#code/MYewdgzgLgBAYgSwKYBsAmARJoBOBDKEHGAXhgAoo8cBzJKALhgFcw0kAzBMJNAGhigwUJAA9GMAMIo8ECIlSZsRAkUngR4gJSkAfDADeAXwBQJ4DLkwACtSTDDJmIPDQczYIRzlqNAIxMbtw0Ar4ATIFQOMGhtADMkdFgNDoGTs4wAPSZMAACUBAAtAg0YERI6c5QABYIEAB0AA44II1+pDC+fpVZOflFJWU4FRkwNXVNLY1hHeE92XkFxaXlPeMNza1xs-Hpps6bbQCEicFmB1NhJzBByecwh3HXtzQmpuaWEFK16DBiImwvrZhg40hdWn5nlEzulDldTnd0rkFOgsLhVDhYVMngjXu8hNBOh0eAB3b4IdDkADkfj8VIEVLCYXpMCpcTiVK05lcIBQSHqKBANB8XIWeEmENIJDIrHYXB4aF6MAA6gAVOAmMUS6ZSsiM5lKgDyAGlNTlxY9dSw2JxuLwlWqNUA

💻 Code

const FieldDecorator = (target: undefined, context: ClassFieldDecoratorContext) => {} // do nothing

class Parent {
 constructor(arg1: string, arg2: string, arg3: string) {
 // @ts-ignore
 this.prop1 = arg1
 // @ts-ignore
 this.prop2 = arg2
 // @ts-ignore
 this.prop3 = arg3
 }
}

class Child extends Parent {
 prop1!: string

 prop2!: string

 @FieldDecorator
 prop3!: string
}

const a = new Child('11', '22', '33')
console.log(a)
// a.prop1 === undefined // WTF
// a.prop2 === '22' // OK
// a.prop3 === undefined // WTF

🙁 Actual behavior

the decorated property and the very first property are undefined despite the fact that they are given a value in the constructor

🙂 Expected behavior

a object should be equal to the following obj object

const obj = {
 prop1: "11"
 prop2: "22"
 prop3: "33"
} 

Additional information about the issue

No response

[fatcerberus]

All three properties should be undefined because per ES spec, class fields declared in a derived class are initialized after the base class constructor runs. prop2 being "22" looks like a bug.

[Edgar-Abgaryan]

Bruce Pascoe (@fatcerberus) Are you saying that even without a decorator, all fields should be undefined? sounds ridiculous. Is this also a bug?

class Parent {
 constructor(arg1: string, arg2: string, arg3: string) {
 // @ts-ignore
 this.prop1 = arg1
 // @ts-ignore
 this.prop2 = arg2
 // @ts-ignore
 this.prop3 = arg3
 }
}

class Child extends Parent {
 prop1!: string

 prop2!: string

 prop3!: string
}

const a = new Child('11', '22', '33')
console.log(a)
// a.prop1 === '11' // bug?
// a.prop2 === '22' // bug?
// a.prop3 === '33' // bug?

https://www.typescriptlang.org/play?#code/MYGwhgzhAEAKYCcCmA7ALtA3gKGtYA9ihGggK7BoEIAUiA5gIwBc0JCAlivQDTQMAmVuy69+CegGZhpUQEosuPNAD0K6AAE0EALQd6KakiV40ACw4QAdAAcEBG42gBecUxOr1W3fsPIP5pa29jYCLm4CHmqa2noGRgEW1nYOkuEMkkoAvtg52KCQMADCFiAAJtBIAB5oqGUw8MjoingpjgCEMpzc2EptAp1ssj19IZKDIj15hMQYYOEoSADu0CUc5TQA5IyMm3ybAgJ70JuSkpty+UQQBCBIViAE9HSX0WDBDk7O3yc7m57QABGZHoAH5sG8PqEXD8DkcAcCwRD1O82mlvq5TucESDwUA

[IllusionMH]

You are using modern decorators in combination with legacy class fields "set" behavior.

Using useDefineForClassFields: true (or changing target to es2020, later doesn't work in playground, but works using tsc locally) brings modern behavior described by wisecerberus

https://www.typescriptlang.org/play?useDefineForClassFields=true#code/MYGwhgzhAEAKYCcCmA7ALtA3gKGtYA9ihGggK7BoEIAUiA5gIwBc0JCAlivQDTQMAmVuy69+CegGZhpUQEosuPNAD0K6AAE0EALQd6KakiV40ACw4QAdAAcEBG42gBecUxOr1W3fsPIP5pa29jYCLm4CHmqa2noGRgEW1nYOkuEMkkoAvtg52KCQMADCFiAAJtBIAB5oqGUw8MjoingpjgCEMpzc2EptAp1ssj19IZKDIj15hMQYYOEoSADu0CUc5TQA5IyMm3ybAgJ70JuSkpty+UQQBCBIViAE9HSX0WDBDk7O3yc7m57QABGZHoAH5sG8PqEXD8DkcAcCwRD1O82mlvq5TucESDwUA

I think it's better to use modern define semantics with modern decorators, or old set semantics with experimental (legacy) decorators, but not mix them.

[Edgar-Abgaryan]

Andrii Dieiev (@IllusionMH) thanks for clarifying. this is much more confusing than I expected. the desire to have fun with new decorators turns into torture

btw. if "mix them" is bad idea then why it is default config?

[RyanCavanaugh]

btw. if "mix them" is bad idea then why it is default config?

The previous "default" behavior was that you couldn't use decorators at all. Once decorators were standardized, you could use them without opting in. The class field behavior also relates to prior older behavior.

[clickclickpwn]

Can you try this with numbers instead of strings? Also, try this with an array as the input instead of three separate this.variables or lines and see if these outputs change to explore more.

[trusktr]

this is much more confusing than I expected. the desire to have fun with new decorators turns into torture

Tip: make sure to always have useDefineForClassFields: true, then everything works exactly like plain JavaScript, set for the future (I like to use esnext for target and module, and use testing to catch cases where I've written syntax that is too new, then adjust my hand-written code and avoid transpiling, using TS only for stripping type annotations).

Running this code in Chrome console shows the define semantics:

class Base {
 constructor() {
 this.foo = 123
 }
}

class Sub extends Base {
 foo
}

console.log(new Sub().foo) // undefined (expected)

btw. if "mix them" is bad idea then why it is default config?

This is a very good point!

TypeScript 5 enables on standard decorators while leaving useDefineForClassFields false by default (due to default target being the super old ES3 target which probably no one needs to default to these days), and this leads to people writing code with new decorators that will be prone to breakage in vanilla JavaScript (f.e. if someone copy/pastes their decorator in another plain JS project, or someone strips types from the code to use it in another plain JS project and it doesn't work).

[rbuckton]

It looks like the bug is in how we emit class fields in useDefineForClassFields: false in the presence of decorators. In #56955 we updated our emit to match the normative change to decorators in pzuraq/ecma262#12. To ensure the correct timing of decorators and initializers, we interleave field initializers with the "extra" initializers added by field decorators by injecting them into the following field's initializer.

The bug here is that we're doing this injection even when useDefineForClassFields is false. As a result, this introduces field assignments to the derived constructor that would have previously been elided.

[StanislavKharchenko]

The workaround here is to use "declare" for the class members.
What is the ETA to get fix of this?