0

I'm currently building a type ahead search. I'm trying to adapt it from the example here https://www.learnrxjs.io/learn-rxjs/recipes/type-ahead but I need to make an http call on keystroke. I'm testing it against timezones from my api. It works, but it seems to be compounding api calls every time I input a keyup.

If I type 'us' it will return results and make two identical api calls. If I add 'a' to make 'usa' it will then make 3 api calls. If I backspace a letter it makes 4, so on and so forth.

From my understanding switchMap is supposed to cancel calls that are going as newer ones come in, but it doesn't seem to do that with my implementation. I cannot for the life of me figure out why it's doing this when I keep going over the example. I tried to use .share() to attempt to consolidate streams but it didn't seem to help.

Search service:

results: any;

search(table: string, page: any, elementId: string) {
    const searchInput = document.getElementById(elementId);
    const keyup$ = fromEvent(searchInput, 'keyup');
    let searchQuery;
    keyup$.pipe(
      debounceTime(500),
      map((e: any) => searchQuery = e.target.value),
      distinctUntilChanged(),
      switchMap(value => this.apiDataService.getSearchData(this.apiDataService.apiBase + `/search?table=${table}&query=${searchQuery}&page=${page}`))
    )
    .subscribe(res => {
      this.results = res.body.data;
    });
  }

apiData service:

getSearchData(dataUrl: string): Observable<any> {
    return this.http.get<[]>(dataUrl, this.httpOptions);
}

html component:

<input id="timezoneSearch" placeholder="Search" [(ngModel)]="search.params" (keyup)="search.search('timezone', '1', 'timezoneSearch')" mdbInput type="text">


<div class="dropdown-item" *ngFor="let option of search.results"
 (click)="accordion.selectOption(obj.currentObject, option.uuid, 'admin', 'ktimezone')">                              
  {{option.name}}
</div>
Dmitry S.
  • 1,544
  • 2
  • 13
  • 22
Goht
  • 46
  • 7
  • You're creating multiple subscribers everytime you keyup - that's not the right way to setup your subscriptions – Michael Kang Mar 13 '21 at 00:09
  • I'm aware I'm creating multiple subscribers, I detailed that in my post. Can you clarify how to set them up properly? – Goht Mar 13 '21 at 04:52
  • because of `(keyup)="search.search(...)"` your `search` function is being called multiple times as you type. That means for each `keyup` event a separate `keyup$` stream is created. It is correct that `switchMap` ensures that you only have one inner observable at a time within *one* stream. But here you have multiple stream with multiple `switchMap` instances that have no connection with each other. – kruschid Mar 13 '21 at 18:50
  • Intuitively I would suggest you to bind the `keyup$` stream to the lifecycle of your view component (similar to @Loops answer). Just keep one subscription for the time the component is displayed and unsubscribe inside `ngOnDestroy` – kruschid Mar 13 '21 at 18:51

2 Answers2

2

You can just subscribe to the input events of the input element:

<input #timezoneSearch placeholder="Search" [(ngModel)]="search.params" mdbInput type="text">
  @ViewChild('timezoneSearch') timezoneSearch: ElementRef;

  ngAfterViewInit() {
    fromEvent(this.timezoneSearch.nativeElement, 'input').pipe(
      debounceTime(500),
      map((e: InputEvent) => e.target.value),
      switchMap(value => this.apiDataService.getSearchData(this.apiDataService.apiBase + `/search?table=${'timezone'}&query=${value}&page=${1}`))
    ).subscribe(
      res => this.results = res.body.data
    );

  }

Furthermore, you don't need to put the results in a field, but instead directly use the Observable in the template:

<div *ngFor="let option of results$ | async"
 ...
</div>
  ngAfterViewInit() {
    this.results$ = fromEvent(this.timezoneSearch.nativeElement, 'input').pipe(
      debounceTime(500),
      map((e: InputEvent) => e.target.value),
      switchMap(value => this.apiDataService.getSearchData(this.apiDataService.apiBase + `/search?table=${'timezone'}&query=${value}&page=${1}`)),
      map(res => res.body.data)
    );
Ozan
  • 4,345
  • 2
  • 23
  • 35
0

Just subscribe once (probably in ngAfterViewInit() since you are accessing the dom):

 const searchInput = document.getElementById('timezoneSearch');
    const keyup$ = fromEvent(searchInput, 'keyup');
    keyup$.pipe(
      debounceTime(500),
      distinctUntilChanged(), // I do not think is actually necessary
      switchMap(value => this.apiDataService.getSearchData(this.apiDataService.apiBase + `/search?table=${'timezone'}&query=${value}&page=${1}`))
    )
    .subscribe(res => {
      this.results = res.body.data;
    });

Another possibility would be to add a take(1) in your pipe, which seems a little odd to me but would provide you with the ability to provide the parameters as you did via html.

Loop
  • 480
  • 2
  • 9
  • That could work but he would need to unsubscribe unless the search field is displayed all the time. `distinctUntilChanged` could be necessary in some cases (eg. selecting with arrow keys). I have the feeling `take(1)` couldn't fix the issue. It still would send multiple search requests with stale parameter (one per additionally created stream). – kruschid Mar 13 '21 at 19:00