DEV Community

Cover image for Don't follow RxJS Best Practices
Nik Poltoratsky
Nik Poltoratsky

Posted on • Updated on

Don't follow RxJS Best Practices

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!

Top comments (39)

Collapse
 
gab profile image
Gabriel Magalh√£es dos Santos

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

Collapse
 
kristijanfistrek profile image
KristijanFiŇ°trek

ūü§£

Collapse
 
benjamindelpech profile image
benjamin DELPECH

i could smell the javascript framework expert feeling superior because he knows some tricks.

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
Nik Poltoratsky

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
Nik Poltoratsky

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
Nik Poltoratsky

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
 
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
 
kristijanfistrek profile image
KristijanFiŇ°trek

Ohh you sneaky, you! The pure function one killed me ūüėÖ

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
Nik Poltoratsky

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
 
elasticrash profile image
Stefanos Kouroupis

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

Collapse
 
nikpoltoratsky profile image
Nik Poltoratsky

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
 
ich5003 profile image
Lucas

I'm still scared someone doesn't get the sarcasm and starts following this ūüėĄ

Collapse
 
stephangalea profile image
Stephan Galea

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

Collapse
 
wlun001 profile image
Wei Lun

Oh gosh

Collapse
 
chris_bertrand profile image
Chris Bertrand

Haha, nice article. A nice checklist to follow!

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!