Skip to content

Commit fd17cf2

Browse files
crisbetojelbourn
authored andcommitted
fix(scrolling): implement ngOnDestroy in ScrollDispatcher (#9608)
Sets up some cleanup logic in the `ScrollDispatcher` in case the module is unloaded.
1 parent b42fcb9 commit fd17cf2

File tree

2 files changed

+34
-6
lines changed

2 files changed

+34
-6
lines changed

src/cdk/scrolling/scroll-dispatcher.spec.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,22 @@ describe('Scroll Dispatcher', () => {
180180
.toBe(4, 'Expected scrollable count to stay the same');
181181
});
182182

183+
it('should remove the global subscription on destroy', () => {
184+
expect(scroll._globalSubscription).toBeNull('Expected no global listeners on init.');
185+
186+
const subscription = scroll.scrolled(0).subscribe(() => {});
187+
188+
expect(scroll._globalSubscription).toBeTruthy(
189+
'Expected global listeners after a subscription has been added.');
190+
191+
scroll.ngOnDestroy();
192+
193+
expect(scroll._globalSubscription).toBeNull(
194+
'Expected global listeners to have been removed after the subscription has stopped.');
195+
196+
subscription.unsubscribe();
197+
});
198+
183199
});
184200
});
185201

src/cdk/scrolling/scroll-dispatcher.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {ElementRef, Injectable, NgZone, Optional, SkipSelf} from '@angular/core';
9+
import {ElementRef, Injectable, NgZone, Optional, SkipSelf, OnDestroy} from '@angular/core';
1010
import {Platform} from '@angular/cdk/platform';
1111
import {Subject} from 'rxjs/Subject';
1212
import {Subscription} from 'rxjs/Subscription';
@@ -26,7 +26,7 @@ export const DEFAULT_SCROLL_TIME = 20;
2626
* Scrollable references emit a scrolled event.
2727
*/
2828
@Injectable()
29-
export class ScrollDispatcher {
29+
export class ScrollDispatcher implements OnDestroy {
3030
constructor(private _ngZone: NgZone, private _platform: Platform) { }
3131

3232
/** Subject for notifying that a registered scrollable reference element has been scrolled. */
@@ -97,14 +97,18 @@ export class ScrollDispatcher {
9797
subscription.unsubscribe();
9898
this._scrolledCount--;
9999

100-
if (this._globalSubscription && !this._scrolledCount) {
101-
this._globalSubscription.unsubscribe();
102-
this._globalSubscription = null;
100+
if (!this._scrolledCount) {
101+
this._removeGlobalListener();
103102
}
104103
};
105104
}) : observableOf<void>();
106105
}
107106

107+
ngOnDestroy() {
108+
this._removeGlobalListener();
109+
this.scrollContainers.forEach((_, container) => this.deregister(container));
110+
}
111+
108112
/**
109113
* Returns an observable that emits whenever any of the
110114
* scrollable ancestors of an element are scrolled.
@@ -146,12 +150,20 @@ export class ScrollDispatcher {
146150
return false;
147151
}
148152

149-
/** Sets up the global scroll and resize listeners. */
153+
/** Sets up the global scroll listeners. */
150154
private _addGlobalListener() {
151155
this._globalSubscription = this._ngZone.runOutsideAngular(() => {
152156
return fromEvent(window.document, 'scroll').subscribe(() => this._scrolled.next());
153157
});
154158
}
159+
160+
/** Cleans up the global scroll listener. */
161+
private _removeGlobalListener() {
162+
if (this._globalSubscription) {
163+
this._globalSubscription.unsubscribe();
164+
this._globalSubscription = null;
165+
}
166+
}
155167
}
156168

157169
/** @docs-private */

0 commit comments

Comments
 (0)