Is it possible to get proper `this` inside a decorator for a method of a class extending Mixin(...)?

[dsechin]

Hi, @tannerntannern!

Thank you for this awesome library, it really helped me and my colleagues 🚀

But I have a small issue. I'm a bit confused on decorating methods in the class extending a mixin (or multiple mixins).

Suppose I have a base class (A) with two methods: methodA and methodB which use A's properties internally. I have another class B that extends A and has a decorated method. What I want to do is to call A.methodA and A.methodB inside the decorator. But I'm getting the following error error "Cannot read properties of undefined" inside the A.methodA call for some reason.

Is there a way to get the proper this (B instance with every property from A) inside the decorator?

Minimal example to reproduce the issue:

import { BehaviorSubject, Observable, of } from 'rxjs';
import { finalize } from 'rxjs/operators';
import { Mixin } from 'ts-mixer';

// I want to decorate methods that return an Observable
type ObservableMethod = (...args: unknown[]) => Observable<unknown>;
type ObservableMethodDecorator = (
  target: any,
  propertyKey: string | symbol,
  descriptor: TypedPropertyDescriptor<ObservableMethod>
) => TypedPropertyDescriptor<ObservableMethod> | void;

function LockUiUntilFinish(): ObservableMethodDecorator {
  return function (
    target: any,
    propertyKey: string,
    descriptor: TypedPropertyDescriptor<ObservableMethod>
  ): TypedPropertyDescriptor<ObservableMethod> {
    const originalFn = descriptor.value;

    descriptor.value = (...args: unknown[]): Observable<unknown> => {
      target?.lockUi?.(); // <------------ at this point target does not have the `isUiLocked$` property

      return originalFn(...args).pipe(finalize(() => target?.unlockUi?.()));
    };

    return descriptor;
  };
}

class Base {
  protected isUiLocked = false;
  public readonly isLoading$ = new BehaviorSubject<boolean>(true);

  public readonly isUiLocked$ = new BehaviorSubject<boolean>(this.isUiLocked);

  public lockUi(): void {
    console.log('lockUi');
    this.isUiLocked = true;

    this.isUiLocked$.next(this.isUiLocked);
  }

  public unlockUi(): void {
    console.log('unlockUi');
    this.isUiLocked = false;

    this.isUiLocked$.next(this.isUiLocked);
  }
}

class Foo extends Mixin(Base) {
  constructor() {
    super();
  }

  @LockUiUntilFinish()
  public test(): Observable<unknown> {
    return of(100);
  }
}

const foo = new Foo();

// I would like to get the following output:
// > lockUi
// > 100
// > unlockUi
foo.test().subscribe(console.log);

[dsechin]

A Stackblitz link to reproduce the issue: https://stackblitz.com/edit/typescript-n3pu44

[tannerntannern]

Hi @dsechin, glad the library has been useful for you. I also appreciate the effort put into the reprex. That said, would you be able to boil it down further, to the absolute minimum? If the root cause of this issue is truly how ts-mixer handles decorators and this, it shouldn't be necessary to use rxjs to reproduce the problem.

