loading...
Cover image for Don't follow RxJS Best Practices

Don't follow RxJS Best Practices

nikpoltoratsky profile image Nikita Updated on ・6 min read

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();
  }
}

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;
  }
}

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();
  }
}

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(); 
  }
}

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(() => ...);
  }
}

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)),
)

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
      });
    });
  });
})

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; 
}

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; 
}

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$;
  }
}

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);
  }
}

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');
  }
}

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');
}

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;
}

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);
  }
}

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))
}

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);
  });
})

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()
    }
  )
})

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!

Discussion

pic
Editor guide
Collapse
gab profile image
Gabriel Magalhães dos Santos

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

Collapse
kristijanfistrek profile image
Collapse
halcaponey profile image
halcaponey
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

Collapse
nikpoltoratsky profile image
Nikita Author

Thank you! Fixed

Collapse
emlautarom1 profile image
Martín Emanuel

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.

Collapse
tptshoe profile image
Mark Flegg

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.

Collapse
devpato profile image
Pato

sameee hahah

Collapse
dexterstpierre profile image
Dexter St-Pierre

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

Collapse
nikpoltoratsky profile image
Nikita Author

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

Collapse
dexterstpierre profile image
Dexter St-Pierre

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

Thread Thread
nikpoltoratsky profile image
Nikita Author

Hmm, then, good luck 😉

Collapse
qm3ster profile image
Mihail Malo

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()
Collapse
medeirosrafael profile image
medeirosrafael

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

Collapse
anduser96 profile image
Andrei Gatej

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.

Collapse
nikpoltoratsky profile image
Nikita Author

Thank you! Fixed

Collapse
qm3ster profile image
Mihail Malo

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
Collapse
kristijanfistrek profile image
KristijanFištrek

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

Collapse
elasticrash profile image
Stefanos Kouroupis

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

Collapse
nikpoltoratsky profile image
Nikita Author

And what is the reason? 😃

Collapse
elasticrash profile image
Stefanos Kouroupis

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

Collapse
innercitypressure profile image
InnerC#ity

Fantastic!! Well done Nikita! :)

Collapse
lcoenenjci profile image
Loic Coenen

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.

Collapse
stephangalea profile image
Stephan Galea

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

Collapse
zerquix18 profile image
I'm Luis! \^-^/

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

Collapse
designpuddle profile image
Chris Bertrand

Haha, nice article. A nice checklist to follow!

Collapse
wlun001 profile image
Wei Lun

Oh gosh

Collapse
carlillo profile image
Carlos Caballero

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

Thanks!

Collapse
ich5003 profile image
Lucas

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

Collapse
vitalyt profile image
Vitaly Tomilov

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.

Collapse
abdallaanasser profile image
Abdalla Abdelnasser

Nice joke 😅
Pure functions and the nested subscribes are hilarious

Collapse
ngclient profile image
ngclient

I like It

Collapse
devpato profile image
Pato

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

Collapse
jogelin profile image
jogelin

I was hating you until the end :D

Collapse
oleksandr profile image
Oleksandr

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

Collapse
nikpoltoratsky profile image
Nikita Author

Definitely! It'll be awesome 😅

Collapse
osnersanchez profile image
osnersanchez

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

Collapse
drflaviopsilva profile image
flavio

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

Collapse
mutembeijoe profile image
Mutembeijoe

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

Collapse
talentlessguy profile image
v 1 r t l

I feel sick