Angular - *ngIf vs simple function calls in template

Question

Sorry if this has already been answered here, but I couldn't find any match for our specific scenario, so here goes!

We've had a discussion in our development team, regarding function calls in angular templates. Now as a general rule of thumb, we agree that you shouldn't do these. However, we've tried to discuss when it might be okay. Let me give you a scenario.

Let's say we have a template block that is wrapped in a ngIf, that checks for multiple parameters, like here:

<ng-template *ngIf="user && user.name && isAuthorized">
 ...
</ng-template>

Would there be a significant difference in performance compared to something like this:

Template:

<ng-template *ngIf="userCheck()">
 ...
</ng-template>

Typescript:

userCheck(): boolean {
  return this.user && this.user.name && this.isAuthorized;
}

So to summarize the question, would the last option have any significant performance cost?

We would prefer to use the 2nd approach, in situations where we need to check more than 2 conditions, but many articles online says function calls are ALWAYS bad in templates, but is it really a problem in this case?

Answer 1

I also tried to avoid functions calls in templates as much as possible, but your question inspired me to do a quick research:

I added another case with caching userCheck() results

*ngIf="isUserChecked"

...
// .ts
isUserChecked = this.userCheck()

Prepared a demo here: https://stackblitz.com/edit/angular-9qgsm9

Surprisingly it looks like there is no difference between

*ngIf="user && user.name && isAuthorized"

And

*ngIf="userCheck()"

...
// .ts
userCheck(): boolean {
  return this.user && this.user.name && this.isAuthorized;
}

And

*ngIf="isUserChecked"

...
// .ts
isUserChecked = this.userCheck()

This looks like it's valid for a simple property checking, but there definitely will be a difference if it comes to any async actions, getters that are waiting for some api for example.

Answer 2

This is a pretty opinionated answer.

The usage of functions like this, is perfectly acceptable. It will make the templates much clearer, and it does not cause any significant overhead. Like JB said before, it will set a much better base for unit testing as well.

I also think that whatever expression you have in your template, will be evaluated as a function by the change detection mechanism, so it doesn't matter if you have it in your template or in your component logic.

Just keep the logic inside the function to a minimum. If you are however wary about any performance impact such a function might have, I strongly advise you to put your ChangeDetectionStrategy to OnPush, which is considered best practice anyways. With this, the function won't be called every cycle, only when an Input changes, some event happens inside the template, etc.

_{(using etc, because I don't know the other reason anymore)}.

---

Personally, again, I think it's even nicer to use the Observables pattern, you can then use the async pipe, and only when a new value gets emitted, the template gets re-evaluated:

userIsAuthorized$ = combineLatest([
  this.user$,
  this.isAuthorized$
]).pipe(
  map(([ user, authorized ]) => !!user && !!user.name && authorized),
  shareReplay({ refCount: true, bufferSize: 1 })
);

You can then just use in the template like this:

<ng-template *ngIf="userIsAuthorized$ | async">
 ...
</ng-template>

---

Yet another option would be to use ngOnChanges, if all the dependent variables to the component are Inputs, and you have a lot of logic going on to calculate a certain template variable (which is not the case you showed):

export class UserComponent implements ngOnChanges {
  userIsAuthorized: boolean = false;

  @Input()
  user?: any;

  @Input()
  isAuthorized?: boolean;

  ngOnChanges(changes: SimpleChanges): void {
    if (changes.user || changes.isAuthorized) {
      this.userIsAuthorized = this.userCheck();
    }
  }

  userCheck(): boolean {
    return this.user && this.user.name && this.isAuthorized || false;
  }
}

Which you can use in your template like this:

<ng-template *ngIf="userIsAuthorized">
 ...
</ng-template>

Answer 3

Is not recommended for many reasons the principal:

To determine whether userCheck() needs to be re-rendered, Angular needs to execute the userCheck() expression to check if its return value has changed.

Because Angular cannot predict whether the return value of userCheck() has changed, it needs to execute the function every time change detection runs.

So if change detection runs 300 times, the function is called 300 times, even if its return value never changes.

Extended explanation and more problems https://medium.com/showpad-engineering/why-you-should-never-use-function-calls-in-angular-template-expressions-e1a50f9c0496

The problem coming when if ur component is big and attend many change events, if you component will be litle and just attend a few events should not be a problem.

Example with observables

user$;
isAuth$
userCheck$;

userCheck$ = user$.pipe(
switchMap((user) => {
    return forkJoin([of(user), isAuth$]);
 }
)
.map(([user, isAuthenticated])=>{
   if(user && user.name && isAuthenticated){
     return true;
   } else {
     return false;
   }
})
);

Then you can use it the observable with async pipe in you code.