Should we avoid nested rxjs operators? One case which I cannot test

Question

I have written the following effect in my Angular app which uses rxjs. On MyActions.myAction, I receive an object containing a property ids - an array of ids - and for each id I want to send an HTTP request via this.myApiService.getResource, which returns an Observable<Resource>. I want then to collect all results in an array, and dispatch another action passing the array.

  public loadResources$: Observable<MyAction> = this.actions$.pipe(
    ofType(MyActions.myAction),
    switchMap(({ ids }) => from(ids).pipe(
      mergeMap(id => this.myApiService.getResource(id)),
      toArray()
    )),
    map(resources) => MyActions.resourcesLoaded({ resources } )),
  );

The code above does the job, but I wonder whether I should avoid nesting two flows of reactive operators, and whether there is a better way to write that.

The reason I wonder that is that I am having problems writing a test for it. I wrote the test below but I cannot make it pass.

 it('should dispatch an resourcesLoaded action with the resources', () => {
      const ids = ['5f7c723832758b859bd8f866'];
      const resources = [{} as Resource];

      const values = {
        l: MyActions.loadResources({ ids }),
        t: ids[0],
        o: MyActions.resourcesLoaded({ resources })
      };

      actions =         hot('--l------------', values);
      const get$ =     cold('  -------t-----', values);
      const expected = cold('---------o-----', values);

      myApiService.getResource.withArgs(ids[0]).returns(get$);

      expect(myEffects.loadResources$).toBeObservable(expected);
    });

The error I get is:

     Expected $.length = 0 to equal 1.
Expected $[0] = undefined to equal Object({ frame: 50, notification: Notification({ kind: 'N', value: { ....
Error: Expected $.length = 0 to equal 1.
Expected $[0] = undefined to equal Object({ frame: 50, notification: Notification({ kind: 'N', value: { ....
    at <Jasmine>
    at compare (http://localhost:9876/Users/jacopolanzoni/Documents/Development/myProject/node_modules/jasmine-marbles/index.js:91:1)
    at <Jasmine>

Expect $.length = 0 to equal 1. Apologies for missing this in the question, I will update it now. Btw, I think it may be toArray waiting for httpClient.get to complete. — Jacopo Lanzoni, Jan 10 '21 at 12:35
No worries. 'toArray waiting for httpClient.get to complete' - aren't you mocking `myApiService.getResource`? I'm assuming that the real `myApiService.getResource` uses HttpClient.get(). But you're right, `toArray` waits for the source to complete, which doesn't happen in your case, because `get$` never completes. Maybe if you change '-t-' into `-(t|)-` in the `get$` obs, will it work? — Andrei Gătej, Jan 10 '21 at 12:44
That's exactly what I did, and the test now passes. Thanks Andrei! Yet what do you think about the method itself? Do you see a better way to write it? I haven't seen many nested operators like that, so I was wondering whether that's good practice or not. — Jacopo Lanzoni, Jan 10 '21 at 12:46
I'd say it depends on what you want to achieve, at least in this case. `of([1,2,3]).pipe(mergeAll(), switchMap(value => http.get(...)))` differs from `of([1,2,3]).pipe(switchMap(ids => from(ids).pipe(mergeMap(...))))`. In the former, each inner obs will be discarded by the next value, so only `3` will resolve. In the second scenario, it will process all of them, because you explode the array in the inner obs(which is _managed_ by swtichMap). — Andrei Gătej, Jan 10 '21 at 12:53
Yeah, I actually need the second options, so I think I am good as I am. If you can expand your last comment in an answer, I'd be happy to accept it as valid answer for my question. — Jacopo Lanzoni, Jan 10 '21 at 12:57

score 0 · Answer 1 · answered Jan 10 '21 at 12:44

I have found out my test was failing because toArray was waiting for the observable returned by getResource (i.e., httpClient.get) to complete. Replacing t with (t|) fixes the test:

 it('should dispatch an resourcesLoaded action with the resources', () => {
      const ids = ['5f7c723832758b859bd8f866'];
      const resources = [{} as Resource];

      const values = {
        l: MyActions.loadResources({ ids }),
        t: ids[0],
        o: MyActions.resourcesLoaded({ resources })
      };

      actions =         hot('--l------------', values);
      const get$ =     cold('  -------(t|)-----', values);
      const expected = cold('---------o-----', values);

      myApiService.getResource.withArgs(ids[0]).returns(get$);

      expect(myEffects.loadResources$).toBeObservable(expected);
    });

Yet, the first part of my question, i.e. whether it's good practice or not to nest operators like that, still stands.

score 0 · Accepted Answer · answered Jan 10 '21 at 16:17

but I wonder whether I should avoid nesting two flows of reactive operators, and whether there is a better way to write that

I'd say it depends on what you want to achieve, at least in this case.

of([1,2,3]).pipe(mergeAll(), switchMap(value => http.get(...)))

differs from

of([1,2,3]).pipe(switchMap(ids => from(ids).pipe(mergeMap(...))))

In the first scenario, each inner observable will be discarded by the next value(except for the last value), so only 3 will resolve.
In the second scenario, it will process all of them, because you explode the array in the inner observable(which is managed by swtichMap, so the only way its inner observable will be discarded is if a new outer value(e.g another array of ids) is emitted by the source).

A case where nesting is not necessary is:

of([1,2,3])
  .pipe(
    // whenever you want to explode an array,
    // it does not matter which higher order operator you use
    // since the operation is **synchronous**
    // so, `mergeAll`, `concatAll`, `switchAll` should work the same
    mergeAll(),

    mergeAll(id => this.apiService.useId(id))
  )

// same as

of([1,2,3])
  .pipe(
    mergeMap(ids => from(ids).pipe(mergeMap(id => this.apiService.useId(id))))
  )

As you can see, in this case, switchMap has been replaced with mergeMap.

Should we avoid nested rxjs operators? One case which I cannot test

2 Answers2