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

nzTable 通过 *ngFor 动态生成列导致 nzSortChange 反复触发 #3603

Closed
AngusFu opened this issue Jun 19, 2019 · 5 comments · Fixed by #3605
Closed

nzTable 通过 *ngFor 动态生成列导致 nzSortChange 反复触发 #3603

AngusFu opened this issue Jun 19, 2019 · 5 comments · Fixed by #3605

Comments

@AngusFu
Copy link
Contributor

AngusFu commented Jun 19, 2019

Reproduction link

https://stackblitz.com/edit/angular-qq6zi1?file=src/app/app.component.ts

Steps to reproduce

  1. 指定 columns: Array<{ key: 'string', title: string }> 用于动态设置 table 所展示的列
  2. 设置 <thead [nzSingleSort]="true" (nzSortChange)="onSortChange($event)">
  3. 通过 *ngFor 生成一系列的 <th nzShowSort [nzSortKey]="col.key">{{ col.title }}</th>
  4. 点击任意 th 进行排序操作,发现只触发一次 onSortChange
  5. 更改 columns 数组,如删除一项
  6. 点击任意 th 进行排序操作,发现 onSortChange 会触发两次
  7. 重复 5、6 操作,发现 onSortChange 会触发三次
  8. 如此等等

视频

What is expected?

nzSortChange 不应当反复触发

What is actually happening?

nzSortChange 反复触发

Environment Info
ng-zorro-antd 7.5.1
Browser Chrome/74.0.3729.169
@zorro-bot
Copy link

zorro-bot bot commented Jun 19, 2019

Translation of this issue:

nzTable causes nzSortChange to be triggered repeatedly by *ngFor dynamically generating columns

Reproduction link