I have some ideas about what the problem could be (e.g., ts-mixer's internal use of arrow functions, which have different semantics around this than regular functions), but that could also just be a red herring -- hard to say until the extraneous details are stripped away!

Thanks in advance for your patience. My time is limited to support this library, and whenever other libraries are involved I get nervous that ts-mixer won't be the culprit and I'll waste hours chasing a non-bug. I'll take a closer look when the bug can be demonstrated without rxjs.

[dsechin]

Hi @dsechin, glad the library has been useful for you. I also appreciate the effort put into the reprex. That said, would you be able to boil it down further, to the absolute minimum? If the root cause of this issue is truly how ts-mixer handles decorators and this, it shouldn't be necessary to use rxjs to reproduce the problem.

I have some ideas about what the problem could be (e.g., ts-mixer's internal use of arrow functions, which have different semantics around this than regular functions), but that could also just be a red herring -- hard to say until the extraneous details are stripped away!

Thanks in advance for your patience. My time is limited to support this library, and whenever other libraries are involved I get nervous that ts-mixer won't be the culprit and I'll waste hours chasing a non-bug. I'll take a closer look when the bug can be demonstrated without rxjs.

Thank you for such a quick reply!

Hmmm, no error without rxjs related parts 😅 (I've updated the Stackblitz: https://stackblitz.com/edit/typescript-n3pu44)

import { Mixin } from 'ts-mixer';

const LockUiUntilFInish = () => {
  return (target: any, propertyKey: string, descriptor: PropertyDescriptor) => {
    const originalFn = descriptor.value;

    descriptor.value = (...args: unknown[]) => {
      target?.lockUi?.();

      setTimeout(() => target?.unlockUi?.());

      return originalFn(...args);
    };

    return descriptor;
  };
};

class Base {
  protected isUiLocked = false;

  public lockUi(): void {
    console.log('lockUi');
    this.isUiLocked = true;
  }

  public unlockUi(): void {
    console.log('unlockUi');
    this.isUiLocked = false;
  }
}

class Foo extends Mixin(Base) {
  constructor() {
    super();
  }

  @LockUiUntilFInish()
  public test(): Promise<unknown> {
    return Promise.resolve(100);
  }
}

const foo = new Foo();

foo.test().then(console.log); 

// Just as expected:
// > lockUi
// > 100
// > unlockUi

I guess I'll need to look for some workaround since I really need to use both ts-mixer and rxjs. Thank you for your help!

[dsechin]

@tannerntannern I'm sorry to bother you further, but I've modified my example a bit and now it shows that actually no properties of the mixin class are available inside the decorator:

(https://stackblitz.com/edit/typescript-n3pu44)

import { Mixin } from 'ts-mixer';

const LockUiUntilFInish = () => {
  return (target: any, propertyKey: string, descriptor: PropertyDescriptor) => {
    const originalFn = descriptor.value;

    descriptor.value = (...args: unknown[]) => {
      target?.lockUi?.();

      setTimeout(() => target?.unlockUi?.());

      return originalFn(...args);
    };

    return descriptor;
  };
};

class Base {
  protected isUiLocked = false;
  protected otherBooleanFlag = true;

  protected someString = '123';
  protected someArray = [];

  public lockUi(): void {
    console.log('Base.lockUi');

    this.isUiLocked = true;

    console.log(this);
    console.log(
      'Expected to see: isUiLocked, otherBooleanFlag, someString, someArray in `this`'
    );
  }

  public unlockUi(): void {
    console.log('Base.unlockUi');

    this.isUiLocked = false;

    console.log(this.isUiLocked);
  }
}

class Base2 {
  protected otherBooleanFlag = true;
  protected isUiLocked = false;

  protected someString = '234';
  protected someArray = [];

  public lockUi(): void {
    console.log('Base2.lockUi');

    this.otherBooleanFlag = true;

    console.log(this);
    console.log(
      'Expected to see: otherBooleanFlag, isUiLocked, someString, someArray in `this`'
    );
  }

  public unlockUi(): void {
    console.log('Base2.unlockUi');

    this.otherBooleanFlag = false;
  }
}

class Foo extends Mixin(Base) {
  constructor() {
    super();
  }

  @LockUiUntilFInish()
  public test(): Promise<unknown> {
    return Promise.resolve(100);
  }
}

class Bar extends Mixin(Base2) {
  constructor() {
    super();
  }

  @LockUiUntilFInish()
  public test(): Promise<unknown> {
    return Promise.resolve(100);
  }
}

console.log('\n\n[Foo]');
const foo = new Foo();
foo.test().then(console.log);

console.log('\n\n[Bar]');
const bar = new Bar();
bar.test().then(console.log);

Maybe this can be fixed by changing the decorator somehow? (e.g. wrapping it into something like @decorate)

[tannerntannern]

Hey @dsechin, the improper this handling actually appears to be in your decorator, not ts-mixer. If you check out the docs on method decorators in TypeScript, target is actually the class prototype, not this (as it appears to be used in your example).

Here's an example based on yours with a corrected LockUiUntilFinish

import { Mixin } from 'ts-mixer';

const LockUiUntilFinish = (target, key, descriptor) => {
  const originalFn = descriptor.value;
  return {
    value: function (...args: unknown[]) {
      this?.lockUi?.();
      return originalFn.apply(this, args).then((result) => {
        this?.unlockUi?.();
        return result;
      });
    },
  };
};

class Base {
  protected isUiLocked = false;
  protected otherProperty = 'hello world';

  public lockUi(): void {
    console.log('Base.lockUi, expecting `this` to contain `isUiLocked` & `otherProperty`');
    console.log(this);
    this.isUiLocked = true;
  }

  public unlockUi(): void {
    console.log('Base.unlockUi, expecting `this` to contain `isUiLocked` & `otherProperty`');
    console.log(this);
    this.isUiLocked = false;
  }
}

class Foo extends Mixin(Base) {
  @LockUiUntilFinish
  public test(): Promise<unknown> {
    return Promise.resolve(100);
  }
}

new Foo().test().then(console.log);

Closing as this still doesn't appear to be ts-mixer-related.

[dsechin]

Thank you, @tannerntannern!

A silly mistake on my end. Thank you once again for the fix and the clarification 🔥