Skip to content

Commit 8331cb5

Browse files
committed
refactor(docs-infra): provide local-/sessionStorage via DI
Previously, we had the same logic in a couple of places to safely access the `Window`'s `local-/sessionStorage` and provide a no-op fallback if necessary. Soon, we will need the same logic for the cookies popup (see angular#42209). This commit reduces code duplication by providing `local-/sessionStorage` as injectables and sharing the logic for accessing them safely. This also makes it easier to mock the storage in tests without having to mess with the actual `Window` object. NOTE: This commit actually decreases the payload size in the `main` bundle by 40B.
1 parent cf403dc commit 8331cb5

File tree

10 files changed

+123
-159
lines changed

10 files changed

+123
-159
lines changed

aio/src/app/app.module.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import { ModeBannerComponent } from 'app/layout/mode-banner/mode-banner.componen
2121
import { GaService } from 'app/shared/ga.service';
2222
import { Logger } from 'app/shared/logger.service';
2323
import { LocationService } from 'app/shared/location.service';
24+
import { STORAGE_PROVIDERS } from 'app/shared/storage.service';
2425
import { NavigationService } from 'app/navigation/navigation.service';
2526
import { DocumentService } from 'app/documents/document.service';
2627
import { SearchService } from 'app/search/search.service';
@@ -187,6 +188,7 @@ export const svgIconProviders = [
187188
ScrollService,
188189
ScrollSpyService,
189190
SearchService,
191+
STORAGE_PROVIDERS,
190192
svgIconProviders,
191193
TocService,
192194
{ provide: CurrentDateToken, useFactory: currentDateProvider },

aio/src/app/layout/notification/notification.component.spec.ts

Lines changed: 6 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@ import { Component, NO_ERRORS_SCHEMA } from '@angular/core';
33
import { ComponentFixture, TestBed } from '@angular/core/testing';
44
import { By } from '@angular/platform-browser';
55
import { CurrentDateToken } from 'app/shared/current-date';
6+
import { LocalStorage, NoopStorage } from 'app/shared/storage.service';
67
import { NotificationComponent } from './notification.component';
7-
import { WindowToken } from 'app/shared/window';
88

99
describe('NotificationComponent', () => {
1010
let component: NotificationComponent;
@@ -14,7 +14,7 @@ describe('NotificationComponent', () => {
1414
TestBed.configureTestingModule({
1515
declarations: [TestComponent, NotificationComponent],
1616
providers: [
17-
{ provide: WindowToken, useClass: MockWindow },
17+
{ provide: LocalStorage, useValue: new NoopStorage() },
1818
{ provide: CurrentDateToken, useValue: now },
1919
],
2020
imports: [NoopAnimationsModule],
@@ -92,7 +92,8 @@ describe('NotificationComponent', () => {
9292
it('should update localStorage key when dismiss is called', () => {
9393
configTestingModule();
9494
createComponent();
95-
const setItemSpy: jasmine.Spy = (TestBed.inject(WindowToken) as MockWindow).localStorage.setItem;
95+
const localStorage = TestBed.inject(LocalStorage);
96+
const setItemSpy = spyOn(localStorage, 'setItem');
9697
component.dismiss();
9798
expect(setItemSpy).toHaveBeenCalledWith('aio-notification/survey-january-2018', 'hide');
9899
});
@@ -105,28 +106,12 @@ describe('NotificationComponent', () => {
105106

106107
it('should not show the notification if the there is a "hide" flag in localStorage', () => {
107108
configTestingModule();
108-
const getItemSpy: jasmine.Spy = (TestBed.inject(WindowToken) as MockWindow).localStorage.getItem;
109-
getItemSpy.and.returnValue('hide');
109+
const localStorage = TestBed.inject(LocalStorage);
110+
const getItemSpy = spyOn(localStorage, 'getItem').and.returnValue('hide');
110111
createComponent();
111112
expect(getItemSpy).toHaveBeenCalledWith('aio-notification/survey-january-2018');
112113
expect(component.showNotification).toBe('hide');
113114
});
114-
115-
it('should not break when cookies are disabled in the browser', () => {
116-
configTestingModule();
117-
118-
// Simulate `window.localStorage` being inaccessible, when cookies are disabled.
119-
const mockWindow: MockWindow = TestBed.inject(WindowToken);
120-
Object.defineProperty(mockWindow, 'localStorage', {
121-
get() { throw new Error('The operation is insecure'); },
122-
});
123-
124-
expect(() => createComponent()).not.toThrow();
125-
expect(component.showNotification).toBe('show');
126-
127-
component.dismiss();
128-
expect(component.showNotification).toBe('hide');
129-
});
130115
});
131116

132117
@Component({
@@ -145,7 +130,3 @@ describe('NotificationComponent', () => {
145130
})
146131
class TestComponent {
147132
}
148-
149-
class MockWindow {
150-
localStorage = jasmine.createSpyObj('localStorage', ['getItem', 'setItem']);
151-
}

aio/src/app/layout/notification/notification.component.ts

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { animate, state, style, trigger, transition } from '@angular/animations';
22
import { Component, EventEmitter, HostBinding, Inject, Input, OnInit, Output } from '@angular/core';
33
import { CurrentDateToken } from 'app/shared/current-date';
4-
import { WindowToken } from 'app/shared/window';
4+
import { LocalStorage } from 'app/shared/storage.service';
55

66
const LOCAL_STORAGE_NAMESPACE = 'aio-notification/';
77

@@ -20,8 +20,6 @@ const LOCAL_STORAGE_NAMESPACE = 'aio-notification/';
2020
]
2121
})
2222
export class NotificationComponent implements OnInit {
23-
private storage: Storage;
24-
2523
@Input() dismissOnContentClick: boolean;
2624
@Input() notificationId: string;
2725
@Input() expirationDate: string;
@@ -30,25 +28,7 @@ export class NotificationComponent implements OnInit {
3028
@HostBinding('@hideAnimation')
3129
showNotification: 'show'|'hide';
3230

33-
constructor(
34-
@Inject(WindowToken) window: Window,
35-
@Inject(CurrentDateToken) private currentDate: Date
36-
) {
37-
try {
38-
this.storage = window.localStorage;
39-
} catch {
40-
// When cookies are disabled in the browser, even trying to access
41-
// `window.localStorage` throws an error. Use a no-op storage.
42-
this.storage = {
43-
length: 0,
44-
clear: () => undefined,
45-
getItem: () => null,
46-
key: () => null,
47-
removeItem: () => undefined,
48-
setItem: () => undefined
49-
};
50-
}
51-
}
31+
constructor(@Inject(LocalStorage) private storage: Storage, @Inject(CurrentDateToken) private currentDate: Date) {}
5232

5333
ngOnInit() {
5434
const previouslyHidden = this.storage.getItem(LOCAL_STORAGE_NAMESPACE + this.notificationId) === 'hide';

aio/src/app/shared/scroll.service.spec.ts

Lines changed: 9 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {Injector} from '@angular/core';
55
import {fakeAsync, tick} from '@angular/core/testing';
66

77
import {ScrollService, topMargin} from './scroll.service';
8+
import {SessionStorage, NoopStorage} from './storage.service';
89

910
describe('ScrollService', () => {
1011
const scrollServiceInstances: ScrollService[] = [];
@@ -20,6 +21,7 @@ describe('ScrollService', () => {
2021
let platformLocation: MockPlatformLocation;
2122
let scrollService: ScrollService;
2223
let location: SpyLocation;
24+
let sessionStorage: Storage;
2325

2426
class MockPlatformLocation {
2527
hash: string;
@@ -46,27 +48,28 @@ describe('ScrollService', () => {
4648
{
4749
provide: ScrollService,
4850
useFactory: createScrollService,
49-
deps: [DOCUMENT, PlatformLocation, ViewportScroller, Location],
51+
deps: [DOCUMENT, PlatformLocation, ViewportScroller, Location, SessionStorage],
5052
},
51-
{provide: Location, useClass: SpyLocation, deps: [] },
53+
{provide: Location, useClass: SpyLocation, deps: []},
5254
{provide: DOCUMENT, useClass: MockDocument, deps: []},
5355
{provide: PlatformLocation, useClass: MockPlatformLocation, deps: []},
5456
{provide: ViewportScroller, useValue: viewportScrollerStub},
55-
{provide: LocationStrategy, useClass: MockLocationStrategy, deps: []}
57+
{provide: LocationStrategy, useClass: MockLocationStrategy, deps: []},
58+
{provide: SessionStorage, useValue: new NoopStorage()},
5659
]
5760
});
5861

5962
platformLocation = injector.get(PlatformLocation);
6063
document = injector.get(DOCUMENT) as unknown as MockDocument;
6164
scrollService = injector.get(ScrollService);
6265
location = injector.get(Location) as unknown as SpyLocation;
66+
sessionStorage = injector.get(SessionStorage);
6367

6468
spyOn(window, 'scrollBy');
6569
});
6670

6771
afterEach(() => {
6872
scrollServiceInstances.forEach(instance => instance.ngOnDestroy());
69-
window.sessionStorage.clear();
7073
});
7174

7275
it('should debounce `updateScrollPositonInHistory()`', fakeAsync(() => {
@@ -92,7 +95,8 @@ describe('ScrollService', () => {
9295
configurable: true,
9396
});
9497
scrollService = createScrollService(
95-
document, platformLocation as PlatformLocation, viewportScrollerStub, location);
98+
document, platformLocation as PlatformLocation, viewportScrollerStub, location,
99+
sessionStorage);
96100

97101
expect(scrollService.supportManualScrollRestoration).toBe(false);
98102
} finally {
@@ -112,32 +116,6 @@ describe('ScrollService', () => {
112116
}
113117
});
114118

115-
it('should not break when cookies are disabled in the browser', () => {
116-
expect(() => {
117-
const originalSessionStorage = Object.getOwnPropertyDescriptor(window, 'sessionStorage') as PropertyDescriptor;
118-
119-
try {
120-
// Simulate `window.sessionStorage` being inaccessible, when cookies are disabled.
121-
Object.defineProperty(window, 'sessionStorage', {
122-
get() {
123-
throw new Error('The operation is insecure');
124-
},
125-
});
126-
127-
const platformLoc = platformLocation as PlatformLocation;
128-
const service = createScrollService(document, platformLoc, viewportScrollerStub, location);
129-
130-
service.updateScrollLocationHref();
131-
expect(service.getStoredScrollLocationHref()).toBeNull();
132-
133-
service.removeStoredScrollInfo();
134-
expect(service.getStoredScrollPosition()).toBeNull();
135-
} finally {
136-
Object.defineProperty(window, 'sessionStorage', originalSessionStorage);
137-
}
138-
}).not.toThrow();
139-
});
140-
141119
describe('#topOffset', () => {
142120
it('should query for the top-bar by CSS selector', () => {
143121
expect(document.querySelector).not.toHaveBeenCalled();

aio/src/app/shared/scroll.service.ts

Lines changed: 3 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import {DOCUMENT, Location, PlatformLocation, PopStateEvent, ViewportScroller} f
22
import {Inject, Injectable, OnDestroy} from '@angular/core';
33
import {fromEvent, Subject} from 'rxjs';
44
import {debounceTime, takeUntil} from 'rxjs/operators';
5+
import {SessionStorage} from './storage.service';
56

67
type ScrollPosition = [number, number];
78
interface ScrollPositionPopStateEvent extends PopStateEvent {
@@ -18,7 +19,6 @@ export class ScrollService implements OnDestroy {
1819
private _topOffset: number|null;
1920
private _topOfPageElement: Element;
2021
private onDestroy = new Subject<void>();
21-
private storage: Storage;
2222

2323
// The scroll position which has to be restored, after a `popstate` event.
2424
poppedStateScrollPosition: ScrollPosition|null = null;
@@ -45,22 +45,8 @@ export class ScrollService implements OnDestroy {
4545

4646
constructor(
4747
@Inject(DOCUMENT) private document: any, private platformLocation: PlatformLocation,
48-
private viewportScroller: ViewportScroller, private location: Location) {
49-
try {
50-
this.storage = window.sessionStorage;
51-
} catch {
52-
// When cookies are disabled in the browser, even trying to access
53-
// `window.sessionStorage` throws an error. Use a no-op storage.
54-
this.storage = {
55-
length: 0,
56-
clear: () => undefined,
57-
getItem: () => null,
58-
key: () => null,
59-
removeItem: () => undefined,
60-
setItem: () => undefined
61-
};
62-
}
63-
48+
private viewportScroller: ViewportScroller, private location: Location,
49+
@Inject(SessionStorage) private storage: Storage) {
6450
// On resize, the toolbar might change height, so "invalidate" the top offset.
6551
fromEvent(window, 'resize')
6652
.pipe(takeUntil(this.onDestroy))
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { Injector } from '@angular/core';
2+
import { LocalStorage, NoopStorage, SessionStorage, STORAGE_PROVIDERS } from './storage.service';
3+
import { WindowToken } from './window';
4+
5+
[
6+
['localStorage', LocalStorage] as const,
7+
['sessionStorage', SessionStorage] as const,
8+
].forEach(([storagePropName, storageToken]) => {
9+
let getStorageSpy: jasmine.Spy;
10+
let injector: Injector;
11+
12+
beforeEach(() => {
13+
getStorageSpy = jasmine.createSpy(`get ${storagePropName}`);
14+
injector = Injector.create({
15+
providers: [
16+
STORAGE_PROVIDERS,
17+
{
18+
provide: WindowToken,
19+
useValue: Object.defineProperty({}, storagePropName, { get: getStorageSpy }),
20+
},
21+
],
22+
});
23+
});
24+
25+
it('should return the storage from `window`', () => {
26+
const mockStorage = { mock: true } as unknown as Storage;
27+
getStorageSpy.and.returnValue(mockStorage);
28+
29+
expect(injector.get(storageToken)).toBe(mockStorage);
30+
});
31+
32+
it('should return a no-op storage if accessing the storage on `window` errors', () => {
33+
getStorageSpy.and.throwError('Can\'t touch this!');
34+
35+
expect(injector.get(storageToken)).toBeInstanceOf(NoopStorage);
36+
});
37+
});
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import { InjectionToken, StaticProvider } from '@angular/core';
2+
import { WindowToken } from './window';
3+
4+
export const LocalStorage = new InjectionToken<Storage>('LocalStorage');
5+
export const SessionStorage = new InjectionToken<Storage>('SessionStorage');
6+
7+
export const STORAGE_PROVIDERS: StaticProvider[] = [
8+
{ provide: LocalStorage, useFactory: (win: Window) => getStorage(win, 'localStorage'), deps: [WindowToken] },
9+
{ provide: SessionStorage, useFactory: (win: Window) => getStorage(win, 'sessionStorage'), deps: [WindowToken] },
10+
];
11+
12+
export class NoopStorage implements Storage {
13+
length = 0;
14+
clear() {}
15+
getItem() { return null; }
16+
key() { return null; }
17+
removeItem() {}
18+
setItem() {}
19+
}
20+
21+
function getStorage(win: Window, storageType: 'localStorage' | 'sessionStorage'): Storage {
22+
// When cookies are disabled in the browser, even trying to access `window[storageType]` throws an
23+
// error. If so, return a no-op storage.
24+
try {
25+
return win[storageType];
26+
} catch {
27+
return new NoopStorage();
28+
}
29+
}

0 commit comments

Comments
 (0)