diff --git a/packages/workbox-build/src/lib/additional-manifest-entries-transform.js b/packages/workbox-build/src/lib/additional-manifest-entries-transform.js index aff04d9e9..57d474761 100644 --- a/packages/workbox-build/src/lib/additional-manifest-entries-transform.js +++ b/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); diff --git a/packages/workbox-build/src/lib/errors.js b/packages/workbox-build/src/lib/errors.js index f9c07e0f3..52ec8a837 100644 --- a/packages/workbox-build/src/lib/errors.js +++ b/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 diff --git a/packages/workbox-precaching/src/PrecacheController.ts b/packages/workbox-precaching/src/PrecacheController.ts index 999ab59de..596d2f2bf 100644 --- a/packages/workbox-precaching/src/PrecacheController.ts +++ b/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); + } + } } } diff --git a/test/all/node/test-prod-builds.mjs b/test/all/node/test-prod-builds.mjs index 13f1edee4..49d887a3c 100644 --- a/test/all/node/test-prod-builds.mjs +++ b/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); } diff --git a/test/workbox-build/node/lib/additional-manifest-entries-transform.js b/test/workbox-build/node/lib/additional-manifest-entries-transform.js index 9d4bffa40..72b611577 100644 --- a/test/workbox-build/node/lib/additional-manifest-entries-transform.js +++ b/test/workbox-build/node/lib/additional-manifest-entries-transform.js @@ -15,34 +15,35 @@ 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'}, @@ -50,11 +51,11 @@ describe(`[workbox-build] lib/additional-manifest-entries-transform.js`, functio 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'], }); }); }); diff --git a/test/workbox-precaching/sw/test-PrecacheController.mjs b/test/workbox-precaching/sw/test-PrecacheController.mjs index 3d64f943b..c68024518 100644 --- a/test/workbox-precaching/sw/test-PrecacheController.mjs +++ b/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();