Unrevisioned precaching warning

packages/workbox-build/src/lib/additional-manifest-entries-transform.js

@@ -14,8 +14,12 @@ module.exports = (additionalManifestEntries) => {
     const stringEntries = new Set();
 
     for (const additionalEntry of additionalManifestEntries) {
+      // Warn about either a string or an object that lacks a precache property.
+      // (An object with a revision property set to null is okay.)
       if (typeof additionalEntry === 'string') {
         stringEntries.add(additionalEntry);
+      } else if (additionalEntry && additionalEntry.revision === undefined) {
+        stringEntries.add(additionalEntry.url);
       }
 
       manifest.push(additionalEntry);

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

@@ -105,12 +105,9 @@ module.exports = {
   'bad-manifest-transforms-return-value': ol`The return value from a
     manifestTransform should be an object with 'manifest' and optionally
     'warnings' properties.`,
-  'string-entry-warning': ol`Some URLs were passed to additionalManifestEntries
-    as strings. Please ensure that these URLs contain versioning info
-    (e.g. 'https://example.com/v1.0/index.js'), as Workbox cannot maintain
-    its own revision info for them. To disable this warning, pass in an
-    object with a 'url' property, and a 'revision' property set to null
-    (e.g. {url: 'https://example.com/v1.0/index.js', revision: null}).`,
+  'string-entry-warning': ol`Some items were passed to additionalManifestEntries
+    without revisioning info. This is generally NOT safe. Learn more at
+    https://bit.ly/wb-precache.`,
   'no-manifest-entries-or-runtime-caching': ol`Couldn't find configuration for
     either precaching or runtime caching. Please ensure that the various glob
     options are set to match one or more files, and/or configure the

packages/workbox-precaching/src/PrecacheController.ts

@@ -66,7 +66,15 @@ class PrecacheController {
       });
     }
 
+    const urlsToWarnAbout: string[] = [];
     for (const entry of entries) {
+      // See https://github.com/GoogleChrome/workbox/issues/2259
+      if (typeof entry === 'string') {
+        urlsToWarnAbout.push(entry);
+      } else if (entry && entry.revision === undefined) {
+        urlsToWarnAbout.push(entry.url);
+      }
+
       const {cacheKey, url} = createCacheKey(entry);
       const cacheMode = (typeof entry !== 'string' && entry.revision) ?
         'reload' : 'default';
@@ -91,6 +99,19 @@ class PrecacheController {
 
       this._urlsToCacheKeys.set(url, cacheKey);
       this._urlsToCacheModes.set(url, cacheMode);
+
+      if (urlsToWarnAbout.length > 0) {
+        const warningMessage = `Workbox is precaching URLs without revision ` +
+          `info: ${urlsToWarnAbout.join(', ')}\nThis is generally NOT safe. ` +
+          `Learn more at https://bit.ly/wb-precache`;
+        if (process.env.NODE_ENV === 'production') {
+          // Use console directly to display this warning without bloating
+          // bundle sizes by pulling in all of the logger codebase in prod.
+          console.warn(warningMessage);
+        } else {
+          logger.warn(warningMessage);
+        }
+      }
     }
   }
 

test/all/node/test-prod-builds.mjs

@@ -25,7 +25,9 @@ describe(`[all] prod builds`, function() {
     const invalidFiles = [];
     buildFiles.forEach((filePath) => {
       const fileContents = fs.readFileSync(filePath).toString();
-      if (fileContents.indexOf(`console`) > -1 ||
+      if ((fileContents.indexOf(`console`) > -1 &&
+           // See https://github.com/GoogleChrome/workbox/issues/2259
+           !filePath.includes('workbox-precaching')) ||
           fileContents.indexOf(`%cworkbox`) > -1) {
         invalidFiles.push(filePath);
       }

test/workbox-build/node/lib/additional-manifest-entries-transform.js

@@ -15,46 +15,47 @@ describe(`[workbox-build] lib/additional-manifest-entries-transform.js`, functio
   function getManifest() {
     return [{
       url: '/first',
+      revision: null,
     }];
   }
 
   it(`should not make any changes when additionalManifestEntries is empty`, function() {
     const transform = additionalManifestEntriesTransform([]);
     expect(transform(getManifest())).to.eql({
-      manifest: [{url: '/first'}],
+      manifest: [{url: '/first', revision: null}],
       warnings: [],
     });
   });
 
   it(`should add the additionalManifestEntries to the end of the existing manifest`, function() {
     const transform = additionalManifestEntriesTransform([
-      {url: '/second'},
-      {url: '/third'},
+      {url: '/second', revision: null},
+      {url: '/third', revision: null},
     ]);
 
     expect(transform(getManifest())).to.eql({
       manifest: [
-        {url: '/first'},
-        {url: '/second'},
-        {url: '/third'},
+        {url: '/first', revision: null},
+        {url: '/second', revision: null},
+        {url: '/third', revision: null},
       ],
       warnings: [],
     });
   });
 
-  it(`should return a warning, along with the modified manifest, when additionalManifestEntries contains a string`, function() {
+  it(`should return a warning, along with the modified manifest, when additionalManifestEntries contains a string or an entry without revision`, function() {
     const transform = additionalManifestEntriesTransform([
       '/second',
       {url: '/third'},
     ]);
 
     expect(transform(getManifest())).to.eql({
       manifest: [
-        {url: '/first'},
+        {url: '/first', revision: null},
         '/second',
         {url: '/third'},
       ],
-      warnings: [errors['string-entry-warning'] + '\n  - /second\n'],
+      warnings: [errors['string-entry-warning'] + '\n  - /second\n  - /third\n'],
     });
   });
 });

test/workbox-precaching/sw/test-PrecacheController.mjs

@@ -155,6 +155,31 @@ describe(`PrecacheController`, function() {
       });
     });
 
+    it(`should log a warning unless an entry has a revision property`, function() {
+      const logObject = process.env.NODE_ENV === 'production' ? console : logger;
+      const warnStub = sandbox.stub(logObject, 'warn');
+
+      const precacheController = new PrecacheController();
+
+      precacheController.addToCacheList([
+        '/should-warn',
+      ]);
+      expect(warnStub.calledOnce).to.be.true;
+      warnStub.resetHistory();
+
+      precacheController.addToCacheList([
+        {url: '/also-should-warn'},
+      ]);
+      expect(warnStub.calledOnce).to.be.true;
+      warnStub.resetHistory();
+
+      precacheController.addToCacheList([
+        {url: '/should-not-warn', revision: null},
+        {url: '/also-should-not-warn', revision: '1234abcd'},
+      ]);
+      expect(warnStub.notCalled).to.be.true;
+    });
+
     it(`should add url + revision objects to cache list`, async function() {
       const precacheController = new PrecacheController();