angular: Angular compiler - custom decorators with ngOnDestroy does not work with AOT build

I’m sorry, but it’s very disappointing. If that’s how it is, we do not have the ability to take advantage of the ability of Decorators. ( And Angular it’s all about decorators )

I wrote and created things that rely on this ability and now I understand that it will not work with AOT.

I do not come to complain, I know you work very hard, but it’s just frustrating.

Thanks for the answer.

As we are moving to AOT by default in the future, this won’t go away.

Sorry, but closing as infeasible.

AOT should be smarter and always consider ngOnInit/ngOnDestroy/ngEtc…, even if the method is not explicitly defined.

Are there any plans for this? Or it will be ignored completely.

After investigating, I found the real problem, and it’s interesting.

In my custom decorator I’m trying to override the ngOnDestroy method, for example:

export function Test() {
  return function ( constructor ) {
   constructor.prototype.ngOnDestroy = function () {
     console.log("ngOnDestroy");
   };
 }
}

But -

1. When working with JIT mode, everything is working fine.

2. When working with AOT - only when I add the ngOnDestroy explicitly, Angular will call my decorated method, but in JIT mode it’s working even if I don’t define this method explicitly.

@Component({
  ...
})
@Test()
export class TodoComponent {
  // When I am in AOT mode, and I don't define this method my decorated method will not run  
  ngOnDestroy() {
  }

}

Any ideas why this is happening?

Could this at least generate an error? I had no idea AOT was causing my app to leak observables due to this.

It seems that ngc compiler fails because it does not really test decorators:

        if (depth === 0) {
          // If depth is 0 we are evaluating the top level expression that is describing element
          // decorator. In this case, it is a decorator we don't understand, such as a custom
          // non-angular decorator, and we should just ignore it.
          return IGNORE;
        }

https://github.com/angular/angular/blob/38a7e0d1c77399f3fa9db5883eb2447e41e2641a/packages/compiler/src/aot/static_reflector.ts#L407-L412

So custom non-angular decorators are ignored: and this explains why the functions in them only work if they work on objects already evaluated by the compiler.

Lack of custom decorators is one of the most annoying things in angular…

There is “jit” flag in module thought that could help but that’s not cool.

I’m working on a library called NgObservable that implements custom lifecycle decorators that are AoT cross-compatible with ViewEngine and Ivy. The tradeoff is that each component must extend the NgObservable base class to get around the AoT limitations, which in turn implements every angular lifecycle hook leading to slightly larger component factories.

It’s still in the works but I’m keen to get feedback. You can grab it on NPM.

@NetanelBasal I think that for now we have to accept things as they are: basically when you building a component of Angular, you should always implement OnInit, and OnDestroy if there are subscriptions. If you are building third-party libraries, I do not think it’s an aggravation that some hooks methods are required.

@lucato Thanks for sharing your idea. Just a suggestion: if you add Injectable or Component to your superclass, you can avoid to implement the constructor with a super call.

But the point is: since Angular has made his own compiler, and as Tobias says, they are thinking of moving to AoT by default, meaning that Angular will no longer be just a framework, but a language to all effects. Think that you can eliminate the constructor and super call adding an Angular decorator: this is really an anti-pattern. If you also think that you can not prototype a function or a property like in the case of decorators it is an anti-pattern for TypeScript and JavaScript, that is, the languages on which it is based. I disagree that it is dynamic creation because it is not based on runtime objects, but is well known at the time of compilation. Is it difficult? Of course. But tsc compiler succeeds, ngc compiler yet no.

@Cole-Beckwith-SP See here: https://github.com/angular/angular/issues/31495

Why not allow us to use comments to tell the static analysis tool to add certain hooks to classes if not already present?

Say we decorate the decorator

/// @angular-aot keep:onInit,onDestroy
function myDecorator(target, key) {
    ...
}

@nasreddineskandrani It’s closed because not implementable with ViewEngine’s structure, it can never be complete even without being closed. It’s much more possible in Ivy but still not that easy.

This is already supported in Ivy, no need for any more idea.

Life-cycle hooks are always checked in runtime in Ivy, but when using decorator it’s not even available at runtime due to evaluation timing. Can only be done in the current (new) decorator proposal, not the deprecated (TypeScript’s) one.

(By the way, also not work in JIT now if decorator order is wrong for ivy’s checking)

Hello! I also wanted to use custom decorators and came up with following solution:

// custom decorator
function JsonRpcService(url){
  return function(target){ /**/ }
}

// custom decorator applyer
function DecorateBy(decorator): { new(): any } {
  console.log('decorate hack works fine, welcome to realize many decorators');
  const resultClass = class {
    constructor() {}
  };
  const decoratedTarget = decorator(resultClass);
  return decoratedTarget ? decoratedTarget : resultClass;
}

@Injectable()
export class VedContractCheckService extends DecorateBy(JsonRpcService('/ved-outsider/jsonrpc')) {
  constructor() {
    // Single restriction - we must call super
    super();
  }
}

As we know - decorators is only functions witch called with target constructor as argument. We can trust inheritance and call the decorators manually.

Hope that solution helps you.

So we can use your custom decorator only if we add the method ngOnDestroy explicitly? Great! 👍

@kmathy If you work with AOT the ngOnDestroy method must be present, even if empty! Otherwise ng build --prod will optimize away any calls to ngOnDestroy, even if the method is added by the @AutoUnsubscribe decorator.

This would be acceptable ONLY IF JIT was decent. As it stands JIT is atrociously slow. This whole AOT is turning into a giant mess.

Since the problem appears because Angular cant find the ngOnDestroy/ngOnChanges functions in class statically one only needs to help Angular out with that. Here is how I managed to fix the problem:

export class ObserveInputsBase implements OnChanges, OnDestroy {
  ngOnChanges(changes: SimpleChanges): void {}
  ngOnDestroy(): void {}
}

@Component({
  ...
})
@Test()
export class TodoComponent extends ObserveInputsBase  {
 constructor(){
   super();
  }
}

where Test is the decorator overwriting OnDestroy/ngOnChanges and ObserveInputsBase gives Angular what needed to later overwrite the corresponding functions. Also note the needed super() call of base class in the constructor.
Hope this helps.

This is a general “problem” with AOT: It relies on being able to statically analyze things like lifecycle hooks, properties with @Input on them, constructor arguments, … As soon as there is dynamic logic that adds new methods / properties, our AOT compiler does not know about this.