0

Several bad practices in Angular

1. Introduction

About 5 months after the ground-breaking release of Angular 2, the next huge update for Angular has become now available: Angular 4 (or Angular due to the development team's concensus that it should be called Angular solely from now on without stating the version number explicitly. At the first place, the “2” was intended to differentiate between AngularJS and the all brand-new Angular Framework, which was introduced with many reassessed and refined concepts.

Indeed, Angular is really awesome. Not only does it provide hell of a lot functionalities out of the box (routing, animations, HTTP module, forms/validations and etc) but also speeds up the development process and most importantly, Angular is really not that hard to learn and embrace (especially with such a powerful tool as Angular CLI).

But like they often says, "a good instrument in incautious hands might one day become a nuclear weapon of mass destruction", thus the topic that I would like to present today shall be several ways and practices in Angular which I suggest you definitely NOT use.

2. Several bad practices

2.1. Not making a REAL use of Angular components

In an Angular ecosystem, components are the essential building blocks serving as the bridge which connects between application's logic and view. But there are occasions that developers strongly overlook the benefits a component provides. Let us consider this example:

@Component({
  selector: 'app-form-control-component',
  template: `
    <div [formGroup]="form">
      <div class="form-control">
        <label>Job Postion</label>
        <input type="text" formControlName="jobPosition" />
      </div>

      <div class="form-control">
        <label>Salary</label>
        <input type="text" formControlName="salary" />
        <span>(USD)</span>
      </div>

      <div class="form-control">
        <label>Qualification required</label>
        <input type="text" formControlName="qualification" />
      </div>
    </div>
  `
})
export class CreateJobComponent implements OnInit {
  
  form: FormGroup;
  
  constructor(private formBuilder: FormBuilder) {

  }
  
  ngOnInit(): {
    this.form = formBuilder.group({
      jobPosition: ['', Validators.required],
      salary: ['', Validators.required],
      qualification: ['', Validators.required]
    });
  }
}

As you can see, we have a little form with three controls, and a template which contains the actual inputs. Each input is put inside a div element, alongside with its label, and the three containers repeat themselves. They are essentially the same, so, maybe, separate them into a component? Now take a look at this:

@Component({
  selector: 'app-form-control-component',
  template: `
    <div class="form-control">
      <label>{{ label }}</label>
      <input type="text" [formControl]="control" />
      <span *ngIf="unit">({{unit}})</span>
    </div>
  `
})
export class SingleControlComponent{
  @Input() control: AbstractControl 
  @Input() label: string;
  @Input() unit: string;
}

So, we have separated a single control to it’s own component, and we have defined inputs to pass data from the parent component, in this case, the form control instance and the label of the input. Let’s revise our first component’s template:

<div>
  <app-form-control-component [control]="form.controls['jobPosition']" [label]="'Job Position'"></app-form-control-component>

  <app-form-control-component [control]="form.controls['salary']" [label]="'Salary'" [unit]="'USD'"></app-single-control-component>

  <app-form-control-component [control]="form.controls['qualification']" [label]="'Qualification'"></app-form-control-component>
</div>

As you reach this part, you might probably think “well, component separation is a basic Angular concept, why would you have you make such simple thing seem bizarre? Everyone knows this, duh...”, but the problem is that many developers are deceived by Angular’s router module: it maps a route to a component, and therefore, people (mostly newbies, but sometimes it happens with more experienced devs too) have the tendency to treat these components as of separate pages. Angular component is NOT a page, it is a piece of the view, and several components together compose a parent view.

Another nasty situation is when you have a small component, mostly without any specific logic at the beginning, however, as new requirements arrive, it just grows larger and larger inevitably. In the nick of time, you had better start thinking of component separation, otherwise, you may end up with an uncontrollable, ugly monstrosity version of the component.

2.2. Abusing .toPromise()

Angular comes with its own out-of-the-box HTTP module for our app to communicate with remote servers. As you should already know by now that Angular uses Rx.js to support HTTP requests, instead of Promises. You know what? The fact is that not everyone is aware of the existence of Rx.js. True. Nevertheless, if you are going to use Angular for a long-term project, it is suggested learning it. Those who are new to Angular tend to just transform Observables returned from API calls in the HTTP module to Promises by using .toPromise() due to their familiarity with Promise. To be honest, that is probably the worst thing you can do to your application, because just for the sake of your laziness you:

  1. Add unnecessary logic to your app. You don’t need to transform an Observable to a Promise, you can work with the stream of data itself. [Oservable Link]

  2. Throw away a lot of disadvantages Rx.js provides: you could cache a response, you could manipulate the data before subscribing to it, you could cancel a request response if it is no longer needed, you could find logical mistakes in the received data and rethrow errors to catch them later on in your app with just one or two lines of code… All miracles happen thanks to Observable. But you preferred .toPromise() instead 😃

2.3. Neglecting on Rx.js

This one is more of a general advice. Rx.js is awesome, and you should consider using it to manipulate your data, events and your application’s overall state with it.

2.4. Carefree DOM manipulation

And this one’s old. Angular does not use directives as much as Angular.js used to (we had lot’s of stuff like ng-click, ng-src, most of them now replaced with Inputs and Outputs), but it still has some: ngIf, ngForOf. The rule of thumb for Angular.js was

Never ever ever do DOM manipulations within a controller

And now, as the concept of controller has completely disappeared, the rule for thumb for the next generation of Angular should be:

Never ever ever ever do DOM manipulations within a component

2.5. Not having interfaces defined for data

Sometimes you may tend to think about the data retrieved from a server/API as any data, that’s it, type any. That’s not really the case. You should define all the types for every data you receive from your backend, because, you know, after all, that’s why Angular choose to be used mainly on TypeScript.

2.6. Data manipulations & transforms within a component

This one’s tricky. Some even do that within a service but I also suggest not doing that either. Services are for API calls, sharing data between components and other utilities. The data manipulations instead should belong to separate model classes. Take a look at this:

interface Book {
  id: number;
  title: string;
}

@Component({
  selector: 'app-form-component',
  template: `...` // our form is here
})
export class FormComponent {

  form: FormGroup;
  books: Array<Book>

  constructor(private formBuilder: FormBuilder){
    this.form = formBuilder.group({
      username: ['', Validators.required]
      favoriteBooks: [[]], 
    });
  }
  
   onSubmit(values){ 
    /* 
      'values' is actually a form value, which represents a user
      but imagine our API does not expect as to send a list of book
      objects, just  a list of id-s, so we have to map the values
    */
    values.favoriteBooks = values.favoriteBooks.map((book: Book) => book.id);
    // then we will send the user data to the server using some service
    
  }
}

Now, this does not look like a catastrophe, just a little data manipulation before sending the values to backend. But imagine if there are lots of Foreign Keys, Many-To-Many fields, lots of data handling, depending on some cases, variables, your app’s state… Your onSubmit method may quickly become a mess. Now consider doing this:

interface Book {
  id: number;
  title: string;
}

interface User {
  username: string;
  favoriteBooks: Array<Book | number>;
}

class UserModel implements User {
  username: string;
  favoriteMovies: Array<Book | number>;

  constructor(user: User){
    this.username = user.username;
    this.favoriteBooks = user.favoriteBooks.map((book: Book) => book.id);
    /*
     we moved the data manipulation to this separate class,
     which is also a valid representation of a User model,
     so no unnecessary clutter here
    */
  }
}

Now, as you see, we have a class, representing a user, with all the manipulations inside it’s constructor. The component will now look like this:

@Component({
  selector: 'app-form-component',
  template: `...` // our form is here
})
export class FormComponent {

  ...

  onSubmit(values: User){
    /*
      now we will just create a new User instance from our form,
      with all the data manipulations done inside the constructor
    */
    let user: UserModel = new UserModel(values);
    // then we will send the user model data to the server using some service
  }

}

And any further data manipulations will go inside the model constructor, hence, not polluting the component’s code. As another rule of thumb , you may look to have a new keyword before each time sending data to a server.

2.7. Not using/Misusing Pipe

I would like to explain this one with an example rightaway. Assume that you have a dropdown box allowing users to select a weight measurement unit. The requirement is that each label represented is always preceded with a slash (/), which looks like ‘1 dollar / kg’ or ‘7 dollars / oz’. Have a look at this:

@Component({
  selector: 'app-dummy-component',
  template: `
    <input type="text" placeholder="Price">
    <dropdown-component [options]="weightUnits"></dropdown-component>
`
})
export class DummyComponent {
   weghtUnits = [{ quantity: 1, unit: 'kg' }, { quantity: 2, unit: 'pound' }];
}

Stupid way:

@Component({
  selector: 'app-dummy-component',
  template: `
    <input type="text" placeholder="Price">
    <dropdown-component [options]="slashedWeightUnits"></dropdown-component>
  `
})
export class DummyComponent {
  weightUnits = [{ quantity: 1, unit: 'kg' }, { quantity: 2, unit: 'pound'  }];
  slashedWeightUnits = [{ quantity: 1, unit: '/kg'}, {quantity: 2, unit: '/pound' }];
  // we just add a new property
}

The output seems nice. Ok, let's imagine what is those values are not just constant values stored inside the components, but, for example, are retrieved from a server? And, of course, creating a new property for every single data mutation will soon see us in a mess. It is really frustrating to solve the problem this way.

Dangerous way:

@Component({
  selector: 'app-dummy-component',
  template: `
    <input type="text" placeholder="Price">
    <dropdown-component [options]="slashedWeightUnits"></dropdown-component>
  `
})
export class DummyComponent {
  weightUnits = [{ quantity: 1, unit: 'kg' }, { quantity: 2, unit: 'pound' }];
  getSlashedWeightUnits() {
    return this.weightUnits.map(weightUnit => ({ 
      unit: '/' + weightUnit.unit,
      quantity: weightUnit.quantity
    }));
  }
  // so now we map existing weight units to a new array
}

This may look like good solution, but in reality, this is even worse. The dropdown will render and look fine, until you try to click on it, and maybe even before that you may notice that it is blinking (yes, blinking!). Why? To understand that, you may need to dive a little into how inputs and outputs work with Angular’s change detection mechanism.

The dropdown component has an options input, and will re-render the dropdown every time that the input’s value changed. Here, the value is determined after a function call, so the change detection mechanism has no way to determine whether it has changed or not, so it will just have to constantly call the function on each change detection iteration, and the dropdown will be constantly re-rendered. Thus, the problem is solved… by creating a bigger problem.

Fine way:

@Pipe({
  name: 'slashed'
})
export class Slashed implements PipeTransform {
  transform(value) {
    return value.map(item => ({
      unit: '/' + item.unit,
      quantity: item.quantity
    }));
  }
}


@Component({
  selector: 'app-dummy-component',
  template: `
    <div>
      <input type="text" placeholder="Price">
      <dropdown-component [options]="(weightUnits | slashed)"></dropdown-component>
    </div>
`
})
export class DummyComponent {
  weightUnits = [{ quantity: 1, unit: 'kg' }, { quantity: 2, unit: 'pound' }];
}

Well, you are of course familiar with the pipes. This is still not a very specific advice (well, the documentation itself tells us to use them in such cases), but the real point I want to make is not the pipes themselves. The point is: I don’t like this solution either. If I have lots of simple, but different data mutations in my app, should I write a Pipe class for each and every one of them? What if most of them are so specific that are only used in one and only one context of a component? This looks like a huge amount of clutter.

A more advanced solution:

@Pipe({
  name: 'map'
})
export class Mapping implements PipeTransform {
  /* 
  this will be a universal pipe for array mappings. You may add more 
  type checkings and runtime checkings to make sure it works correctly everywhere
  */
  transform(value, mappingFunction: Function) {
    return mappingFunction(value);
  }
}


@Component({
  selector: 'app-dummy-component',
  template: `
    <div>
      <input type="text" placeholder="Price">
      <dropdown-component [options]="(weightUnits | map : addSlash)"></dropdown-component>
    </div>
`
})
export class DummyComponent {
  weightUnits = [{ quantity: 1, label: 'kg' }, { quantity: 2, label: 'pound' }];
  
  addSlash(units) {
    return units.map(unit => ({
      unit: '/' + unit.unit,
      quantity: unit.quantity
    });
  }
}

What’s the difference? Well a pipe calls its transform method when and only when the data changes. As long as weightUnits do not change, the pipe will be invoked just one time instead of on every change detection iteration.

I don’t state you must have just one or two mapping Pipes and nothing else, but you should have more custom pipes on more complex stuff (working with datetime, etc) and where reuse is crucial, and for more component-specific manipulations you may consider having a universal pipe.

3. References

Rxjs Angular Training


All rights reserved

Viblo
Hãy đăng ký một tài khoản Viblo để nhận được nhiều bài viết thú vị hơn.
Đăng kí