Skip to content

Commit

Permalink
test(docs-infra): more thoroughly clean up after ScrollService tests (
Browse files Browse the repository at this point in the history
#33937)

By clearing `sessionStorage` and unsubscribing from `Location` events,
after each test, we reduce the possibility of potential
[spooky action at a distance][1]-type of failures in the future.

This does not have an impact on the actual app, since `ScrollService` is
currently expected to live throughout the lifetime of the app. Still,
unsubscribing from `Location` events keeps the code consistent with how
other `ScrollService` listeners are handled (e.g. for `window` events).

[1]: https://en.wikipedia.org/wiki/Action_at_a_distance_(computer_programming)

PR Close #33937
  • Loading branch information
gkalpak authored and alxhub committed Nov 20, 2019
1 parent b7fd86e commit 9e45203
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 34 deletions.
104 changes: 75 additions & 29 deletions aio/src/app/shared/scroll.service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,10 @@ describe('ScrollService', () => {
spyOn(window, 'scrollBy');
});

afterEach(() => scrollServiceInstances.forEach(instance => instance.ngOnDestroy()));
afterEach(() => {
scrollServiceInstances.forEach(instance => instance.ngOnDestroy());
window.sessionStorage.clear();
});

it('should debounce `updateScrollPositonInHistory()`', fakeAsync(() => {
const updateScrollPositionInHistorySpy = spyOn(scrollService, 'updateScrollPositionInHistory');
Expand All @@ -78,25 +81,6 @@ describe('ScrollService', () => {
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', () => {
if (scrollService.supportManualScrollRestoration) {
expect(window.history.scrollRestoration).toBe('manual');
Expand All @@ -106,18 +90,26 @@ describe('ScrollService', () => {
});

it('should not break when cookies are disabled in the browser', () => {
// Simulate `window.sessionStorage` being inaccessible, when cookies are disabled.
spyOnProperty(window, 'sessionStorage', 'get').and.throwError('The operation is insecure');

expect(() => {
const platformLoc = platformLocation as PlatformLocation;
const service = createScrollService(document, platformLoc, viewportScrollerStub, location);
const originalSessionStorage = Object.getOwnPropertyDescriptor(window, 'sessionStorage')!;

try {
// Simulate `window.sessionStorage` being inaccessible, when cookies are disabled.
Object.defineProperty(window, 'sessionStorage', {
get() { throw new Error('The operation is insecure'); },
});

service.updateScrollLocationHref();
expect(service.getStoredScrollLocationHref()).toBeNull();
const platformLoc = platformLocation as PlatformLocation;
const service = createScrollService(document, platformLoc, viewportScrollerStub, location);

service.removeStoredScrollInfo();
expect(service.getStoredScrollPosition()).toBeNull();
service.updateScrollLocationHref();
expect(service.getStoredScrollLocationHref()).toBeNull();

service.removeStoredScrollInfo();
expect(service.getStoredScrollPosition()).toBeNull();
} finally {
Object.defineProperty(window, 'sessionStorage', originalSessionStorage);
}
}).not.toThrow();
});

Expand Down Expand Up @@ -433,4 +425,58 @@ describe('ScrollService', () => {
expect(scrollToTopSpy).not.toHaveBeenCalled();
}));
});

describe('once destroyed', () => {
it('should stop updating scroll position', 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 stop updating the stored location href', () => {
const updateScrollLocationHrefSpy = spyOn(scrollService, 'updateScrollLocationHref');

window.dispatchEvent(new Event('beforeunload'));
expect(updateScrollLocationHrefSpy).toHaveBeenCalledTimes(1);

window.dispatchEvent(new Event('beforeunload'));
expect(updateScrollLocationHrefSpy).toHaveBeenCalledTimes(2);

updateScrollLocationHrefSpy.calls.reset();
scrollService.ngOnDestroy();

window.dispatchEvent(new Event('beforeunload'));
expect(updateScrollLocationHrefSpy).not.toHaveBeenCalled();
});

it('should stop scrolling on `hashchange` events', () => {
const scrollToPositionSpy = spyOn(scrollService, 'scrollToPosition');

location.simulateHashChange('foo');
expect(scrollToPositionSpy).toHaveBeenCalledTimes(1);

location.simulateHashChange('bar');
expect(scrollToPositionSpy).toHaveBeenCalledTimes(2);

scrollToPositionSpy.calls.reset();
scrollService.ngOnDestroy();

location.simulateHashChange('baz');
expect(scrollToPositionSpy).not.toHaveBeenCalled();
});
});
});
13 changes: 8 additions & 5 deletions aio/src/app/shared/scroll.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,14 @@ export class ScrollService implements OnDestroy {
.pipe(takeUntil(this.onDestroy))
.subscribe(() => this.updateScrollLocationHref());

// Change scroll restoration strategy to `manual` if it's supported
// Change scroll restoration strategy to `manual` if it's supported.
if (this.supportManualScrollRestoration) {
history.scrollRestoration = 'manual';
// we have to detect forward and back navigation thanks to popState event
this.location.subscribe((event: ScrollPositionPopStateEvent) => {
// the type is `hashchange` when the fragment identifier of the URL has changed. It allows us to go to position
// just before a click on an anchor

// We have to detect forward and back navigation thanks to popState event.
const locationSubscription = this.location.subscribe((event: ScrollPositionPopStateEvent) => {
// The type is `hashchange` when the fragment identifier of the URL has changed. It allows
// us to go to position just before a click on an anchor.
if (event.type === 'hashchange') {
this.scrollToPosition();
} else {
Expand All @@ -96,6 +97,8 @@ export class ScrollService implements OnDestroy {
this.poppedStateScrollPosition = event.state ? event.state.scrollPosition : null;
}
});

this.onDestroy.subscribe(() => locationSubscription.unsubscribe());
}

// If this was not a reload, discard the stored scroll info.
Expand Down

0 comments on commit 9e45203

Please sign in to comment.