kamilkisiela / apollo-angular

A fully-featured, production ready caching GraphQL client for Angular and every GraphQL server 🎁
https://apollo-angular.com
MIT License
1.5k stars 309 forks source link

WatchQuery dispose leaking #469

Closed stephenlautier closed 5 years ago

stephenlautier commented 6 years ago

When using the .watchQuery, is there a way to dispose it? Not sure if its a bug or I'm misusing it, but it seems to be leaking without somehow disposing it.

I followed the documentation but all the usages doesnt have any dispose, I also unsubscribe from the .valueChanges.subscribe but it's still alive.

This is what I'm doing:

image

If you would like to see how I'm using it exactly you can do so on https://github.com/sketch7/angular-skeleton-app/blob/feature/odin-gql/src/app/areas/heroes/heroes.component.ts#L30 Unfortunately on that branch, with the API you cannot just run it. p.s. I also tried to put everything in the UI for the sake of it, but same issue

kamilkisiela commented 6 years ago

I failed to reproduce it by simply using Apollo service inside a component.

https://stackblitz.com/edit/apollo-angular-469

Apollo Dev Tools shows one watched query and I get only one log.

stephenlautier commented 6 years ago

@kamilkisiela in that it seems its works fine, I will try to use that API in my repo and see if it works properly or not.

kamilkisiela commented 6 years ago

@stephenlautier How it works? Maybe you could prepare a small reproduction of it?

stephenlautier commented 6 years ago

yes i will try replicate what you had in mine, and ill update you

stephenlautier commented 6 years ago

Just an update, so far I've changed my implementation to simply use the gql api you used, and did some changes to comment out some private lib we have... and its working fine now.

I can give last shot again at work exactly how it was, and maybe reinstall node modules and see if it happens agian.. but i had tested this several times and it was reproducible 100% - if i didnt had the screenshot I would think I was hallucinating =(

intellix commented 6 years ago

think I'm seeing something similar locally. Not sure if it's related entirely but I have the following:

private subscriptions = new Subscription();
private gamesQueryRef = this.apollo.watchQuery({ query, variables });

games$: Observable<any> = this.gamesQueryRef.valueChanges.pipe(
  map(({ data }) => data.games.edges.map(edge => edge.node)),
  shareReplay(1),
);

countdown$ = this.games$.pipe(
  map(games => new Date(games[0].scheduledAt)),
  share(),
);

ngOnInit() {
  this.subscriptions.add(this.games$.pipe(take(1)).subscribe(console.log));
  this.subscribeToMore();
}

ngOnDestroy() {
  this.subscriptions.unsubscribe();
}

subscribeToMore() {
  this.gamesQueryRef.subscribeToMore({ document, updateQuery: (prev: any, { subscriptionData }) => { ... });
}

With the shareReplay(1) and subscribeToMore, leaving the page doesn't stop the subscription so they're piling up. With share() it does clean up but returning with instant cache causes countdown$ to miss the emit.

There's no view and the ngOnDestroy is definitely run so I'd say that apollo-angular or apollo-client isn't able to clean up the subscribeToMore for some reason. It's getting late but I'll try to create a reproduction over the weekend outside of my codebase

ntziolis commented 6 years ago

Seeing the same behaviour still, anybody figured out a way to stop the leaking?

intellix commented 6 years ago

My issue in particular was probably shareReplay not unsubscribing: https://github.com/ReactiveX/rxjs/issues/3336

kamilkisiela commented 5 years ago

Is this still an issue?

stephenlautier commented 5 years ago

@kamilkisiela doest seems like it, I will close it, if anyone has the same issue will reopen if needed

gituser3000 commented 2 years ago

shareReplay(1)causes leaks, but as far as i understand it should not (using destroy subjects\ async pipe) . shareReplay({refCount: true}) fixes this, but this solution may not be appropriate