Code Smells in Angular — Deep Dive — Part I
In one of my previous post I have talked about Code Smells in Angular [1]. This time I would like to elaborate more on this topic and provide techniques to avoid those mistakes. Specifically, I will talk about code smells that negatively influence the runtime-performance.
What are Code Smells?
Code Smells are mistakes that may affect not only the runtime- or the build-time performance, but also the maintainability of any project. To put it in a nutshell, the total count of WTFs during a code review.
In this blog post I would like to take a closer look at code smells that affect the runtime performance and elaborate more on the top 5 code smells of one of my previous posts.
1.) Default Change-Detection-Strategy instead of OnPush
2.) Binding methods in the template
3.) No trackBy in *ngFor
4.) The use of detectChanges instead of markForCheck
5.) Multiple subscriptions
1. Default Change-Detection-Strategy instead of On-Push
In general, the view must be in sync with the data-model. Therefore, each time the model changes the view has to be updated.
In Angular this is achieved via Zone.js and Change Detection. Zone.js attaches to anything that could lead to a change of the application state (setTimeout, setInterval, click, focus, mouseevent, …). This is also called Meta-Monkey Patch. Additionally, Zone.js provides an execution context. Now, for each async operation in the so called NgZone the ChangeDetection is triggered.
How does this work?
In Angular the application state is represented as component tree. Each time the model is updated, Angular detects the change and every component in the tree is checked. By default, the tree will be traversed from bottom to top and all components in the component-tree will be marked as dirty. Then Zone.js kicks in and triggers a tick, which means that all components marked as dirty will be re-rendered from top to bottom.
In order to not always re-render all the components in the component tree, Angular has introduced the Change-Detection-Strategy On-Push. With On-Push only the parent-components of the component, where the change happened, will be marked as dirty (see the violet arrows in the picture below) and re-rendered.
Summing up, if the user clicks on the shopping cart icon, Angular will be notified about the change and does not only re-render the JourneyListItem-component, but also the JourneyList- and the app-component. This is definitely a performance improvement, however we can do better. One example would be the use of RxAngular [2]. RxAngular introduces different structural directives (rxLet, rxFor, …) that will only re-render the parts of the template that is affected by the the change. The other one is called Signals in combination with On-Push [3].
<mat-list-item>
<h4 mat-line>{{journey?.header}}</h4>
<p mat-line>{{journey?.content}}</p>
<button
type="button"
mat-icon-button
(click)="addAction.emit()">
<mat-icon aria-label="Side nav toggle icon">add_shopping_cart</mat-icon>
</button>
</mat-list-item>@Component({
selector: 'journey-list-item',
templateUrl: './journey-list-item.component.html',
styleUrls: ['./journey-list-item.component.scss'],
changeDetection: ChangeDetectionStrategy.OnPush
})
export class JourneyListItemComponent {
@Input() journey: Journey;
@Output() addAction = new EventEmitter<void>();
}So, what does the OnPush-Strategy exactly mean for the example above. The re-rendering will be only triggered, if
- the input reference has changed
- an event handler is triggered (self or children)
- the change detection is triggered manually
- or the async pipe emits a new value
Therefore, please make sure to always use the Change-Detection-Strategy On-Push, instead of the default one. This can also be enforced by eslint-rules. The following code snippet shows an example of an .eslintrc.js.
require('@rushstack/eslint-patch/modern-module-resolution');
module.exports = {
extends: ['@xyz/eslint-config', 'prettier', 'plugin:prettier/recommended'],
plugins: ['change-detection-strategy', 'prettier'],
rules: {
'change-detection-strategy/on-push': 'error',
'@typescript-eslint/no-explicit-any': 'warn',
'@typescript-eslint/no-unsafe-assignment': 'warn',
'@typescript-eslint/strict-boolean-expressions': 'warn',
'@typescript-eslint/no-unsafe-member-access': 'warn',
},
...
]2. Binding methods in the template
Let’s say that we have a function called getTotalCosts() that calculates the total costs of the journey. If we use this function directly in the template, Angular will always have to call this method each time the Change Detection is triggered.
Why is that so?
Because Angular does not keep track of the return value of getTotalCosts(). It’s not a pure function. Therefore, Angular has to call this function each time the Change Detection is triggered. Of course one might add the ChangeDetection-Strategy On-Push. Then, the Change Detection will not be triggered as often as with the default one. However, still, the function will be triggered too often.
<mat-list-item>
<h4 mat-line>{{journey.titel}}</h4>
<p mat-line>{{getTotalCosts(journey.days)}}</p>
<button
type="button"
mat-icon-button
(click)="addAction.emit()">
<mat-icon aria-label="Side nav toggle icon">add_shopping_cart</mat-icon>
</button>
</mat-list-item>@Component({
selector: 'journey-list-item',
templateUrl: './journey-list-item.component.html',
styleUrls: ['./journey-list-item.component.scss']
})
export class JourneyListItemComponent {
@Input() journey: Journey;
@Output() addAction = new EventEmitter<void>();
private readonly pricePerDay = 120;
getTotalCosts(days: number): number {
return days * this.pricePerDay;
}
}In order to fix this one might use a pure pipe. Means, that the transform function will only be triggered if the input-values change.
@Pipe({name: 'totalCosts', pure: true})
export class TotalCostsPipe implements PipeTransform {
transform(days: number, pricePerDay: number): number {
return days * pricePerDay;
}
}Another solution would be the use of RxJS. The following code shows how this could be achieved.
<mat-list-item>
<p mat-line>{{totalCosts$ | async}}</p>
<button
type="button"
mat-icon-button
(click)="addAction.emit()">
<mat-icon aria-label="Side nav toggle icon">add_shopping_cart</mat-icon>
</button>
</mat-list-item> @Component({
selector: 'journey-list-item',
templateUrl: './journey-list-item.component.html',
styleUrls: ['./journey-list-item.component.scss']
})
export class JourneyListItemComponent {
@Input() journey$: Observable<Reise>;
@Output() addAction = new EventEmitter<void>();
private readonly pricePerDay = 120;
totalCosts$ = this.journey$.pipe(
map((journey) => journey.days),
distinctUntilChanged(),
map((days) => this.pricePerDay * days));
}3. No trackBy in *ngFor
Angular does not know, by default, at which point a DOM element has to be re-rendered. Therefore, each time something is added to the list below, the whole list gets re-rendered.
<div *ngFor="let journey of (journeys$ | async)">
<span>journey.titel</span>
</div>In order to get rid of this problem Angular has introduced the trackBy directive. However, keep in mind, that the function to pass must be a pure one. Means, that there are not side-effects.
<div *ngFor="let journey of (journeys$ | async); trackBy: journeyTrackBy">
<span>journey.titel</span>
</div>journeyTrackBy(index: number, journey: Journey): number {
return journey.id;
}With the trackBy directive Angular does not have to re-render the whole list, but only the journey that is added to the list.
Additionally, with the new control flow, which was introduced with Angular 17, the trackBy function was renamed to track and is not optional anymore. Moreover, in contrast to*ngFor the track expression can be defined inline.
<div>
@for (journey of (journeys$ | async); track journey.id) {
<span>{{ journey.titel }}</span>
}
</div>Imagine that one has a big list with virtual scrolling enabled. Then, the visible journeys may not always have to be re-rendered, but only the journeys that get visible during the scrolling.
4. The use of detectChanges instead of markForCheck
As described in the first code smell chapter above (see Default Change-Detection-Strategy instead of On-Push), a click on a button will trigger the dirty-marking in the component-tree. Then Zone.js kicks in and triggers a tick. This tick performs detectChanges, which will lead to the re-rendering of the affected component and it’s children (see the code below).
@Injectable()
export class ApplicationRef {
constructor() {
this._zone.onMicrotaskEmpty
.subscribe({next: () => { this._zone.run(() => { this.tick(); }) }});
}
tick(): void {
for (let view of this._views) {
view.detectChanges();
}
}
}However, one might not always want to re-render all the children. Therefore, we Angular introduced markForCheck. This method only marks the view and it’s ancestors as dirty and on the next change life cycle the component will be re-rendered.
If we have a look at the internals of Angular, specifically at the async-pipe, we see that not detectChanges, but markForCheck is called. Means, that the component is only marked as dirty without instantly triggering the re-rendering. So keep in mind to use markForCheck, or the async-pipe as often as possible.
AsyncPipe.prototype._updateLatestValue = function (async, value) {
if (async === this._obj) {
this._latestValue = value;
this._ref.markForCheck();
}
};Of course one might not always want to re-render the entire component, but only the parts of the view that changed. However, all structural directives (*ngIf, *ngFor, …) trigger detectChanges by default.
EmbeddedView.detectChanges();Therefore, RxAngular has introduced new structural directives (*rxLet, *rxIf, *rxFor), which will only re-render the parts of the component affected by the change.
@Component({
selector: 'app-child',
template: `
<span *rxLet="value$"; let value>
{{value.type}}
</span>`
})
export class ChildComponent { }Angular will also provide a solution to this problem. However, not based on RxJs, like it’s the case with RxAngular, but on Signals. In future, we will get signal-based components, which will do the job. In the meantime, Signals in combination with the Change-Detection-Strategy On-Push will already provide a local change detection [3].
5. Multiple subscriptions
Another code smell that I have encountered so far are multiple subscriptions, which may lead to unwanted side-effects. The following code snippet shows such an example. The service-call for retrieving the journey is called multiple times.
@Component({
selector: 'journey-list-item',
templateUrl: './journey-list-item.component.html',
styleUrls: ['./journey-list-item.component.scss'],
})
export class JourneyListItemComponent {
@Input()
set journeyId(value: number) {
this.journeyId$.next(value);
}
@Output() addAction = new EventEmitter<void>();
private journeyId$ = new BehaviorSubject<Journey | undefined>(undefined);
journey$ = this.journeyId$.pipe(
switchMap((id) => this.journeyService.getJourney(id))
);
}<h1>{{(journey$ | async).titel}}</h1>
<div *ngIf="let journey of (journey$ | async)">
<span>journey?.description</span>
</div>How can we fix this?
One way would be to introduce distinctUntilChanged in combination with shareReplay. Another solution would be to remove the multiple subscriptions in the template.
@Component({
selector: 'journey-list-item',
templateUrl: './journey-list-item.component.html',
styleUrls: ['./journey-list-item.component.scss'],
})
export class JourneyListItemComponent implements OnDestroy {
@Input()
set journeyId(value: number) {
this.journeyId$.next(value);
}
@Output() addAction = new EventEmitter<void>();
private readonly journeyId$ = new BehaviorSubject<Journey | undefined>(undefined);
journey$ = this.journeyId$.pipe(
distinctUntilChanged(),
switchMap((id) => this.journeyService.getJourney(id)),
shareReplay(1)
);
constructor(private readonly journeyService: ReiseService) {
}
ngOnDestroy() {
this.destroy$.next();
this.destroy$.complete();
}
...
}In addition, as long as all subscriptions are handled via the async-pipe there is no need to unsubscribe from observables. However, if there is a manual subscription in the component takeUntil in combination with a subject has to be used.
private readonly destroy$ = new Subject<void>();
constructor(private readonly journeyService: ReiseService) {
this.journeyId$.pipe(
distinctUntilChanged(),
switchMap((id) => this.journeyService.getJourney(id)),
shareReplay(1),
takeUntil(this.destroy$)
);
}
ngOnDestroy() {
this.destroy$.next();
this.destroy$.complete();
}However, since Angular 16 no more boiler-plate code has to be defined, but takeUntilDestroyed has to be used instead.
journey: Journey | undefined = undefined;
constructor(private readonly journeyService: JourneyService) {
this.journeyId$.pipe(
distinctUntilChanged(),
switchMap((id) => this.journeyService.getJourney(id)),
shareReplay(1),
takeUntilDestroyed()
).subscribe((journey) => this.journey = journey);
}Summing up
I have taken a closer look on the top 5 code smells I have encountered so far regarding the runtime-performance. And I have shown some examples how to avoid these mistakes.
Keep posted about the next part — the next blog post — a deep dive into common mistakes that negatively influence the maintainability of any project.
Links
[1] https://medium.com/bitsrc/code-smells-in-angular-ce73bf7db072
[2] https://www.rx-angular.io/
[3] https://medium.com/ngconf/local-change-detection-in-angular-410d82b38664
