test(docs-infra): clean up global listeners after `ScrollService` tests (#31390)

The `ScrollService` sets up some global `window` listeners.

Previously, these listeners were never unregistered. This was not a
problem in the real app, because the `ScrollService` instance exists for
the lifetime of a user session. In tests, however, where the `window`
instance is among all tests, the listeners would survive the
`ScrollService` tests. This, in addition to the fact that we used a mock
`ViewportScroller` which did not return the expected type from
`getScrollPosition()`, caused errors to be thrown in unrelated tests
(i.e. whenever a scroll event was emitted on `window`). See [here][1]
for an example failure.

This commit fixes it by adding an `ngOnDestroy()` method that
unregisters the listeners and ensuring it is called after each
`ScrollService` test.

[1]: https://circleci.com/gh/angular/angular/381649

PR Close #31390
This commit is contained in:
George Kalpakas 2019-07-03 17:47:07 +03:00 committed by Matias Niemelä
parent 9b29ca95a2
commit 0c00c94f34
2 changed files with 52 additions and 6 deletions

View File

@ -51,6 +51,8 @@ describe('ScrollService', () => {
spyOn(window, 'scrollBy'); spyOn(window, 'scrollBy');
}); });
afterEach(() => scrollService.ngOnDestroy());
it('should debounce `updateScrollPositonInHistory()`', fakeAsync(() => { it('should debounce `updateScrollPositonInHistory()`', fakeAsync(() => {
const updateScrollPositionInHistorySpy = spyOn(scrollService, 'updateScrollPositionInHistory'); const updateScrollPositionInHistorySpy = spyOn(scrollService, 'updateScrollPositionInHistory');
@ -65,6 +67,25 @@ describe('ScrollService', () => {
expect(updateScrollPositionInHistorySpy).toHaveBeenCalledTimes(1); expect(updateScrollPositionInHistorySpy).toHaveBeenCalledTimes(1);
})); }));
it('should stop updating scroll position once destroyed', fakeAsync(() => {
const updateScrollPositionInHistorySpy = spyOn(scrollService, 'updateScrollPositionInHistory');
window.dispatchEvent(new Event('scroll'));
tick(250);
expect(updateScrollPositionInHistorySpy).toHaveBeenCalledTimes(1);
window.dispatchEvent(new Event('scroll'));
tick(250);
expect(updateScrollPositionInHistorySpy).toHaveBeenCalledTimes(2);
updateScrollPositionInHistorySpy.calls.reset();
scrollService.ngOnDestroy();
window.dispatchEvent(new Event('scroll'));
tick(250);
expect(updateScrollPositionInHistorySpy).not.toHaveBeenCalled();
}));
it('should set `scrollRestoration` to `manual` if supported', () => { it('should set `scrollRestoration` to `manual` if supported', () => {
if (scrollService.supportManualScrollRestoration) { if (scrollService.supportManualScrollRestoration) {
expect(window.history.scrollRestoration).toBe('manual'); expect(window.history.scrollRestoration).toBe('manual');
@ -112,6 +133,23 @@ describe('ScrollService', () => {
expect(scrollService.topOffset).toBe(100 + topMargin); expect(scrollService.topOffset).toBe(100 + topMargin);
expect(document.querySelector).toHaveBeenCalled(); expect(document.querySelector).toHaveBeenCalled();
}); });
it('should stop updating on resize once destroyed', () => {
let clientHeight = 50;
(document.querySelector as jasmine.Spy).and.callFake(() => ({clientHeight}));
expect(scrollService.topOffset).toBe(50 + topMargin);
clientHeight = 100;
window.dispatchEvent(new Event('resize'));
expect(scrollService.topOffset).toBe(100 + topMargin);
scrollService.ngOnDestroy();
clientHeight = 200;
window.dispatchEvent(new Event('resize'));
expect(scrollService.topOffset).toBe(100 + topMargin);
});
}); });
describe('#topOfPageElement', () => { describe('#topOfPageElement', () => {

View File

@ -1,7 +1,7 @@
import { DOCUMENT, Location, PlatformLocation, PopStateEvent, ViewportScroller } from '@angular/common'; import { DOCUMENT, Location, PlatformLocation, PopStateEvent, ViewportScroller } from '@angular/common';
import { Injectable, Inject } from '@angular/core'; import { Injectable, Inject, OnDestroy } from '@angular/core';
import { fromEvent } from 'rxjs'; import { fromEvent, Subject } from 'rxjs';
import { debounceTime } from 'rxjs/operators'; import { debounceTime, takeUntil } from 'rxjs/operators';
type ScrollPosition = [number, number]; type ScrollPosition = [number, number];
interface ScrollPositionPopStateEvent extends PopStateEvent { interface ScrollPositionPopStateEvent extends PopStateEvent {
@ -14,10 +14,11 @@ export const topMargin = 16;
* A service that scrolls document elements into view * A service that scrolls document elements into view
*/ */
@Injectable() @Injectable()
export class ScrollService { export class ScrollService implements OnDestroy {
private _topOffset: number | null; private _topOffset: number | null;
private _topOfPageElement: Element; private _topOfPageElement: Element;
private onDestroy = new Subject<void>();
// The scroll position which has to be restored, after a `popstate` event. // The scroll position which has to be restored, after a `popstate` event.
poppedStateScrollPosition: ScrollPosition | null = null; poppedStateScrollPosition: ScrollPosition | null = null;
@ -49,10 +50,13 @@ export class ScrollService {
private viewportScroller: ViewportScroller, private viewportScroller: ViewportScroller,
private location: Location) { private location: Location) {
// On resize, the toolbar might change height, so "invalidate" the top offset. // On resize, the toolbar might change height, so "invalidate" the top offset.
fromEvent(window, 'resize').subscribe(() => this._topOffset = null); fromEvent(window, 'resize')
.pipe(takeUntil(this.onDestroy))
.subscribe(() => this._topOffset = null);
fromEvent(window, 'scroll') fromEvent(window, 'scroll')
.pipe(debounceTime(250)).subscribe(() => this.updateScrollPositionInHistory()); .pipe(debounceTime(250), takeUntil(this.onDestroy))
.subscribe(() => this.updateScrollPositionInHistory());
// Change scroll restoration strategy to `manual` if it's supported // Change scroll restoration strategy to `manual` if it's supported
if (this.supportManualScrollRestoration) { if (this.supportManualScrollRestoration) {
@ -75,6 +79,10 @@ export class ScrollService {
} }
} }
ngOnDestroy() {
this.onDestroy.next();
}
/** /**
* Scroll to the element with id extracted from the current location hash fragment. * Scroll to the element with id extracted from the current location hash fragment.
* Scroll to top if no hash. * Scroll to top if no hash.