Get Off My Lawn
Get Off My Lawn

Reputation: 36299

Setter/Getter not running on second decorator of property

I have 2 decorators, and when I attach them to a property, only the first one defined executes it's setter/getter properties. The inner function itself runs the Init Decorator 1 and Init Decorator 2. What is causing the second decorator to not execute the setter/getter?

Here is how I defined my two decorators:

export function Decorator1(): any {
  return (target: any, propertyKey: string, descriptor: PropertyDescriptor) => {
    descriptor = descriptor || {};
    console.log('Init Decorator 1');
    descriptor.get = function (this: any) { console.log('Get Decorator 1'); }
    descriptor.set = function (this: any) { console.log('Set Decorator 1'); }
    return descriptor;
  }
}
export function Decorator2(): any {
  return (target: any, propertyKey: string, descriptor: PropertyDescriptor) => {
    descriptor = descriptor || {};
    console.log('Init Decorator 2');
    descriptor.get = function (this: any) { console.log('Get Decorator 2'); }
    descriptor.set = function (this: any) { console.log('Set Decorator 2'); }
    return descriptor;
  }
}

I am then using the decorators as follows:

export class Test {
  @Decorator1()
  @Decorator2()
  code = '';

  constructor() {
    setTimeout(() => this.code = '123', 2000);
  }
}

new Test();

Playground Example

[LOG]: "Init Decorator 2" 
[LOG]: "Init Decorator 1" 
[LOG]: "Set Decorator 1" 
[LOG]: "Set Decorator 1" 

Upvotes: 0

Views: 400

Answers (1)

jcalz
jcalz

Reputation: 327774

According to the TypeScript documentation for accessor decorators, when such a decorator returns a value, it replaces the property descriptor for the relevant property. And when you compose decorators, they run bottom-to-top.

Thus, in your example code

@Decorator1()
@Decorator2()
code = '';

you are first taking the initially undefined property descriptor and replacing it with your Decorator2 descriptor. Then you are taking that Decorator2 descriptor, and modifying it by reassigning its get and set properties. Reassigning properties does not preserve the prior property in any way. The set() method containing "Set Decorator 2" has been thrown away. Oops.


It's up to you how you want to compose property descriptors. Presumably you should analyze the descriptor value passed in to your decorator and preserve its get and set properties however you see fit. For example:

return (target: any, propertyKey: string, descriptor: PropertyDescriptor) => {
  descriptor = descriptor || {};
  const prevGet = descriptor.get;
  const prevSet = descriptor.set;
  console.log('Init Decorator N');
  descriptor.get = function (this: any) {
    console.log('Get Decorator N');
    if (prevGet) prevGet.call(this);
  }
  descriptor.set = function (this: any, v) {
    console.log('Set Decorator N');
    if (prevSet) prevSet.call(this, v);
  }
  return descriptor;
}

Here we hold onto the previous values of get and set and, if they are defined, calling them inside the new get and set methods. Note that I'm not worrying about what the previous methods return, and, especially for get you probably would. And it's up to you what order to run them in; here I run the new method logic before the existing one. The important bit here is that you need to explicitly care about the prior descriptor state if you which to preserve it somehow.

Using this approach gives the following results:

[LOG]: "Init Decorator 2" // Decorator2 runs first
[LOG]: "Init Decorator 1" // Then Decorator1, 
[LOG]: "Set Decorator 1" // Decorator1's logic runs first 
[LOG]: "Set Decorator 2" // Then Decorator2's logic is run inside of Decorator1's set
[LOG]: "Set Decorator 1" // ditto
[LOG]: "Set Decorator 2" // ditto

Playground link to code

Upvotes: 1

Related Questions