Angular Bad Practices: Revisited

Post Editor

This article explores the patterns in Angular applications that make your code less readable and maintainable, like polluting the ngOnInit or writing useless directive selectors.

6 min read
post

Angular Bad Practices: Revisited

This article explores the patterns in Angular applications that make your code less readable and maintainable, like polluting the ngOnInit or writing useless directive selectors.

post
post
6 min read

Three years ago I published this article about bad practices often used by developers in Angular applications. If you have not read that article, it may be beneficial to read it first before this one.

Today, I want to focus on some other patterns that make our components/directives/services and other parts of our Angular apps less readable and harder to reason about. Without further ado, let’s get started!

Polluting the ngOnInit
Link to this section

ngOnInit may be the most important lifecycle hook in Angular components; it is used to initialize data, setup some listeners, create connections and so on. But sometimes this may make our lifecycle hook too overwhelming:

<>Copy
@Component({ selector: 'some', template: 'template', }) export class SomeComponent implements OnInit, OnDestroy { @ViewChild('btn') buttonRef: ElementRef<HTMLButtonElement>; form = this.formBuilder.group({ firstName: [''], lastName: [''], age: [''], occupation: [''], }) destroy$ = new Subject<void>(); constructor( private readonly service: Service, private formBuilder: FormBuilder, ) {} ngOnInit() { this.service.getSomeData().subscribe(res => { // handle response }); this.service.getSomeOtherData().subscribe(res => { // LOTS of logic may go here }); this.form.get('age').valueChanges.pipe( map(age => +age), takeUntil(this.destroy$), ).subscribe(age => { if (age >= 18) { // do some stuff } else { // do other stuff } }); this.form.get('occupation').valueChanges.pipe( filter(occupation => ['engineer', 'doctor', 'actor'].indexOf(occupation) > -1), takeUntil(this.destroy$), ).subscribe(occupation => { // Do some heavy lifting here }); combineLatest( this.form.get('firstName').valueChanges, this.form.get('lastName').valueChanges, ).pipe( debounceTime(300), map(([firstName, lastName]) => `${firstName} ${lastName}`), switchMap(fullName => this.service.getUser(fullName)), takeUntil(this.destroy$), ).subscribe(user => { // Do some stuff }); fromEvent(this.buttonRef.nativeElement, 'click').pipe( takeUntil(this.destroy$), ).subscribe(event => { // handle event }) } ngOnDestroy() { this.destroy$.next(); } }

Take a look at this component. It does not have very many methods — actually, it has only two lifecycles. But the ngOnInit method is, frankly speaking, terrifying. It subscribes to different form change events, fromEvent streams, it also loads lots of data. It has 40 lines of code, but we have actually omitted the contents of the subscribe callbacks; with them, it may easily be 100+ lines, which already goes against most soft guidelines. Also, we usually work with other methods and not the ngOnInit, so we will need better access to the other methods, but now we would have to scroll through this whole mess to reach them (or close/reopen ngOnInit every time we need to see it). Also, finding something inside the ngOnInit method itself becomes harder because there are so many mixed concepts and tasks.

Now take a look at this revised version of the same component:

<>Copy
@Component({ selector: 'some', template: 'template', }) export class SomeComponent implements OnInit, OnDestroy { @ViewChild('btn') buttonRef: ElementRef<HTMLButtonElement>; form = this.formBuilder.group({ firstName: [''], lastName: [''], age: [''], occupation: [''], }) destroy$ = new Subject<void>(); constructor( private readonly service: Service, private formBuilder: FormBuilder, ) {} ngOnInit() { this.loadInitialData(); this.setupFormListeners(); this.setupEventListeners(); } private setupFormListeners() { this.form.get('age').valueChanges.pipe( map(age => +age), takeUntil(this.destroy$), ).subscribe(age => { if (age >= 18) { // do some stuff } else { // do other stuff } }); this.form.get('occupation').valueChanges.pipe( filter(occupation => ['engineer', 'doctor', 'actor'].indexOf(occupation) > -1), takeUntil(this.destroy$), ).subscribe(occupation => { // Do some heavy lifting here }); combineLatest( this.form.get('firstName').valueChanges, this.form.get('lastName').valueChanges, ).pipe( debounceTime(300), map(([firstName, lastName]) => `${firstName} ${lastName}`), switchMap(fullName => this.service.getUser(fullName)), takeUntil(this.destroy$), ).subscribe(user => { // Do some stuff }); } private loadInitialData() { this.service.getSomeData().subscribe(res => { // handle response }); this.service.getSomeOtherData().subscribe(res => { // LOTS of logic may go here }); } private setupEventListeners() { fromEvent(this.buttonRef.nativeElement, 'click').pipe( takeUntil(this.destroy$), ).subscribe(event => { // handle event }) } ngOnDestroy() { this.destroy$.next(); } }

The logic of the component is the same, but how we arrange our code is different. Now the ngOnInit method calls for three different methods to load the initial data from services, setup form change listeners and setup DOM event listeners (if needed). After this change, reading the component from scratch becomes easier (read the ngOnInit — understand what it gets started at a glance, and — if you need implementation details — visit the corresponding methods). Finding the source of the bugs is also relatively easier: if form listeners don’t work correctly — go to setupFormListeners and so on.

Don’t pollute your ngOnInit method — split it into parts!

Writing useless directive selectors
Link to this section

Angular directives are a powerful tool which allows us to apply custom logic to different HTML elements. In doing so, we utilize css selectors, which actually gives us way more power than we care to realize. Here is an example: imagine a directive that checks if the corresponding element’s formControl has errors and applies some style on it — let’s call it an ErrorHighlightDirective. Now let’s say we give it an attribute selector, say [errorHighlight]. It works fine, but we now have to find all form elements with formControl attribute and put our [errorHighlight] on them, which is a tedious task. But we can, of course use the [formControl] directive attribute selector itself, so out directive will look like this:

<>Copy
@Directive({ selector: '[formControl],[formControlName]' }) export class ErrorHighlightDirective { // implementation }

Now our directive will automatically bind to all form controls in our module.

But the usage does not end there. Imagine we want to apply a shaky animation to all formControls that have a has-error class on them. We can easily write a directive and bind it using a class selector: .has-error.

Use better selectors for your directives to avoid cluttering your HTML with unnecessary attributes

Logic inside a service constructor
Link to this section

Services are classes, and as such, have a constructor, which is usually used to inject dependencies. But sometimes developers write some code/initialization logic inside it too. And sometimes this is not the best idea, and here’s why.

Imagine a service that creates and maintains a socket connection, sends data to the server in real time and dispatches even from the server. Here is a naive implementation:

<>Copy
@Injectable() class SocketService { private connection: SocketConnection; constructor() { this.connection = openWebSocket(); // implementation details omitted } subscribe(eventName: string, cb: (event: SocketEvent) => any) { this.connection.on(eventName, cb); } send<T extends any>(event: string, payload: T) { this.connection.send(event, payload); } }

This basic service creates a socket connection and handles interactions with it. Notice anything off?

The problem is, any time a new instance of this service is created, a new connection will open. And this just may not be the case we want!

Actually, lots of times an app will use a single socket connection, so when we use this service inside lazy loaded modules, we will get a new open connection. To avoid this, we need to remove the initialization logic from this constructor, and find another way to share the connection between lazy loaded modules. Also, we might want to have a method that allows us to reload the connection at will (essentially reopen it, for example if it unexpectedly closes):

<>Copy
@Injectable() class SocketService { constructor( private connection: SocketConnection // the SocketConnection itself is provided in the root of the App and is the same everywhere ) { } // handle to reload a socket, naive implementation openConnection() { this.connection = openWebSocket(); } subscribe(eventName: string, cb: (event: SocketEvent) => any) { this.connection.on(eventName, cb); } send<T extends any>(event: string, payload: T) { this.connection.send(event, payload); } }

Adding new state when you can derive it from the existing state
Link to this section

Every component has its state — a set of properties that hold data essential to render the UI. State is the most important logical part of our app, so handling it correctly has great benefits.

The state can be described as original and derived. Original state can be described as independent data that exists on itself — for example, the state of being signed in. Derived state is entirely dependent on some piece of original state — for example, a text notice that says ‘Please sign in’ if the user is logged out or ‘Sign out’ if the user is signed in. Essentially, we don’t need to hold that text value anywhere — anytime we need it, we can calculate it based on the authentication state. So this piece of code:

<>Copy
@Component({ selector: 'some', template: '<button>{{ text }}</button>', }) export class SomeComponent { isAuth = false; text = 'Sign Out'; constructor( private authService: AuthService, ) {} ngOnInit() { this.authService.authChange.subscribe(auth => { this.isAuth = auth; this.text = this.isAuth ? 'Sign Out' : 'Sign In'; }); } }

will turn into this:

<>Copy
@Component({ selector: 'some', template: `<button>{{ isAuth ? 'Sign Out' : 'Sign In' }}</button>`, }) export class SomeComponent { isAuth = false; constructor( private authService: AuthService, ) {} ngOnInit() { this.authService.authChange.subscribe(auth => this.isAuth = auth); } }

As you can see, the text property was derived state and was completely unnecessary. Removing it made out code easier to read and reason about.

Don’t create separate variables and properties to store derived state; calculate it whenever necessary instead

This one may seem a bit easy to spot, but when dealing with increasingly complex data, even the most experienced developers sometimes make this mistake, especially with RxJS streams. In this article I explore how this concept should be handled in RxJS Angular apps.

Conclusion
Link to this section

There are lots of mistakes one can do when writing an application with Angular. But some mistakes are very common and become patterns, that are reused and abused. Learning about the most common ones and how to avoid them can be very beneficial for our Angular apps.

Share

About the author

author_image

Senior Angular developer from Armenia. Passionate about WebDev, Football, Chess and Music

author_image

About the author

Armen Vardanyan

Senior Angular developer from Armenia. Passionate about WebDev, Football, Chess and Music

About the author

author_image

Senior Angular developer from Armenia. Passionate about WebDev, Football, Chess and Music

Looking for a JS job?
Job logo
PDQ team| Senior JavaScript developer (Angular/Node)

SD Solutions

Ukraine
Remote
$60k - $80k
Job logo
Senior Full-Stack Developer (Node+Angular)

A-Listware

Ukraine
Remote
$48k - $78k
Job logo
Senior Full stack (Angular+Node)

Monolith

Ukraine
Remote
$60k - $84k
Job logo
AngularJS Developer/.net Core - Remote Contract

InfoMagnus

United States
Remote
$115k - $134k
More jobs
NxAngularCli
NxAngularCli
NxAngularCli

Featured articles

Angularpost
13 September 20218 min read
Tracking user interaction area

Explore one of the most complex pieces of Taiga UI — ActiveZone directive that keeps an eye on what region user is working with. It touches on low-level native DOM events API, advanced RxJS and Dependency Injection, ShadowDOM and more!

Angularpost
13 September 20218 min read
Tracking user interaction area

Explore one of the most complex pieces of Taiga UI — ActiveZone directive that keeps an eye on what region user is working with. It touches on low-level native DOM events API, advanced RxJS and Dependency Injection, ShadowDOM and more!

Read more
AngularpostTracking user interaction area

13 September 2021

8 min read

Explore one of the most complex pieces of Taiga UI — ActiveZone directive that keeps an eye on what region user is working with. It touches on low-level native DOM events API, advanced RxJS and Dependency Injection, ShadowDOM and more!

Read more
Angularpost
7 September 202122 min read
Designing Angular architecture - Container-Presentation pattern

Designing architecture could be tricky, especially in the agile world, where requirement changes are frequent. So your design has to support that and provides extendibility without the need for serious modification. In such cases, you will find the Container-Presentation pattern instrumental.

micro frontendspost
6 September 202125 min read
Taking micro-frontends to the next level

The micro-frontends concept has been out there for quite a while. We’ve been using this architecture in Wix since around 2013, long before it was even given this name. In this article I’d like to share some of the things we did in order to evolve the concept of developing big scale micro-frontends.

micro frontendspost
6 September 202125 min read
Taking micro-frontends to the next level

The micro-frontends concept has been out there for quite a while. We’ve been using this architecture in Wix since around 2013, long before it was even given this name. In this article I’d like to share some of the things we did in order to evolve the concept of developing big scale micro-frontends.

Read more
micro frontendspostTaking micro-frontends to the next level

6 September 2021

25 min read

The micro-frontends concept has been out there for quite a while. We’ve been using this architecture in Wix since around 2013, long before it was even given this name. In this article I’d like to share some of the things we did in order to evolve the concept of developing big scale micro-frontends.

Read more