Moving some validation logic to be more clear about the flow

Author: weaverryan

lib/config-generator.js

@@ -276,7 +276,8 @@ class ConfigGenerator {
          */
         let manifestPrefix = this.webpackConfig.manifestKeyPrefix;
         if (null === manifestPrefix) {
-            manifestPrefix = pathUtil.generateManifestKeyPrefix(this.webpackConfig);
+            // by convention, we remove the opening slash on the manifest keys
+            manifestPrefix = this.webpackConfig.publicPath.replace(/^\//,'');
         }
         plugins.push(new ManifestPlugin({
             basePath: manifestPrefix,

lib/config/path-util.js

@@ -13,7 +13,7 @@ const path = require('path');
 
 module.exports = {
     /**
-     * Determines the "contentBase" used for the devServer.
+     * Determines the "contentBase" to use for the devServer.
      *
      * @param {WebpackConfig} webpackConfig
      * @return {String}
@@ -30,6 +30,10 @@ module.exports = {
          *   * outputPath = /var/www/public/build
          *   * publicPath = /build/
          *      => contentBase should be /var/www/public
+         *
+         * At this point, if the publicPath is non-standard (e.g. it contains
+         * a sub-directory or is absolute), then the user will already see
+         * an error that they must set the manifestKeyPrefix.
          */
 
         // start with outputPath, then join publicPath with it, see if it equals outputPath
@@ -44,7 +48,7 @@ module.exports = {
             }
         }
 
-        throw new Error(`Unable to determine contentBase option for webpack's devServer configuration. The publicPath (${webpackConfig.publicPath}) string does not exist in the outputPath (${webpackConfig.outputPath}), and so the "document root" cannot be determined.`);
+        throw new Error(`Unable to determine contentBase option for webpack's devServer configuration. The ${webpackConfig.manifestKeyPrefix ? 'manifestKeyPrefix' : 'publicPath'} (${webpackConfig.manifestKeyPrefix ? webpackConfig.manifestKeyPrefix : webpackConfig.publicPath}) string does not exist in the outputPath (${webpackConfig.outputPath}), and so the "document root" cannot be determined.`);
     },
 
     /**
@@ -64,9 +68,13 @@ module.exports = {
      * ok to use the publicPath as the manifestKeyPrefix.
      *
      * @param {WebpackConfig} webpackConfig
-     * @return {String}
      */
-    generateManifestKeyPrefix(webpackConfig) {
+    validatePublicPathAndManifestKeyPrefix(webpackConfig) {
+        if (webpackConfig.manifestKeyPrefix !== null) {
+            // nothing to check - they have manually set the key prefix
+            return;
+        }
+
         if (webpackConfig.publicPath.includes('://')) {
             /*
              * If publicPath is absolute, you probably don't want your manifests.json
@@ -103,8 +111,5 @@ module.exports = {
 
             throw new Error(`Cannot determine how to prefix the keys in manifest.json. Call Encore.setManifestKeyPrefix() to choose what path (e.g. ${suggestion}) to use when building your manifest keys. This is caused by setOutputPath() (${outputPath}) and setPublicPath() (${publicPath}) containing paths that don't seem compatible.`);
         }
-
-        // by convention, we remove the opening slash on the manifest keys
-        return webpackConfig.publicPath.replace(/^\//,'');
     }
 };

lib/config/validator.js

@@ -9,6 +9,8 @@
 
 'use strict';
 
+const pathUtil = require('./path-util');
+
 class Validator {
     /**
      * @param {WebpackConfig} webpackConfig
@@ -20,6 +22,8 @@ class Validator {
     validate() {
         this._validateBasic();
 
+        this._validatePublicPathAndManifestKeyPrefix();
+
         this._validateDevServer();
     }
 
@@ -37,6 +41,10 @@ class Validator {
         }
     }
 
+    _validatePublicPathAndManifestKeyPrefix() {
+        pathUtil.validatePublicPathAndManifestKeyPrefix(this.webpackConfig);
+    }
+
     _validateDevServer() {
         if (this.webpackConfig.useVersioning && this.webpackConfig.useDevServer()) {
             throw new Error('Don\'t enable versioning with the dev-server. A good setting is Encore.enableVersioning(Encore.isProduction()).');

test/config/path-util.js

@@ -89,15 +89,15 @@ describe('path-util getContentBase()', () => {
         });
     });
 
-    describe('generateManifestKeyPrefix', () => {
+    describe('validatePublicPathAndManifestKeyPrefix', () => {
         it('manifestKeyPrefix is correctly not required on windows', () => {
             const config = createConfig();
             config.outputPath = 'C:\\projects\\webpack-encore\\web\\build';
             config.setPublicPath('/build/');
             config.addEntry('main', './main');
 
-            const actualPrefix = pathUtil.generateManifestKeyPrefix(config);
-            expect(actualPrefix).to.equal('build/');
+            // NOT throwing an error is the assertion
+            pathUtil.validatePublicPathAndManifestKeyPrefix(config);
         });
 
         it('with absolute publicPath, manifestKeyPrefix must be set', () => {
@@ -108,7 +108,7 @@ describe('path-util getContentBase()', () => {
             config.setPublicPath('https://cdn.example.com');
 
             expect(() => {
-                pathUtil.generateManifestKeyPrefix(config);
+                pathUtil.validatePublicPathAndManifestKeyPrefix(config);
             }).to.throw('Cannot determine how to prefix the keys in manifest.json. Call Encore.setManifestKeyPrefix() to choose what path (e.g. build/) to use');
         });
 
@@ -120,7 +120,7 @@ describe('path-util getContentBase()', () => {
             config.setPublicPath('/subdirectory/build');
 
             expect(() => {
-                pathUtil.generateManifestKeyPrefix(config);
+                pathUtil.validatePublicPathAndManifestKeyPrefix(config);
             }).to.throw('Cannot determine how to prefix the keys in manifest.json. Call Encore.setManifestKeyPrefix() to choose what path (e.g. build/) to use');
         });
     });