refactor(docs-infra): provide `local-/sessionStorage` via DI (#42259)

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 #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.

PR Close #42259

aio/src/app/app.module.ts

@@ -21,6 +21,7 @@ import { ModeBannerComponent } from 'app/layout/mode-banner/mode-banner.componen
 import { GaService } from 'app/shared/ga.service';
 import { Logger } from 'app/shared/logger.service';
 import { LocationService } from 'app/shared/location.service';
+import { STORAGE_PROVIDERS } from 'app/shared/storage.service';
 import { NavigationService } from 'app/navigation/navigation.service';
 import { DocumentService } from 'app/documents/document.service';
 import { SearchService } from 'app/search/search.service';
@@ -187,6 +188,7 @@ export const svgIconProviders = [
     ScrollService,
     ScrollSpyService,
     SearchService,
+    STORAGE_PROVIDERS,
     svgIconProviders,
     TocService,
     { provide: CurrentDateToken, useFactory: currentDateProvider },

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

@@ -3,8 +3,8 @@ import { Component, NO_ERRORS_SCHEMA } from '@angular/core';
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 import { By } from '@angular/platform-browser';
 import { CurrentDateToken } from 'app/shared/current-date';
+import { LocalStorage, NoopStorage } from 'app/shared/storage.service';
 import { NotificationComponent } from './notification.component';
-import { WindowToken } from 'app/shared/window';
 
 describe('NotificationComponent', () => {
   let component: NotificationComponent;
@@ -14,7 +14,7 @@ describe('NotificationComponent', () => {
     TestBed.configureTestingModule({
       declarations: [TestComponent, NotificationComponent],
       providers: [
-        { provide: WindowToken, useClass: MockWindow },
+        { provide: LocalStorage, useValue: new NoopStorage() },
         { provide: CurrentDateToken, useValue: now },
       ],
       imports: [NoopAnimationsModule],
@@ -92,7 +92,8 @@ describe('NotificationComponent', () => {
   it('should update localStorage key when dismiss is called', () => {
     configTestingModule();
     createComponent();
-    const setItemSpy: jasmine.Spy = (TestBed.inject(WindowToken) as MockWindow).localStorage.setItem;
+    const localStorage = TestBed.inject(LocalStorage);
+    const setItemSpy = spyOn(localStorage, 'setItem');
     component.dismiss();
     expect(setItemSpy).toHaveBeenCalledWith('aio-notification/survey-january-2018', 'hide');
   });
@@ -105,28 +106,12 @@ describe('NotificationComponent', () => {
 
   it('should not show the notification if the there is a "hide" flag in localStorage', () => {
     configTestingModule();
-    const getItemSpy: jasmine.Spy = (TestBed.inject(WindowToken) as MockWindow).localStorage.getItem;
+    const localStorage = TestBed.inject(LocalStorage);
-    getItemSpy.and.returnValue('hide');
+    const getItemSpy = spyOn(localStorage, 'getItem').and.returnValue('hide');
     createComponent();
     expect(getItemSpy).toHaveBeenCalledWith('aio-notification/survey-january-2018');
     expect(component.showNotification).toBe('hide');
   });
-
-  it('should not break when cookies are disabled in the browser', () => {
-    configTestingModule();
-
-    // Simulate `window.localStorage` being inaccessible, when cookies are disabled.
-    const mockWindow: MockWindow = TestBed.inject(WindowToken);
-    Object.defineProperty(mockWindow, 'localStorage', {
-      get() { throw new Error('The operation is insecure'); },
-    });
-
-    expect(() => createComponent()).not.toThrow();
-    expect(component.showNotification).toBe('show');
-
-    component.dismiss();
-    expect(component.showNotification).toBe('hide');
-  });
 });
 
 @Component({
@@ -145,7 +130,3 @@ describe('NotificationComponent', () => {
 })
 class TestComponent {
 }
-
-class MockWindow {
-  localStorage = jasmine.createSpyObj('localStorage', ['getItem', 'setItem']);
-}

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

@@ -1,7 +1,7 @@
 import { animate, state, style, trigger, transition } from '@angular/animations';
 import { Component, EventEmitter, HostBinding, Inject, Input, OnInit, Output } from '@angular/core';
 import { CurrentDateToken } from 'app/shared/current-date';
-import { WindowToken } from 'app/shared/window';
+import { LocalStorage } from 'app/shared/storage.service';
 
 const LOCAL_STORAGE_NAMESPACE = 'aio-notification/';
 
@@ -20,8 +20,6 @@ const LOCAL_STORAGE_NAMESPACE = 'aio-notification/';
   ]
 })
 export class NotificationComponent implements OnInit {
-  private storage: Storage;
-
   @Input() dismissOnContentClick: boolean;
   @Input() notificationId: string;
   @Input() expirationDate: string;
@@ -30,25 +28,7 @@ export class NotificationComponent implements OnInit {
   @HostBinding('@hideAnimation')
   showNotification: 'show'|'hide';
 
-  constructor(
+  constructor(@Inject(LocalStorage) private storage: Storage, @Inject(CurrentDateToken) private currentDate: Date) {}
-    @Inject(WindowToken) window: Window,
-    @Inject(CurrentDateToken) private currentDate: Date
-  ) {
-    try {
-      this.storage = window.localStorage;
-    } catch {
-      // When cookies are disabled in the browser, even trying to access
-      // `window.localStorage` throws an error. Use a no-op storage.
-      this.storage = {
-        length: 0,
-        clear: () => undefined,
-        getItem: () => null,
-        key: () => null,
-        removeItem: () => undefined,
-        setItem: () => undefined
-      };
-    }
-  }
 
   ngOnInit() {
     const previouslyHidden = this.storage.getItem(LOCAL_STORAGE_NAMESPACE + this.notificationId) === 'hide';

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

@@ -5,6 +5,7 @@ import {Injector} from '@angular/core';
 import {fakeAsync, tick} from '@angular/core/testing';
 
 import {ScrollService, topMargin} from './scroll.service';
+import {SessionStorage, NoopStorage} from './storage.service';
 
 describe('ScrollService', () => {
   const scrollServiceInstances: ScrollService[] = [];
@@ -20,6 +21,7 @@ describe('ScrollService', () => {
   let platformLocation: MockPlatformLocation;
   let scrollService: ScrollService;
   let location: SpyLocation;
+  let sessionStorage: Storage;
 
   class MockPlatformLocation {
     hash: string;
@@ -46,13 +48,14 @@ describe('ScrollService', () => {
         {
           provide: ScrollService,
           useFactory: createScrollService,
-          deps: [DOCUMENT, PlatformLocation, ViewportScroller, Location],
+          deps: [DOCUMENT, PlatformLocation, ViewportScroller, Location, SessionStorage],
         },
-        {provide: Location, useClass: SpyLocation, deps: [] },
+        {provide: Location, useClass: SpyLocation, deps: []},
         {provide: DOCUMENT, useClass: MockDocument, deps: []},
         {provide: PlatformLocation, useClass: MockPlatformLocation, deps: []},
         {provide: ViewportScroller, useValue: viewportScrollerStub},
-        {provide: LocationStrategy, useClass: MockLocationStrategy, deps: []}
+        {provide: LocationStrategy, useClass: MockLocationStrategy, deps: []},
+        {provide: SessionStorage, useValue: new NoopStorage()},
       ]
     });
 
@@ -60,13 +63,13 @@ describe('ScrollService', () => {
     document = injector.get(DOCUMENT) as unknown as MockDocument;
     scrollService = injector.get(ScrollService);
     location = injector.get(Location) as unknown as SpyLocation;
+    sessionStorage = injector.get(SessionStorage);
 
     spyOn(window, 'scrollBy');
   });
 
   afterEach(() => {
     scrollServiceInstances.forEach(instance => instance.ngOnDestroy());
-    window.sessionStorage.clear();
   });
 
   it('should debounce `updateScrollPositonInHistory()`', fakeAsync(() => {
@@ -92,7 +95,8 @@ describe('ScrollService', () => {
         configurable: true,
       });
       scrollService = createScrollService(
-          document, platformLocation as PlatformLocation, viewportScrollerStub, location);
+          document, platformLocation as PlatformLocation, viewportScrollerStub, location,
+          sessionStorage);
 
       expect(scrollService.supportManualScrollRestoration).toBe(false);
     } finally {
@@ -112,32 +116,6 @@ describe('ScrollService', () => {
     }
   });
 
-  it('should not break when cookies are disabled in the browser', () => {
-    expect(() => {
-      const originalSessionStorage = Object.getOwnPropertyDescriptor(window, 'sessionStorage') as PropertyDescriptor;
-
-      try {
-        // Simulate `window.sessionStorage` being inaccessible, when cookies are disabled.
-        Object.defineProperty(window, 'sessionStorage', {
-          get() {
-            throw new Error('The operation is insecure');
-          },
-        });
-
-        const platformLoc = platformLocation as PlatformLocation;
-        const service = createScrollService(document, platformLoc, viewportScrollerStub, location);
-
-        service.updateScrollLocationHref();
-        expect(service.getStoredScrollLocationHref()).toBeNull();
-
-        service.removeStoredScrollInfo();
-        expect(service.getStoredScrollPosition()).toBeNull();
-      } finally {
-        Object.defineProperty(window, 'sessionStorage', originalSessionStorage);
-      }
-    }).not.toThrow();
-  });
-
   describe('#topOffset', () => {
     it('should query for the top-bar by CSS selector', () => {
       expect(document.querySelector).not.toHaveBeenCalled();

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

@@ -2,6 +2,7 @@ import {DOCUMENT, Location, PlatformLocation, PopStateEvent, ViewportScroller} f
 import {Inject, Injectable, OnDestroy} from '@angular/core';
 import {fromEvent, Subject} from 'rxjs';
 import {debounceTime, takeUntil} from 'rxjs/operators';
+import {SessionStorage} from './storage.service';
 
 type ScrollPosition = [number, number];
 interface ScrollPositionPopStateEvent extends PopStateEvent {
@@ -18,7 +19,6 @@ export class ScrollService implements OnDestroy {
   private _topOffset: number|null;
   private _topOfPageElement: Element;
   private onDestroy = new Subject<void>();
-  private storage: Storage;
 
   // The scroll position which has to be restored, after a `popstate` event.
   poppedStateScrollPosition: ScrollPosition|null = null;
@@ -45,22 +45,8 @@ export class ScrollService implements OnDestroy {
 
   constructor(
       @Inject(DOCUMENT) private document: any, private platformLocation: PlatformLocation,
-      private viewportScroller: ViewportScroller, private location: Location) {
+      private viewportScroller: ViewportScroller, private location: Location,
-    try {
+      @Inject(SessionStorage) private storage: Storage) {
-      this.storage = window.sessionStorage;
-    } catch {
-      // When cookies are disabled in the browser, even trying to access
-      // `window.sessionStorage` throws an error. Use a no-op storage.
-      this.storage = {
-        length: 0,
-        clear: () => undefined,
-        getItem: () => null,
-        key: () => null,
-        removeItem: () => undefined,
-        setItem: () => undefined
-      };
-    }
-
     // On resize, the toolbar might change height, so "invalidate" the top offset.
     fromEvent(window, 'resize')
         .pipe(takeUntil(this.onDestroy))

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

@@ -0,0 +1,37 @@
+import { Injector } from '@angular/core';
+import { LocalStorage, NoopStorage, SessionStorage, STORAGE_PROVIDERS } from './storage.service';
+import { WindowToken } from './window';
+
+[
+  ['localStorage', LocalStorage] as const,
+  ['sessionStorage', SessionStorage] as const,
+].forEach(([storagePropName, storageToken]) => {
+  let getStorageSpy: jasmine.Spy;
+  let injector: Injector;
+
+  beforeEach(() => {
+    getStorageSpy = jasmine.createSpy(`get ${storagePropName}`);
+    injector = Injector.create({
+      providers: [
+        STORAGE_PROVIDERS,
+        {
+          provide: WindowToken,
+          useValue: Object.defineProperty({}, storagePropName, { get: getStorageSpy }),
+        },
+      ],
+    });
+  });
+
+  it('should return the storage from `window`', () => {
+    const mockStorage = { mock: true } as unknown as Storage;
+    getStorageSpy.and.returnValue(mockStorage);
+
+    expect(injector.get(storageToken)).toBe(mockStorage);
+  });
+
+  it('should return a no-op storage if accessing the storage on `window` errors', () => {
+    getStorageSpy.and.throwError('Can\'t touch this!');
+
+    expect(injector.get(storageToken)).toBeInstanceOf(NoopStorage);
+  });
+});

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

@@ -0,0 +1,29 @@
+import { InjectionToken, StaticProvider } from '@angular/core';
+import { WindowToken } from './window';
+
+export const LocalStorage = new InjectionToken<Storage>('LocalStorage');
+export const SessionStorage = new InjectionToken<Storage>('SessionStorage');
+
+export const STORAGE_PROVIDERS: StaticProvider[] = [
+  { provide: LocalStorage, useFactory: (win: Window) => getStorage(win, 'localStorage'), deps: [WindowToken] },
+  { provide: SessionStorage, useFactory: (win: Window) => getStorage(win, 'sessionStorage'), deps: [WindowToken] },
+];
+
+export class NoopStorage implements Storage {
+  length = 0;
+  clear() {}
+  getItem() { return null; }
+  key() { return null; }
+  removeItem() {}
+  setItem() {}
+}
+
+function getStorage(win: Window, storageType: 'localStorage' | 'sessionStorage'): Storage {
+  // When cookies are disabled in the browser, even trying to access `window[storageType]` throws an
+  // error. If so, return a no-op storage.
+  try {
+    return win[storageType];
+  } catch {
+    return new NoopStorage();
+  }
+}

aio/src/app/shared/theme-picker/theme-toggle.component.spec.ts

@@ -1,43 +1,8 @@
 import { ComponentFixture, TestBed } from '@angular/core/testing';
 import { MatIconModule } from '@angular/material/icon';
 import { By } from '@angular/platform-browser';
-import { ThemeStorage, ThemeToggleComponent } from './theme-toggle.component';
+import { LocalStorage, NoopStorage } from '../storage.service';
-
+import { storageKey as themeStorageKey, ThemeToggleComponent } from './theme-toggle.component';
-class FakeThemeStorage implements ThemeStorage {
-  fakeStorage: string | null = null;
-
-  getThemePreference(): string | null {
-    return this.fakeStorage;
-  }
-
-  setThemePreference(isDark: boolean): void {
-    this.fakeStorage = String(isDark);
-  }
-}
-
-// Verify that FakeThemeStorage behaves like ThemeStorage would
-describe('FakeThemeStorage', () => {
-  let themeStorage: ThemeStorage;
-
-  beforeEach(() => {
-    themeStorage = new FakeThemeStorage();
-  });
-
-  it('should have null stored initially', () => {
-    expect(themeStorage.getThemePreference()).toBeNull();
-  });
-
-  it('should store true as a string if isDark is true', () => {
-    themeStorage.setThemePreference(true);
-    expect(themeStorage.getThemePreference()).toBe('true');
-  });
-
-  it('should store false as a string if isDark is false', () => {
-    themeStorage.setThemePreference(false);
-    expect(themeStorage.getThemePreference()).toBe('false');
-  });
-});
-
 
 describe('ThemeToggleComponent', () => {
   let component: ThemeToggleComponent;
@@ -47,6 +12,7 @@ describe('ThemeToggleComponent', () => {
     TestBed.configureTestingModule({
       declarations: [ ThemeToggleComponent ],
       imports: [ MatIconModule ],
+      providers: [ { provide: LocalStorage, useValue: new NoopStorage() } ],
     });
 
     fixture = TestBed.createComponent(ThemeToggleComponent);
@@ -82,6 +48,30 @@ describe('ThemeToggleComponent', () => {
     expect(component.getToggleLabel()).toBe('Switch to light mode');
   });
 
+  it('should store the theme in `localStorage`', () => {
+    const storage = TestBed.inject(LocalStorage);
+    const setItemSpy = spyOn(storage, 'setItem');
+
+    component.toggleTheme();
+    expect(setItemSpy).toHaveBeenCalledWith(themeStorageKey, 'true');
+
+    component.toggleTheme();
+    expect(setItemSpy).toHaveBeenCalledWith(themeStorageKey, 'false');
+  });
+
+  it('should initialize the theme from `localStorage`', () => {
+    const storage = TestBed.inject(LocalStorage);
+    const getItemSpy = spyOn(storage, 'getItem').withArgs(themeStorageKey);
+
+    getItemSpy.and.returnValue('false');
+    const component1 = TestBed.createComponent(ThemeToggleComponent).componentInstance;
+    expect(component1.isDark).toBe(false);
+
+    getItemSpy.and.returnValue('true');
+    const component2 = TestBed.createComponent(ThemeToggleComponent).componentInstance;
+    expect(component2.isDark).toBe(true);
+  });
+
   // Helpers
   function getToggleButton(): HTMLButtonElement {
     return fixture.debugElement.query(By.css('button')).nativeElement;

aio/src/app/shared/theme-picker/theme-toggle.component.ts

@@ -1,27 +1,8 @@
 import { DOCUMENT } from '@angular/common';
-import { Component, Inject, Injectable } from '@angular/core';
+import { Component, Inject } from '@angular/core';
+import { LocalStorage } from 'app/shared/storage.service';
 
-/** Injectable facade around localStorage for theme preference to make testing easier. */
+export const storageKey = 'aio-theme';
-@Injectable({ providedIn: 'root' })
-export class ThemeStorage {
-  getThemePreference(): string | null {
-    // Wrap localStorage access in try/catch because user agents can block localStorage. If it is
-    // blocked, we treat it as if no preference was previously stored.
-    try {
-      return localStorage.getItem('aio-theme');
-    } catch {
-      return null;
-    }
-  }
-
-  setThemePreference(isDark: boolean): void {
-    // Wrap localStorage access in try/catch because user agents can block localStorage. If it
-    // fails, we persist nothing.
-    try {
-      localStorage.setItem('aio-theme', String(isDark));
-    } catch { }
-  }
-}
 
 @Component({
   selector: 'aio-theme-toggle',
@@ -37,7 +18,7 @@ export class ThemeStorage {
 export class ThemeToggleComponent {
   isDark = false;
 
-  constructor(@Inject(DOCUMENT) private document: Document, private readonly themeStorage: ThemeStorage) {
+  constructor(@Inject(DOCUMENT) private document: Document, @Inject(LocalStorage) private storage: Storage) {
     this.initializeThemeFromPreferences();
   }
 
@@ -48,7 +29,7 @@ export class ThemeToggleComponent {
 
   private initializeThemeFromPreferences(): void {
     // Check whether there's an explicit preference in localStorage.
-    const storedPreference = this.themeStorage.getThemePreference();
+    const storedPreference = this.storage.getItem(storageKey);
 
     // If we do have a preference in localStorage, use that. Otherwise,
     // initialize based on the prefers-color-scheme media query.
@@ -86,6 +67,6 @@ export class ThemeToggleComponent {
       customLinkElement.href = `${this.getThemeName()}-theme.css`;
     }
 
-    this.themeStorage.setThemePreference(this.isDark);
+    this.storage.setItem(storageKey, String(this.isDark));
   }
 }

goldens/size-tracking/aio-payloads.json

@@ -3,7 +3,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2017": 4619,
-        "main-es2017": 454043,
+        "main-es2017": 454003,
         "polyfills-es2017": 55210
       }
     }
@@ -12,7 +12,7 @@
     "master": {
       "uncompressed": {
         "runtime-es2017": 4619,
-        "main-es2017": 454178,
+        "main-es2017": 454138,
         "polyfills-es2017": 55348
       }
     }