Codelyzer: Feature request: disallow explicit calls to lifecycle hooks

[lazarljubenovic]

Disallow code like

method() {
  this.ngOnDestroy()
}
ngOnDestroy() { /* cleanup code here */}

Instead, we should write something like

cleanup() { /* cleanup code here */}
method() {
  this.cleanup()
}
ngOnDestroy() {
  this.cleanup()
}

As Rob put it, "don't call your own hooks, because one day, you'll be debugging something and you'll be really confused why hooks are firing unexpectedly/out of order/whatever".

Thought it would be a nice addition to codelyzer.

[mgechev]

I like this proposal. Lets implement it.

[mgechev]

Btw, this is not very trivial to solve in the generic case. We need to check if given object instance implements a lifecycle hook interface and later, it's method is being invoked somewhere.

However, I think a basic check, such as:

• Find all method invocations in a class.
• Look for method names reserved by lifecycle hooks.
• Report a warning if find one.

Seems good enough.

[chaosmonster]

How can I disable this rule for tests?

I can see the benefit for application code, but within specs I actually do want to call lifecycle hooks to test them.

[michaeljota]

@mgechev I second the question. How can this be disabled just for testing? (Aside for the obvious solution of turning it off in a comment)

[mgechev]

There are two options:

• Comments in the beginning of the file
• Two tslint config files - one for tests which does not include this rule and another for your app, which includes the rule and extends the original one

[michaeljota]

@mgechev Just for you to notice, that Angular CLI seems to ignore the linterOptions options setting of tslint. I know that this is also an Angular CLI problem, but the solution of having two tslint config files won't work. Thanks any way.

[mgechev]

If there's no issue for this in the Angular CLI repo, it might be worth opening.

[michaeljota]

angular/angular-cli#11951