[https://stackblitz.com/edit/angular-qq6zi1?file=src/app/app.component.ts](https://stackblitz.com/edit/angular-qq6zi1?file=src/app/app. Component.ts)

Steps to reproduce

  1. Specify columns: Array<{ key: 'string', title: string }> to dynamically set the columns displayed by table
  2. Set <thead [nzSingleSort]="true" (nzSortChange)="onSortChange($event)">
  3. Generate a series of <th nzShowSort [nzSortKey]="col.key">{{ col.title }}</th> by *ngFor`.
  4. Click on any of the th sorting operations and find that only one time is called onSortChange.
  5. Change the columns array, such as deleting an item
  6. Click on any th to sort and find that onSortChange will fire twice.
  7. Repeat steps 5 and 6 and find that onSortChange will trigger three times.
  8. So, etc.

[Video] (https://s3.ssl.qhres.com/static/18b135736269b69b.mp4)

What is expected?

nzSortChange should not be triggered repeatedly

What is actually happening?

nzSortChange is triggered repeatedly

Environment Info
ng-zorro-antd 7.5.1
Browser Chrome/74.0.3729.169

@AngusFu
Copy link
Contributor Author

AngusFu commented Jun 19, 2019

@vthinkxie 大概定位了 bug 的原因。主要在这里:components/table/nz-thead.component.ts#L66

flatMap(() =>
  merge<{ key: string; value: string }>(...this.listOfNzThComponent.map(th => th.nzSortChangeWithKey))
),

写了段原理类似的代码 ——

import { merge, Subject } from "rxjs";
import { startWith, flatMap } from "rxjs/operators";

const subject = new Subject();
const array = [new Subject(), new Subject(), new Subject()];

subject
  .pipe(
    startWith(true),
    flatMap(() => {
      console.log(array.length);
      return merge(...array);
    })
  )
  .subscribe(console.log);

array[0].next("hello");
array[1].next("world");
array.pop().next("!");

subject.next(123);
array[0].next("hello");
array[1].next("world");

执行效果如下:

image

@AngusFu
Copy link
Contributor Author

AngusFu commented Jun 19, 2019

上面的例子 flatMap 换成 switchMap 就好了。我提个 PR ?

import { merge, Subject } from "rxjs";
import { startWith, switchMap } from "rxjs/operators";

const subject = new Subject();
const array = [new Subject(), new Subject(), new Subject()];

subject
  .pipe(
    startWith(true),
    switchMap(() => {
      console.log(array.length);
      return merge(...array);
    })
  )
  .subscribe(console.log);

array[0].next("hello");
array[1].next("world");
array.pop().next("!");

subject.next(123);
array[0].next("hello");
array[1].next("world");

image

@vthinkxie
Copy link
Member

vthinkxie commented Jun 19, 2019

@AngusFu 非常感谢

@AngusFu
Copy link
Contributor Author

AngusFu commented Jun 19, 2019

@vthinkxie 🙂 刚刚搜了下,貌似整个 repo 中还有另外三个地方使用了 flatMap,其出现的场景大体和本 issue 类似((<ContentChildren>something).changes )。table 中有一个,submenu 和 select 中也各有一个。建议可以确认排查下。

https://github.com/NG-ZORRO/ng-zorro-antd/search?q=flatMap&unscoped_q=flatMap

hsuanxyz added a commit to hsuanxyz/ng-zorro-antd that referenced this issue Jun 21, 2019
* chore: sync ant-design v3.19.6 (NG-ZORRO#3602)

* fix(module:table): fix sortChange with dynamic columns (NG-ZORRO#3603) (NG-ZORRO#3605)

close NG-ZORRO#3603

* chore: sync with antd 3.19 (NG-ZORRO#3590)

* chore: sync with antd 3.19

* test(module:page-header): fix test

* chore: refactor commit hook (NG-ZORRO#3610)

* chore: refactor commit hook

* chore(packaging): clean dependencie

* chore(packaging): clean dev dependencie

* chore: refactor commit hook (NG-ZORRO#3610)

* chore: refactor commit hook

* chore(packaging): clean dependencie

* chore(packaging): clean dev dependencie

* chore: refactor commit hook (NG-ZORRO#3610)

* chore: refactor commit hook

* chore(packaging): clean dependencie

* chore(packaging): clean dev dependencie

* build: refactor scripts

ref NG-ZORRO#2474

* chore: update ci config

* ci: fix integration

* ci: add travis_wait

* ci: fix ci

* ci: fix ci

* ci: fix ci

* ci: fix ci

* ci: fix ci

* chore: fix type

* build: add site release scripts

* build: _

* build: add `test:watch` task

* build: fix `test:watch` task

* chore: bump node to 12.1.0

* ci: add now.json
hsuanxyz added a commit to hsuanxyz/ng-zorro-antd that referenced this issue Jun 21, 2019
* chore: sync ant-design v3.19.6 (NG-ZORRO#3602)

* fix(module:table): fix sortChange with dynamic columns (NG-ZORRO#3603) (NG-ZORRO#3605)

close NG-ZORRO#3603

* chore: sync with antd 3.19 (NG-ZORRO#3590)

* chore: sync with antd 3.19

* test(module:page-header): fix test

* chore: refactor commit hook (NG-ZORRO#3610)

* chore: refactor commit hook

* chore(packaging): clean dependencie

* chore(packaging): clean dev dependencie

* chore: refactor commit hook (NG-ZORRO#3610)

* chore: refactor commit hook

* chore(packaging): clean dependencie

* chore(packaging): clean dev dependencie

* chore: refactor commit hook (NG-ZORRO#3610)

* chore: refactor commit hook

* chore(packaging): clean dependencie

* chore(packaging): clean dev dependencie

* build: refactor scripts

ref NG-ZORRO#2474

* chore: update ci config

* ci: fix integration

* ci: add travis_wait

* ci: fix ci

* ci: fix ci

* ci: fix ci

* ci: fix ci

* ci: fix ci

* chore: fix type

* build: add site release scripts

* build: _

* build: add `test:watch` task

* build: fix `test:watch` task

* chore: bump node to 12.1.0

* ci: add now.json
hsuanxyz added a commit to hsuanxyz/ng-zorro-antd that referenced this issue Jun 21, 2019
* chore: sync ant-design v3.19.6 (NG-ZORRO#3602)

* fix(module:table): fix sortChange with dynamic columns (NG-ZORRO#3603) (NG-ZORRO#3605)

close NG-ZORRO#3603

* chore: sync with antd 3.19 (NG-ZORRO#3590)

* chore: sync with antd 3.19

* test(module:page-header): fix test

* chore: refactor commit hook (NG-ZORRO#3610)

* chore: refactor commit hook

* chore(packaging): clean dependencie

* chore(packaging): clean dev dependencie

* chore: refactor commit hook (NG-ZORRO#3610)

* chore: refactor commit hook

* chore(packaging): clean dependencie

* chore(packaging): clean dev dependencie

* chore: refactor commit hook (NG-ZORRO#3610)

* chore: refactor commit hook

* chore(packaging): clean dependencie

* chore(packaging): clean dev dependencie

* build: refactor scripts

ref NG-ZORRO#2474

* chore: update ci config

* ci: fix integration

* ci: add travis_wait

* ci: fix ci

* ci: fix ci

* ci: fix ci

* ci: fix ci

* ci: fix ci

* chore: fix type

* build: add site release scripts

* build: _

* build: add `test:watch` task

* build: fix `test:watch` task

* chore: bump node to 12.1.0

* ci: add now.json

* ci: add now.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants