From 0dbe0a406933bcc60dfd8617bcca5dd5f356a14a Mon Sep 17 00:00:00 2001 From: Beatriz de Miguel Date: Mon, 4 Dec 2017 11:03:06 +0100 Subject: [PATCH 1/6] [Workbox-Build] Raise an error when an unusual error is encountered when attempting to read a directory and follow symlinked directories when expanding ** patterns --- packages/workbox-build/src/lib/get-file-details.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/workbox-build/src/lib/get-file-details.js b/packages/workbox-build/src/lib/get-file-details.js index 351834c66..819ea7a1f 100644 --- a/packages/workbox-build/src/lib/get-file-details.js +++ b/packages/workbox-build/src/lib/get-file-details.js @@ -21,12 +21,22 @@ const errors = require('./errors'); const getFileSize = require('./get-file-size'); const getFileHash = require('./get-file-hash'); -module.exports = ({globDirectory, globIgnores, globPattern}) => { +module.exports = (globOptions) => { + const { + globDirectory, + globIgnores, + globPattern, + globFollows, + globStrict, + } = globOptions; let globbedFiles; try { globbedFiles = glob.sync(globPattern, { cwd: globDirectory, ignore: globIgnores, + follow: typeof globFollows !== 'undefined'? globFollows : true, + strict: typeof globStrict !== 'undefined'? globStrict : true, + }); } catch (err) { throw new Error(errors['unable-to-glob-files'] + ` '${err.message}'`); From 9077a55e8155d3c2b32244a36b195e3e89196aa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Gonz=C3=A1lez=20Rus?= Date: Mon, 4 Dec 2017 12:03:53 +0100 Subject: [PATCH 2/6] Change globFollows option to globFollow --- packages/workbox-build/src/lib/get-file-details.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/workbox-build/src/lib/get-file-details.js b/packages/workbox-build/src/lib/get-file-details.js index 819ea7a1f..a4951f774 100644 --- a/packages/workbox-build/src/lib/get-file-details.js +++ b/packages/workbox-build/src/lib/get-file-details.js @@ -26,7 +26,7 @@ module.exports = (globOptions) => { globDirectory, globIgnores, globPattern, - globFollows, + globFollow, globStrict, } = globOptions; let globbedFiles; @@ -34,7 +34,7 @@ module.exports = (globOptions) => { globbedFiles = glob.sync(globPattern, { cwd: globDirectory, ignore: globIgnores, - follow: typeof globFollows !== 'undefined'? globFollows : true, + follow: typeof globFollow !== 'undefined'? globFollow : true, strict: typeof globStrict !== 'undefined'? globStrict : true, }); From 9366e95d5c5cd88cd626f64b0131fa9febad8860 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Gonz=C3=A1lez=20Rus?= Date: Mon, 4 Dec 2017 12:04:14 +0100 Subject: [PATCH 3/6] Add unit testing for globFollow and globStrict --- .../node/lib/get-file-details.js | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/test/workbox-build/node/lib/get-file-details.js b/test/workbox-build/node/lib/get-file-details.js index 51899af9f..b0c1e7fd6 100644 --- a/test/workbox-build/node/lib/get-file-details.js +++ b/test/workbox-build/node/lib/get-file-details.js @@ -88,4 +88,46 @@ describe(`[workbox-build] lib/get-file-details.js`, function() { size: SIZE, }]); }); + + it(`should call sync with follow and strict options by default`, function() { + const getFileDetails = proxyquire(MODULE_PATH, { + 'glob': { + sync: (pattern, options) => { + expect(options.follow).to.be.true; + expect(options.strict).to.be.true; + + return [FILE1]; + }, + }, + './get-file-size': (value) => SIZE, + './get-file-hash': (value) => HASH, + }); + + getFileDetails({ + globDirectory: GLOB_DIRECTORY, + globPattern: GLOB_PATTERN, + }); + }); + + it(`should call sync with follow and strict options with value`, function() { + const getFileDetails = proxyquire(MODULE_PATH, { + 'glob': { + sync: (pattern, options) => { + expect(options.follow).to.be.false; + expect(options.strict).to.be.false; + + return [FILE1]; + }, + }, + './get-file-size': (value) => SIZE, + './get-file-hash': (value) => HASH, + }); + + getFileDetails({ + globDirectory: GLOB_DIRECTORY, + globPattern: GLOB_PATTERN, + globFollow: false, + globStrict: false, + }); + }); }); From 3129073a78596d509a13fb99e55da140d2c6228e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Gonz=C3=A1lez=20Rus?= Date: Mon, 4 Dec 2017 12:04:51 +0100 Subject: [PATCH 4/6] Update doc with globFollow and globStrict options --- packages/workbox-build/src/_types.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/workbox-build/src/_types.js b/packages/workbox-build/src/_types.js index e7f6e7395..9ebb14173 100644 --- a/packages/workbox-build/src/_types.js +++ b/packages/workbox-build/src/_types.js @@ -132,6 +132,19 @@ import './_version.mjs'; * * E.g. `['**\/ignored.html']` * + * @property {Boolean} [globFollow=true] Follow symlinked directories when + * expanding ** patterns. Note that this can result in a lot of duplicate + * references in the presence of cyclic links. + * + * E.g. `true` + * + * @property {Boolean} [globStrict=true] When an unusual error is encountered + * when attempting to read a directory, the process will just continue on in + * search of other matches. Set the strict option to raise an error in these + * cases. + * + * E.g. `true` + * * @property {Object} [templatedUrls] * If a URL is rendered generated based on some server-side logic, its contents * may depend on multiple files or on some other unique string value. From bd8e9323c9d568609f715a2fc3f901bbd0c3496d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Gonz=C3=A1lez=20Rus?= Date: Tue, 5 Dec 2017 08:48:38 +0100 Subject: [PATCH 5/6] Update doc issues from pull-request comments --- packages/workbox-build/src/_types.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/workbox-build/src/_types.js b/packages/workbox-build/src/_types.js index 9ebb14173..b3946c6e4 100644 --- a/packages/workbox-build/src/_types.js +++ b/packages/workbox-build/src/_types.js @@ -133,18 +133,14 @@ import './_version.mjs'; * E.g. `['**\/ignored.html']` * * @property {Boolean} [globFollow=true] Follow symlinked directories when - * expanding ** patterns. Note that this can result in a lot of duplicate + * expanding '**' patterns. Note that this can result in a lot of duplicate * references in the presence of cyclic links. * - * E.g. `true` - * * @property {Boolean} [globStrict=true] When an unusual error is encountered * when attempting to read a directory, the process will just continue on in * search of other matches. Set the strict option to raise an error in these * cases. * - * E.g. `true` - * * @property {Object} [templatedUrls] * If a URL is rendered generated based on some server-side logic, its contents * may depend on multiple files or on some other unique string value. From b31725799743fde6d8662f696da8379b41b2d652 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Gonz=C3=A1lez=20Rus?= Date: Thu, 7 Dec 2017 13:00:27 +0100 Subject: [PATCH 6/6] [test] Add test for globFollow option --- .../src/entry-points/options/base-schema.js | 2 + .../src/entry-points/options/defaults.js | 2 + .../workbox-build/src/lib/get-file-details.js | 7 +- .../src/lib/get-file-manifest-entries.js | 10 ++- .../node/entry-points/generate-sw-string.js | 2 + .../node/entry-points/generate-sw.js | 77 +++++++++++++++++++ .../node/entry-points/get-manifest.js | 33 ++++++++ .../node/entry-points/inject-manifest.js | 2 + .../node/entry-points/options/validate.js | 8 +- .../node/lib/get-file-details.js | 42 ---------- 10 files changed, 136 insertions(+), 49 deletions(-) diff --git a/packages/workbox-build/src/entry-points/options/base-schema.js b/packages/workbox-build/src/entry-points/options/base-schema.js index 3d391be71..886b6757a 100644 --- a/packages/workbox-build/src/entry-points/options/base-schema.js +++ b/packages/workbox-build/src/entry-points/options/base-schema.js @@ -21,8 +21,10 @@ const defaults = require('./defaults'); // Define some common constrains used by all methods. module.exports = joi.object().keys({ dontCacheBustUrlsMatching: joi.object().type(RegExp), + globFollow: joi.boolean().default(defaults.globFollow), globIgnores: joi.array().items(joi.string()).default(defaults.globIgnores), globPatterns: joi.array().items(joi.string()).default(defaults.globPatterns), + globStrict: joi.boolean().default(defaults.globStrict), manifestTransforms: joi.array().items(joi.func().arity(1)), maximumFileSizeToCacheInBytes: joi.number().min(1) .default(defaults.maximumFileSizeToCacheInBytes), diff --git a/packages/workbox-build/src/entry-points/options/defaults.js b/packages/workbox-build/src/entry-points/options/defaults.js index 8b29335a2..91966689c 100644 --- a/packages/workbox-build/src/entry-points/options/defaults.js +++ b/packages/workbox-build/src/entry-points/options/defaults.js @@ -15,8 +15,10 @@ */ module.exports = { + globFollow: true, globIgnores: ['node_modules/**/*'], globPatterns: ['**/*.{js,css,html}'], + globStrict: true, // Use a different default for generateSWString. generateSWStringGlobPatterns: [], maximumFileSizeToCacheInBytes: 2 * 1024 * 1024, diff --git a/packages/workbox-build/src/lib/get-file-details.js b/packages/workbox-build/src/lib/get-file-details.js index a4951f774..35b260f1b 100644 --- a/packages/workbox-build/src/lib/get-file-details.js +++ b/packages/workbox-build/src/lib/get-file-details.js @@ -24,19 +24,18 @@ const getFileHash = require('./get-file-hash'); module.exports = (globOptions) => { const { globDirectory, + globFollow, globIgnores, globPattern, - globFollow, globStrict, } = globOptions; let globbedFiles; try { globbedFiles = glob.sync(globPattern, { cwd: globDirectory, + follow: globFollow, ignore: globIgnores, - follow: typeof globFollow !== 'undefined'? globFollow : true, - strict: typeof globStrict !== 'undefined'? globStrict : true, - + strict: globStrict, }); } catch (err) { throw new Error(errors['unable-to-glob-files'] + ` '${err.message}'`); diff --git a/packages/workbox-build/src/lib/get-file-manifest-entries.js b/packages/workbox-build/src/lib/get-file-manifest-entries.js index f2bbcd4d4..aeb05c2c0 100644 --- a/packages/workbox-build/src/lib/get-file-manifest-entries.js +++ b/packages/workbox-build/src/lib/get-file-manifest-entries.js @@ -26,8 +26,10 @@ const getStringDetails = require('./get-string-details'); module.exports = async ({ dontCacheBustUrlsMatching, globDirectory, + globFollow, globIgnores, globPatterns, + globStrict, manifestTransforms, maximumFileSizeToCacheInBytes, modifyUrlPrefix, @@ -48,8 +50,10 @@ module.exports = async ({ fileDetails = globPatterns.reduce((accumulated, globPattern) => { const globbedFileDetails = getFileDetails({ globDirectory, - globPattern, + globFollow, globIgnores, + globPattern, + globStrict, }); globbedFileDetails.forEach((fileDetails) => { @@ -74,8 +78,10 @@ module.exports = async ({ try { const globbedFileDetails = getFileDetails({ globDirectory, - globPattern, + globFollow, globIgnores, + globPattern, + globStrict, }); return previous.concat(globbedFileDetails); } catch (error) { diff --git a/test/workbox-build/node/entry-points/generate-sw-string.js b/test/workbox-build/node/entry-points/generate-sw-string.js index 2d300d59d..541de687c 100644 --- a/test/workbox-build/node/entry-points/generate-sw-string.js +++ b/test/workbox-build/node/entry-points/generate-sw-string.js @@ -19,8 +19,10 @@ describe(`[workbox-build] entry-points/generate-sw-string.js (End to End)`, func 'directoryIndex', 'dontCacheBustUrlsMatching', 'globDirectory', + 'globFollow', 'globIgnores', 'globPatterns', + 'globStrict', 'ignoreUrlParametersMatching', 'injectionPointRegexp', 'manifestTransforms', diff --git a/test/workbox-build/node/entry-points/generate-sw.js b/test/workbox-build/node/entry-points/generate-sw.js index 7e8062f67..33177098a 100644 --- a/test/workbox-build/node/entry-points/generate-sw.js +++ b/test/workbox-build/node/entry-points/generate-sw.js @@ -26,8 +26,10 @@ describe(`[workbox-build] entry-points/generate-sw.js (End to End)`, function() 'clientsClaim', 'directoryIndex', 'dontCacheBustUrlsMatching', + 'globFollow', 'globIgnores', 'globPatterns', + 'globStrict', 'ignoreUrlParametersMatching', 'importScripts', 'importWorkboxFromCDN', @@ -372,6 +374,81 @@ describe(`[workbox-build] entry-points/generate-sw.js (End to End)`, function() }]], }}); }); + + it(`should use defaults when all the required parameters are present, with symlinks`, async function() { + const swDest = tempy.file(); + const globDirectory = tempy.directory(); + + await fse.ensureSymlink(GLOB_DIR, path.join(globDirectory, 'link')); + + const options = Object.assign({}, BASE_OPTIONS, { + globDirectory, + swDest, + }); + + const {count, size} = await generateSW(options); + + expect(count).to.eql(6); + expect(size).to.eql(2421); + await validateServiceWorkerRuntime({swFile: swDest, expectedMethodCalls: { + importScripts: [[WORKBOX_SW_CDN_URL]], + suppressWarnings: [[]], + precacheAndRoute: [[[{ + url: 'link/index.html', + revision: '3883c45b119c9d7e9ad75a1b4a4672ac', + }, { + url: 'link/page-1.html', + revision: '544658ab25ee8762dc241e8b1c5ed96d', + }, { + url: 'link/page-2.html', + revision: 'a3a71ce0b9b43c459cf58bd37e911b74', + }, { + url: 'link/styles/stylesheet-1.css', + revision: '934823cbc67ccf0d67aa2a2eeb798f12', + }, { + url: 'link/styles/stylesheet-2.css', + revision: '884f6853a4fc655e4c2dc0c0f27a227c', + }, { + url: 'link/webpackEntry.js', + revision: 'd41d8cd98f00b204e9800998ecf8427e', + }], {}]], + }}); + }); + + it(`should use defaults when all the required parameters are present, with 'globFollow' and symlinks`, async function() { + const swDest = tempy.file(); + const globDirectory = tempy.directory(); + + await fse.ensureSymlink(GLOB_DIR, path.join(globDirectory, 'link')); + + const options = Object.assign({}, BASE_OPTIONS, { + globDirectory, + globFollow: false, + swDest, + }); + + const {count, size} = await generateSW(options); + + expect(count).to.eql(4); + expect(size).to.eql(2352); + await validateServiceWorkerRuntime({swFile: swDest, expectedMethodCalls: { + importScripts: [[WORKBOX_SW_CDN_URL]], + suppressWarnings: [[]], + precacheAndRoute: [[[{ + url: 'link/index.html', + revision: '3883c45b119c9d7e9ad75a1b4a4672ac', + }, { + url: 'link/page-1.html', + revision: '544658ab25ee8762dc241e8b1c5ed96d', + }, { + url: 'link/page-2.html', + revision: 'a3a71ce0b9b43c459cf58bd37e911b74', + }, { + url: 'link/webpackEntry.js', + revision: 'd41d8cd98f00b204e9800998ecf8427e', + }], {}]], + }}); + }); }); describe(`[workbox-build] behavior with 'runtimeCaching'`, function() { diff --git a/test/workbox-build/node/entry-points/get-manifest.js b/test/workbox-build/node/entry-points/get-manifest.js index 87f87b400..5a2e43cc5 100644 --- a/test/workbox-build/node/entry-points/get-manifest.js +++ b/test/workbox-build/node/entry-points/get-manifest.js @@ -1,5 +1,7 @@ const expect = require('chai').expect; +const fse = require('fs-extra'); const path = require('path'); +const tempy = require('tempy'); const getManifest = require('../../../../packages/workbox-build/src/entry-points/get-manifest'); @@ -11,8 +13,10 @@ describe(`[workbox-build] entry-points/get-manifest.js (End to End)`, function() const REQUIRED_PARAMS = ['globDirectory']; const SUPPORTED_PARAMS = [ 'dontCacheBustUrlsMatching', + 'globFollow', 'globIgnores', 'globPatterns', + 'globStrict', 'manifestTransforms', 'maximumFileSizeToCacheInBytes', 'modifyUrlPrefix', @@ -259,5 +263,34 @@ describe(`[workbox-build] entry-points/get-manifest.js (End to End)`, function() expect(count).to.eql(2); expect(size).to.eql(50); }); + + it(`should use defaults when all the required parameters are present, with 'globFollow' and symlinks`, async function() { + const globDirectory = tempy.directory(); + + await fse.ensureSymlink(SRC_DIR, path.join(globDirectory, 'link')); + + const options = Object.assign({}, BASE_OPTIONS, { + globDirectory, + globFollow: false, + }); + + const {count, size, manifestEntries} = await getManifest(options); + + expect(manifestEntries).to.deep.equal([{ + url: 'link/index.html', + revision: '3883c45b119c9d7e9ad75a1b4a4672ac', + }, { + url: 'link/page-1.html', + revision: '544658ab25ee8762dc241e8b1c5ed96d', + }, { + url: 'link/page-2.html', + revision: 'a3a71ce0b9b43c459cf58bd37e911b74', + }, { + url: 'link/webpackEntry.js', + revision: 'd41d8cd98f00b204e9800998ecf8427e', + }]); + expect(count).to.eql(4); + expect(size).to.eql(2352); + }); }); }); diff --git a/test/workbox-build/node/entry-points/inject-manifest.js b/test/workbox-build/node/entry-points/inject-manifest.js index 1321131e4..1730c9fe4 100644 --- a/test/workbox-build/node/entry-points/inject-manifest.js +++ b/test/workbox-build/node/entry-points/inject-manifest.js @@ -21,8 +21,10 @@ describe(`[workbox-build] entry-points/inject-manifest.js (End to End)`, functio ]; const SUPPORTED_PARAMS = [ 'dontCacheBustUrlsMatching', + 'globFollow', 'globIgnores', 'globPatterns', + 'globStrict', 'injectionPointRegexp', 'manifestTransforms', 'maximumFileSizeToCacheInBytes', diff --git a/test/workbox-build/node/entry-points/options/validate.js b/test/workbox-build/node/entry-points/options/validate.js index 1ede5ed2f..099eb8527 100644 --- a/test/workbox-build/node/entry-points/options/validate.js +++ b/test/workbox-build/node/entry-points/options/validate.js @@ -24,8 +24,10 @@ describe(`[workbox-build] entry-points/options/validate.js`, function() { const options = validate({}, baseSchema); expect(options).to.eql({ + globFollow: true, globIgnores: ['node_modules/**/*'], globPatterns: ['**/*.{js,css,html}'], + globStrict: true, maximumFileSizeToCacheInBytes: 2097152, }); }); @@ -35,9 +37,11 @@ describe(`[workbox-build] entry-points/options/validate.js`, function() { const options = validate({maximumFileSizeToCacheInBytes}, baseSchema); expect(options).to.eql({ - maximumFileSizeToCacheInBytes, + globFollow: true, globIgnores: ['node_modules/**/*'], globPatterns: ['**/*.{js,css,html}'], + globStrict: true, + maximumFileSizeToCacheInBytes, }); }); @@ -47,8 +51,10 @@ describe(`[workbox-build] entry-points/options/validate.js`, function() { expect(options).to.eql({ dontCacheBustUrlsMatching, + globFollow: true, globIgnores: ['node_modules/**/*'], globPatterns: ['**/*.{js,css,html}'], + globStrict: true, maximumFileSizeToCacheInBytes: 2097152, }); }); diff --git a/test/workbox-build/node/lib/get-file-details.js b/test/workbox-build/node/lib/get-file-details.js index b0c1e7fd6..51899af9f 100644 --- a/test/workbox-build/node/lib/get-file-details.js +++ b/test/workbox-build/node/lib/get-file-details.js @@ -88,46 +88,4 @@ describe(`[workbox-build] lib/get-file-details.js`, function() { size: SIZE, }]); }); - - it(`should call sync with follow and strict options by default`, function() { - const getFileDetails = proxyquire(MODULE_PATH, { - 'glob': { - sync: (pattern, options) => { - expect(options.follow).to.be.true; - expect(options.strict).to.be.true; - - return [FILE1]; - }, - }, - './get-file-size': (value) => SIZE, - './get-file-hash': (value) => HASH, - }); - - getFileDetails({ - globDirectory: GLOB_DIRECTORY, - globPattern: GLOB_PATTERN, - }); - }); - - it(`should call sync with follow and strict options with value`, function() { - const getFileDetails = proxyquire(MODULE_PATH, { - 'glob': { - sync: (pattern, options) => { - expect(options.follow).to.be.false; - expect(options.strict).to.be.false; - - return [FILE1]; - }, - }, - './get-file-size': (value) => SIZE, - './get-file-hash': (value) => HASH, - }); - - getFileDetails({ - globDirectory: GLOB_DIRECTORY, - globPattern: GLOB_PATTERN, - globFollow: false, - globStrict: false, - }); - }); });