Don't follow RxJS Best Practices
Nik Poltoratsky

Nik Poltoratsky @nikpoltoratsky

About: Open-Source Contributor | Tech Author | Developer Advocate for uibakery.io

Location:
Vilnius, Lithuania
Joined:
Jul 9, 2019

Don't follow RxJS Best Practices

Publish Date: Jul 15 '19
260 41

Nowadays more and more developers learn RxJS and use it properly with best practices in mind. But we shouldn't. All those so-called best practices require to learn something new and to add additional code in your projects.
Moreover, using the best practices we're risking to create a good code base and make your teammates happy! 🌈
Stop being a gray mass! Break the rules! Stop using best practices!

Here are my suggestions to you on how to deal with those so-called RxJS best practices in Angular:


Don't unsubscribe

Everybody says that we have to always unsubscribe from observables to prevent memory leaks.

But I can't agree with it. Seriously, who decided that you have to unsubscribe from observables? You don't have to do that. Let's play a game! Which unsubscribe implementation of those Angular components is the best?

That one with takeUntil operator?



@Component({ ... })
export class MyComponent implements OnInit, OnDestroy {

  private destroyed$ = new Subject();

  ngOnInit() {
    myInfiniteStream$
      .pipe(takeUntil(this.destroyed$))
      .subscribe(() => ...);
  }

  ngOnDestroy() {
    this.destroyed$.next();
    this.destroyed$.complete();
  }
}


Enter fullscreen mode Exit fullscreen mode

Or that one with takeWhile operator?



@Component({ ... })
export class MyComponent implements OnInit, OnDestroy {
  private alive = true;
  ngOnInit() {
    myInfiniteStream$
      .pipe(takeWhile(() => this.alive))
      .subscribe(() => ...);
  }
  ngOnDestroy() {
    this.alive = false;
  }
}


Enter fullscreen mode Exit fullscreen mode

Exactly! Neither! Both takeWhile and takeUntil operators are implicit and may be hard to read 🤓 (sarcasm). The best solution is to store each subscription in a separate variable and then unsubscribe on component destroy in an explicit way:



@Component({ ... })
export class MyComponent implements OnInit, OnDestroy {

  private subscription;

  ngOnInit() {
    this.subscription = myInfiniteStream$
      .subscribe(() => ...);
  }

  ngOnDestroy() {
    this.subscription.unsubscribe();
  }
}


Enter fullscreen mode Exit fullscreen mode

That works especially good in cases when you have multiple subscriptions:



Component({ ... })
export class MyComponent implements OnInit, OnDestroy {

  private subscription1;
  private subscription2;
  private subscription3;
  private subscription4;
  private subscription5;

  ngOnInit() {
    this.subscription1 = myInfiniteStream1$
      .subscribe(() => ...);
        this.subscription2 = myInfiniteStream2$
      .subscribe(() => ...);
        this.subscription3 = myInfiniteStream3$
      .subscribe(() => ...);
        this.subscription4 = myInfiniteStream4$
      .subscribe(() => ...);
        this.subscription5 = myInfiniteStream5$
      .subscribe(() => ...);
  }

  ngOnDestroy() {
    this.subscription1.unsubscribe();
    this.subscription2.unsubscribe();
    this.subscription3.unsubscribe();
    this.subscription4.unsubscribe();
    this.subscription5.unsubscribe(); 
  }
}


Enter fullscreen mode Exit fullscreen mode

But that solution is not perfect yet. What could be done better? How do you feel? How could we make that code more clean and readable?
Yeah, I have the answer for you! Let's remove all that ugly unsubscribe statements at all.



@Component({ ... })
export class MyComponent implements OnInit {

  ngOnInit() {
    myInfiniteStream$
      .subscribe(() => ...);
  }
}


Enter fullscreen mode Exit fullscreen mode

Excellent! We've removed all the redundant code and now it looks simpler and even saves us a bit of memory on our hard drives. But what will happen with myInfiniteStream$ subscription?

Forget about it! 😅 Let's leave that job for the garbage collector, otherwise, why does it exist, right?


Use subscribe inside subscribe inside subscribe inside…

Everybody says that we should use *Map operators to chain observables instead of subscribing inside subscribes to prevent callback hell.

