perf(scroll-dispatcher): avoid triggering change detection on scroll (#3687)

* Avoids triggering change detection when listening to global scroll events in the `ScrollDispatcher`.
* Avoids triggering change detection when scrolling inside of `Scrollable` instances.
* Switches a `ScrollDispatcher` test to use spies, instead of toggling booleans.
This commit is contained in:
Kristiyan Kostadinov
2017-03-27 19:16:12 +02:00
committed by Jeremy Elbourn
parent 75996b51b5
commit 5c2b44903d
5 changed files with 67 additions and 21 deletions
@@ -534,7 +534,9 @@ describe('ConnectedPositionStrategy', () => {
fakeElementRef,
{originX: 'start', originY: 'bottom'},
{overlayX: 'start', overlayY: 'top'});
strategy.withScrollableContainers([new Scrollable(new FakeElementRef(scrollable), null)]);
strategy.withScrollableContainers([
new Scrollable(new FakeElementRef(scrollable), null, null, null)]);
positionChangeHandler = jasmine.createSpy('positionChangeHandler');
onPositionChangeSubscription = strategy.onPositionChange.subscribe(positionChangeHandler);
@@ -41,15 +41,15 @@ describe('Scroll Dispatcher', () => {
it('should notify through the directive and service that a scroll event occurred',
fakeAsync(() => {
let hasDirectiveScrollNotified = false;
// Listen for notifications from scroll directive
let scrollable = fixture.componentInstance.scrollable;
scrollable.elementScrolled().subscribe(() => { hasDirectiveScrollNotified = true; });
const scrollable = fixture.componentInstance.scrollable;
const directiveSpy = jasmine.createSpy('directive scroll callback');
scrollable.elementScrolled().subscribe(directiveSpy);
// Listen for notifications from scroll service with a throttle of 100ms
const throttleTime = 100;
let hasServiceScrollNotified = false;
scroll.scrolled(throttleTime, () => { hasServiceScrollNotified = true; });
const serviceSpy = jasmine.createSpy('service scroll callback');
scroll.scrolled(throttleTime, serviceSpy);
// Emit a scroll event from the scrolling element in our component.
// This event should be picked up by the scrollable directive and notify.
@@ -57,16 +57,38 @@ describe('Scroll Dispatcher', () => {
dispatchFakeEvent(fixture.componentInstance.scrollingElement.nativeElement, 'scroll');
// The scrollable directive should have notified the service immediately.
expect(hasDirectiveScrollNotified).toBe(true);
expect(directiveSpy).toHaveBeenCalled();
// Verify that the throttle is used, the service should wait for the throttle time until
// sending the notification.
expect(hasServiceScrollNotified).toBe(false);
expect(serviceSpy).not.toHaveBeenCalled();
// After the throttle time, the notification should be sent.
tick(throttleTime);
expect(hasServiceScrollNotified).toBe(true);
expect(serviceSpy).toHaveBeenCalled();
}));
it('should not execute the global events in the Angular zone', () => {
const spy = jasmine.createSpy('zone unstable callback');
const subscription = fixture.ngZone.onUnstable.subscribe(spy);
scroll.scrolled(0, () => {});
dispatchFakeEvent(document, 'scroll');
dispatchFakeEvent(window, 'resize');
expect(spy).not.toHaveBeenCalled();
subscription.unsubscribe();
});
it('should not execute the scrollable events in the Angular zone', () => {
const spy = jasmine.createSpy('zone unstable callback');
const subscription = fixture.ngZone.onUnstable.subscribe(spy);
dispatchFakeEvent(fixture.componentInstance.scrollingElement.nativeElement, 'scroll');
expect(spy).not.toHaveBeenCalled();
subscription.unsubscribe();
});
});
describe('Nested scrollables', () => {
@@ -1,4 +1,4 @@
import {Injectable, ElementRef, Optional, SkipSelf} from '@angular/core';
import {Injectable, ElementRef, Optional, SkipSelf, NgZone} from '@angular/core';
import {Scrollable} from './scrollable';
import {Subject} from 'rxjs/Subject';
import {Observable} from 'rxjs/Observable';
@@ -17,6 +17,8 @@ export const DEFAULT_SCROLL_TIME = 20;
*/
@Injectable()
export class ScrollDispatcher {
constructor(private _ngZone: NgZone) { }
/** Subject for notifying that a registered scrollable reference element has been scrolled. */
_scrolled: Subject<void> = new Subject<void>();
@@ -69,10 +71,12 @@ export class ScrollDispatcher {
this._scrolledCount++;
if (!this._globalSubscription) {
this._globalSubscription = Observable.merge(
Observable.fromEvent(window.document, 'scroll'),
Observable.fromEvent(window, 'resize')
).subscribe(() => this._notify());
this._globalSubscription = this._ngZone.runOutsideAngular(() => {
return Observable.merge(
Observable.fromEvent(window.document, 'scroll'),
Observable.fromEvent(window, 'resize')
).subscribe(() => this._notify());
});
}
// Note that we need to do the subscribing from here, in order to be able to remove
@@ -118,13 +122,14 @@ export class ScrollDispatcher {
}
}
export function SCROLL_DISPATCHER_PROVIDER_FACTORY(parentDispatcher: ScrollDispatcher) {
return parentDispatcher || new ScrollDispatcher();
export function SCROLL_DISPATCHER_PROVIDER_FACTORY(parentDispatcher: ScrollDispatcher,
ngZone: NgZone) {
return parentDispatcher || new ScrollDispatcher(ngZone);
}
export const SCROLL_DISPATCHER_PROVIDER = {
// If there is already a ScrollDispatcher available, use that. Otherwise, provide a new one.
provide: ScrollDispatcher,
deps: [[new Optional(), new SkipSelf(), ScrollDispatcher]],
deps: [[new Optional(), new SkipSelf(), ScrollDispatcher], NgZone],
useFactory: SCROLL_DISPATCHER_PROVIDER_FACTORY
};
+20 -3
View File
@@ -1,5 +1,6 @@
import {Directive, ElementRef, OnInit, OnDestroy} from '@angular/core';
import {Directive, ElementRef, OnInit, OnDestroy, NgZone, Renderer} from '@angular/core';
import {Observable} from 'rxjs/Observable';
import {Subject} from 'rxjs/Subject';
import {ScrollDispatcher} from './scroll-dispatcher';
import 'rxjs/add/observable/fromEvent';
@@ -13,22 +14,38 @@ import 'rxjs/add/observable/fromEvent';
selector: '[cdk-scrollable]'
})
export class Scrollable implements OnInit, OnDestroy {
private _elementScrolled: Subject<Event> = new Subject();
private _scrollListener: Function;
constructor(private _elementRef: ElementRef,
private _scroll: ScrollDispatcher) {}
private _scroll: ScrollDispatcher,
private _ngZone: NgZone,
private _renderer: Renderer) {}
ngOnInit() {
this._scrollListener = this._ngZone.runOutsideAngular(() => {
return this._renderer.listen(this.getElementRef().nativeElement, 'scroll', (event: Event) => {
this._elementScrolled.next(event);
});
});
this._scroll.register(this);
}
ngOnDestroy() {
this._scroll.deregister(this);
if (this._scrollListener) {
this._scrollListener();
this._scrollListener = null;
}
}
/**
* Returns observable that emits when a scroll event is fired on the host element.
*/
elementScrolled(): Observable<any> {
return Observable.fromEvent(this._elementRef.nativeElement, 'scroll');
return this._elementScrolled.asObservable();
}
getElementRef(): ElementRef {
+1 -1
View File
@@ -5,7 +5,7 @@ import {
} from './event-objects';
/** Shorthand to dispatch a fake event on a specified node. */
export function dispatchFakeEvent(node: Node, type: string) {
export function dispatchFakeEvent(node: Node | Window, type: string) {
node.dispatchEvent(createFakeEvent(type));
}