From 3f8cf5935b6c83508c189052bfe2f310f78302f3 Mon Sep 17 00:00:00 2001 From: Jeff Posnick Date: Fri, 19 Jun 2020 17:14:41 -0400 Subject: [PATCH 1/8] Attempted type normalization --- packages/workbox-core/src/types.ts | 69 ++++++++++++------- .../workbox-routing/src/NavigationRoute.ts | 10 +-- packages/workbox-routing/src/RegExpRoute.ts | 9 ++- packages/workbox-routing/src/Route.ts | 11 +-- packages/workbox-routing/src/Router.ts | 32 +++++---- packages/workbox-routing/src/_types.ts | 17 ----- packages/workbox-routing/src/registerRoute.ts | 12 ++-- .../workbox-routing/src/setCatchHandler.ts | 8 ++- .../workbox-routing/src/setDefaultHandler.ts | 8 ++- .../src/utils/normalizeHandler.ts | 7 +- packages/workbox-strategies/src/Strategy.ts | 27 ++++---- .../workbox-strategies/src/StrategyHandler.ts | 14 ++-- 12 files changed, 122 insertions(+), 102 deletions(-) diff --git a/packages/workbox-core/src/types.ts b/packages/workbox-core/src/types.ts index ea5779e1b..443821d43 100644 --- a/packages/workbox-core/src/types.ts +++ b/packages/workbox-core/src/types.ts @@ -23,10 +23,10 @@ export type PluginState = MapLikeObject; * Options passed to a `RouteMatchCallback` function. */ export interface RouteMatchCallbackOptions { - url: URL; - sameOrigin: boolean; + event: ExtendableEvent; request: Request; - event?: ExtendableEvent; + sameOrigin: boolean; + url: URL; } /** @@ -48,12 +48,23 @@ export interface RouteMatchCallback { * Options passed to a `RouteHandlerCallback` function. */ export interface RouteHandlerCallbackOptions { - request: Request | string; - url?: URL; + event: ExtendableEvent; + request: Request; + url: URL; params?: string[] | MapLikeObject; +} + +/** + * Options passed to a `ManualHandlerCallback` function. + */ +export interface ManualHandlerCallbackOptions { + request: Request | string; event?: ExtendableEvent; } +export type HandlerCallbackOptions = + RouteHandlerCallbackOptions | ManualHandlerCallbackOptions; + /** * The "handler" callback is invoked whenever a `Router` matches a URL/Request * to a `Route` via its `RouteMatchCallback`. This handler callback should @@ -66,6 +77,18 @@ export interface RouteHandlerCallback { (options: RouteHandlerCallbackOptions): Promise; } +/** + * The "handler" callback is invoked whenever a `Router` matches a URL/Request + * to a `Route` via its `RouteMatchCallback`. This handler callback should + * return a `Promise` that resolves with a `Response`. + * + * If a non-empty array or object is returned by the `RouteMatchCallback` it + * will be passed in as this handler's `options.params` argument. + */ +export interface HandlerCallback { + (options: HandlerCallbackOptions): Promise; +} + /** * An object with a `handle` method of type `RouteHandlerCallback`. * @@ -95,10 +118,10 @@ export interface HandlerWillStartCallback { export interface CacheDidUpdateCallbackParam { cacheName: string; - oldResponse?: Response | null; newResponse: Response; request: Request; event?: ExtendableEvent; + oldResponse?: Response | null; state?: PluginState; } @@ -107,10 +130,10 @@ export interface CacheDidUpdateCallback { } export interface CacheKeyWillBeUsedCallbackParam { - request: Request; mode: string; - params?: any; + request: Request; event?: ExtendableEvent; + params?: any; state?: PluginState; } @@ -119,8 +142,8 @@ export interface CacheKeyWillBeUsedCallback { } export interface CacheWillUpdateCallbackParam { - response: Response; request: Request; + response: Response; event?: ExtendableEvent; state?: PluginState; } @@ -132,9 +155,9 @@ export interface CacheWillUpdateCallback { export interface CachedResponseWillBeUsedCallbackParam { cacheName: string; request: Request; - matchOptions?: CacheQueryOptions; cachedResponse?: Response; event?: ExtendableEvent; + matchOptions?: CacheQueryOptions; state?: PluginState; } @@ -143,8 +166,8 @@ export interface CachedResponseWillBeUsedCallback { } export interface FetchDidFailCallbackParam { - originalRequest: Request; error: Error; + originalRequest: Request; request: Request; event?: ExtendableEvent; state?: PluginState; @@ -188,8 +211,8 @@ export interface HandlerWillRespondCallback { export interface HandlerDidRespondCallbackParam { request: Request; - response?: Response; event?: ExtendableEvent; + response?: Response; state?: PluginState; } @@ -199,9 +222,9 @@ export interface HandlerDidRespondCallback { export interface HandlerDidCompleteCallbackParam { request: Request; - response?: Response; error?: Error; event?: ExtendableEvent; + response?: Response; state?: PluginState; } @@ -214,29 +237,29 @@ export interface HandlerDidCompleteCallback { * cache operations. */ export interface WorkboxPlugin { - handlerWillStart?: HandlerWillStartCallback; cacheDidUpdate?: CacheDidUpdateCallback; + cachedResponseWillBeUsed?: CachedResponseWillBeUsedCallback; cacheKeyWillBeUsed?: CacheKeyWillBeUsedCallback; cacheWillUpdate?: CacheWillUpdateCallback; - cachedResponseWillBeUsed?: CachedResponseWillBeUsedCallback; fetchDidFail?: FetchDidFailCallback; fetchDidSucceed?: FetchDidSucceedCallback; - requestWillFetch?: RequestWillFetchCallback; - handlerWillRespond?: HandlerWillRespondCallback; - handlerDidRespond?: HandlerDidRespondCallback; handlerDidComplete?: HandlerDidCompleteCallback; + handlerDidRespond?: HandlerDidRespondCallback; + handlerWillRespond?: HandlerWillRespondCallback; + handlerWillStart?: HandlerWillStartCallback; + requestWillFetch?: RequestWillFetchCallback; } export interface WorkboxPluginCallbackParam { - handlerWillStart: HandlerWillStartCallbackParam; cacheDidUpdate: CacheDidUpdateCallbackParam; + cachedResponseWillBeUsed: CachedResponseWillBeUsedCallbackParam; cacheKeyWillBeUsed: CacheKeyWillBeUsedCallbackParam; cacheWillUpdate: CacheWillUpdateCallbackParam; - cachedResponseWillBeUsed: CachedResponseWillBeUsedCallbackParam; fetchDidFail: FetchDidFailCallbackParam; fetchDidSucceed: FetchDidSucceedCallbackParam; - requestWillFetch: RequestWillFetchCallbackParam; - handlerWillRespond: HandlerWillRespondCallbackParam; - handlerDidRespond: HandlerDidRespondCallbackParam; handlerDidComplete: HandlerDidCompleteCallbackParam; + handlerDidRespond: HandlerDidRespondCallbackParam; + handlerWillRespond: HandlerWillRespondCallbackParam; + handlerWillStart: HandlerWillStartCallbackParam; + requestWillFetch: RequestWillFetchCallbackParam; } diff --git a/packages/workbox-routing/src/NavigationRoute.ts b/packages/workbox-routing/src/NavigationRoute.ts index 110c36d9a..91ae6a587 100644 --- a/packages/workbox-routing/src/NavigationRoute.ts +++ b/packages/workbox-routing/src/NavigationRoute.ts @@ -8,8 +8,10 @@ import {assert} from 'workbox-core/_private/assert.js'; import {logger} from 'workbox-core/_private/logger.js'; +import {RouteHandler, RouteMatchCallbackOptions} from 'workbox-core/types.js'; + import {Route} from './Route.js'; -import {Handler, MatchCallbackOptions} from './_types.js'; + import './_version.js'; export interface NavigationRouteMatchOptions { @@ -55,7 +57,7 @@ class NavigationRoute extends Route { * match the URL's pathname and search parameter, the route will handle the * request (assuming the denylist doesn't match). */ - constructor(handler: Handler, + constructor(handler: RouteHandler, {allowlist = [/./], denylist = []}: NavigationRouteMatchOptions = {}) { if (process.env.NODE_ENV !== 'production') { assert!.isArrayOfClass(allowlist, RegExp, { @@ -72,7 +74,7 @@ class NavigationRoute extends Route { }); } - super((options: MatchCallbackOptions) => this._match(options), handler); + super((options: RouteMatchCallbackOptions) => this._match(options), handler); this._allowlist = allowlist; this._denylist = denylist; @@ -88,7 +90,7 @@ class NavigationRoute extends Route { * * @private */ - private _match({url, request}: MatchCallbackOptions): boolean { + private _match({url, request}: RouteMatchCallbackOptions): boolean { if (request && request.mode !== 'navigate') { return false; } diff --git a/packages/workbox-routing/src/RegExpRoute.ts b/packages/workbox-routing/src/RegExpRoute.ts index c5b5d7ffd..a808723cf 100644 --- a/packages/workbox-routing/src/RegExpRoute.ts +++ b/packages/workbox-routing/src/RegExpRoute.ts @@ -8,9 +8,12 @@ import {assert} from 'workbox-core/_private/assert.js'; import {logger} from 'workbox-core/_private/logger.js'; +import {RouteHandler, RouteMatchCallback, RouteMatchCallbackOptions} + from 'workbox-core/types.js'; + import {HTTPMethod} from './utils/constants.js'; import {Route} from './Route.js'; -import {Handler, MatchCallbackOptions, MatchCallback} from './_types.js'; + import './_version.js'; @@ -41,7 +44,7 @@ class RegExpRoute extends Route { * @param {string} [method='GET'] The HTTP method to match the Route * against. */ - constructor(regExp: RegExp, handler: Handler, method?: HTTPMethod) { + constructor(regExp: RegExp, handler: RouteHandler, method?: HTTPMethod) { if (process.env.NODE_ENV !== 'production') { assert!.isInstance(regExp, RegExp, { moduleName: 'workbox-routing', @@ -51,7 +54,7 @@ class RegExpRoute extends Route { }); } - const match: MatchCallback = ({url}: MatchCallbackOptions) => { + const match: RouteMatchCallback = ({url}: RouteMatchCallbackOptions) => { const result = regExp.exec(url.href); // Return immediately if there's no match. diff --git a/packages/workbox-routing/src/Route.ts b/packages/workbox-routing/src/Route.ts index da8a728e1..899e33dba 100644 --- a/packages/workbox-routing/src/Route.ts +++ b/packages/workbox-routing/src/Route.ts @@ -9,7 +9,8 @@ import {assert} from 'workbox-core/_private/assert.js'; import {HTTPMethod, defaultMethod, validMethods} from './utils/constants.js'; import {normalizeHandler} from './utils/normalizeHandler.js'; -import {Handler, HandlerObject, MatchCallback} from './_types.js'; +import {RouteHandler, RouteHandlerObject, RouteMatchCallback} + from 'workbox-core/types.js'; import './_version.js'; @@ -23,8 +24,8 @@ import './_version.js'; * @memberof module:workbox-routing */ class Route { - handler: HandlerObject; - match: MatchCallback; + handler: RouteHandlerObject; + match: RouteMatchCallback; method: HTTPMethod; /** @@ -39,8 +40,8 @@ class Route { * against. */ constructor( - match: MatchCallback, - handler: Handler, + match: RouteMatchCallback, + handler: RouteHandler, method: HTTPMethod = defaultMethod) { if (process.env.NODE_ENV !== 'production') { assert!.isType(match, 'function', { diff --git a/packages/workbox-routing/src/Router.ts b/packages/workbox-routing/src/Router.ts index 6ade5e12e..2f44cd4fb 100644 --- a/packages/workbox-routing/src/Router.ts +++ b/packages/workbox-routing/src/Router.ts @@ -8,8 +8,12 @@ import {assert} from 'workbox-core/_private/assert.js'; import {getFriendlyURL} from 'workbox-core/_private/getFriendlyURL.js'; -import {Handler, HandlerObject, HandlerCallbackOptions, MatchCallbackOptions} - from './_types.js'; +import { + RouteHandler, + RouteHandlerObject, + RouteHandlerCallbackOptions, + RouteMatchCallbackOptions, +} from 'workbox-core/types.js'; import {HTTPMethod, defaultMethod} from './utils/constants.js'; import {logger} from 'workbox-core/_private/logger.js'; import {normalizeHandler} from './utils/normalizeHandler.js'; @@ -46,8 +50,8 @@ interface CacheURLsMessageData { */ class Router { private readonly _routes: Map; - private readonly _defaultHandlerMap: Map; - private _catchHandler?: HandlerObject; + private readonly _defaultHandlerMap: Map; + private _catchHandler?: RouteHandlerObject; /** * Initializes a new Router. @@ -120,7 +124,7 @@ class Router { } const request = new Request(...entry); - return this.handleRequest({request}); + return this.handleRequest({request, event}); // TODO(philipwalton): TypeScript errors without this typecast for // some reason (probably a bug). The real type here should work but @@ -142,17 +146,16 @@ class Router { * appropriate Route's handler. * * @param {Object} options - * @param {Request} options.request The request to handle (this is usually - * from a fetch event, but it does not have to be). - * @param {FetchEvent} [options.event] The event that triggered the request, - * if applicable. + * @param {Request} options.request The request to handle. + * @param {ExtendableEvent} options.event The event that triggered the + * request. * @return {Promise|undefined} A promise is returned if a * registered route can handle the request. If there is no matching * route and there's no `defaultHandler`, `undefined` is returned. */ handleRequest({request, event}: { request: Request; - event?: ExtendableEvent; + event: ExtendableEvent; }): Promise | undefined { if (process.env.NODE_ENV !== 'production') { assert!.isInstance(request, Request, { @@ -272,8 +275,9 @@ class Router { * They are populated if a matching route was found or `undefined` * otherwise. */ - findMatchingRoute({url, sameOrigin, request, event}: MatchCallbackOptions): - {route?: Route; params?: HandlerCallbackOptions['params']} { + findMatchingRoute( + {url, sameOrigin, request, event}: RouteMatchCallbackOptions): + {route?: Route; params?: RouteHandlerCallbackOptions['params']} { const routes = this._routes.get(request.method as HTTPMethod) || []; for (const route of routes) { let params; @@ -317,7 +321,7 @@ class Router { * @param {string} [method='GET'] The HTTP method to associate with this * default handler. Each method has its own default. */ - setDefaultHandler(handler: Handler, method: HTTPMethod = defaultMethod) { + setDefaultHandler(handler: RouteHandler, method: HTTPMethod = defaultMethod) { this._defaultHandlerMap.set(method, normalizeHandler(handler)); } @@ -328,7 +332,7 @@ class Router { * @param {module:workbox-routing~handlerCallback} handler A callback * function that returns a Promise resulting in a Response. */ - setCatchHandler(handler: Handler) { + setCatchHandler(handler: RouteHandler) { this._catchHandler = normalizeHandler(handler); } diff --git a/packages/workbox-routing/src/_types.ts b/packages/workbox-routing/src/_types.ts index 15b1f2b12..284f9c089 100644 --- a/packages/workbox-routing/src/_types.ts +++ b/packages/workbox-routing/src/_types.ts @@ -6,25 +6,8 @@ https://opensource.org/licenses/MIT. */ -import { - RouteHandler, - RouteHandlerObject, - RouteHandlerCallback, - RouteHandlerCallbackOptions, - RouteMatchCallback, - RouteMatchCallbackOptions -} from 'workbox-core/types.js'; import './_version.js'; - export { - RouteHandler as Handler, - RouteHandlerObject as HandlerObject, - RouteHandlerCallback as HandlerCallback, - RouteHandlerCallbackOptions as HandlerCallbackOptions, - RouteMatchCallback as MatchCallback, - RouteMatchCallbackOptions as MatchCallbackOptions, -} - // * * * IMPORTANT! * * * // ------------------------------------------------------------------------- // // jdsoc type definitions cannot be declared above TypeScript definitions or diff --git a/packages/workbox-routing/src/registerRoute.ts b/packages/workbox-routing/src/registerRoute.ts index e4eed51f7..4b36d7801 100644 --- a/packages/workbox-routing/src/registerRoute.ts +++ b/packages/workbox-routing/src/registerRoute.ts @@ -8,11 +8,13 @@ import {logger} from 'workbox-core/_private/logger.js'; import {WorkboxError} from 'workbox-core/_private/WorkboxError.js'; +import {RouteHandler, RouteMatchCallback} from 'workbox-core/types.js'; + import {Route} from './Route.js'; import {RegExpRoute} from './RegExpRoute.js'; import {HTTPMethod} from './utils/constants.js'; import {getOrCreateDefaultRouter} from './utils/getOrCreateDefaultRouter.js'; -import {Handler, MatchCallback} from './_types.js'; + import './_version.js'; @@ -36,8 +38,8 @@ import './_version.js'; * @memberof module:workbox-routing */ function registerRoute( - capture: RegExp | string | MatchCallback | Route, - handler?: Handler, + capture: RegExp | string | RouteMatchCallback | Route, + handler?: RouteHandler, method?: HTTPMethod): Route { let route; @@ -69,7 +71,7 @@ function registerRoute( } } - const matchCallback: MatchCallback = ({url}) => { + const matchCallback: RouteMatchCallback = ({url}) => { if (process.env.NODE_ENV !== 'production') { if ((url.pathname === captureUrl.pathname) && (url.origin !== captureUrl.origin)) { @@ -107,4 +109,4 @@ function registerRoute( return route; } -export {registerRoute} +export {registerRoute}; diff --git a/packages/workbox-routing/src/setCatchHandler.ts b/packages/workbox-routing/src/setCatchHandler.ts index 958838f3c..aa07748c2 100644 --- a/packages/workbox-routing/src/setCatchHandler.ts +++ b/packages/workbox-routing/src/setCatchHandler.ts @@ -6,8 +6,10 @@ https://opensource.org/licenses/MIT. */ +import {RouteHandler} from 'workbox-core/types.js'; + import {getOrCreateDefaultRouter} from './utils/getOrCreateDefaultRouter.js'; -import {Handler} from './_types.js'; + import './_version.js'; /** @@ -19,9 +21,9 @@ import './_version.js'; * * @memberof module:workbox-routing */ -function setCatchHandler(handler: Handler) { +function setCatchHandler(handler: RouteHandler) { const defaultRouter = getOrCreateDefaultRouter(); defaultRouter.setCatchHandler(handler); } -export {setCatchHandler} +export {setCatchHandler}; diff --git a/packages/workbox-routing/src/setDefaultHandler.ts b/packages/workbox-routing/src/setDefaultHandler.ts index 7938f942e..bc1b70217 100644 --- a/packages/workbox-routing/src/setDefaultHandler.ts +++ b/packages/workbox-routing/src/setDefaultHandler.ts @@ -6,8 +6,10 @@ https://opensource.org/licenses/MIT. */ +import {RouteHandler} from 'workbox-core/types.js'; + import {getOrCreateDefaultRouter} from './utils/getOrCreateDefaultRouter.js'; -import {Handler} from './_types.js'; + import './_version.js'; @@ -23,9 +25,9 @@ import './_version.js'; * * @memberof module:workbox-routing */ -function setDefaultHandler(handler: Handler) { +function setDefaultHandler(handler: RouteHandler) { const defaultRouter = getOrCreateDefaultRouter(); defaultRouter.setDefaultHandler(handler); } -export {setDefaultHandler} +export {setDefaultHandler}; diff --git a/packages/workbox-routing/src/utils/normalizeHandler.ts b/packages/workbox-routing/src/utils/normalizeHandler.ts index 1ae679873..e9a8172b1 100644 --- a/packages/workbox-routing/src/utils/normalizeHandler.ts +++ b/packages/workbox-routing/src/utils/normalizeHandler.ts @@ -7,7 +7,8 @@ */ import {assert} from 'workbox-core/_private/assert.js'; -import {Handler, HandlerObject} from '../_types.js'; +import {RouteHandler, RouteHandlerObject} from 'workbox-core/types.js'; + import '../_version.js'; @@ -19,8 +20,8 @@ import '../_version.js'; * @private */ export const normalizeHandler = ( - handler: Handler -): HandlerObject => { + handler: RouteHandler +): RouteHandlerObject => { if (handler && typeof handler === 'object') { if (process.env.NODE_ENV !== 'production') { assert!.hasMethod(handler, 'handle', { diff --git a/packages/workbox-strategies/src/Strategy.ts b/packages/workbox-strategies/src/Strategy.ts index 61aa9416a..abc960b98 100644 --- a/packages/workbox-strategies/src/Strategy.ts +++ b/packages/workbox-strategies/src/Strategy.ts @@ -7,10 +7,12 @@ */ import {cacheNames} from 'workbox-core/_private/cacheNames.js'; -import {MapLikeObject, RouteHandlerObject, RouteHandlerCallbackOptions, WorkboxPlugin} from 'workbox-core/types.js'; +import {HandlerCallbackOptions, RouteHandlerObject, WorkboxPlugin} + from 'workbox-core/types.js'; + import {StrategyHandler} from './StrategyHandler.js'; -import './_version.js'; +import './_version.js'; export interface StrategyOptions { cacheName?: string; @@ -19,13 +21,6 @@ export interface StrategyOptions { matchOptions?: CacheQueryOptions; } -type StrategyHandlerOptions = { - request: Request; - event?: ExtendableEvent; - response?: Response; - params?: string[] | MapLikeObject; -} - /** * An abstract base class that all other strategy classes must extend from: * @@ -115,7 +110,7 @@ abstract class Strategy implements RouteHandlerObject { * @param {URL} [options.url] * @param {*} [options.params] */ - handle(options: FetchEvent | RouteHandlerCallbackOptions): Promise { + handle(options: HandlerCallbackOptions): Promise { const [responseDone] = this.handleAll(options); return responseDone; } @@ -140,7 +135,7 @@ abstract class Strategy implements RouteHandlerObject { * promises that can be used to determine when the response resolves as * well as when the handler has completed all its work. */ - handleAll(options: FetchEvent | RouteHandlerCallbackOptions): [ + handleAll(options: HandlerCallbackOptions | FetchEvent): [ Promise, Promise, ] { @@ -150,12 +145,14 @@ abstract class Strategy implements RouteHandlerObject { event: options, request: options.request, }; - } else if (typeof options.request === 'string') { - // `options.request` can be a string, similar to what `fetch()` accepts. - options.request = new Request(options.request); } - const {event, request, params} = options as StrategyHandlerOptions; + const event = options.event; + const request = typeof options.request === 'string' ? + new Request(options.request) : + options.request; + const params = 'params' in options ? options.params : undefined; + const handler = new StrategyHandler(this, {event, request, params}); const responseDone = this._getResponse(handler, request, event); diff --git a/packages/workbox-strategies/src/StrategyHandler.ts b/packages/workbox-strategies/src/StrategyHandler.ts index 43d3a811c..223e5c31e 100644 --- a/packages/workbox-strategies/src/StrategyHandler.ts +++ b/packages/workbox-strategies/src/StrategyHandler.ts @@ -12,16 +12,16 @@ import {getFriendlyURL} from 'workbox-core/_private/getFriendlyURL.js'; import {logger} from 'workbox-core/_private/logger.js'; import {timeout} from 'workbox-core/_private/timeout.js'; import {WorkboxError} from 'workbox-core/_private/WorkboxError.js'; -import {MapLikeObject, RouteHandlerCallbackOptions, WorkboxPlugin, WorkboxPluginCallbackParam} from 'workbox-core/types.js'; +import { + HandlerCallbackOptions, + MapLikeObject, + WorkboxPlugin, + WorkboxPluginCallbackParam, +} from 'workbox-core/types.js'; import {Strategy} from './Strategy.js'; import './_version.js'; - -export interface StrategyHandlerOptions extends RouteHandlerCallbackOptions { - request: Request; -} - function toRequest(input: RequestInfo) { return (typeof input === 'string') ? new Request(input) : input; } @@ -64,7 +64,7 @@ class StrategyHandler { * [match callback]{@link module:workbox-routing~matchCallback}, * (if applicable). */ - constructor(strategy: Strategy, options: StrategyHandlerOptions) { + constructor(strategy: Strategy, options: HandlerCallbackOptions) { /** * The request the strategy is performing (passed to the strategy's * `handle()` or `handleAll()` method). From f6cecf266436b4b199d6a80ece4f09a8515609d0 Mon Sep 17 00:00:00 2001 From: Jeff Posnick Date: Wed, 24 Jun 2020 14:14:07 -0400 Subject: [PATCH 2/8] Review feedback --- packages/workbox-core/src/types.ts | 6 +++--- packages/workbox-precaching/src/PrecacheController.ts | 10 +++++----- packages/workbox-precaching/src/utils/cacheWrapper.ts | 2 +- packages/workbox-precaching/src/utils/fetchWrapper.ts | 2 +- packages/workbox-strategies/src/Strategy.ts | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/workbox-core/src/types.ts b/packages/workbox-core/src/types.ts index 443821d43..d19451603 100644 --- a/packages/workbox-core/src/types.ts +++ b/packages/workbox-core/src/types.ts @@ -58,8 +58,8 @@ export interface RouteHandlerCallbackOptions { * Options passed to a `ManualHandlerCallback` function. */ export interface ManualHandlerCallbackOptions { + event: ExtendableEvent; request: Request | string; - event?: ExtendableEvent; } export type HandlerCallbackOptions = @@ -85,8 +85,8 @@ export interface RouteHandlerCallback { * If a non-empty array or object is returned by the `RouteMatchCallback` it * will be passed in as this handler's `options.params` argument. */ -export interface HandlerCallback { - (options: HandlerCallbackOptions): Promise; +export interface ManualHandlerCallback { + (options: ManualHandlerCallbackOptions): Promise; } /** diff --git a/packages/workbox-precaching/src/PrecacheController.ts b/packages/workbox-precaching/src/PrecacheController.ts index a364c79cd..688a0f2a7 100644 --- a/packages/workbox-precaching/src/PrecacheController.ts +++ b/packages/workbox-precaching/src/PrecacheController.ts @@ -235,15 +235,15 @@ class PrecacheController { * install event. * * @param {Object} options - * @param {Event} [options.event] The install event (if needed). + * @param {Event} options.event The install event. * @param {Array} [options.plugins] Plugins to be used for fetching * and caching during install. * @return {Promise} */ async install({event, plugins}: { - event?: ExtendableEvent; + event: ExtendableEvent; plugins?: WorkboxPlugin[]; - } = {}) { + }) { if (process.env.NODE_ENV !== 'production') { if (plugins) { assert!.isArray(plugins, { @@ -339,8 +339,8 @@ class PrecacheController { * @param {Object} options * @param {string} options.cacheKey The string to use a cache key. * @param {string} options.url The URL to fetch and cache. + * @param {Event} options.event The install event. * @param {string} [options.cacheMode] The cache mode for the network request. - * @param {Event} [options.event] The install event (if passed). * @param {string} [options.integrity] The value to use for the `integrity` * field when making the request. */ @@ -348,7 +348,7 @@ class PrecacheController { cacheKey: string; url: string; cacheMode: "reload" | "default" | "no-store" | "no-cache" | "force-cache" | "only-if-cached" | undefined; - event?: ExtendableEvent; + event: ExtendableEvent; integrity?: string; }) { const request = new Request(url, { diff --git a/packages/workbox-precaching/src/utils/cacheWrapper.ts b/packages/workbox-precaching/src/utils/cacheWrapper.ts index c5fb16bb9..5c44be3d8 100644 --- a/packages/workbox-precaching/src/utils/cacheWrapper.ts +++ b/packages/workbox-precaching/src/utils/cacheWrapper.ts @@ -15,7 +15,7 @@ import '../_version.js'; interface MatchWrapperOptions { cacheName: string; request: Request; - event?: ExtendableEvent; + event: ExtendableEvent; plugins?: WorkboxPlugin[]; matchOptions?: CacheQueryOptions; } diff --git a/packages/workbox-precaching/src/utils/fetchWrapper.ts b/packages/workbox-precaching/src/utils/fetchWrapper.ts index e775a5f35..ff723ee7e 100644 --- a/packages/workbox-precaching/src/utils/fetchWrapper.ts +++ b/packages/workbox-precaching/src/utils/fetchWrapper.ts @@ -14,7 +14,7 @@ import '../_version.js'; interface WrappedFetchOptions { request: Request | string; - event?: ExtendableEvent; + event: ExtendableEvent; plugins?: WorkboxPlugin[]; fetchOptions?: RequestInit; } diff --git a/packages/workbox-strategies/src/Strategy.ts b/packages/workbox-strategies/src/Strategy.ts index abc960b98..a93581659 100644 --- a/packages/workbox-strategies/src/Strategy.ts +++ b/packages/workbox-strategies/src/Strategy.ts @@ -110,7 +110,7 @@ abstract class Strategy implements RouteHandlerObject { * @param {URL} [options.url] * @param {*} [options.params] */ - handle(options: HandlerCallbackOptions): Promise { + handle(options: FetchEvent | HandlerCallbackOptions): Promise { const [responseDone] = this.handleAll(options); return responseDone; } From b4737367864759b5a3b3f7ee801007474acb73bb Mon Sep 17 00:00:00 2001 From: Jeff Posnick Date: Fri, 26 Jun 2020 11:24:15 -0400 Subject: [PATCH 3/8] Test fix --- test/workbox-precaching/sw/test-PrecacheController.mjs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/workbox-precaching/sw/test-PrecacheController.mjs b/test/workbox-precaching/sw/test-PrecacheController.mjs index 487f16d7b..1450c846b 100644 --- a/test/workbox-precaching/sw/test-PrecacheController.mjs +++ b/test/workbox-precaching/sw/test-PrecacheController.mjs @@ -237,8 +237,11 @@ describe(`PrecacheController`, function() { describe('install()', function() { it('should be fine when calling with empty precache list', async function() { + const event = new ExtendableEvent('install'); + spyOnEvent(event); + const precacheController = new PrecacheController(); - return precacheController.install(); + return precacheController.install({event}); }); it('should precache assets (with cache busting via search params)', async function() { From 613b2f85a57a6c16155d39e253af28637e825d28 Mon Sep 17 00:00:00 2001 From: Jeff Posnick Date: Fri, 26 Jun 2020 11:50:52 -0400 Subject: [PATCH 4/8] Require event in more places --- packages/workbox-core/src/types.ts | 22 ++++++++-------- packages/workbox-strategies/src/Strategy.ts | 4 +-- .../workbox-strategies/src/StrategyHandler.ts | 26 ++++++++++++------- .../sw/test-StrategyHandler.mjs | 17 ++++++++++++ 4 files changed, 47 insertions(+), 22 deletions(-) diff --git a/packages/workbox-core/src/types.ts b/packages/workbox-core/src/types.ts index d19451603..e05d4e332 100644 --- a/packages/workbox-core/src/types.ts +++ b/packages/workbox-core/src/types.ts @@ -108,7 +108,7 @@ export type RouteHandler = RouteHandlerCallback | RouteHandlerObject; export interface HandlerWillStartCallbackParam { request: Request; - event?: ExtendableEvent; + event: ExtendableEvent; state?: PluginState; } @@ -120,7 +120,7 @@ export interface CacheDidUpdateCallbackParam { cacheName: string; newResponse: Response; request: Request; - event?: ExtendableEvent; + event: ExtendableEvent; oldResponse?: Response | null; state?: PluginState; } @@ -132,7 +132,7 @@ export interface CacheDidUpdateCallback { export interface CacheKeyWillBeUsedCallbackParam { mode: string; request: Request; - event?: ExtendableEvent; + event: ExtendableEvent; params?: any; state?: PluginState; } @@ -144,7 +144,7 @@ export interface CacheKeyWillBeUsedCallback { export interface CacheWillUpdateCallbackParam { request: Request; response: Response; - event?: ExtendableEvent; + event: ExtendableEvent; state?: PluginState; } @@ -156,7 +156,7 @@ export interface CachedResponseWillBeUsedCallbackParam { cacheName: string; request: Request; cachedResponse?: Response; - event?: ExtendableEvent; + event: ExtendableEvent; matchOptions?: CacheQueryOptions; state?: PluginState; } @@ -169,7 +169,7 @@ export interface FetchDidFailCallbackParam { error: Error; originalRequest: Request; request: Request; - event?: ExtendableEvent; + event: ExtendableEvent; state?: PluginState; } @@ -180,7 +180,7 @@ export interface FetchDidFailCallback { export interface FetchDidSucceedCallbackParam { request: Request; response: Response; - event?: ExtendableEvent; + event: ExtendableEvent; state?: PluginState; } @@ -190,7 +190,7 @@ export interface FetchDidSucceedCallback { export interface RequestWillFetchCallbackParam { request: Request; - event?: ExtendableEvent; + event: ExtendableEvent; state?: PluginState; } @@ -201,7 +201,7 @@ export interface RequestWillFetchCallback { export interface HandlerWillRespondCallbackParam { request: Request; response: Response; - event?: ExtendableEvent; + event: ExtendableEvent; state?: PluginState; } @@ -211,7 +211,7 @@ export interface HandlerWillRespondCallback { export interface HandlerDidRespondCallbackParam { request: Request; - event?: ExtendableEvent; + event: ExtendableEvent; response?: Response; state?: PluginState; } @@ -223,7 +223,7 @@ export interface HandlerDidRespondCallback { export interface HandlerDidCompleteCallbackParam { request: Request; error?: Error; - event?: ExtendableEvent; + event: ExtendableEvent; response?: Response; state?: PluginState; } diff --git a/packages/workbox-strategies/src/Strategy.ts b/packages/workbox-strategies/src/Strategy.ts index a93581659..bb7f3cf87 100644 --- a/packages/workbox-strategies/src/Strategy.ts +++ b/packages/workbox-strategies/src/Strategy.ts @@ -162,7 +162,7 @@ abstract class Strategy implements RouteHandlerObject { return [responseDone, handlerDone]; } - async _getResponse(handler: StrategyHandler, request: Request, event?: ExtendableEvent) { + async _getResponse(handler: StrategyHandler, request: Request, event: ExtendableEvent) { await handler.runCallbacks('handlerWillStart', {event, request}); let response = await this._handle(request, handler); @@ -172,7 +172,7 @@ abstract class Strategy implements RouteHandlerObject { return response; } - async _awaitComplete(responseDone: Promise, handler: StrategyHandler, request: Request, event?: ExtendableEvent) { + async _awaitComplete(responseDone: Promise, handler: StrategyHandler, request: Request, event: ExtendableEvent) { let response; let error; diff --git a/packages/workbox-strategies/src/StrategyHandler.ts b/packages/workbox-strategies/src/StrategyHandler.ts index 223e5c31e..1f8aba975 100644 --- a/packages/workbox-strategies/src/StrategyHandler.ts +++ b/packages/workbox-strategies/src/StrategyHandler.ts @@ -6,6 +6,7 @@ https://opensource.org/licenses/MIT. */ +import {assert} from 'workbox-core/_private/assert.js'; import {Deferred} from 'workbox-core/_private/Deferred.js'; import {executeQuotaErrorCallbacks} from 'workbox-core/_private/executeQuotaErrorCallbacks.js'; import {getFriendlyURL} from 'workbox-core/_private/getFriendlyURL.js'; @@ -38,7 +39,7 @@ function toRequest(input: RequestInfo) { class StrategyHandler { public request!: Request; public url?: URL; - public event?: ExtendableEvent; + public event: ExtendableEvent; public params?: any; private readonly _strategy: Strategy; @@ -56,9 +57,9 @@ class StrategyHandler { * * @param {module:workbox-strategies.Strategy} strategy * @param {Object} options + * @param {ExtendableEvent} options.event An event associated with this + * request. * @param {Request} [options.request] The request the strategy is performing. - * @param {FetchEvent} [options.event] The event that triggered the request - * (if applicable). * @param {URL} [options.url] A `URL` instance of `request.url`, if passed. * @param {*} [options.params] Parameters returned by the Route's * [match callback]{@link module:workbox-routing~matchCallback}, @@ -74,11 +75,10 @@ class StrategyHandler { * @memberof module:workbox-strategies.StrategyHandler */ /** - * The event that triggered the request (if passed to the strategy's - * `handle()` or `handleAll()` method). + * The event associated with this request. * @name event * @instance - * @type {ExtendableEvent|undefined} + * @type {ExtendableEvent} * @memberof module:workbox-strategies.StrategyHandler */ /** @@ -103,8 +103,18 @@ class StrategyHandler { * @type {*|undefined} * @memberof module:workbox-strategies.StrategyHandler */ + if (process.env.NODE_ENV !== 'production') { + assert!.isInstance(options.event, ExtendableEvent, { + moduleName: 'workbox-strategies', + className: 'StrategyHandler', + funcName: 'constructor', + paramName: 'options.event', + }); + } + Object.assign(this, options); + this.event = options.event; this._strategy = strategy; this._handlerDeferred = new Deferred(); this._extendLifetimePromises = []; @@ -117,9 +127,7 @@ class StrategyHandler { this._pluginStateMap.set(plugin, {}); } - if (this.event) { - this.event.waitUntil(this._handlerDeferred.promise); - } + this.event.waitUntil(this._handlerDeferred.promise); } /** diff --git a/test/workbox-strategies/sw/test-StrategyHandler.mjs b/test/workbox-strategies/sw/test-StrategyHandler.mjs index dd6845628..d96b87b56 100644 --- a/test/workbox-strategies/sw/test-StrategyHandler.mjs +++ b/test/workbox-strategies/sw/test-StrategyHandler.mjs @@ -55,6 +55,23 @@ describe(`StrategyHandler`, function() { }); describe('constructor()', function() { + it(`should throw when called without an 'event' parameter in dev`, async function() { + if (process.env.NODE_ENV === 'production') { + return this.skip(); + } + + await expectError( + () => new StrategyHandler(new TestStrategy(), {}), + 'incorrect-class', + (error) => { + expect(error.details).to.have.property('moduleName').that.equals('workbox-strategies'); + expect(error.details).to.have.property('className').that.equals('StrategyHandler'); + expect(error.details).to.have.property('funcName').that.equals('constructor'); + expect(error.details).to.have.property('paramName').that.equals('options.event'); + }, + ); + }); + it('creates an object with the correct public properties', function() { const handler = createStrategyHandler(); From d9a026368bb263be72a6af2f798073b692490f53 Mon Sep 17 00:00:00 2001 From: Jeff Posnick Date: Fri, 26 Jun 2020 11:52:26 -0400 Subject: [PATCH 5/8] Linting --- test/workbox-strategies/sw/test-StrategyHandler.mjs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/workbox-strategies/sw/test-StrategyHandler.mjs b/test/workbox-strategies/sw/test-StrategyHandler.mjs index d96b87b56..67aa7caf0 100644 --- a/test/workbox-strategies/sw/test-StrategyHandler.mjs +++ b/test/workbox-strategies/sw/test-StrategyHandler.mjs @@ -59,7 +59,7 @@ describe(`StrategyHandler`, function() { if (process.env.NODE_ENV === 'production') { return this.skip(); } - + await expectError( () => new StrategyHandler(new TestStrategy(), {}), 'incorrect-class', From 5f5325f9c834483b57a93ca15f61b36ef4ed738c Mon Sep 17 00:00:00 2001 From: Philip Walton Date: Fri, 26 Jun 2020 14:15:57 -0700 Subject: [PATCH 6/8] Update jsdoc comments --- .../src/BroadcastCacheUpdate.ts | 2 +- .../src/BroadcastUpdatePlugin.ts | 2 +- packages/workbox-routing/src/Router.ts | 4 ++-- packages/workbox-routing/src/_types.ts | 14 +++++++------- packages/workbox-strategies/src/NetworkFirst.ts | 4 ++-- packages/workbox-strategies/src/Strategy.ts | 14 +++++++++----- packages/workbox-strategies/src/StrategyHandler.ts | 8 ++++---- 7 files changed, 26 insertions(+), 22 deletions(-) diff --git a/packages/workbox-broadcast-update/src/BroadcastCacheUpdate.ts b/packages/workbox-broadcast-update/src/BroadcastCacheUpdate.ts index fb7a92a7b..47a76b0bb 100644 --- a/packages/workbox-broadcast-update/src/BroadcastCacheUpdate.ts +++ b/packages/workbox-broadcast-update/src/BroadcastCacheUpdate.ts @@ -105,7 +105,7 @@ class BroadcastCacheUpdate { * @param {Request} options.request The request. * @param {string} options.cacheName Name of the cache the responses belong * to. This is included in the broadcast message. - * @param {Event} [options.event] event An optional event that triggered + * @param {Event} options.event event The event that triggered * this possible cache update. * @return {Promise} Resolves once the update is sent. */ diff --git a/packages/workbox-broadcast-update/src/BroadcastUpdatePlugin.ts b/packages/workbox-broadcast-update/src/BroadcastUpdatePlugin.ts index d2f0ebb59..a017e8860 100644 --- a/packages/workbox-broadcast-update/src/BroadcastUpdatePlugin.ts +++ b/packages/workbox-broadcast-update/src/BroadcastUpdatePlugin.ts @@ -50,7 +50,7 @@ class BroadcastUpdatePlugin implements WorkboxPlugin { * @param {Response} [options.oldResponse] The previous cached value, if any. * @param {Response} options.newResponse The new value in the cache. * @param {Request} options.request The request that triggered the update. - * @param {Request} [options.event] The event that triggered the update. + * @param {Request} options.event The event that triggered the update. */ cacheDidUpdate: WorkboxPlugin['cacheDidUpdate'] = async (options) => { dontWaitFor(this._broadcastUpdate.notifyIfUpdated(options)); diff --git a/packages/workbox-routing/src/Router.ts b/packages/workbox-routing/src/Router.ts index 2f44cd4fb..2ba7aafd9 100644 --- a/packages/workbox-routing/src/Router.ts +++ b/packages/workbox-routing/src/Router.ts @@ -270,7 +270,7 @@ class Router { * @param {Object} options * @param {URL} options.url * @param {Request} options.request The request to match. - * @param {Event} [options.event] The corresponding event (unless N/A). + * @param {Event} options.event The corresponding event. * @return {Object} An object with `route` and `params` properties. * They are populated if a matching route was found or `undefined` * otherwise. @@ -310,7 +310,7 @@ class Router { /** * Define a default `handler` that's called when no routes explicitly * match the incoming request. - * + * * Each HTTP method ('GET', 'POST', etc.) gets its own default handler. * * Without a default handler, unmatched requests will go against the diff --git a/packages/workbox-routing/src/_types.ts b/packages/workbox-routing/src/_types.ts index 284f9c089..c10384719 100644 --- a/packages/workbox-routing/src/_types.ts +++ b/packages/workbox-routing/src/_types.ts @@ -30,12 +30,12 @@ import './_version.js'; * * @callback ~matchCallback * @param {Object} context + * @param {Request} context.request The corresponding request. * @param {URL} context.url The request's URL. + * @param {ExtendableEvent} context.event The corresponding event that triggered + * the request. * @param {boolean} context.sameOrigin The result of comparing `url.origin` - * against the current origin. - * @param {Request} context.request The corresponding request. - * @param {FetchEvent} [context.event] The corresponding event that triggered - * the request, if available. + * against the current origin. * @return {*} To signify a match, return a truthy value. * * @memberof module:workbox-routing @@ -54,9 +54,9 @@ import './_version.js'; * @callback ~handlerCallback * @param {Object} context * @param {Request|string} context.request The corresponding request. - * @param {URL} [context.url] The URL that matched, if available. - * @param {FetchEvent} [context.event] The corresponding event that triggered - * the request, if available. + * @param {URL} context.url The URL that matched, if available. + * @param {ExtendableEvent} context.event The corresponding event that triggered + * the request. * @param {Object} [context.params] Array or Object parameters returned by the * Route's [match callback]{@link module:workbox-routing~matchCallback}. * This will be undefined if an empty array or object were returned. diff --git a/packages/workbox-strategies/src/NetworkFirst.ts b/packages/workbox-strategies/src/NetworkFirst.ts index 1d6cbd0e0..353d3948a 100644 --- a/packages/workbox-strategies/src/NetworkFirst.ts +++ b/packages/workbox-strategies/src/NetworkFirst.ts @@ -147,7 +147,7 @@ class NetworkFirst extends Strategy { * @param {Object} options * @param {Request} options.request * @param {Array} options.logs A reference to the logs array - * @param {Event} [options.event] + * @param {Event} options.event * @return {Promise} * * @private @@ -183,7 +183,7 @@ class NetworkFirst extends Strategy { * @param {number|undefined} options.timeoutId * @param {Request} options.request * @param {Array} options.logs A reference to the logs Array. - * @param {Event} [options.event] + * @param {Event} options.event * @return {Promise} * * @private diff --git a/packages/workbox-strategies/src/Strategy.ts b/packages/workbox-strategies/src/Strategy.ts index bb7f3cf87..553253860 100644 --- a/packages/workbox-strategies/src/Strategy.ts +++ b/packages/workbox-strategies/src/Strategy.ts @@ -104,9 +104,11 @@ abstract class Strategy implements RouteHandlerObject { * Alternatively, this method can be used in a standalone `FetchEvent` * listener by passing it to `event.respondWith()`. * - * @param {Object} options + * @param {FetchEvent|Object} options A `FetchEvent` or an object with the + * properties listed below. * @param {Request|string} options.request A request to run this strategy for. - * @param {ExtendableEvent} [options.event] + * @param {ExtendableEvent} options.event The event associated with the + * request. * @param {URL} [options.url] * @param {*} [options.params] */ @@ -126,16 +128,18 @@ abstract class Strategy implements RouteHandlerObject { * You can await the `done` promise to ensure any extra work performed by * the strategy (usually caching responses) completes successfully. * - * @param {Object} options + * @param {FetchEvent|Object} options A `FetchEvent` or an object with the + * properties listed below. * @param {Request|string} options.request A request to run this strategy for. - * @param {ExtendableEvent} [options.event] + * @param {ExtendableEvent} options.event The event associated with the + * request. * @param {URL} [options.url] * @param {*} [options.params] * @return {Array} A tuple of [response, done] * promises that can be used to determine when the response resolves as * well as when the handler has completed all its work. */ - handleAll(options: HandlerCallbackOptions | FetchEvent): [ + handleAll(options: FetchEvent | HandlerCallbackOptions): [ Promise, Promise, ] { diff --git a/packages/workbox-strategies/src/StrategyHandler.ts b/packages/workbox-strategies/src/StrategyHandler.ts index 1f8aba975..46053d503 100644 --- a/packages/workbox-strategies/src/StrategyHandler.ts +++ b/packages/workbox-strategies/src/StrategyHandler.ts @@ -57,11 +57,11 @@ class StrategyHandler { * * @param {module:workbox-strategies.Strategy} strategy * @param {Object} options - * @param {ExtendableEvent} options.event An event associated with this + * @param {Request|string} options.request A request to run this strategy for. + * @param {ExtendableEvent} options.event The event associated with the * request. - * @param {Request} [options.request] The request the strategy is performing. - * @param {URL} [options.url] A `URL` instance of `request.url`, if passed. - * @param {*} [options.params] Parameters returned by the Route's + * @param {URL} [options.url] + * @param {*} [options.params] * [match callback]{@link module:workbox-routing~matchCallback}, * (if applicable). */ From 5601bd1ac9d358148460ae6e423306200782e68d Mon Sep 17 00:00:00 2001 From: Jeff Posnick Date: Tue, 30 Jun 2020 13:28:18 -0400 Subject: [PATCH 7/8] WIP test cleanup --- .../sw/test-createHandler.mjs | 21 ++++++--- .../sw/test-createHandlerBoundToURL.mjs | 15 +++--- .../sw/utils/test-cacheWrapper.mjs | 46 ++++++++++++++++++- 3 files changed, 68 insertions(+), 14 deletions(-) diff --git a/test/workbox-precaching/sw/test-createHandler.mjs b/test/workbox-precaching/sw/test-createHandler.mjs index 294470b47..d74e27902 100644 --- a/test/workbox-precaching/sw/test-createHandler.mjs +++ b/test/workbox-precaching/sw/test-createHandler.mjs @@ -6,10 +6,11 @@ https://opensource.org/licenses/MIT. */ -import {createHandler} from 'workbox-precaching/createHandler.mjs'; -import {precache} from 'workbox-precaching/precache.mjs'; import {resetDefaultPrecacheController} from './resetDefaultPrecacheController.mjs'; +import {spyOnEvent} from '../../../infra/testing/helpers/extendable-event-utils.mjs'; +import {createHandler} from 'workbox-precaching/createHandler.mjs'; +import {precache} from 'workbox-precaching/precache.mjs'; describe(`createHandler()`, function() { const sandbox = sinon.createSandbox(); @@ -27,8 +28,11 @@ describe(`createHandler()`, function() { precache([]); const handler = createHandler(false); + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + return expectError(async () => { - await handler({request: new Request('/cache-miss')}); + await handler({event, request: new Request('/cache-miss')}); }, 'missing-precache-entry', (error) => { expect(error.details.url).to.eql(`${location.origin}/cache-miss`); expect(error.details.cacheName).to.eql(`workbox-precache-v2-${location.origin}/test/workbox-precaching/sw/`); @@ -55,22 +59,25 @@ describe(`createHandler()`, function() { {url: '/url4', revision: 'def456'}, ]); + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + const handler = createHandler(); - const response1 = await handler({request: new Request('/url1')}); + const response1 = await handler({event, request: new Request('/url1')}); expect(matchStub.calledOnce).to.be.true; expect(matchStub.firstCall.args[0].url).to.eql(`${location.origin}/url1`); expect(fetchStub.notCalled).to.be.true; expect(await response1.text()).to.eql('response 1'); - const response2 = await handler({request: new Request('/url2')}); + const response2 = await handler({event, request: new Request('/url2')}); expect(matchStub.calledTwice).to.be.true; expect(matchStub.secondCall.args[0].url).to.eql(`${location.origin}/url2?__WB_REVISION__=abc123`); expect(fetchStub.notCalled).to.be.true; expect(await response2.text()).to.eql('response 2'); - const response3 = await handler({request: new Request('/url3')}); + const response3 = await handler({event, request: new Request('/url3')}); expect(matchStub.calledThrice).to.be.true; expect(matchStub.thirdCall.args[0].url).to.eql(`${location.origin}/url3`); @@ -78,7 +85,7 @@ describe(`createHandler()`, function() { expect(fetchStub.firstCall.args[0].url).to.eql(`${location.origin}/url3`); expect(await response3.text()).to.eql('response 3'); - const response4 = await handler({request: new Request('/url4')}); + const response4 = await handler({event, request: new Request('/url4')}); expect(matchStub.callCount).to.eql(4); // Call #3 is the fourth call due to zero-indexing. diff --git a/test/workbox-precaching/sw/test-createHandlerBoundToURL.mjs b/test/workbox-precaching/sw/test-createHandlerBoundToURL.mjs index 09f73c59b..60fc911ec 100644 --- a/test/workbox-precaching/sw/test-createHandlerBoundToURL.mjs +++ b/test/workbox-precaching/sw/test-createHandlerBoundToURL.mjs @@ -6,10 +6,11 @@ https://opensource.org/licenses/MIT. */ -import {createHandlerBoundToURL} from 'workbox-precaching/createHandlerBoundToURL.mjs'; -import {precache} from 'workbox-precaching/precache.mjs'; import {resetDefaultPrecacheController} from './resetDefaultPrecacheController.mjs'; +import {spyOnEvent} from '../../../infra/testing/helpers/extendable-event-utils.mjs'; +import {createHandlerBoundToURL} from 'workbox-precaching/createHandlerBoundToURL.mjs'; +import {precache} from 'workbox-precaching/precache.mjs'; describe(`createHandlerBoundToURL()`, function() { const sandbox = sinon.createSandbox(); @@ -51,9 +52,11 @@ describe(`createHandlerBoundToURL()`, function() { {url: '/url4', revision: 'def456'}, ]); + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); const handler1 = createHandlerBoundToURL('/url1'); - const response1 = await handler1({}); + const response1 = await handler1({event}); expect(matchStub.calledOnce).to.be.true; expect(matchStub.firstCall.args[0].url).to.eql(`${location.origin}/url1`); @@ -61,7 +64,7 @@ describe(`createHandlerBoundToURL()`, function() { expect(await response1.text()).to.eql('response 1'); const handler2 = createHandlerBoundToURL('/url2'); - const response2 = await handler2({}); + const response2 = await handler2({event}); expect(matchStub.calledTwice).to.be.true; expect(matchStub.secondCall.args[0].url).to.eql(`${location.origin}/url2?__WB_REVISION__=abc123`); @@ -69,7 +72,7 @@ describe(`createHandlerBoundToURL()`, function() { expect(await response2.text()).to.eql('response 2'); const handler3 = createHandlerBoundToURL('/url3'); - const response3 = await handler3({}); + const response3 = await handler3({event}); expect(matchStub.calledThrice).to.be.true; expect(matchStub.thirdCall.args[0].url).to.eql(`${location.origin}/url3`); @@ -78,7 +81,7 @@ describe(`createHandlerBoundToURL()`, function() { expect(await response3.text()).to.eql('response 3'); const handler4 = createHandlerBoundToURL('/url4'); - const response4 = await handler4({}); + const response4 = await handler4({event}); expect(matchStub.callCount).to.eql(4); // Call #3 is the fourth call due to zero-indexing. diff --git a/test/workbox-precaching/sw/utils/test-cacheWrapper.mjs b/test/workbox-precaching/sw/utils/test-cacheWrapper.mjs index 7f06c895a..9006203d2 100644 --- a/test/workbox-precaching/sw/utils/test-cacheWrapper.mjs +++ b/test/workbox-precaching/sw/utils/test-cacheWrapper.mjs @@ -10,7 +10,6 @@ import {registerQuotaErrorCallback} from 'workbox-core/registerQuotaErrorCallbac import {cacheWrapper} from 'workbox-precaching/utils/cacheWrapper.mjs'; import {spyOnEvent} from '../../../../infra/testing/helpers/extendable-event-utils.mjs'; - describe(`cacheWrapper`, function() { const sandbox = sinon.createSandbox(); @@ -29,6 +28,9 @@ describe(`cacheWrapper`, function() { // TODO Add Error Case Tests (I.e. bad input) it(`should work with a request and response`, async function() { + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + const testCache = await caches.open('TEST-CACHE'); const cacheOpenStub = sandbox.stub(self.caches, 'open'); const cachePutStub = sandbox.stub(testCache, 'put'); @@ -38,6 +40,7 @@ describe(`cacheWrapper`, function() { const putRequest = new Request('/test/string'); const putResponse = new Response('Response for /test/string'); await cacheWrapper.put({ + event, cacheName: 'TODO-CHANGE-ME', request: putRequest, response: putResponse, @@ -57,6 +60,9 @@ describe(`cacheWrapper`, function() { // This covers opaque responses (0) and partial content responses (206). for (const status of [0, 206]) { it(`should not cache response.status of ${status} by default`, async function() { + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + const cacheName = 'test-cache'; const testCache = await caches.open(cacheName); const cacheOpenStub = sandbox.stub(self.caches, 'open').resolves(testCache); @@ -71,6 +77,7 @@ describe(`cacheWrapper`, function() { await cacheWrapper.put({ cacheName, + event, request: putRequest, response: putResponse, }); @@ -83,6 +90,9 @@ describe(`cacheWrapper`, function() { it(`should throw when trying to cache POST requests in dev mode`, async function() { if (process.env.NODE_ENV === 'production') this.skip(); + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + const testCache = await caches.open('TEST-CACHE'); const cacheOpenStub = sandbox.stub(self.caches, 'open'); const cachePutStub = sandbox.stub(testCache, 'put'); @@ -96,6 +106,7 @@ describe(`cacheWrapper`, function() { await expectError(async () => { await cacheWrapper.put({ + event, cacheName: 'CACHE NAME', request: putRequest, response: putResponse, @@ -107,6 +118,9 @@ describe(`cacheWrapper`, function() { }); it(`should call cacheDidUpdate`, async function() { + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + const firstPlugin = { cacheDidUpdate: () => {}, }; @@ -124,6 +138,7 @@ describe(`cacheWrapper`, function() { }); await cacheWrapper.put({ + event, cacheName: 'TODO-CHANGE-ME', request: putRequest, response: putResponse, @@ -152,6 +167,7 @@ describe(`cacheWrapper`, function() { }); await cacheWrapper.put({ + event, cacheName: 'TODO-CHANGE-ME', request: putRequest, response: putResponseUpdate, @@ -174,6 +190,9 @@ describe(`cacheWrapper`, function() { }); it(`should call cacheWillUpdate`, async function() { + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + const firstPluginResponse = new Response('Response for /test/string/1'); const firstPlugin = { cacheWillUpdate: () => { @@ -196,6 +215,7 @@ describe(`cacheWrapper`, function() { spyOnEvent(fetchEvent); await cacheWrapper.put({ + event, cacheName: 'TODO-CHANGE-ME', request: putRequest, response: putResponse, @@ -224,6 +244,9 @@ describe(`cacheWrapper`, function() { }); it(`should call cacheKeyWillBeUsed`, async function() { + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + const cacheName = 'cacheKeyWillBeUsed-test-cache'; const cache = await caches.open(cacheName); sandbox.stub(caches, 'open').resolves(cache); @@ -246,6 +269,7 @@ describe(`cacheWrapper`, function() { const response = new Response('Test response.'); await cacheWrapper.put({ + event, response, cacheName, request: initialRequest, @@ -275,6 +299,9 @@ describe(`cacheWrapper`, function() { }); it(`should call the quota exceeded callbacks when there's a QuotaExceeded error`, async function() { + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + const callback1 = sandbox.stub(); registerQuotaErrorCallback(callback1); const callback2 = sandbox.stub(); @@ -287,6 +314,7 @@ describe(`cacheWrapper`, function() { try { await cacheWrapper.put({ + event, cacheName, request: 'ignored', response: new Response(), @@ -300,6 +328,9 @@ describe(`cacheWrapper`, function() { }); it(`should not call the quota exceeded callbacks when there's a non-QuotaExceeded error`, async function() { + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + const callback = sandbox.stub(); registerQuotaErrorCallback(callback); @@ -310,6 +341,7 @@ describe(`cacheWrapper`, function() { try { await cacheWrapper.put({ + event, cacheName, request: 'ignored', response: new Response(), @@ -324,6 +356,9 @@ describe(`cacheWrapper`, function() { describe(`.match()`, function() { it(`should use the matchOptions that were provided to put()`, async function() { + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + const matchOptions = { ignoreSearch: true, }; @@ -332,6 +367,7 @@ describe(`cacheWrapper`, function() { const matchSpy = sandbox.spy(self.caches, 'match'); await cacheWrapper.put({ + event, cacheName, matchOptions, plugins: [{ @@ -348,6 +384,9 @@ describe(`cacheWrapper`, function() { }); it(`should call cachedResponseWillBeUsed`, async function() { + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + const options = {}; const matchCacheName = 'MATCH-CACHE-NAME'; const matchRequest = new Request('/test/string'); @@ -402,6 +441,7 @@ describe(`cacheWrapper`, function() { await openCache.put(matchRequest, matchResponse); const result = await cacheWrapper.match({ + event, cacheName: matchCacheName, request: matchRequest, matchOptions: options, @@ -421,6 +461,9 @@ describe(`cacheWrapper`, function() { }); it(`should call cacheKeyWillBeUsed`, async function() { + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + const cacheName = 'cacheKeyWillBeUsed-test-cache'; const cacheMatchStub = sandbox.stub(self.caches, 'match').resolves( new Response('Test response.')); @@ -441,6 +484,7 @@ describe(`cacheWrapper`, function() { const initialRequest = new Request('/noPlugin'); await cacheWrapper.match({ + event, cacheName, request: initialRequest, plugins: [ From 278edaee1577974a8e0621c75f7be27b8cb8c609 Mon Sep 17 00:00:00 2001 From: Jeff Posnick Date: Tue, 30 Jun 2020 13:39:57 -0400 Subject: [PATCH 8/8] Fix up more tests --- .../sw/utils/test-cacheWrapper.mjs | 4 --- .../sw/utils/test-fetchWrapper.mjs | 27 ++++++++++++++++--- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/test/workbox-precaching/sw/utils/test-cacheWrapper.mjs b/test/workbox-precaching/sw/utils/test-cacheWrapper.mjs index 9006203d2..39b5af414 100644 --- a/test/workbox-precaching/sw/utils/test-cacheWrapper.mjs +++ b/test/workbox-precaching/sw/utils/test-cacheWrapper.mjs @@ -190,9 +190,6 @@ describe(`cacheWrapper`, function() { }); it(`should call cacheWillUpdate`, async function() { - const event = new ExtendableEvent('fetch'); - spyOnEvent(event); - const firstPluginResponse = new Response('Response for /test/string/1'); const firstPlugin = { cacheWillUpdate: () => { @@ -215,7 +212,6 @@ describe(`cacheWrapper`, function() { spyOnEvent(fetchEvent); await cacheWrapper.put({ - event, cacheName: 'TODO-CHANGE-ME', request: putRequest, response: putResponse, diff --git a/test/workbox-precaching/sw/utils/test-fetchWrapper.mjs b/test/workbox-precaching/sw/utils/test-fetchWrapper.mjs index 4851e10cd..1a0254ff2 100644 --- a/test/workbox-precaching/sw/utils/test-fetchWrapper.mjs +++ b/test/workbox-precaching/sw/utils/test-fetchWrapper.mjs @@ -9,7 +9,6 @@ import {fetchWrapper} from 'workbox-precaching/utils/fetchWrapper.mjs'; import {spyOnEvent} from '../../../../infra/testing/helpers/extendable-event-utils.mjs'; - describe(`fetchWrapper`, function() { const sandbox = sinon.createSandbox(); @@ -21,9 +20,12 @@ describe(`fetchWrapper`, function() { // TODO Add Error Case Tests (I.e. bad input) it(`should work with request string`, async function() { + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + const stub = sandbox.stub(self, 'fetch').callsFake(() => new Response()); - await fetchWrapper.fetch({request: '/test/string'}); + await fetchWrapper.fetch({event, request: '/test/string'}); expect(stub.callCount).to.equal(1); const fetchRequest = stub.args[0][0]; @@ -31,9 +33,12 @@ describe(`fetchWrapper`, function() { }); it(`should work with Request instance`, async function() { + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + const stub = sandbox.stub(self, 'fetch').callsFake(() => new Response()); - await fetchWrapper.fetch({request: new Request('/test/request')}); + await fetchWrapper.fetch({event, request: new Request('/test/request')}); expect(stub.callCount).to.equal(1); const fetchRequest = stub.args[0][0]; @@ -41,6 +46,9 @@ describe(`fetchWrapper`, function() { }); it(`should use fetchOptions`, async function() { + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + const stub = sandbox.stub(self, 'fetch').callsFake(() => new Response()); const exampleOptions = { @@ -51,6 +59,7 @@ describe(`fetchWrapper`, function() { body: 'Example Body', }; await fetchWrapper.fetch({ + event, request: '/test/fetchOptions', fetchOptions: exampleOptions, }); @@ -63,6 +72,9 @@ describe(`fetchWrapper`, function() { }); it(`should ignore fetchOptions when request.mode === 'navigate'`, async function() { + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + // See https://github.com/GoogleChrome/workbox/issues/1796 const fetchStub = sandbox.stub(self, 'fetch').resolves(new Response()); @@ -78,6 +90,7 @@ describe(`fetchWrapper`, function() { Object.defineProperty(request, 'mode', {value: 'navigate'}); await fetchWrapper.fetch({ + event, fetchOptions, request, }); @@ -128,6 +141,9 @@ describe(`fetchWrapper`, function() { }); it(`should throw a meaningful error on bad requestWillFetch plugin`, async function() { + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + const fetchStub = sandbox.stub(self, 'fetch').callsFake(() => new Response()); const errorPlugin = { requestWillFetch: (request) => { @@ -138,6 +154,7 @@ describe(`fetchWrapper`, function() { await expectError(() => { return fetchWrapper.fetch({ + event, request: '/test/requestWillFetch/0', plugins: [errorPlugin], }); @@ -151,6 +168,9 @@ describe(`fetchWrapper`, function() { }); it(`should call fetchDidFail method in plugins`, async function() { + const event = new ExtendableEvent('fetch'); + spyOnEvent(event); + sandbox.stub(self, 'fetch').callsFake(() => { return Promise.reject(new Error('Injected Error.')); }); @@ -178,6 +198,7 @@ describe(`fetchWrapper`, function() { try { await fetchWrapper.fetch({ + event, request: '/test/failingRequest/0', plugins: [ firstPlugin,