But I can't agree with it. Seriously, why not? Why should we use all those switchMap/mergeMap operators? How do you feel about that code? Easy to read? Do you really like your teammates so much?



getUser().pipe(
  switchMap(user => getDetails(user)),
  switchMap(details => getPosts(details)),
  switchMap(posts => getComments(posts)),
)


Enter fullscreen mode Exit fullscreen mode

Don't you think it too neat and cute? You shouldn't write code that way! You have another choice, take a look here:



getUser().subscribe(user => {
  getDetails(user).subscribe(details => {
    getPosts(details).subscribe(posts => {
      getComments(posts).subscribe(comments => {  

        // handle all the data here
      });
    });
  });
})


Enter fullscreen mode Exit fullscreen mode

Much better, huh?! Always write code this way if you hate your teammates and don't want to learn new RxJS operators.

Be bright! Let your team members feel a bit of nostalgia with callback hell.


Never use pure functions

Everybody says that we should use pure functions to make our code predictable and easier to test.

But I can't agree with it. Seriously, why should you use pure functions? Testability? Composability? It's hard, it would be much easier to affect the global world. Let's take a look at the example:



function calculateTax(tax: number, productPrice: number) {
 return (productPrice * (tax / 100)) + productPrice; 
}


Enter fullscreen mode Exit fullscreen mode

For instance, we have a function which calculates a tax - it's a pure function it will always return the same result for the same parameters. It's easy to test and compose with other functions. But, do we really need that behavior? I don't think so. It would be easier to use a function without parameters:



window.tax = 20;
window.productPrice = 200;

function calculateTax() {
 return (productPrice * (tax / 100)) + productPrice; 
}


Enter fullscreen mode Exit fullscreen mode

Indeed, what can go wrong? 😉


Always subscribe manually, don't use async

Everybody says that we have to use async pipe in Angular templates to facilitate subscriptions management in components.

But I can't agree with it. We've already discussed subscriptions management with takeUntil and takeWhile and agreed that these operators are from an evil one. Though, why should we treat async pipe another way?



@Component({  
  template: `
    <span>{{ data$ | async }}</span>
  `,
})
export class MyComponent implements OnInit {

  data$: Observable<Data>;

  ngOnInit() {
    this.data$ = myInfiniteStream$;
  }
}


Enter fullscreen mode Exit fullscreen mode

Do you see that? Clean, readable, easy to maintain code! Argh. It's not allowed. As for me, it would be much better to put the data in local variable and just use that variable in the template.



@Component({  
  template: `
    <span>{{ data }}</span>
  `,
})
export class MyComponent implements OnInit {
  data;

  ngOnInit() {

    myInfiniteStream$
      .subscribe(data => this.data = data);
  }
}


Enter fullscreen mode Exit fullscreen mode

Expose subjects from your services

There is a pretty common practice to use Observable Data Services in Angular:



@Injectable({ providedIn: 'root' })
export class DataService {

  private data: BehaviorSubject = new BehaviorSubject('bar');

  readonly data$: Observable = this.data.asObservable();

  foo() {
    this.data$.next('foo');
  }

  bar() {
    this.data$.next('bar');
  }
}


Enter fullscreen mode Exit fullscreen mode

Here we're exposing data stream as observable. Just to make sure it can be changed only through a data service interface. But it confuses people.

You want to change the data - you have to change the data.

Why add additional methods if we can change the data on the place? Let's rewrite the service to make it easier to use;



@Injectable({ providedIn: 'root' })
export class DataService {
  public data$: BehaviorSubject = new BehaviorSubject('bar');
}


Enter fullscreen mode Exit fullscreen mode

Yeah! Do you see that? Our data service became smaller and easier to read! Also, now we can put almost anything in our data stream! Awesome, don't you think so?🔥


Always pass streams to child components

Have you ever heard about Smart/Dump components pattern, that can help us to decouple components from each other? Also, that pattern prevents child component from triggering actions in parent components:



@Component({
  selector: 'app-parent',
  template: `
    <app-child [data]="data$ | async"></app-child>
  `,
})
class ParentComponent implements OnInit {

  data$: Observable<Data>;

  ngOnInit() {
    this.data$ = this.http.get(...);
  }
}

@Component({
  selector: 'app-child',
})
class ChildComponent {
  @Input() data: Data;
}


Enter fullscreen mode Exit fullscreen mode

