Override more function validation (#2911)

* Override more fn validation

* Opt-out of linting

gulp-tasks/build-node-packages.js

@@ -71,10 +71,19 @@ async function generateWorkboxBuildJSONSchema(packagePath) {
       schema.properties.include.items = {};
     }
 
+    // See https://github.com/GoogleChrome/workbox/issues/2910
+    if (schema.definitions.OnSyncCallback) {
+      schema.definitions.OnSyncCallback = {};
+    }
+
     if (schema.definitions.RouteMatchCallback) {
       schema.definitions.RouteMatchCallback = {};
     }
 
+    if (schema.definitions.RouteHandlerCallback) {
+      schema.definitions.RouteHandlerCallback = {};
+    }
+
     // See https://github.com/GoogleChrome/workbox/issues/2901
     if (schema.definitions.WorkboxPlugin) {
       for (const plugin of Object.keys(

packages/workbox-build/src/lib/errors.ts

@@ -120,4 +120,5 @@ export const errors = {
     configure a custom cacheName.`,
   'manifest-transforms': ol`When using manifestTransforms, you must provide
     an array of functions.`,
+  'invalid-handler-string': ol`The handler name provided is not valid: `,
 };

packages/workbox-build/src/lib/validate-options.ts

@@ -40,7 +40,12 @@ export class WorkboxConfigError extends Error {
   }
 }
 
-function validate<T>(input: unknown, methodName: MethodNames): T {
+// Some methods need to do follow-up validation using the JSON schema,
+// so return both the validated options and then schema.
+function validate<T>(
+  input: unknown,
+  methodName: MethodNames,
+): [T, JSONSchemaType<T>] {
   // Don't mutate input: https://github.com/GoogleChrome/workbox/issues/2158
   const inputCopy = Object.assign({}, input);
   // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
@@ -49,7 +54,7 @@ function validate<T>(input: unknown, methodName: MethodNames): T {
   if (validate(inputCopy)) {
     // All methods support manifestTransforms, so validate it here.
     ensureValidManifestTransforms(inputCopy);
-    return inputCopy;
+    return [inputCopy, jsonSchema];
   }
 
   const betterErrors = betterAjvErrors({
@@ -126,23 +131,65 @@ function ensureValidRuntimeCachingOrGlobDirectory(
   }
 }
 
+// This is... messy, because we can't rely on the built-in ajv validation for
+// runtimeCaching.handler, as it needs to accept {} (i.e. any) due to
+// https://github.com/GoogleChrome/workbox/pull/2899
+// So we need to perform validation when a string (not a function) is used.
+function ensureValidStringHandler(
+  options: GenerateSWOptions | WebpackGenerateSWOptions,
+  jsonSchema: JSONSchemaType<GenerateSWOptions | WebpackGenerateSWOptions>,
+): void {
+  let validHandlers: Array<string> = [];
+  /* eslint-disable */
+  for (const handler of jsonSchema.definitions?.RuntimeCaching?.properties
+    ?.handler?.anyOf || []) {
+    if ('enum' in handler) {
+      validHandlers = handler.enum;
+      break;
+    }
+  }
+  /* eslint-enable */
+
+  for (const runtimeCaching of options.runtimeCaching || []) {
+    if (
+      typeof runtimeCaching.handler === 'string' &&
+      !validHandlers.includes(runtimeCaching.handler)
+    ) {
+      throw new WorkboxConfigError(
+        errors['invalid-handler-string'] + runtimeCaching.handler,
+      );
+    }
+  }
+}
+
 export function validateGenerateSWOptions(input: unknown): GenerateSWOptions {
-  const validatedOptions = validate<GenerateSWOptions>(input, 'GenerateSW');
+  const [validatedOptions, jsonSchema] = validate<GenerateSWOptions>(
+    input,
+    'GenerateSW',
+  );
   ensureValidNavigationPreloadConfig(validatedOptions);
   ensureValidCacheExpiration(validatedOptions);
   ensureValidRuntimeCachingOrGlobDirectory(validatedOptions);
+  ensureValidStringHandler(validatedOptions, jsonSchema);
 
   return validatedOptions;
 }
 
 export function validateGetManifestOptions(input: unknown): GetManifestOptions {
-  return validate<GetManifestOptions>(input, 'GetManifest');
+  const [validatedOptions] = validate<GetManifestOptions>(input, 'GetManifest');
+
+  return validatedOptions;
 }
 
 export function validateInjectManifestOptions(
   input: unknown,
 ): InjectManifestOptions {
-  return validate<InjectManifestOptions>(input, 'InjectManifest');
+  const [validatedOptions] = validate<InjectManifestOptions>(
+    input,
+    'InjectManifest',
+  );
+
+  return validatedOptions;
 }
 
 // The default `exclude: [/\.map$/, /^manifest.*\.js$/]` value can't be
@@ -157,13 +204,14 @@ export function validateWebpackGenerateSWOptions(
     },
     input,
   );
-  const validatedOptions = validate<WebpackGenerateSWOptions>(
+  const [validatedOptions, jsonSchema] = validate<WebpackGenerateSWOptions>(
     inputWithExcludeDefault,
     'WebpackGenerateSW',
   );
 
   ensureValidNavigationPreloadConfig(validatedOptions);
   ensureValidCacheExpiration(validatedOptions);
+  ensureValidStringHandler(validatedOptions, jsonSchema);
 
   return validatedOptions;
 }
@@ -178,8 +226,10 @@ export function validateWebpackInjectManifestOptions(
     },
     input,
   );
-  return validate<WebpackInjectManifestOptions>(
+  const [validatedOptions] = validate<WebpackInjectManifestOptions>(
     inputWithExcludeDefault,
     'WebpackInjectManifest',
   );
+
+  return validatedOptions;
 }

packages/workbox-build/src/schema/GenerateSWOptions.json

@@ -407,11 +407,7 @@
         "urlPattern"
       ]
     },
-    "RouteHandlerCallback": {
-      "description": "The \"handler\" callback is invoked whenever a `Router` matches a URL/Request\nto a `Route` via its `RouteMatchCallback`. This handler callback should\nreturn a `Promise` that resolves with a `Response`.\n\nIf a non-empty array or object is returned by the `RouteMatchCallback` it\nwill be passed in as this handler's `options.params` argument.",
-      "type": "object",
-      "additionalProperties": false
-    },
+    "RouteHandlerCallback": {},
     "RouteHandlerObject": {
       "description": "An object with a `handle` method of type `RouteHandlerCallback`.\n\nA `Route` object can be created with either an `RouteHandlerCallback`\nfunction or this `RouteHandler` object. The benefit of the `RouteHandler`\nis it can be extended (as is done by the `workbox-strategies` package).",
       "type": "object",
@@ -437,10 +433,7 @@
       },
       "additionalProperties": false
     },
-    "OnSyncCallback": {
-      "type": "object",
-      "additionalProperties": false
-    },
+    "OnSyncCallback": {},
     "BroadcastCacheUpdateOptions": {
       "type": "object",
       "properties": {

packages/workbox-build/src/schema/GetManifestOptions.json

@@ -299,11 +299,7 @@
         "urlPattern"
       ]
     },
-    "RouteHandlerCallback": {
-      "description": "The \"handler\" callback is invoked whenever a `Router` matches a URL/Request\nto a `Route` via its `RouteMatchCallback`. This handler callback should\nreturn a `Promise` that resolves with a `Response`.\n\nIf a non-empty array or object is returned by the `RouteMatchCallback` it\nwill be passed in as this handler's `options.params` argument.",
-      "type": "object",
-      "additionalProperties": false
-    },
+    "RouteHandlerCallback": {},
     "RouteHandlerObject": {
       "description": "An object with a `handle` method of type `RouteHandlerCallback`.\n\nA `Route` object can be created with either an `RouteHandlerCallback`\nfunction or this `RouteHandler` object. The benefit of the `RouteHandler`\nis it can be extended (as is done by the `workbox-strategies` package).",
       "type": "object",
@@ -329,10 +325,7 @@
       },
       "additionalProperties": false
     },
-    "OnSyncCallback": {
-      "type": "object",
-      "additionalProperties": false
-    },
+    "OnSyncCallback": {},
     "BroadcastCacheUpdateOptions": {
       "type": "object",
       "properties": {

packages/workbox-build/src/schema/InjectManifestOptions.json

@@ -311,11 +311,7 @@
         "urlPattern"
       ]
     },
-    "RouteHandlerCallback": {
-      "description": "The \"handler\" callback is invoked whenever a `Router` matches a URL/Request\nto a `Route` via its `RouteMatchCallback`. This handler callback should\nreturn a `Promise` that resolves with a `Response`.\n\nIf a non-empty array or object is returned by the `RouteMatchCallback` it\nwill be passed in as this handler's `options.params` argument.",
-      "type": "object",
-      "additionalProperties": false
-    },
+    "RouteHandlerCallback": {},
     "RouteHandlerObject": {
       "description": "An object with a `handle` method of type `RouteHandlerCallback`.\n\nA `Route` object can be created with either an `RouteHandlerCallback`\nfunction or this `RouteHandler` object. The benefit of the `RouteHandler`\nis it can be extended (as is done by the `workbox-strategies` package).",
       "type": "object",
@@ -341,10 +337,7 @@
       },
       "additionalProperties": false
     },
-    "OnSyncCallback": {
-      "type": "object",
-      "additionalProperties": false
-    },
+    "OnSyncCallback": {},
     "BroadcastCacheUpdateOptions": {
       "type": "object",
       "properties": {

packages/workbox-build/src/schema/WebpackGenerateSWOptions.json

@@ -386,11 +386,7 @@
         "urlPattern"
       ]
     },
-    "RouteHandlerCallback": {
-      "description": "The \"handler\" callback is invoked whenever a `Router` matches a URL/Request\nto a `Route` via its `RouteMatchCallback`. This handler callback should\nreturn a `Promise` that resolves with a `Response`.\n\nIf a non-empty array or object is returned by the `RouteMatchCallback` it\nwill be passed in as this handler's `options.params` argument.",
-      "type": "object",
-      "additionalProperties": false
-    },
+    "RouteHandlerCallback": {},
     "RouteHandlerObject": {
       "description": "An object with a `handle` method of type `RouteHandlerCallback`.\n\nA `Route` object can be created with either an `RouteHandlerCallback`\nfunction or this `RouteHandler` object. The benefit of the `RouteHandler`\nis it can be extended (as is done by the `workbox-strategies` package).",
       "type": "object",
@@ -416,10 +412,7 @@
       },
       "additionalProperties": false
     },
-    "OnSyncCallback": {
-      "type": "object",
-      "additionalProperties": false
-    },
+    "OnSyncCallback": {},
     "BroadcastCacheUpdateOptions": {
       "type": "object",
       "properties": {

packages/workbox-build/src/schema/WebpackInjectManifestOptions.json

@@ -298,11 +298,7 @@
         "urlPattern"
       ]
     },
-    "RouteHandlerCallback": {
-      "description": "The \"handler\" callback is invoked whenever a `Router` matches a URL/Request\nto a `Route` via its `RouteMatchCallback`. This handler callback should\nreturn a `Promise` that resolves with a `Response`.\n\nIf a non-empty array or object is returned by the `RouteMatchCallback` it\nwill be passed in as this handler's `options.params` argument.",
-      "type": "object",
-      "additionalProperties": false
-    },
+    "RouteHandlerCallback": {},
     "RouteHandlerObject": {
       "description": "An object with a `handle` method of type `RouteHandlerCallback`.\n\nA `Route` object can be created with either an `RouteHandlerCallback`\nfunction or this `RouteHandler` object. The benefit of the `RouteHandler`\nis it can be extended (as is done by the `workbox-strategies` package).",
       "type": "object",
@@ -328,10 +324,7 @@
       },
       "additionalProperties": false
     },
-    "OnSyncCallback": {
-      "type": "object",
-      "additionalProperties": false
-    },
+    "OnSyncCallback": {},
     "BroadcastCacheUpdateOptions": {
       "type": "object",
       "properties": {

test/workbox-build/node/generate-sw.js

@@ -921,18 +921,25 @@ describe(`[workbox-build] generate-sw.js (End to End)`, function() {
           errors['invalid-network-timeout-seconds']);
     });
 
-    it(`should support passing in a function for cachedResponseWillBeUsed`, async function() {
+    it(`should support passing in a function when allowed`, async function() {
       const swDest = tempy.file({extension: 'js'});
-      const handler = 'CacheFirst';
+      const handler = () => {};
+      const urlPattern = () => {};
 
       const runtimeCachingOptions = {
+        backgroundSync: {
+          name: 'test',
+          options: {
+            onSync: () => {},
+          },
+        },
         plugins: [{
-          cachedResponseWillBeUsed: async () => {},
+          cachedResponseWillBeUsed: () => {},
         }],
       };
       const runtimeCaching = [{
-        urlPattern: REGEXP_URL_PATTERN,
         handler,
+        urlPattern,
         options: runtimeCachingOptions,
       }];
       const options = Object.assign({}, BASE_OPTIONS, {
@@ -968,7 +975,7 @@ describe(`[workbox-build] generate-sw.js (End to End)`, function() {
           revision: /^[0-9a-f]{32}$/,
         }], {}]],
         registerRoute: [
-          [REGEXP_URL_PATTERN, {name: handler}, DEFAULT_METHOD],
+          [urlPattern.toString(), handler.toString(), DEFAULT_METHOD],
         ],
       }});
     });