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

182 Views Asked by At

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>
2

There are 2 best solutions below

0
Andrei Gătej On BEST ANSWER

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.

0
Jacopo Lanzoni On

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.