Do you like it? Your teammates also like it. In case you want to revenge them, you need to rewrite your code in the following way:



@Component({
  selector: 'app-parent',
  template: `
    <app-child [data$]="data$"></app-child>
  `,
})
class ParentComponent {

  data$ = this.http.get(...);
  ...
}

@Component({
  selector: 'app-child',
})
class ChildComponent implements OnInit {

  @Input() data$: Observable<Data>;

  data: Data;

  ngOnInit(){
    // Trigger data fetch only here
    this.data$.subscribe(data => this.data = data);
  }
}


Enter fullscreen mode Exit fullscreen mode

Do you see that? We're not handling subscriptions in the parent component anymore. We're just passing subscription to the child component.
If you follow that practice your team members will cry tears of blood during debugging, believe me.


Marble diagrams? No, it's not for you

Do you know what are marble diagrams? No? It's good for you!

Let's assume we wrote the following function and going to test it:



export function numTwoTimes(obs: Observable<number>) {
  return obs.pipe(map((x: number) => x * 2))
}


Enter fullscreen mode Exit fullscreen mode

Many of us will use marble diagrams to test the function:



it('multiplies each number by 2', () => { 
  createScheduler().run(({ cold, expectObservable }) => {
    const values = { a: 1, b: 2, c: 3, x: 2, y: 4, z: 6 }
    const numbers$ = cold('a-b-c-|', values) as Observable<number>;
    const resultDiagram = 'x-y-z-|';
    expectObservable(numTwoTimes(numbers$)).toBe(resultDiagram, values);
  });
})


Enter fullscreen mode Exit fullscreen mode

But, who the hell wants to learn a new concept of marble diagrams? Who wants to write clean and laconic code? Let's rewrite the test in a common manner.



it('multiplies each number by 2', done => {
  const numbers$ = interval(1000).pipe(
    take(3),
    map(n => n + 1)
  )
  // This emits: -1-2-3-|

  const numbersTwoTimes$ = numTwoTimes(numbers$)

  const results: number[] = []

  numbersTwoTimes$.subscribe(
    n => {
      results.push(n)
    },
    err => {
      done(err)
    },
    () => {
      expect(results).toEqual([ 2, 4, 6 ])
      done()
    }
  )
})


Enter fullscreen mode Exit fullscreen mode

Yeah! It looks one hundred times better now!


Conclusion

You're a hero if you've read all the advice above. But. Well. If you recognized your train of thoughts, I have a piece of bad news for you. It was a joke.

Please, never do what I said in that article. Never let your teammates cry and hate you. Always strive to be a decent and neat person. Save the world - use patterns and best practices!

I just decided to cheer you up and make your day a little bit better. Hopefully, you like it.

Stay tuned and let me know if you have any particular Angular topics you would like to hear about!

