From fe135e11984c8a081c44e9027185d6f964e64a0f Mon Sep 17 00:00:00 2001 From: George Kalpakas Date: Thu, 8 Jul 2021 16:25:15 +0300 Subject: [PATCH] refactor(service-worker): remove duplicate `Context` type (in favor of `ExtendableEvent`) (#42736) This commit removes the duplicate `Context` interface and uses the `ExtendableEvent` interface instead. This is in preparation of switching from our custom typings to the official TypeScript typings (`lib.webworker.d.ts`). PR Close #42736 --- packages/service-worker/worker/src/adapter.ts | 11 --- .../service-worker/worker/src/app-version.ts | 10 +-- packages/service-worker/worker/src/assets.ts | 4 +- packages/service-worker/worker/src/data.ts | 18 ++--- .../worker/test/prefetch_spec.ts | 18 +++-- .../service-worker/worker/testing/scope.ts | 73 +++++++++++++++---- 6 files changed, 85 insertions(+), 49 deletions(-) diff --git a/packages/service-worker/worker/src/adapter.ts b/packages/service-worker/worker/src/adapter.ts index c711436d1e..ad56aee12a 100644 --- a/packages/service-worker/worker/src/adapter.ts +++ b/packages/service-worker/worker/src/adapter.ts @@ -104,14 +104,3 @@ export class Adapter { }); } } - -/** - * An event context in which an operation is taking place, which allows - * the delaying of Service Worker shutdown until certain triggers occur. - */ -export interface Context { - /** - * Delay shutdown of the Service Worker until the given promise resolves. - */ - waitUntil(fn: Promise): void; -} diff --git a/packages/service-worker/worker/src/app-version.ts b/packages/service-worker/worker/src/app-version.ts index e02c4e1d44..b6ad441279 100644 --- a/packages/service-worker/worker/src/app-version.ts +++ b/packages/service-worker/worker/src/app-version.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Adapter, Context} from './adapter'; +import {Adapter} from './adapter'; import {CacheState, NormalizedUrl, UpdateCacheStatus, UpdateSource} from './api'; import {AssetGroup, LazyAssetGroup, PrefetchAssetGroup} from './assets'; import {DataGroup} from './data'; @@ -135,7 +135,7 @@ export class AppVersion implements UpdateSource { } } - async handleFetch(req: Request, context: Context): Promise { + async handleFetch(req: Request, event: ExtendableEvent): Promise { // Check the request against each `AssetGroup` in sequence. If an `AssetGroup` can't handle the // request, // it will return `null`. Thus, the first non-null response is the SW's answer to the request. @@ -152,7 +152,7 @@ export class AppVersion implements UpdateSource { } // No response has been found yet. Maybe this group will have one. - return group.handleFetch(req, context); + return group.handleFetch(req, event); }, Promise.resolve(null)); // The result of the above is the asset response, if there is any, or null otherwise. Return the @@ -170,7 +170,7 @@ export class AppVersion implements UpdateSource { return resp; } - return group.handleFetch(req, context); + return group.handleFetch(req, event); }, Promise.resolve(null)); // If the data caching group returned a response, go with it. @@ -195,7 +195,7 @@ export class AppVersion implements UpdateSource { // This was a navigation request. Re-enter `handleFetch` with a request for // the URL. - return this.handleFetch(this.adapter.newRequest(this.indexUrl), context); + return this.handleFetch(this.adapter.newRequest(this.indexUrl), event); } return null; diff --git a/packages/service-worker/worker/src/assets.ts b/packages/service-worker/worker/src/assets.ts index b6650ff0e6..c29707a45f 100644 --- a/packages/service-worker/worker/src/assets.ts +++ b/packages/service-worker/worker/src/assets.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Adapter, Context} from './adapter'; +import {Adapter} from './adapter'; import {CacheState, NormalizedUrl, UpdateCacheStatus, UpdateSource, UrlMetadata} from './api'; import {Database, Table} from './database'; import {CacheTable} from './db-cache'; @@ -114,7 +114,7 @@ export abstract class AssetGroup { /** * Process a request for a given resource and return it, or return null if it's not available. */ - async handleFetch(req: Request, ctx: Context): Promise { + async handleFetch(req: Request, _event: ExtendableEvent): Promise { const url = this.adapter.normalizeUrl(req.url); // Either the request matches one of the known resource URLs, one of the patterns for // dynamically matched URLs, or neither. Determine which is the case for this request in diff --git a/packages/service-worker/worker/src/data.ts b/packages/service-worker/worker/src/data.ts index ef41458a3c..636d6321fa 100644 --- a/packages/service-worker/worker/src/data.ts +++ b/packages/service-worker/worker/src/data.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {Adapter, Context} from './adapter'; +import {Adapter} from './adapter'; import {Database, Table} from './database'; import {CacheTable} from './db-cache'; import {DebugHandler} from './debug'; @@ -294,7 +294,7 @@ export class DataGroup { * Process a fetch event and return a `Response` if the resource is covered by this group, * or `null` otherwise. */ - async handleFetch(req: Request, ctx: Context): Promise { + async handleFetch(req: Request, event: ExtendableEvent): Promise { // Do nothing if (!this.patterns.some(pattern => pattern.test(req.url))) { return null; @@ -314,9 +314,9 @@ export class DataGroup { // Handle the request with whatever strategy was selected. switch (this.config.strategy) { case 'freshness': - return this.handleFetchWithFreshness(req, ctx, lru); + return this.handleFetchWithFreshness(req, event, lru); case 'performance': - return this.handleFetchWithPerformance(req, ctx, lru); + return this.handleFetchWithPerformance(req, event, lru); default: throw new Error(`Unknown strategy: ${this.config.strategy}`); } @@ -337,7 +337,7 @@ export class DataGroup { } } - private async handleFetchWithPerformance(req: Request, ctx: Context, lru: LruList): + private async handleFetchWithPerformance(req: Request, event: ExtendableEvent, lru: LruList): Promise { let res: Response|null|undefined = null; @@ -348,7 +348,7 @@ export class DataGroup { res = fromCache.res; // Check the age of the resource. if (this.config.refreshAheadMs !== undefined && fromCache.age >= this.config.refreshAheadMs) { - ctx.waitUntil(this.safeCacheResponse(req, this.safeFetch(req), lru)); + event.waitUntil(this.safeCacheResponse(req, this.safeFetch(req), lru)); } } @@ -367,7 +367,7 @@ export class DataGroup { res = this.adapter.newResponse(null, {status: 504, statusText: 'Gateway Timeout'}); // Cache the network response eventually. - ctx.waitUntil(this.safeCacheResponse(req, networkFetch, lru)); + event.waitUntil(this.safeCacheResponse(req, networkFetch, lru)); } else { // The request completed in time, so cache it inline with the response flow. await this.safeCacheResponse(req, res, lru); @@ -376,7 +376,7 @@ export class DataGroup { return res; } - private async handleFetchWithFreshness(req: Request, ctx: Context, lru: LruList): + private async handleFetchWithFreshness(req: Request, event: ExtendableEvent, lru: LruList): Promise { // Start with a network fetch. const [timeoutFetch, networkFetch] = this.networkFetchWithTimeout(req); @@ -392,7 +392,7 @@ export class DataGroup { // If the network fetch times out or errors, fall back on the cache. if (res === undefined) { - ctx.waitUntil(this.safeCacheResponse(req, networkFetch, lru, true)); + event.waitUntil(this.safeCacheResponse(req, networkFetch, lru, true)); // Ignore the age, the network response will be cached anyway due to the // behavior of freshness. diff --git a/packages/service-worker/worker/test/prefetch_spec.ts b/packages/service-worker/worker/test/prefetch_spec.ts index ceef0b5d05..9531efe7d4 100644 --- a/packages/service-worker/worker/test/prefetch_spec.ts +++ b/packages/service-worker/worker/test/prefetch_spec.ts @@ -12,7 +12,7 @@ import {IdleScheduler} from '../src/idle'; import {MockCache} from '../testing/cache'; import {MockRequest} from '../testing/fetch'; import {MockFileSystemBuilder, MockServerStateBuilder, tmpHashTable, tmpManifestSingleAssetGroup} from '../testing/mock'; -import {SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; +import {MockExtendableEvent, SwTestHarness, SwTestHarnessBuilder} from '../testing/scope'; (function() { // Skip environments that don't support the minimum APIs needed to run the SW tests. @@ -33,6 +33,8 @@ const scope = new SwTestHarnessBuilder().withServerState(server).build(); const db = new CacheDatabase(scope); +const testEvent = new MockExtendableEvent('test'); + describe('prefetch assets', () => { let group: PrefetchAssetGroup; @@ -50,8 +52,8 @@ describe('prefetch assets', () => { it('fully caches the two files', async () => { await group.initializeFully(); scope.updateServerState(); - const res1 = await group.handleFetch(scope.newRequest('/foo.txt'), scope); - const res2 = await group.handleFetch(scope.newRequest('/bar.txt'), scope); + const res1 = await group.handleFetch(scope.newRequest('/foo.txt'), testEvent); + const res2 = await group.handleFetch(scope.newRequest('/bar.txt'), testEvent); expect(await res1!.text()).toEqual('this is foo'); expect(await res2!.text()).toEqual('this is bar'); }); @@ -63,14 +65,14 @@ describe('prefetch assets', () => { freshScope, freshScope, idle, manifest.assetGroups![0], tmpHashTable(manifest), new CacheDatabase(freshScope), 'test'); await group.initializeFully(); - const res1 = await group.handleFetch(scope.newRequest('/foo.txt'), scope); - const res2 = await group.handleFetch(scope.newRequest('/bar.txt'), scope); + const res1 = await group.handleFetch(scope.newRequest('/foo.txt'), testEvent); + const res2 = await group.handleFetch(scope.newRequest('/bar.txt'), testEvent); expect(await res1!.text()).toEqual('this is foo'); expect(await res2!.text()).toEqual('this is bar'); }); it('caches properly if resources are requested before initialization', async () => { - const res1 = await group.handleFetch(scope.newRequest('/foo.txt'), scope); - const res2 = await group.handleFetch(scope.newRequest('/bar.txt'), scope); + const res1 = await group.handleFetch(scope.newRequest('/foo.txt'), testEvent); + const res2 = await group.handleFetch(scope.newRequest('/bar.txt'), testEvent); expect(await res1!.text()).toEqual('this is foo'); expect(await res2!.text()).toEqual('this is bar'); scope.updateServerState(); @@ -90,7 +92,7 @@ describe('prefetch assets', () => { it('CacheQueryOptions are passed through', async () => { await group.initializeFully(); const matchSpy = spyOn(MockCache.prototype, 'match').and.callThrough(); - await group.handleFetch(scope.newRequest('/foo.txt'), scope); + await group.handleFetch(scope.newRequest('/foo.txt'), testEvent); expect(matchSpy).toHaveBeenCalledWith(new MockRequest('/foo.txt'), {ignoreVary: true}); }); }); diff --git a/packages/service-worker/worker/testing/scope.ts b/packages/service-worker/worker/testing/scope.ts index fb92476c3d..0b507d59e7 100644 --- a/packages/service-worker/worker/testing/scope.ts +++ b/packages/service-worker/worker/testing/scope.ts @@ -8,7 +8,7 @@ import {Subject} from 'rxjs'; -import {Adapter, Context} from '../src/adapter'; +import {Adapter} from '../src/adapter'; import {AssetGroupConfig, Manifest} from '../src/manifest'; import {sha1} from '../src/sha1'; @@ -105,8 +105,7 @@ export class MockClients implements Clients { async claim(): Promise {} } -export class SwTestHarness extends Adapter implements Context, - ServiceWorkerGlobalScope { +export class SwTestHarness extends Adapter implements ServiceWorkerGlobalScope { readonly clients = new MockClients(); private eventHandlers = new Map(); private skippedWaiting = false; @@ -239,8 +238,6 @@ export class SwTestHarness extends Adapter implements Context, this.skippedWaiting = true; } - waitUntil(promise: Promise): void {} - handleFetch(req: Request, clientId = ''): [Promise, Promise] { if (!this.eventHandlers.has('fetch')) { throw new Error('No fetch handler registered'); @@ -376,7 +373,49 @@ export class ConfigBuilder { } } -class OneTimeContext implements Context { +class MockEvent implements Event { + readonly AT_TARGET = -1; + readonly BUBBLING_PHASE = -1; + readonly CAPTURING_PHASE = -1; + readonly NONE = -1; + + readonly bubbles = false; + cancelBubble = false; + readonly cancelable = false; + readonly composed = false; + readonly currentTarget = null; + readonly defaultPrevented = false; + readonly eventPhase = -1; + readonly isTrusted = false; + returnValue = false; + readonly srcElement = null; + readonly target = null; + readonly timeStamp = Date.now(); + + constructor(readonly type: string) {} + + composedPath(): EventTarget[] { + this.notImplemented(); + } + initEvent(type: string, bubbles?: boolean, cancelable?: boolean): void { + this.notImplemented(); + } + preventDefault(): void { + this.notImplemented(); + } + stopImmediatePropagation(): void { + this.notImplemented(); + } + stopPropagation(): void { + this.notImplemented(); + } + + private notImplemented(): never { + throw new Error('Method not implemented in `MockEvent`.'); + } +} + +export class MockExtendableEvent extends MockEvent implements ExtendableEvent { private queue: Promise[] = []; waitUntil(promise: Promise): void { @@ -392,14 +431,12 @@ class OneTimeContext implements Context { } } -class MockExtendableEvent extends OneTimeContext {} - class MockFetchEvent extends MockExtendableEvent { response: Promise = Promise.resolve(undefined); constructor( readonly request: Request, readonly clientId: string, readonly resultingClientId: string) { - super(); + super('fetch'); } respondWith(promise: Promise): Promise { @@ -410,13 +447,13 @@ class MockFetchEvent extends MockExtendableEvent { class MockMessageEvent extends MockExtendableEvent { constructor(readonly data: Object, readonly source: MockClient|null) { - super(); + super('message'); } } class MockPushEvent extends MockExtendableEvent { constructor(private _data: Object) { - super(); + super('push'); } data = { json: () => this._data, @@ -425,11 +462,19 @@ class MockPushEvent extends MockExtendableEvent { class MockNotificationEvent extends MockExtendableEvent { constructor(private _notification: any, readonly action?: string) { - super(); + super('notification'); } readonly notification = {...this._notification, close: () => undefined}; } -class MockInstallEvent extends MockExtendableEvent {} +class MockInstallEvent extends MockExtendableEvent { + constructor() { + super('install'); + } +} -class MockActivateEvent extends MockExtendableEvent {} +class MockActivateEvent extends MockExtendableEvent { + constructor() { + super('activate'); + } +}