From 3bd53440b39a8e42cc14f8891e3d1ed8ed7a8b86 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 20 Jul 2017 21:02:58 -0400 Subject: [PATCH 1/2] Enforcing that the manifest keys NEVER start with "/" --- lib/WebpackConfig.js | 12 ++++++++++++ test/WebpackConfig.js | 12 ++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/lib/WebpackConfig.js b/lib/WebpackConfig.js index 706ec597..41e70605 100644 --- a/lib/WebpackConfig.js +++ b/lib/WebpackConfig.js @@ -107,6 +107,18 @@ class WebpackConfig { } setManifestKeyPrefix(manifestKeyPrefix) { + /* + * Normally, we make sure that the manifest keys don't start + * with an opening "/" ever... for consistency. If you need + * to manually specify the manifest key (e.g. because you're + * publicPath is absolute), we're *still* adding this restriction, + * to help avoid a user accidentally adding an opening "/" + * and changing their keys from what they looked like before. + */ + if (manifestKeyPrefix.indexOf('/') === 0) { + throw new Error(`For consistency, the value passed to setManifestKeyPrefix() cannot start with "/". Try setManifestKeyPrefix('${manifestKeyPrefix.replace(/^\//, '')}') instead.`); + } + // guarantee a single trailing slash, except for blank strings if (manifestKeyPrefix !== '') { manifestKeyPrefix = manifestKeyPrefix.replace(/\/$/, ''); diff --git a/test/WebpackConfig.js b/test/WebpackConfig.js index 8cfdccea..f8bec427 100644 --- a/test/WebpackConfig.js +++ b/test/WebpackConfig.js @@ -148,10 +148,10 @@ describe('WebpackConfig object', () => { it('You can set it!', () => { const config = createConfig(); - config.setManifestKeyPrefix('/foo'); + config.setManifestKeyPrefix('foo'); // trailing slash added - expect(config.manifestKeyPrefix).to.equal('/foo/'); + expect(config.manifestKeyPrefix).to.equal('foo/'); }); it('You can set it blank', () => { @@ -161,6 +161,14 @@ describe('WebpackConfig object', () => { // trailing slash not added expect(config.manifestKeyPrefix).to.equal(''); }); + + it('You cannot use an opening slash', () => { + const config = createConfig(); + + expect(() => { + config.setManifestKeyPrefix('/foo/'); + }).to.throw('the value passed to setManifestKeyPrefix() cannot start with "/"'); + }); }); describe('addEntry', () => { From baa70b1e690a6503787fb8631e1079c712dddac9 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Wed, 9 Aug 2017 21:21:31 -0400 Subject: [PATCH 2/2] softening to a warning --- lib/WebpackConfig.js | 9 +++++---- test/WebpackConfig.js | 10 ++++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/lib/WebpackConfig.js b/lib/WebpackConfig.js index 41e70605..dd3b7816 100644 --- a/lib/WebpackConfig.js +++ b/lib/WebpackConfig.js @@ -11,6 +11,7 @@ const path = require('path'); const fs = require('fs'); +const logger = require('./logger'); /** * @param {RuntimeConfig} runtimeConfig @@ -111,12 +112,12 @@ class WebpackConfig { * Normally, we make sure that the manifest keys don't start * with an opening "/" ever... for consistency. If you need * to manually specify the manifest key (e.g. because you're - * publicPath is absolute), we're *still* adding this restriction, - * to help avoid a user accidentally adding an opening "/" - * and changing their keys from what they looked like before. + * publicPath is absolute), it's easy to accidentally add + * an opening slash (thereby changing your key prefix) without + * intending to. Hence, the warning. */ if (manifestKeyPrefix.indexOf('/') === 0) { - throw new Error(`For consistency, the value passed to setManifestKeyPrefix() cannot start with "/". Try setManifestKeyPrefix('${manifestKeyPrefix.replace(/^\//, '')}') instead.`); + logger.warning(`The value passed to setManifestKeyPrefix "${manifestKeyPrefix}" starts with "/". This is allowed, but since the key prefix does not normally start with a "/", you may have just changed the prefix accidentally.`); } // guarantee a single trailing slash, except for blank strings diff --git a/test/WebpackConfig.js b/test/WebpackConfig.js index f8bec427..fcb0dad5 100644 --- a/test/WebpackConfig.js +++ b/test/WebpackConfig.js @@ -15,6 +15,7 @@ const RuntimeConfig = require('../lib/config/RuntimeConfig'); const path = require('path'); const fs = require('fs'); const webpack = require('webpack'); +const logger = require('../lib/logger'); function createConfig() { const runtimeConfig = new RuntimeConfig(); @@ -162,12 +163,13 @@ describe('WebpackConfig object', () => { expect(config.manifestKeyPrefix).to.equal(''); }); - it('You cannot use an opening slash', () => { + it('You can use an opening slash, but get a warning', () => { const config = createConfig(); - expect(() => { - config.setManifestKeyPrefix('/foo/'); - }).to.throw('the value passed to setManifestKeyPrefix() cannot start with "/"'); + logger.reset(); + logger.quiet(); + config.setManifestKeyPrefix('/foo/'); + expect(logger.getMessages().warning).to.have.lengthOf(1); }); });