Doesn't Cancel Observable using async

[DrakeAnglin]

See example Below.

Expected results: When using the Async pipe and navigating to a new route, any pending requests should be canceled.

Actual Results: Pending requests stay open until completed. This causes issues in performance, since when you navigate to a new route the old request(s) is taking up request threads.

In the example below, you can see when you navigate from route1 to route2 the request is canceled when not using the decorator. Adding the decorator to the request keeps the request open even after navigating away. I understand that you may want to go back to that request later, but if it doesn't complete i dont think it should be kept.

This same thing happens when you make a different request to a cached function, the original request is still pending and is not canceled, even if you only have the default one request set to cache. I would only expect that it would keep the previous request if the max cache count is set to more than one. this becomes a large performance issue when you have an app that makes multiple new requests after some parameter changes, all the old request are still pending and are processed, then the new request are sent off.

Example

[angelnikolov]

Thank you for reporting this issue! Even at a first glance this seems like a performance regression indeed. I am currently out on a holiday but once I get back I will take a look:)

[angelnikolov]

@DrakeAnglin Just to update you, I am working on some changes which will solve that issue for you. Except more next week :)

[angelnikolov]

@DrakeAnglin I might have something but now that I do I am starting to think if this is the correct behaviour. You can check the branch diff here. However, think about the following case: You have two identical components on the same page, both requesting data from the same endpoint, at the same time, but with different parameters. Imagine they are User Cards loading user profile info for two different users from /api/users/${userId}. If we just cancel the first request in favor of the second, the first user-card will remain empty. I don't think this is a good thing to implement in such a low level caching functionality and would rather leave that to the user. However, I am still going to take a look at why the requests still go out, even if the caller has unsubscribed (route has changed). The issue with the unsubscription from other routes lays within the shareReplay operator I've used, but let me dig deeper.

[angelnikolov]

Okay the shareReplay is definitely an issue, since it will actually replay the previous request, even though you expect it to be stopped and let the new one take its place.

[angelnikolov]

@DrakeAnglin, can you please update to 1.2.7 and verify if the issue has been fixed? Thanks!

[DrakeAnglin]

Ok so I have tested and found some issues. Though the above issue is fixed, this has introduced a issue with the observable not being cached properly and a new call to the server being generated when multiple requests are made.

The "shareReplay" pipe hade changed in version 5.5 and now no longer unsubscribes automatically. The solution is to use the "publishReplay" and "refCount" pipes.

I was able to fix this on my local computer by adding in place of the shareReplay pipe you had before. publishReplay(1), refCount(),

This article goes into more detail: RxJS: How to Use refCount

[robert-king]

I think i'm getting something similar. When my app loads, I get many services calling getUser() at the same time, which causes many duplicate HTTP calls at the same time.

@Cacheable()
  public getUser(): Observable<UserDetailsViewModel> {
    return this.userDetailService.UserDetailGetUserDetails()
      .pipe(
        retry(3),
        catchError((e) => {
          this.showErrorService.showError(
            'Fatal Error', 'Error loading user profile, please try again later'
          );
          return throwError(e);
        })
      );
  }

my code before which worked, however i'd like to use the decorator instead:

@Injectable({
  providedIn: 'root'
})
export class UserService {
  private user$: Observable<UserDetailsViewModel> = null;

  constructor(
    private userDetailService: UserDetailService,
    private showErrorService: ShowErrorService,
  ) {}

  public getUser(): Observable<UserDetailsViewModel> {
    if (this.user$ === null) {
      this.user$ = this.userDetailService.UserDetailGetUserDetails()
        .pipe(
          retry(3),
          publishReplay(1),
          refCount(),
          catchError((e) => {
            this.showErrorService.showError(
              'Fatal Error', 'Error loading user profile, please try again later'
            );
            return of(e);
          })
        );
    }
    return this.user$;
  }

  public clearUserCache() {
    this.user$ = null;
  }

  public isStudent(): Observable<boolean> {
    return this.getUser().pipe(map(
      (user: UserDetailsViewModel) => user.adfsUserType === 2
    ));
  }

  public isParent(): Observable<boolean> {
    return this.getUser().pipe(map(
      (user: UserDetailsViewModel) => user.adfsUserType === 1
    ));
  }
}

@Pipe({
  name: 'isParent'
})
export class IsParentPipe implements PipeTransform {
  constructor(
    private userService: UserService,
  ) {
  }

  transform(value: any, args?: any): Observable<boolean> {
    return this.userService.isParent();
  }

}

The decorator does work well though if there's not a one-to-many fanout.

[angelnikolov]

I can confirm that the issue exists. Let me dig deeper :) Thanks guys!

[angelnikolov]

@DrakeAnglin @robert-king Published 1.2.8. Thank you for your reports and I hope this resolved your issues :)