Pagination. Call pageChange to many times on Init

[kolkov]

1.0.0-alpha.8
why pageChange calls to much at view init with NAN as event?

What could be the problem?

[pkozlowski-opensource]

What could be the problem?

No idea without a minimal reproduce scenario: https://github.com/ng-bootstrap/ng-bootstrap/blob/master/README.md#you-think-youve-found-a-bug

[kolkov]

Once this code here affects, it is not clear ...

getActorsList(): void{
      this.actorService
        .getActorsList(this.currentPage, this.itemsPerPage)
        .subscribe(actorList => {
          this.actors = actorList.Actors;
          this.totalItems = actorList.Total;
      });
      /*this.actors = [{"Id":1,"PersonId":0,"Alias":"","ActorAvatarUrl":"","CreatedAt":"2016-08-11T22:28:40+03:00","UpdatedAt":"2016-08-12T00:25:01+03:00","VideoTotal":0,"PhotoTotal":0,"UserId":0,"LastName":"Бондарчук","FirstName":"Федор","Patronymic":"Сергеевич","Birthday":"1967-05-09T00:00:00+03:00","Gender":"male","AvatarUrl":"/img/actor/1/4aedc48e-4379-4f41-aefa-28e64a160652.png"},{"Id":2,"PersonId":0,"Alias":"","ActorAvatarUrl":"","CreatedAt":"2016-08-12T00:24:31+03:00","UpdatedAt":"2016-08-12T00:24:31+03:00","VideoTotal":0,"PhotoTotal":0,"UserId":0,"LastName":"Меньшов","FirstName":"Владимир","Patronymic":"Валентинович","Birthday":"1939-09-17T00:00:00+03:00","Gender":"male","AvatarUrl":"/img/actor/2/4ef89444-34d7-46d0-99f9-d0a666e188e0.png"},{"Id":3,"PersonId":0,"Alias":"fdgfdgf","ActorAvatarUrl":"","CreatedAt":"2016-10-11T18:02:05+03:00","UpdatedAt":"2016-10-11T18:02:05+03:00","VideoTotal":0,"PhotoTotal":0,"UserId":0,"LastName":"dgdg","FirstName":"dgdgdf","Patronymic":"dgfdgfg","Birthday":"2016-09-28T00:00:00+03:00","Gender":"male","AvatarUrl":"/img/actor/3/0fce612e-9ce6-4fa4-99a0-177f47672132.png"},{"Id":4,"PersonId":0,"Alias":"Иванушка","ActorAvatarUrl":"","CreatedAt":"2016-10-12T00:16:10+03:00","UpdatedAt":"2016-10-12T00:16:10+03:00","VideoTotal":0,"PhotoTotal":0,"UserId":0,"LastName":"Иванов","FirstName":"Иван","Patronymic":"Иванович","Birthday":"2010-02-02T00:00:00+03:00","Gender":"male","AvatarUrl":"/img/actor/4/1e7c0bb6-bd24-406b-a754-77a118a15552.png"},{"Id":5,"PersonId":0,"Alias":"Петя","ActorAvatarUrl":"","CreatedAt":"2016-10-12T03:23:29+03:00","UpdatedAt":"2016-10-12T03:23:29+03:00","VideoTotal":0,"PhotoTotal":0,"UserId":0,"LastName":"Петров","FirstName":"Петр","Patronymic":"Петрович","Birthday":"2013-10-01T00:00:00+03:00","Gender":"male","AvatarUrl":"/img/actor/5/cc4c18b0-0883-4c58-8d4c-c7f1eb57c7aa.png"}];
      this.totalItems = 6;*/
    }

In the service:

getActorsList(currentPage, itemsPerPage): Observable<ActorList>{

    let params = new URLSearchParams();
    params.set('page', currentPage);
    params.set('limit', itemsPerPage);
    let options = new RequestOptions({
      headers: this.headers,
      search: params
    });

    return this.http.get('/api/v1/actor/pagination', options)
      .map((response: Response) => response.json());
  }

что здесь может быть не так ума не приложу... (

[kolkov]

Plunker without service

[kolkov]

working plunker
http://plnkr.co/edit/Mv30YF7iMiqKo5VfnFM7?p=preview

[kolkov]

When I come back to the version "@ng-bootstrap/ng-bootstrap": "=1.0.0-alpha.7", problem is disappeared.

[kolkov]

Very strange construction:

ng-bootstrap/src/pagination/pagination.ts

Line 139 in 84d555c

this._setPageInRange(this.page);

[wesleycho]

My guess is what is happening is that totalItems is not set initially, so inside the pagination, this.collectionSize is not a number, which transforms this._pageCount into NaN when ngOnChanges first runs. A simple workaround is to give it an initial value of 0, as in this Plunker.

Given that, I don't think this is an issue with the library, unless we want to guard against this situation and default to 0 if Math.ceil results in NaN, which may not be the worst thing in the world.

[pkozlowski-opensource]

Given that, I don't think this is an issue with the library, unless we want to guard against this situation and default to 0 if Math.ceil results in NaN, which may not be the worst thing in the world.

Yup, this wouldn't hurt.

[pkozlowski-opensource]

Actually, @kolkov is right - with 84d555c we are calling _setPageInRange more often and this one in turns raises an event. NaN case is another story and should be fixed as well.

@maxokorokov could you take a look at this one as it is regression introduced by 84d555c? I also agree with @kolkov that invoking noOnChanges is a bizarre construct, we should probably extract common code into a separate method.

[maxokorokov]

Actually we were emitting multiple times only because current page was NaN and NaN !== NaN is true...

The real issue for me is that totaltems and other inputs are not initialized → so I suggest we throw exceptions for non-numeric values in this case as in pull request above

[pkozlowski-opensource]

so I suggest we throw exceptions for non-numeric values in this case as in pull request above

I'm not sure throwing is the best option here as it will break all the people that initialize data asynchronously from the server-side... Maybe we should just assume that there are 0 pages if one of the inputs is invalid? It doesn't sound great either, but yeh, we need to make sure that we support async usage scenarios...

[maxokorokov]

I don't see the how it is related to async usage scenarios at all. I mean you can do whatever you want if you initialize your collection with an empty one, or your collection size with 0, right?

It's about guarding against incorrect inputs:

• should we eat them silently and still render something (what?)
• or should we tell that we expect something else

For me the second way seems clearer; But I don't have a really strong opinion on this one - it's just have to be consistent across components.

[Civile]

In my specific case I resolved giving a default value to my page property (this.page = 1). It was undefined and now its 1, this way I prevent the automatic change from undefined to 1 and the resulting multiple call of the event (pageChange).