jfcere / ngx-malihu-scrollbar

Angular 2+ scrollbar customization using Malihu jQuery Custom Scrollbar plugin
https://jfcere.github.io/ngx-malihu-scrollbar
MIT License
58 stars 17 forks source link

The malihu scroll bar seems keep firing endless events, which makes the angular DoCheck life circle hook runs over and over again #16

Closed rdkmaster closed 7 years ago

rdkmaster commented 7 years ago

reproduce this issue

follow this to reproduce this issue, all code is based on the repo:

  1. add the following lines to the head of malihu-scrollbar-demo.component.html
<button (click)="get()">click</button>
<div *ngFor="let theme of themes; trackBy: track">
  <!--do nothing here-->
</div>
  1. add the following line in the class MalihuScrollbarDemoComponent defined in malihu-scrollbar-demo.component.ts
  track() {
    if (!window['cc']) {
      window['cc'] = 1;
    }
    window['cc']++;
  }

  get() {
    console.log(window['cc']);
  }
  1. recompile the repo, open the console of the browser, wait for seconds and click the button at the head of the page. it look like this in my browser:

image

  1. if everything goes right, your console should look like this (btw I clicked several times )

image

this is a series performance problem

as we known, the ngFor implements the DoCheck life cicle hook, which makes the ngFor very sensitive to any change, because that after any event, the ngDoCheck of ngFor will be invoked. It seems that malihu keep firing event and make angular executes the ngDoCheck hook time and time again.

How to fix this?

jfcere commented 7 years ago

Hi @rdkmaster,

Thanks for pointing it out, following your steps I've been able to improve the performance.

Please update to v1.2.1 and give me some feedback.

image

rdkmaster commented 7 years ago

I've look into the source code of malihu scrollbar, I found that the scrollbar's implementation is using setTimeout to perform a loop check and thus to perform scrolling, it update in 50ms in default. That is to say, the implementation will invoke setTimeout time and time again as soon as it initialized.

As we have already known, everytime after setTimeout, zone of Angular will run the change detection, and the DoCheck life cycle will be invoked, and this is the reason that causes this issue. I check latest commit of malihu scrollbar, it is more that one year ago, and I think the author is not likely to fix this issue, I also sadly think this issue is NOT fixable, because it is the base of the implementation.

I've decided to give up this scrollbar implementation.

Btw, you can take a look at my repo, we've made 30 components based on Angular including a scrollbar directive, and the number is increasing, you can find it here https://github.com/rdkmaster/jigsaw, join us if you like it.

rdkmaster commented 7 years ago

@jfcere you need to leave at least one scrollbar in the view, this is the key to this issue, remove all scrollbars in the view and angular will not initialize malihu-scrollbar anymore, and the setTimeout will not be invoked.

jfcere commented 7 years ago

I did pretty much the best I could do on my side by improving the change detection for this Angular wrap but of course it won't fix the existing behavior or the original Malihu Scrollbar plugin.