The other day, one of my coworkers detected a strange behavior inside our application. When he added RouterLinkActive
to a link, the application stopped rendering. However when the directive was removed, the application worked correctly.
Instead of immediately reading the solution, I created a challenge inside AngularChallenges for those who want to try resolving and understanding the source of the bug first. After that, you can come back to this article to compare your solution with mine and understand what went wrong.
To better understand the problem, below is a small reproduction of the issue:
interface MenuItem {
path: string;
name: string;
}
@Component({
selector: 'app-nav',
standalone: true,
imports: [RouterLink, NgFor, RouterLinkActive],
template: `
<ng-container *ngFor="let menu of menus">
<a
[routerLink]="menu.path"
[routerLinkActive]="isSelected"
>
{{ menu.name }}
</a>
</ng-container>
`,
})
export class NavigationComponent {
@Input() menus!: MenuItem[];
}
@Component({
standalone: true,
imports: [NavigationComponent, NgIf, AsyncPipe],
template: `
<ng-container *ngIf="info$ | async as info">
<ng-container *ngIf="info !== null; else noInfo">
<app-nav [menus]="getMenu(info)" />
</ng-container>
</ng-container>
<ng-template #noInfo>
<app-nav [menus]="getMenu('')" />
</ng-template>
`,
})
export class MainNavigationComponent {
private fakeBackend = inject(FakeServiceService);
readonly info$ = this.fakeBackend.getInfoFromBackend();
getMenu(prop: string) {
return [
{ path: '/foo', name: `Foo ${prop}` },
{ path: '/bar', name: `Bar ${prop}` },
];
}
}
MainNavigationComponent
will display NavigationComponent
and pass a list of MenuItem
as an argument depending on the return of an HTTP request. When our HTTP request returns, we call the getMenu
function with an empty string if there is no info, or with info if it's not empty.
NavigationComponent
will iterate over MenuItem
and create a link for each item using RouterLink
and RouterLinkActive
for routing.
At first sight, this code seems correct, but applying RouterLinkActive
to each link breaks the rendering with no errors in the console.
What could be happening? 🤯
To better understand the issue, let's break down RouterLinkActive
with the code that is causing the infinite rendering loop:
import { Directive } from '@angular/core';
@Directive({
selector: '[fake]',
standalone: true,
})
export class FakeRouterLinkActiveDirective {
constructor(private readonly cdr: ChangeDetectorRef) {
queueMicrotask(() => {
this.cdr.markForCheck();
});
}
}
Inside RouterLinkActive
, we call this.cdr.markForCheck()
to mark our component as dirty. However, we make this function call within a different micro task. Once our current macro task ends, Angular will trigger a new change detection cycle within the following micro task.
Having this information, can you spot the issue now ?
Since Angular runs a new change detection cycle, the framework rechecks every binding, causing new function calls. This means that the getMenu
function inside our MainNavigationComponent
will be called again, and it returns a new instance of MenuItems
.
But that's not all.
NavigationComponent
iterates over the array using the NgFor
directive. As a new instance of MenuItem
is passed as an Input
to the component, NgFor
recreates its list. NgFor
destroys all DOM elements inside the list and recreates them. This causes the recreation of the RouterLinkActive
instance, leading to another round of change detection, and this will be infinite.
We can avoid this by using the trackBy
function inside the NgFor
directive. This function tracks one property on the element, and checks if that property still exists within the new array. NgFor
will only DESTROY or CREATE an element if the property no longer exists or did not exist previously. Adding the trackBy
function in our case will correct the issue of infinite re-rendering.
If you are always forgetting the trackBy
function, I invite you to read this article.
However, even if the trackBy
function resolves this error, creating a new instance of MenuItem
at each change detection cycle is bad practice.
One way of avoiding this would be to create a menuItem
class property, but this would create imperative code and lead to spaghetti code.
The best way is to take a more declarative approach. Let's see how to refactor the code in a more declarative way:
@Component({
standalone: true,
imports: [NavigationComponent, AsyncPipe],
template: ` <app-nav [menus]="(menus$ | async) ?? []" /> `,
})
export class MainNavigationComponent {
private fakeBackend = inject(FakeServiceService);
readonly menus$ = this.fakeBackend
.getInfoFromBackend()
.pipe(map((info) => this.getMenu(info ?? '')));
getMenu(prop: string) {
return [
{ path: '/foo', name: `Foo ${prop}` },
{ path: '/bar', name: `Bar ${prop}` },
];
}
}
Our menus$
property is now defined in a single place, and will update when getInfoFromBackend
returns. menu$
will not get recomputed at each change detection cycle and only a single instance will be created during the entire lifetime of MainNavigationComponent
. The code looks simpler, doesn't it ?
You have probably heard that calling functions inside templates is a bad practice, and in most cases, it is. While you can call a function to access nested properties of an object, that should be one of the only exceptions. Try to avoid calling functions inside your template bindings, or be sure to really understand what you are doing and all the side effects that can be created by this function call. When attempting to mutate data through a function call, it should trigger a warning inside your head. Most of the time, there is a better declarative approach to your problem. Declarative programming is a different state of mind, but you should aim for it. Persist, and your code will become clearer and simpler, and both your coworkers and your future self will thank you for that.
I hope this article sheds some light on the consequences of calling functions within template bindings.
Notes: In the future, Angular with "signal" will reduce that risk. With "signal" being memoized, it will save you from recreating new instances. 🔥
You can find me on Twitter or Github.Don't hesitate to reach out to me if you have any questions.
Top comments (4)
Thought this would be the usual "calling functions in the templates is bad because it affects performance" article, but this is a really good example of a problem that can emerge + solutions, great read!
thanks Armen 🙏.
Using signals most places, it's amazing