From 895f47a9c2da03ef9e0e97e05bf48011b6a61e24 Mon Sep 17 00:00:00 2001 From: Georgios Kalpakas Date: Thu, 4 May 2017 00:01:22 +0300 Subject: [PATCH] refactor(aio): remove work-around for browsers without ServiceWorker support This essentially reverts #15731, which is no longer necessary after angular/mobile-toolkit@eeb4b22 (which is included in v1.0.0-beta.10). --- .../app/sw-updates/noop-ng-service-worker.ts | 35 -- aio/src/app/sw-updates/sw-updates.module.ts | 2 - .../app/sw-updates/sw-updates.service.spec.ts | 341 ++++++++---------- 3 files changed, 144 insertions(+), 234 deletions(-) delete mode 100644 aio/src/app/sw-updates/noop-ng-service-worker.ts diff --git a/aio/src/app/sw-updates/noop-ng-service-worker.ts b/aio/src/app/sw-updates/noop-ng-service-worker.ts deleted file mode 100644 index d94b6c3c8d..0000000000 --- a/aio/src/app/sw-updates/noop-ng-service-worker.ts +++ /dev/null @@ -1,35 +0,0 @@ -// Alternative to NgServiceWorker when the browser doesn't support NgServiceWorker -// -// Many browsers do not support ServiceWorker (e.g, Safari). -// The Angular NgServiceWorker assumes that the browser supports ServiceWorker -// and starts talking to it immediately in its constructor without checking if it exists. -// Merely injecting the `NgServiceWorker` is an exception in any browser w/o ServiceWorker. -// -// Solution: when the browser doesn't support service worker and a class injects `NgServiceWorker` -// substitute the inert `NoopNgServiceWorker`. - -import { Injector } from '@angular/core'; -import { NgServiceWorker } from '@angular/service-worker'; - -import { Observable } from 'rxjs/Observable'; -import { of } from 'rxjs/observable/of'; - -export class NoopNgServiceWorker { - // Service worker is supported if `navigator['serviceWorker'] is defined. - isServiceWorkerSupported = !!navigator['serviceWorker']; - - checkForUpdate() { return of(false); } - activateUpdate(version: string) { return of(false); } -} - -export abstract class NgServiceWorkerForReals {} - -export function NgServiceWorkerFactory(injector: Injector, nsw: NoopNgServiceWorker) { - return nsw.isServiceWorkerSupported ? injector.get(NgServiceWorkerForReals) : nsw; -} - -export const noopNgServiceWorkerProviders = [ - NoopNgServiceWorker, - { provide: NgServiceWorkerForReals, useClass: NgServiceWorker }, - { provide: NgServiceWorker, useFactory: NgServiceWorkerFactory, - deps: [Injector, NoopNgServiceWorker] }]; diff --git a/aio/src/app/sw-updates/sw-updates.module.ts b/aio/src/app/sw-updates/sw-updates.module.ts index 71b53ea6e3..e0aa61d9f2 100644 --- a/aio/src/app/sw-updates/sw-updates.module.ts +++ b/aio/src/app/sw-updates/sw-updates.module.ts @@ -3,7 +3,6 @@ import { MdSnackBarModule } from '@angular/material'; import { ServiceWorkerModule } from '@angular/service-worker'; import { globalProvider } from './global.value'; -import { noopNgServiceWorkerProviders } from './noop-ng-service-worker'; import { SwUpdateNotificationsService } from './sw-update-notifications.service'; import { SwUpdatesService } from './sw-updates.service'; @@ -15,7 +14,6 @@ import { SwUpdatesService } from './sw-updates.service'; ], providers: [ globalProvider, - noopNgServiceWorkerProviders, SwUpdateNotificationsService, SwUpdatesService ] diff --git a/aio/src/app/sw-updates/sw-updates.service.spec.ts b/aio/src/app/sw-updates/sw-updates.service.spec.ts index 29993cc42e..e66f3ae456 100644 --- a/aio/src/app/sw-updates/sw-updates.service.spec.ts +++ b/aio/src/app/sw-updates/sw-updates.service.spec.ts @@ -1,20 +1,16 @@ import { ReflectiveInjector } from '@angular/core'; import { fakeAsync, tick } from '@angular/core/testing'; import { NgServiceWorker } from '@angular/service-worker'; -import { of } from 'rxjs/observable/of'; import { Subject } from 'rxjs/Subject'; import 'rxjs/add/operator/take'; -import { NgServiceWorkerForReals, NoopNgServiceWorker, noopNgServiceWorkerProviders } from './noop-ng-service-worker'; import { SwUpdatesService } from './sw-updates.service'; describe('SwUpdatesService', () => { let injector: ReflectiveInjector; let service: SwUpdatesService; let sw: MockNgServiceWorker; - let nsw: NoopNgServiceWorker; let checkInterval: number; - let isServiceWorkerSupportedInTest: boolean; // Helpers // NOTE: @@ -23,19 +19,13 @@ describe('SwUpdatesService', () => { // `setup()` in a `beforeEach()` block. We use the `run()` helper to call it inside each test' zone. const setup = () => { injector = ReflectiveInjector.resolveAndCreate([ - noopNgServiceWorkerProviders, - { provide: NgServiceWorkerForReals, useClass: MockNgServiceWorker }, - { provide: NoopNgServiceWorker, useClass: MockNoopNgServiceWorker }, + { provide: NgServiceWorker, useClass: MockNgServiceWorker }, SwUpdatesService ]); - nsw = injector.get(NoopNgServiceWorker); - // Set whether service worker exists before getting the SwUpdatesService! - nsw.isServiceWorkerSupported = isServiceWorkerSupportedInTest; - service = injector.get(SwUpdatesService); + sw = injector.get(NgServiceWorker); checkInterval = (service as any).checkInterval; - sw = injector.get(NgServiceWorkerForReals); }; const tearDown = () => service.ngOnDestroy(); const run = specFn => () => { @@ -44,212 +34,178 @@ describe('SwUpdatesService', () => { tearDown(); }; - describe('when service worker is supported', () => { - beforeEach(() => { - isServiceWorkerSupportedInTest = true; - }); + it('should create', run(() => { + expect(service).toBeTruthy(); + })); - it('should create', run(() => { - expect(service).toBeTruthy(); + it('should immediatelly check for updates when instantiated', run(() => { + expect(sw.checkForUpdate).toHaveBeenCalled(); + })); + + it('should schedule a new check if there is no update available', fakeAsync(run(() => { + sw.checkForUpdate.calls.reset(); + + sw.$$checkForUpdateSubj.next(false); + expect(sw.checkForUpdate).not.toHaveBeenCalled(); + + tick(checkInterval); + expect(sw.checkForUpdate).toHaveBeenCalled(); + }))); + + it('should not schedule a new check if there is an update available', fakeAsync(run(() => { + sw.checkForUpdate.calls.reset(); + + sw.$$checkForUpdateSubj.next(true); + expect(sw.checkForUpdate).not.toHaveBeenCalled(); + + tick(checkInterval); + expect(sw.checkForUpdate).not.toHaveBeenCalled(); + }))); + + describe('#activateUpdate()', () => { + it('should return a promise', run(() => { + expect(service.activateUpdate()).toEqual(jasmine.any(Promise)); })); - it('should call the NgServiceWorker', run(() => { - // does not call the Angular ServiceWorker - expect(sw.checkForUpdate).toHaveBeenCalled(); - // calls the noop Angular ServiceWorker instead - expect(nsw.checkForUpdate).not.toHaveBeenCalled(); + it('should call `NgServiceWorker.activateUpdate()`', run(() => { + expect(sw.activateUpdate).not.toHaveBeenCalled(); + + service.activateUpdate(); + expect(sw.activateUpdate).toHaveBeenCalled(); })); - it('should immediatelly check for updates when instantiated', run(() => { - expect(sw.checkForUpdate).toHaveBeenCalled(); + it('should not pass a specific version to `NgServiceWorker.activateUpdate()`', run(() => { + (service.activateUpdate as Function)('foo'); + expect(sw.activateUpdate).toHaveBeenCalledWith(null); })); - it('should schedule a new check if there is no update available', fakeAsync(run(() => { + it('should resolve the promise with the activation outcome', fakeAsync(run(() => { + let outcome; + + service.activateUpdate().then(v => outcome = v); + sw.$$activateUpdateSubj.next(true); + tick(); + expect(outcome).toBe(true); + + service.activateUpdate().then(v => outcome = v); + sw.$$activateUpdateSubj.next(false); + tick(); + expect(outcome).toBe(false); + }))); + + it('should schedule a new check (if the activation succeeded)', fakeAsync(run(() => { sw.checkForUpdate.calls.reset(); - sw.$$checkForUpdateSubj.next(false); + service.activateUpdate(); + + tick(checkInterval); + expect(sw.checkForUpdate).not.toHaveBeenCalled(); + + sw.$$activateUpdateSubj.next(true); expect(sw.checkForUpdate).not.toHaveBeenCalled(); tick(checkInterval); expect(sw.checkForUpdate).toHaveBeenCalled(); }))); - it('should not schedule a new check if there is an update available', fakeAsync(run(() => { + it('should schedule a new check (if the activation failed)', fakeAsync(run(() => { sw.checkForUpdate.calls.reset(); - sw.$$checkForUpdateSubj.next(true); - expect(sw.checkForUpdate).not.toHaveBeenCalled(); + service.activateUpdate(); tick(checkInterval); expect(sw.checkForUpdate).not.toHaveBeenCalled(); - }))); - describe('#activateUpdate()', () => { - it('should return a promise', run(() => { - expect(service.activateUpdate()).toEqual(jasmine.any(Promise)); - })); - - it('should call `NgServiceWorker.activateUpdate()`', run(() => { - expect(sw.activateUpdate).not.toHaveBeenCalled(); - - service.activateUpdate(); - expect(sw.activateUpdate).toHaveBeenCalled(); - })); - - it('should not pass a specific version to `NgServiceWorker.activateUpdate()`', run(() => { - (service.activateUpdate as Function)('foo'); - expect(sw.activateUpdate).toHaveBeenCalledWith(null); - })); - - it('should resolve the promise with the activation outcome', fakeAsync(run(() => { - let outcome; - - service.activateUpdate().then(v => outcome = v); - sw.$$activateUpdateSubj.next(true); - tick(); - expect(outcome).toBe(true); - - service.activateUpdate().then(v => outcome = v); - sw.$$activateUpdateSubj.next(false); - tick(); - expect(outcome).toBe(false); - }))); - - it('should schedule a new check (if the activation succeeded)', fakeAsync(run(() => { - sw.checkForUpdate.calls.reset(); - - service.activateUpdate(); - - tick(checkInterval); - expect(sw.checkForUpdate).not.toHaveBeenCalled(); - - sw.$$activateUpdateSubj.next(true); - expect(sw.checkForUpdate).not.toHaveBeenCalled(); - - tick(checkInterval); - expect(sw.checkForUpdate).toHaveBeenCalled(); - }))); - - it('should schedule a new check (if the activation failed)', fakeAsync(run(() => { - sw.checkForUpdate.calls.reset(); - - service.activateUpdate(); - - tick(checkInterval); - expect(sw.checkForUpdate).not.toHaveBeenCalled(); - - sw.$$activateUpdateSubj.next(false); - expect(sw.checkForUpdate).not.toHaveBeenCalled(); - - tick(checkInterval); - expect(sw.checkForUpdate).toHaveBeenCalled(); - }))); - }); - - describe('#isUpdateAvailable', () => { - let emittedValues: boolean[]; - - // Helpers - const withSubscription = specFn => () => { - emittedValues = []; - service.isUpdateAvailable.subscribe(v => emittedValues.push(v)); - specFn(); - }; - - - it('should emit `false/true` when there is/isn\'t an update available', - fakeAsync(run(withSubscription(() => { - expect(emittedValues).toEqual([]); - - sw.$$checkForUpdateSubj.next(false); - expect(emittedValues).toEqual([false]); - - tick(checkInterval); - sw.$$checkForUpdateSubj.next(true); - expect(emittedValues).toEqual([false, true]); - }))) - ); - - it('should emit only when the value has changed', - fakeAsync(run(withSubscription(() => { - expect(emittedValues).toEqual([]); - - sw.$$checkForUpdateSubj.next(false); - expect(emittedValues).toEqual([false]); - - tick(checkInterval); - sw.$$checkForUpdateSubj.next(false); - expect(emittedValues).toEqual([false]); - - tick(checkInterval); - sw.$$checkForUpdateSubj.next(false); - expect(emittedValues).toEqual([false]); - }))) - ); - - it('should emit `false` after a successful activation', - fakeAsync(run(withSubscription(() => { - sw.$$checkForUpdateSubj.next(true); - expect(emittedValues).toEqual([true]); - - service.activateUpdate(); - sw.$$activateUpdateSubj.next(true); - - expect(emittedValues).toEqual([true, false]); - }))) - ); - - it('should emit `false` after a failed activation', - fakeAsync(run(withSubscription(() => { - sw.$$checkForUpdateSubj.next(true); - expect(emittedValues).toEqual([true]); - - service.activateUpdate(); - sw.$$activateUpdateSubj.next(false); - - expect(emittedValues).toEqual([true, false]); - }))) - ); - - it('should not emit a new value after activation if already `false`', - fakeAsync(run(withSubscription(() => { - sw.$$checkForUpdateSubj.next(false); - expect(emittedValues).toEqual([false]); - - service.activateUpdate(); - sw.$$activateUpdateSubj.next(true); - - expect(emittedValues).toEqual([false]); - }))) - ); - }); - }); - - describe('when service worker isn\'t supported (Safari)', () => { - - beforeEach(() => { - isServiceWorkerSupportedInTest = false; - }); - - it('should create', run(() => { - expect(service).toBeTruthy(); - })); - - it('should call the NoopNgServiceWorker', run(() => { - // does not call the Angular ServiceWorker + sw.$$activateUpdateSubj.next(false); expect(sw.checkForUpdate).not.toHaveBeenCalled(); - // calls the noop Angular ServiceWorker instead - expect(nsw.checkForUpdate).toHaveBeenCalled(); - })); + tick(checkInterval); + expect(sw.checkForUpdate).toHaveBeenCalled(); + }))); }); + describe('#isUpdateAvailable', () => { + let emittedValues: boolean[]; + + // Helpers + const withSubscription = specFn => () => { + emittedValues = []; + service.isUpdateAvailable.subscribe(v => emittedValues.push(v)); + specFn(); + }; + + + it('should emit `false/true` when there is/isn\'t an update available', + fakeAsync(run(withSubscription(() => { + expect(emittedValues).toEqual([]); + + sw.$$checkForUpdateSubj.next(false); + expect(emittedValues).toEqual([false]); + + tick(checkInterval); + sw.$$checkForUpdateSubj.next(true); + expect(emittedValues).toEqual([false, true]); + }))) + ); + + it('should emit only when the value has changed', + fakeAsync(run(withSubscription(() => { + expect(emittedValues).toEqual([]); + + sw.$$checkForUpdateSubj.next(false); + expect(emittedValues).toEqual([false]); + + tick(checkInterval); + sw.$$checkForUpdateSubj.next(false); + expect(emittedValues).toEqual([false]); + + tick(checkInterval); + sw.$$checkForUpdateSubj.next(false); + expect(emittedValues).toEqual([false]); + }))) + ); + + it('should emit `false` after a successful activation', + fakeAsync(run(withSubscription(() => { + sw.$$checkForUpdateSubj.next(true); + expect(emittedValues).toEqual([true]); + + service.activateUpdate(); + sw.$$activateUpdateSubj.next(true); + + expect(emittedValues).toEqual([true, false]); + }))) + ); + + it('should emit `false` after a failed activation', + fakeAsync(run(withSubscription(() => { + sw.$$checkForUpdateSubj.next(true); + expect(emittedValues).toEqual([true]); + + service.activateUpdate(); + sw.$$activateUpdateSubj.next(false); + + expect(emittedValues).toEqual([true, false]); + }))) + ); + + it('should not emit a new value after activation if already `false`', + fakeAsync(run(withSubscription(() => { + sw.$$checkForUpdateSubj.next(false); + expect(emittedValues).toEqual([false]); + + service.activateUpdate(); + sw.$$activateUpdateSubj.next(true); + + expect(emittedValues).toEqual([false]); + }))) + ); + }); }); // Mocks class MockNgServiceWorker { - $$activateUpdateSubj = new Subject(); $$checkForUpdateSubj = new Subject(); @@ -259,12 +215,3 @@ class MockNgServiceWorker { checkForUpdate = jasmine.createSpy('MockNgServiceWorker.checkForUpdate') .and.callFake(() => this.$$checkForUpdateSubj.take(1)); } - -class MockNoopNgServiceWorker extends NoopNgServiceWorker { - constructor() { - super(); - this.isServiceWorkerSupported = true; // assume it is by default - spyOn(this, 'activateUpdate').and.callThrough(); - spyOn(this, 'checkForUpdate').and.callThrough(); - } -}