Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Best practices for RxJS inside non-trivial Angular components

Our team is in the process of migrating from AngularJS to Angular (4).

Inside our Angular components, we use the RxJS library to work on observables returned from the Http, ActivatedRoute, etc. services or subjects receiving values from inputs on the user interface. When some processing is required on the values produced by these observables, we map or combine them into new observables to produce "derived" observables. Nothing special here.

We end up with components in which almost all the public properties bound from the templates are observables.

However, this technique seems to have major drawbacks:

  • It requires a very good knowledge and understanding of the RxJS library and many tasks that formerly seemed simple have become very complex, with a lot of time spent debugging/optimizing observables subscriptions.
  • Our templates are now filled with async pipes and we regularly run into Angular bugs like this one: https://github.com/angular/angular/issues/10165.

To give you an example, here is an simple case (not the worst, by far): the template of a pagination component. Notice the large number of async pipes, some of which are nested inside a structural directive, itself asynchronous (which causes the bug I just mentionned).

<nav aria-label="Pagination" class="mt-5" *ngIf="($items | async)?.length">
  <ul class="pagination">
    <li class="page-item" [class.disabled]="(currentPage$ | async) === 1">
      <a class="page-link" [routerLink]="" [queryParams]="{ page: (currentPage$ | async) - 1 }" queryParamsHandling="merge">Previous</a>
    </li>
    <li *ngFor="let page of (pages$ | async)" class="page-item" [class.active]="page === (currentPage$ | async)">
      <a class="page-link app-page-number" [routerLink]="" [queryParams]="{ page: page }" queryParamsHandling="merge">{{ page }} <span class="sr-only" *ngIf="page === (currentPage$ | async)">(current)</span></a>
    </li>
    <li class="page-item" [class.disabled]="(currentPage$ | async) === (lastPage$ | async)">
      <a class="page-link" [routerLink]="" [queryParams]="{ page: (currentPage$ | async) + 1 }" queryParamsHandling="merge">Next</a>
    </li>
  </ul>
</nav>

Changing the "derived" observables into simple (non-observable) properties, filled from a .subscribe() helps reduce code complexity, as well as the number of async pipes inside the template. But this doesn’t feel like a satisfying solution.

Besides, we encounter some new problems related to change detection with the OnPush strategy. Since we turned our observables into "normal" properties and removed the async pipes, Angular is not aware anymore that we are changing these properties and we often have to call ChangeDetectorRef.detectChanges().

We have so far not been able to find clear best practices on how to deal with RxJS inside non-trivial components. Would you have any suggestions on how to get around these difficulties? Could Redux be the solution to (some) of our problems?

Edit: It turned out that the aforementioned "bug" is actually a misuse of the RxJS library on our part. See my contribution to the (GitHub issue) for more details.

like image 912
Mathieu Renda Avatar asked Nov 02 '17 17:11

Mathieu Renda


1 Answers

Change detection

The problem of *ngIf="(obs | async)" is one of change detection. The variable obs does not itself change when emitting, coupled with this being an expression in the template, makes it hard for change detection to detect.

A couple of principles are worth thinking of in this scenario:

  • Observable variables are "pipes", i.e wrappers for values passing through them not the changing values themselves. The two are often equated, but that's akin to thinking that an an array and array elements are the same thing.

  • RxJs is an 'external' (non-Angular) library. With these libraries, we need to be aware of whether changes to data handled by the library are visible to Angular change detection.

A fix for #10165 is

<div style="background-color: green;" *ngIf="trigger">{{(val1 | async)}}</div>
<div style="background-color: green;" *ngIf="!trigger">{{(val2 | async)}}</div>

ngOnInit() {
  this.trigger = this.ifObservable.subscribe();
}

Dealing with async data

On the wider issue of 'observables everywhere', you are quite right but is this not a problem inherent in the async nature of web apps?

It would be interesting to contrast your old AngularJS code to the new Angular code. Is there an increase in complexity for the same set of feature implementation?

A principle I like to apply is to handle 'one-shot' observables such as http.get by subscribing to them, and keep 'multi-shot' observables connected via async pipes.

OnPush

This is essentially a way to dial-down the amount of automatic change detection going on, and hence speed up the app. Of course, that means you may have to be fire change detection manually more often - speed vs complexity trade-off.

Redux

While Redux stores generally expose state as observables (so you still need async pipe in the template), it does remove complexity around multiple points of updates to state, which may mean less observables are required (or at least the observables are abstracted away from the components).

The only Redux flavour I've looked at which doesn't involve de-asyncing data in the template is Mobex, which essentially converts simple variables into objects with getters and setters, which contain the logic to watch for the async changes and unwrap them. But it has problems/caveats too with arrays.

Simplifying the template

One way you might simplify the template you've shown (I haven't tested this) is to wrap it in a child component and pass in the unwrapped values

<nav aria-label="Pagination" class="mt-5" *ngIf="items.length">
  <ul class="pagination">
    <li class="page-item" [class.disabled]="currentPage === 1">
      <a class="page-link" [routerLink]="" [queryParams]="{ page: currentPage - 1 }" queryParamsHandling="merge">Previous</a>
    </li>
    <li *ngFor="let page of pages" class="page-item" [class.active]="page === currentPage">
      <a class="page-link app-page-number" [routerLink]="" [queryParams]="{ page: page }" queryParamsHandling="merge">{{ page }} <span class="sr-only" *ngIf="page === currentPage">(current)</span></a>
    </li>
    <li class="page-item" [class.disabled]="currentPage === lastPage">
      <a class="page-link" [routerLink]="" [queryParams]="{ page: currentPage + 1 }" queryParamsHandling="merge">Next</a>
    </li>
  </ul>
</nav>


export class PageComponent {

  @Input() currentPage = 0;
  @Input() pages = [];
  @Input() items = [];

and in the parent,

<page-component [pages]="pages$ | async" [items]="items$ | async" [currentPage]="currentPage$ | async" >

Esstentially, @Input hooks into change detection, even with onPush strategy.
Be careful to give child inputs default values.

like image 163
Richard Matsen Avatar answered Nov 10 '22 05:11

Richard Matsen