ngOnChanges best practice - always use SimpleChanges - always

ngOnChanges(): "A lifecycle hook that is called when any data-bound property of a directive changes. Define an ngOnChanges() method to handle the changes."

We use this lifecycle hook to respond to changes to our @Input() variables. However, I'm seeing this method abused in the wild with, what I consider, an anti-pattern.

Let's start with an example...

import { Component, Input, OnChanges } from "@angular/core";

@Component({
  selector: "app-my-comp",
  templateUrl: "./my-comp.component.html"
})
export class MyComponent implements OnChanges {
  @Input() myFirstInputParameter: string;

  constructor() {}

  ngOnChanges() {
    this.doSomething(this.myFirstInputParameter);
  }

  private doSomething(input: string) {}
}

This works fine. If the value of myInputParameter changes, our OnChanges fires and doSomething is called. Now let's add a second @Input...

import { Component, Input, OnChanges } from "@angular/core";

@Component({
  selector: "app-my-comp",
  templateUrl: "./my-comp.component.html"
})
export class MyComponent implements OnChanges {
  @Input() myFirstInputParameter: string;
  @Input() mySecondInputParameter: string;

  constructor() {}

  ngOnChanges() {
    this.doSomething(this.myFirstInputParameter);
  }

  private doSomething(input: string) {}
}

This code now has a problem. OnChanges gets called twice - once for every @Input. What if the first time it's called is for mySecondInputParameter? Well, we'd be calling doSomething with 'undefined' since myFirstInputParameter hasn't been set yet. The way I typically see this handled is like this...

  OnChanges() {
    if (this.myFirstInputParameter) {
      this.doSomething(this.myFirstInputParameter);
    }
  }

And this is the anti-pattern. Or worse, it's just plain wrong. Consider what happens if the first value to be set happens to be myFirstInputParameter. Well, our OnChanges fires, finds myFirstInputParameter is set and calls doSomething(). Then OnChanges fires again because mySecondInputParameter is set, finds that myFirstInputParameter is set and calls doSomething() again. This was not intended by the developer. If further @Input()'s are added, then doSomething() gets called even more. Does the angular developer really control the order that ngOnChanges() is called for each @Input? I'd say not. So writing code that assumes a particular order is bad. In fact, depending on your app, the order of calls to OnChanges may not be consistent and then your app may act inconsistently with race conditions.

The solution is to use SimpleChanges. And further to that - to ALWAYS use SimpleChanges. Lets see how SimpleChanges actually works. It's quite a simple change ;-)

https://angular.io/api/core/SimpleChanges

ngOnChanges(changes: SimpleChanges) {
    for (const propName in changes) {
      if (changes.hasOwnProperty(propName)) {
        switch (propName) {
          case 'myFirstInputParameter': {
            this.doSomething(change.currentValue)
          }
        }
      }
    }
  }

This looks a bit verbose but it fixes our problem. ngOnChanges still gets called multiple times, but crucially the changes parameter tells us which @Input() caused the call. SimpleChanges also happens to provide the old value of the @Input() if there has been multiple changes to that value - pretty handy.

I propose that we should always take the SimpleChanges parameter in our ngOnChanges methods. Even if we only have a single @Input() on our component. I don't want code that assumes there will always be only one @Input(). That would be forcing the next developer to handle my @Input() if they want to add their own. It would be so easy to introduce a bug by adding another @Input() to a component and forgetting to check ngOnChanges() for the handling of the other @Input()'s. So take responsibility of your @Input() and embrace SimpleChanges.