3

I am passing a subject as an observable from a service to a resolver. The service may complete the observable when an http request completes or when it finds that the data it needs is cached. In the second case, where the data is cached it looks like the observable completes too early because the route transition does not happen. if i put the requestSubject.complete() inside a setTimeout with some timeout it works but otherwise it doesnt.

//resolve function from resolver
resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): any {
  const id = +route.paramMap.get('id');
  const language = route.queryParamMap.get('language');
  const request = this.service.get(id, language);
  request.pipe(share()).subscribe(
    () => {},
    () => { this.router.navigateByUrl(AppRoutingConfig.search); });
  return request;
}


//the service get function
get(id: number, language: string): Observable<any> {
  const requestSubject = new Subject();
  if (!isDataCached(id)) {
    const url = `some url`;
    const sub = this.http.get(url).subscribe((item: any) => {
      requestSubject.next(1);
    }, () => {
        requestSubject.error(0);
    }, () => {
      requestSubject.complete();
    });
  } else {
    requestSubject.complete();
  }
  return requestSubject.asObservable();
}
Sibi John
  • 475
  • 6
  • 22

2 Answers2

3

looks like the observable completes too early because the route transition does not happen

I'd say that the Subject does not do anything.

If the data is not cached, requestSubject.complete(); will run, which will synchronously emit a complete notification to all its subscribers.

This means that returning the requestSubject after won't do anything, because the requestSubject won't emit a value.

It works when the data is not cached because the by the time that Subject emits and completes, it would already have a subscriber which can decide whether or not the route transition should continue.

For example, this is how Angular internally subscribes to all the observables provided in a resolve property:

return from(keys).pipe(
      mergeMap(
          (key: string) => getResolver(resolve[key], futureARS, futureRSS, moduleInjector)
                               .pipe(tap((value: any) => {
                                 data[key] = value;
                               }))),
      takeLast(1),
      mergeMap(() => {
        // Ensure all resolvers returned values, otherwise don't emit any "next" and just complete
        // the chain which will cancel navigation
        if (Object.keys(data).length === keys.length) {
          return of(data);
        }
        return EMPTY;
      }),
  );

Source,

as you can see, if the resolve() fn didn't emit any values, an EMPTY observable will be emitted, which will in turn cause the navigation to be cancelled.


I'd also modify a bit your resolve function:

resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): any {
  const id = +route.paramMap.get('id');
  const language = route.queryParamMap.get('language');
  const request$ = this.service.get(id, language);

  return request$.pipe(
    tap(
      null, 
      err => this.router.navigateByUrl(AppRoutingConfig.search),
    ),
  );
}

I considered share() to be unnecessary, as you'll have only one subscriber.


So, a quick fix(which I wouldn't recommend at all) I think would be to also send the cached value when it's the case:

// Inside `service.get()`

else {
  // The value is cached

  of(getCachedValue(id)).pipe(
    subscribeOn(asyncScheduler) // delay(0) would also work
  ).subscribe(v => {
    requestSubject.next(v)
    requestSubject.complete()
  })
}

return requestSubject.

Another solution would be to consider adding the caching layer at the interceptors level, as I think this will simplify a lot your code. If you decide to switch to this approach, your resolve() fn can return something like this.service.get(...), because eveerything will be intercepted by the interceptors so you can return from there the cached values.

Andrei Gătej
  • 11,116
  • 1
  • 14
  • 31
2

In your case instead of Subject you need to use ReplaySubject, it remembers the last value and will emit it once someone subscribes.

Nevertheless I would change the get() to use a caching state for the stream.

//resolve function from resolver
resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): any {
  const id = +route.paramMap.get('id');
  const language = route.queryParamMap.get('language');
  return this.service.get(id, language).pipe(
    catchError(() => {
      this.router.navigateByUrl(AppRoutingConfig.search);
      return EMPTY; // to suppress the stream
    }),
  );
}


//the service get function
private readonly cache = new Map(); // our cache for urls


get(id: number, language: string): Observable<any> {
  const url = `some url`;

  // if we had the id - we return its observable.
  if (this.cache.has(id)) {
    return cache.get(id);
  }

  // otherwise we create a replay subject.
  const status$ = new ReplaySubject(1);
  this.cache.set(id, status$);

  this.http.get(url).subscribe(
    () => status$.next(),
    e => status$.error(e),
    () => requestSubject.complete(),
  );

  return status$.asObservable();
}
satanTime
  • 12,631
  • 1
  • 25
  • 73
  • Yeah, but if the `BehaviorSubject` completes (as it does when the value is cached), the subscriber will only receive the complete notification. Using `ReplaySubject(N)` will send the latest N values, although the subject has completed([source](https://github.com/ReactiveX/rxjs/blob/master/src/internal/ReplaySubject.ts#L115)). – Andrei Gătej Jun 01 '20 at 13:37
  • True, changed it to the `ReplaySubject`. – satanTime Jun 01 '20 at 15:17