Comments 41 total

  • Wei Lun
    Wei LunJul 15, 2019

    Oh gosh

  • Gabriel Magalhães dos Santos
    Gabriel Magalhães dos SantosJul 15, 2019

    I could smell the junior's developer happiness reading this article

  • Dexter St-Pierre
    Dexter St-PierreJul 15, 2019

    We have code in our codebase that is like this.. And the people that wrote it were senior developers...

    • Nik Poltoratsky
      Nik PoltoratskyJul 15, 2019

      I’m not telling you mustn't do that. In some particular cases, it is acceptable, so hopefully, everything is okay with your codebase 😁

      • Dexter St-Pierre
        Dexter St-PierreJul 15, 2019

        Eh, in the case of our code base it is not acceptable cases. But we are working on fixing the issues :D

  • KristijanFištrek
    KristijanFištrekJul 15, 2019

    Ohh you sneaky, you! The pure function one killed me 😅

  • Carlos Caballero
    Carlos CaballeroJul 15, 2019

    Oh!! Thanks! I've a post to recommend to my students!

    Thanks!

  • halcaponey
    halcaponeyJul 15, 2019
    getUser().pipe(
      switchMap(user => getDetails(user)),
      switchMap(details => getPosts(details)),
      switchMap(posts => getComments(details)),
    )
    

    should be

    getUser().pipe(
      switchMap(user => getDetails(user)),
      switchMap(details => getPosts(details)),
      switchMap(posts => getComments(posts)),
    )
    

    but really should be

    getUser().pipe(
      switchMap(getDetails),
      switchMap(getPosts),
      switchMap(getComments),
    )
    

    Good article, made me smile

  • InnerC#ity
    InnerC#ityJul 15, 2019

    Fantastic!! Well done Nikita! :)

  • Lucas
    LucasJul 15, 2019

    I'm still scared someone doesn't get the sarcasm and starts following this 😄

  • Martín Emanuel
    Martín EmanuelJul 15, 2019

    For a moment I tought you were serious. But when you said no to pure functions I had to stop reading. Not gonna lie, you got me at the beginning.

    • Mark Flegg
      Mark FleggJul 18, 2019

      Haha, same here... at first I was thinking, "this is interesting," then "these ideas are a bit strange," then when I got to the no pure functions part I had to scroll to the bottom to see if it was a joke.

    • Pato
      PatoOct 31, 2019

      sameee hahah

  • I'm Luis! \^-^/
    I'm Luis! \^-^/Jul 16, 2019

    I'm just getting started and I spotted the sarcasm. This is a great article for starters, thanks!

  • Stefanos Kouroupis
    Stefanos KouroupisJul 16, 2019

    Smack me as much as you want but I really hate the async pipe

    • Nik Poltoratsky
      Nik PoltoratskyJul 16, 2019

      And what is the reason? 😃

      • Stefanos Kouroupis
        Stefanos KouroupisJul 16, 2019

        psychological, I had weird/torturing issues in older versions of Angular. Maybe I need to try it again :P

  • Chris Bertrand
    Chris BertrandJul 16, 2019

    Haha, nice article. A nice checklist to follow!

  • Mutembeijoe
    MutembeijoeJul 16, 2019

    You had me at never unsubscribe, but was back to my mind at nested subscriptions and polluting the global space

  • medeirosrafael
    medeirosrafaelJul 17, 2019

    Nice. But in pipe(takeWhile(this.alive)) should be: pipe(takeWhile(() => this.alive)).

    • Nik Poltoratsky
      Nik PoltoratskyJul 17, 2019

      Thank you! Fixed

      • Mihail Malo
        Mihail MaloMar 22, 2020

        pls fix

        Also, setting that value to false won’t be enough. You must emit another value so the observer can unsubscribe when the cb function returns false.



        too
    • Andrei Gatej
      Andrei GatejAug 17, 2019

      Also, setting that value to false won’t be enough. You must emit another value so the observer can unsubscribe when the cb function returns false.

  • jogelin
    jogelinJul 18, 2019

    I was hating you until the end :D

  • Stephan Galea
    Stephan GaleaJul 21, 2019

    This article reminds me of this :) youtube.com/watch?v=X0tjziAQfNQ

  • Oleksandr
    OleksandrJul 30, 2019

    Marble example is super! Can I use same example in my talk?

  • ngclient
    ngclientSep 19, 2019

    I like It

  • Vitaly Tomilov
    Vitaly TomilovSep 28, 2019

    Best practice that I discovered with RxJS - do not use it at all. It is a flawed attempt at trying to turn everything into a cryptic stream.

  • osnersanchez
    osnersanchezOct 18, 2019

    I need an article on good practices with the safe navigation operator for next week <3 Thank you

  • Pato
    PatoOct 31, 2019

    OMG!!! I hate you! hahaha i was like who is this kid? he doesn't know what he is talking about, then I read the entire article lol that was funny

  • Abdalla Abdelnasser
    Abdalla AbdelnasserJan 15, 2020

    Nice joke 😅
    Pure functions and the nested subscribes are hilarious

  • Mihail Malo
    Mihail MaloMar 22, 2020

    Is this actually a good one?

    takeWhile(() => this.alive)
    

    Wouldn't this only trigger once the source tries to push a value, so if a value is never pushed the subscription is never garbage collected?
    Seems vastly inferior to

    destroyed$ = new Subject()
    
  • Loic Coenen
    Loic CoenenAug 18, 2020

    There's something I really don't get though. For me the difference between dumb and smart component is that the later doesn't communicate directly with the store or any other services, not that we can't pass down observable.

  • flavio
    flavioSep 12, 2020

    kkkkkk man, what a great post. congrats!! in a cool manner you make me see some things about this kind of good practice.

Add comment