Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

typeahead subscription #2382

Closed
danielGz opened this issue Aug 8, 2017 · 2 comments
Closed

typeahead subscription #2382

danielGz opened this issue Aug 8, 2017 · 2 comments

Comments

@danielGz
Copy link

danielGz commented Aug 8, 2017

I just stumbled upon some question when thinking about any possible memory leak in my application:
Since the typeahead Observable is provided we never get a chance to unsubscribe from it as the subscription happens inside the directive code, should we need to worry about this?

I implemented my own solution and so far it seems that no negative secondary effects were caused:

 protected asyncActions(): void {
    this.typeaheadSubscription=this.keyUpEventEmitter
      .debounceTime(this.typeaheadWaitMs)
      .mergeMap(() => this.typeahead)
      .subscribe(
        (matches: any[]) => {
          this.finalizeAsyncCall(matches);
        },
        (err: any) => {
          console.error(err);
        }
      );
  }

and inside ngOnDestroy :

public ngOnDestroy(): any {
    this._typeahead.dispose();
    this.typeaheadSubscription.unsubscribe();    
  }
@valorkin
Copy link
Member

or like here

this._subscriptions.push(this._state
.isOpenChange.subscribe((value: boolean) => this.isOpen = value));
// populate disabled state
this._subscriptions.push(this._state
.isDisabledChange
.subscribe((value: boolean) => this.isDisabled = value || null));
}
ngOnDestroy(): void {
for (const sub of this._subscriptions) {
sub.unsubscribe();
}
}

anyway, you are right and this is
a good reason for small PR :)

krooshua added a commit to krooshua/ngx-bootstrap that referenced this issue Aug 24, 2017
valor-software#2382

Also fixed bug, when navigating to other router before asyncActions() end.
@danielGz
Copy link
Author

Thanks!

valorkin pushed a commit that referenced this issue Aug 29, 2017
#2382

Also fixed bug, when navigating to other router before asyncActions() end.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants