Adds non-fatal warnings to the build tools (#1325)

* WIP.

* Hopefully working.

* Some linting.

* Formatting fix.

* Review feedback.

* Fixing a test post-merge.

infra/testing/validator/service-worker-runtime.js

@@ -97,21 +97,21 @@ function validateMethodCalls({methodsToSpies, expectedMethodCalls}) {
  * this method will reject with a description of what failed.
  *
  * @param {string} [swFile]
- * @param {string} [swCode]
+ * @param {string} [swString]
  * @param {Object} expectedMethodCalls
  * @return {Promise} Resolves if all of the expected method calls were made.
  */
-module.exports = async ({swFile, swCode, expectedMethodCalls}) => {
-  assert((swFile || swCode) && !(swFile && swCode),
-    `Set swFile or swCode, but not both.`);
+module.exports = async ({swFile, swString, expectedMethodCalls}) => {
+  assert((swFile || swString) && !(swFile && swString),
+    `Set swFile or swString, but not both.`);
 
   if (swFile) {
-    swCode = await fse.readFile(swFile, 'utf8');
+    swString = await fse.readFile(swFile, 'utf8');
   }
 
   const {context, methodsToSpies} = setupSpiesAndContext();
 
-  vm.runInNewContext(swCode, context);
+  vm.runInNewContext(swString, context);
 
   validateMethodCalls({methodsToSpies, expectedMethodCalls});
 };

packages/workbox-build/package.json

@@ -26,6 +26,7 @@
     "glob": "^7.1.2",
     "joi": "^11.1.1",
     "lodash.template": "^4.4.0",
+    "pretty-bytes": "^4.0.2",
     "workbox-background-sync": "^3.0.0-beta.0",
     "workbox-broadcast-cache-update": "^3.0.0-beta.0",
     "workbox-cache-expiration": "^3.0.0-beta.0",

packages/workbox-build/src/entry-points/generate-sw-string.js

@@ -25,19 +25,23 @@ const validate = require('./options/validate');
  *
  * @param {Object} config Please refer to the
  * [configuration guide](https://developers.google.com/web/tools/workbox/modules/workbox-build#generateswstring_mode).
- * @return {Promise<String>} A populated service worker template, based on the
- * other configuration options provided.
+ * @return {Promise<{swString: String, warnings: Array<String>}>} A promise that
+ * resolves once the service worker template is populated. The `swString`
+ * property contains a string representation of the full service worker code.
+ * Any non-fatal warning messages will be returned via `warnings`.
  *
  * @memberof module:workbox-build
  */
 async function generateSWString(config) {
   const options = validate(config, generateSWStringSchema);
 
-  const {manifestEntries} = await getFileManifestEntries(options);
+  const {manifestEntries, warnings} = await getFileManifestEntries(options);
 
-  return populateSWTemplate(Object.assign({
+  const swString = await populateSWTemplate(Object.assign({
     manifestEntries,
   }, options));
+
+  return {swString, warnings};
 }
 
 module.exports = generateSWString;

packages/workbox-build/src/entry-points/generate-sw.js

@@ -36,10 +36,12 @@ const writeServiceWorkerUsingDefaultTemplate =
  *
  * @param {Object} config Please refer to the
  * [configuration guide](https://developers.google.com/web/tools/workbox/modules/workbox-build#full_generatesw_config).
- * @return {Promise<{count: Number, size: Number}>} A promise that resolves once
- * the service worker file has been written to `swDest`. The `size` property
- * contains the aggregate size of all the precached entries, in bytes, and the
- * `count` property contains the total number of precached entries.
+ * @return {Promise<{count: Number, size: Number, warnings: Array<String>}>}
+ * A promise that resolves once the service worker file has been written to
+ * `swDest`. The `size` property contains the aggregate size of all the
+ * precached entries, in bytes, and the `count` property contains the total
+ * number of precached entries. Any non-fatal warning messages will be returned
+ * via `warnings`.
  *
  * @memberof module:workbox-build
  */
@@ -69,13 +71,14 @@ async function generateSW(config) {
     options.modulePathPrefix = workboxDirectoryName;
   }
 
-  const {count, size, manifestEntries} = await getFileManifestEntries(options);
+  const {count, size, manifestEntries, warnings} =
+    await getFileManifestEntries(options);
 
   await writeServiceWorkerUsingDefaultTemplate(Object.assign({
     manifestEntries,
   }, options));
 
-  return {count, size};
+  return {count, size, warnings};
 }
 
 module.exports = generateSW;

packages/workbox-build/src/entry-points/get-manifest.js

@@ -26,19 +26,21 @@ const validate = require('./options/validate');
  * @param {Object} config Please refer to the
  * [configuration guide](https://developers.google.com/web/tools/workbox/modules/workbox-build#getmanifest_mode).
  * @return {Promise<{manifestEntries: Array<ManifestEntry>,
- * count: Number, size: Number}>} A promise that resolves once the precache
- * manifest is determined. The `size` property contains the aggregate size of
- * all the precached entries, in bytes, the `count` property contains the total
- * number of precached entries, and the `manifestEntries` property contains all
- * the `ManifestEntry` items.
+ * count: Number, size: Number, warnings: Array<String>}>} A promise that
+ * resolves once the precache manifest is determined. The `size` property
+ * contains the aggregate size of all the precached entries, in bytes, the
+ * `count` property contains the total number of precached entries, and the
+ * `manifestEntries` property contains all the `ManifestEntry` items. Any
+ * non-fatal warning messages will be returned via `warnings`.
  *
  * @memberof module:workbox-build
  */
 async function getManifest(config) {
   const options = validate(config, getManifestSchema);
 
-  const {manifestEntries, count, size} = await getFileManifestEntries(options);
-  return {manifestEntries, count, size};
+  const {manifestEntries, count, size, warnings} =
+    await getFileManifestEntries(options);
+  return {manifestEntries, count, size, warnings};
 }
 
 module.exports = getManifest;

packages/workbox-build/src/entry-points/inject-manifest.js

@@ -36,10 +36,12 @@ const validate = require('./options/validate');
  *
  * @param {Object} config Please refer to the
  * [configuration guide](https://developers.google.com/web/tools/workbox/modules/workbox-build#full_injectmanifest_config).
- * @return {Promise<{count: Number, size: Number}>} A promise that resolves once
- * the service worker file has been written to `swDest`. The `size` property
- * contains the aggregate size of all the precached entries, in bytes, and the
- * `count` property contains the total number of precached entries.
+ * @return {Promise<{count: Number, size: Number, warnings: Array<String>}>}
+ * A promise that resolves once the service worker file has been written to
+ * `swDest`. The `size` property contains the aggregate size of all the
+ * precached entries, in bytes, and the `count` property contains the total
+ * number of precached entries. Any non-fatal warning messages will be returned
+ * via `warnings`.
  *
  * @memberof module:workbox-build
  */
@@ -52,7 +54,8 @@ async function injectManifest(config) {
 
   const globalRegexp = new RegExp(options.injectionPointRegexp, 'g');
 
-  const {count, size, manifestEntries} = await getFileManifestEntries(options);
+  const {count, size, manifestEntries, warnings} =
+    await getFileManifestEntries(options);
   let swFileContents;
   try {
     swFileContents = await fse.readFile(config.swSrc, 'utf8');
@@ -84,7 +87,7 @@ async function injectManifest(config) {
 
   await fse.writeFile(config.swDest, swFileContents);
 
-  return {count, size};
+  return {count, size, warnings};
 }
 
 module.exports = injectManifest;

packages/workbox-build/src/lib/filter-files.js

@@ -33,33 +33,38 @@ const noRevisionForUrlsMatchingTransform =
  * @example <caption>A transformation that prepended the origin of a CDN for any
  * URL starting with '/assets/' could be implemented as:</caption>
  *
- * const cdnTransform = (manifestEntries) => manifestEntries.map(entry => {
- *   const cdnOrigin = 'https://example.com';
- *   if (entry.url.startsWith('/assets/')) {
- *     entry.url = cdnOrigin + entry.url;
- *   }
- *   return entry;
- * });
+ * const cdnTransform = (manifestEntries) => {
+ *   const manifest = manifestEntries.map(entry => {
+ *     const cdnOrigin = 'https://example.com';
+ *     if (entry.url.startsWith('/assets/')) {
+ *       entry.url = cdnOrigin + entry.url;
+ *     }
+ *     return entry;
+ *   });
+ *   return {manifest, warnings: []};
+ * };
  *
  * @example <caption>A transformation that removes the revision field when the
  * URL contains an 8-character hash surrounded by '.', indicating that it
  * already contains revision information:</caption>
  *
  * const removeRevisionTransform = (manifestEntries) => {
- *   return manifestEntries.map(entry => {
+ *   const manifest = manifestEntries.map(entry => {
  *     const hashRegExp = /\.\w{8}\./;
  *     if (entry.url.match(hashRegExp)) {
  *       delete entry.revision;
  *     }
  *     return entry;
  *   });
+ *   return {manifest, warnings: []};
  * };
  *
  * @callback ManifestTransform
  * @param {Array<ManifestEntry>} manifestEntries The full array of entries,
  * prior to the current transformation.
- * @return {Array<ManifestEntry>} The array of entries with the transformation
- * applied.
+ * @return {{manifest: Array<ManifestEntry>, warnings: Array<String>|undefined}}
+ * The array of entries with the transformation applied, and optionally, any
+ * warnings that should be reported back to the build tool.
  *
  * @memberof module:workbox-build
  */
@@ -71,6 +76,8 @@ module.exports = ({
   maximumFileSizeToCacheInBytes,
   modifyUrlPrefix,
 }) => {
+  let allWarnings = [];
+
   // Take the array of fileDetail objects and convert it into an array of
   // {url, revision, size} objects, with \ replaced with /.
   const normalizedManifest = fileDetails.map((fileDetails) => {
@@ -99,10 +106,12 @@ module.exports = ({
   // Any additional manifestTransforms that were passed will be applied last.
   transformsToApply = transformsToApply.concat(manifestTransforms || []);
 
-  // Apply the transformations sequentially.
-  const transformedManifest = transformsToApply.reduce(
-    (previousManifest, transform) => transform(previousManifest),
-    normalizedManifest);
+  let transformedManifest = normalizedManifest;
+  for (const transform of transformsToApply) {
+    const {manifest, warnings} = transform(transformedManifest);
+    transformedManifest = manifest;
+    allWarnings = allWarnings.concat(warnings || []);
+  }
 
   // Generate some metadata about the manifest before we clear out the size
   // properties from each entry.
@@ -117,5 +126,6 @@ module.exports = ({
     count,
     size,
     manifestEntries: transformedManifest,
+    warnings: allWarnings,
   };
 };

packages/workbox-build/src/lib/get-file-details.js

@@ -42,7 +42,7 @@ module.exports = (globOptions) => {
   }
 
   if (globbedFiles.length === 0) {
-    throw new Error(errors['useless-glob-pattern'] +
+    throw new Error(errors['useless-glob-pattern'] + ' ' +
       JSON.stringify({globDirectory, globPattern, globIgnores}, null, 2));
   }