Allowing an absolute publicPath with dev-server, but showing a warning

symfony · Jul 21, 2017 · 92e22af · 92e22af
1 parent 830fdb5
commit 92e22af
Show file tree

Hide file tree

Showing 10 changed files with 122 additions and 36 deletions.
diff --git a/bin/encore.js b/bin/encore.js
@@ -10,7 +10,6 @@
 
 'use strict';
 
-const path = require('path');
 const parseRuntime = require('../lib/config/parse-runtime');
 const context = require('../lib/context');
 const chalk = require('chalk');

diff --git a/index.js b/index.js
@@ -14,13 +14,17 @@ const configGenerator = require('./lib/config-generator');
 const validator = require('./lib/config/validator');
 const PrettyError = require('pretty-error');
 const runtimeConfig = require('./lib/context').runtimeConfig;
+const logger = require('./lib/logger');
 
 // at this time, the encore executable should have set the runtimeConfig
 if (!runtimeConfig) {
     throw new Error('Are you trying to require index.js directly?');
 }
 
 let webpackConfig = new WebpackConfig(runtimeConfig);
+if (runtimeConfig.verbose) {
+    logger.verbose();
+}
 
 module.exports = {
     /**

diff --git a/lib/WebpackConfig.js b/lib/WebpackConfig.js
@@ -86,23 +86,11 @@ class WebpackConfig {
     }
 
     setPublicPath(publicPath) {
-        /*
-         * Do not allow absolute URLs *and* the webpackDevServer
-         * to be used at the same time. The webpackDevServer basically
-         * provides the publicPath (and so in those cases, publicPath)
-         * is simply used as the default manifestKeyPrefix.
-         */
-        if (publicPath.includes('://')) {
-            if (this.useDevServer() && false === this.runtimeConfig.devServerKeepPublicPath) {
-                throw new Error('You cannot pass an absolute URL to setPublicPath() and use the dev-server at the same time. This is because the public path is automatically set to point to the dev server. Try using Encore.isProduction() to only configure your absolute publicPath for production. Or, if you want to override this behavior, pass the --keep-public-path option to allow this.');
-            }
-        } else {
-            if (publicPath.indexOf('/') !== 0) {
-                // technically, not starting with "/" is legal, but not
-                // what you want in most cases. Let's not let the user make
-                // a mistake (and we can always change this later).
-                throw new Error('The value passed to setPublicPath() must start with "/" or be a full URL (http://...)');
-            }
+        if (publicPath.includes('://') === false && publicPath.indexOf('/') !== 0) {
+            // technically, not starting with "/" is legal, but not
+            // what you want in most cases. Let's not let the user make
+            // a mistake (and we can always change this later).
+            throw new Error('The value passed to setPublicPath() must start with "/" or be a full URL (http://...)');
         }
 
         // guarantee a single trailing slash
@@ -129,13 +117,20 @@ class WebpackConfig {
      * @returns {string}
      */
     getRealPublicPath() {
-        // if we're using webpack-dev-server, use it & add the publicPath
-        if (this.useDevServer() && false === this.runtimeConfig.devServerKeepPublicPath) {
-            // avoid 2 middle slashes
-            return this.runtimeConfig.devServerUrl.replace(/\/$/,'') + this.publicPath;
+        if (!this.useDevServer()) {
+            return this.publicPath;
+        }
+
+        if (this.runtimeConfig.devServerKeepPublicPath) {
+            return this.publicPath;
+        }
+
+        if (this.publicPath.includes('://')) {
+            return this.publicPath;
         }
 
-        return this.publicPath;
+        // if using dev-server, prefix the publicPath with the dev server URL
+        return this.runtimeConfig.devServerUrl.replace(/\/$/,'') + this.publicPath;
     }
 
     addEntry(name, src) {

diff --git a/lib/config-generator.js b/lib/config-generator.js
@@ -384,8 +384,8 @@ class ConfigGenerator {
             publicPath: this.webpackConfig.publicPath,
             // avoid CORS concerns trying to load things like fonts from the dev server
             headers: { 'Access-Control-Allow-Origin': '*' },
-            // required by FriendlyErrorsWebpackPlugin
             hot: this.webpackConfig.useHotModuleReplacementPlugin(),
+            // required by FriendlyErrorsWebpackPlugin
             quiet: true,
             compress: true,
             historyApiFallback: true,

diff --git a/lib/config/RuntimeConfig.js b/lib/config/RuntimeConfig.js
@@ -25,6 +25,7 @@ class RuntimeConfig {
         this.babelRcFileExists = null;
 
         this.helpRequested = false;
+        this.verbose = false;
     }
 }
 

diff --git a/lib/config/parse-runtime.js b/lib/config/parse-runtime.js
@@ -29,14 +29,18 @@ module.exports = function(argv, cwd) {
         case 'dev':
             runtimeConfig.isValidCommand = true;
             runtimeConfig.environment = 'dev';
+            runtimeConfig.verbose = true;
             break;
         case 'production':
             runtimeConfig.isValidCommand = true;
             runtimeConfig.environment = 'production';
+            runtimeConfig.verbose = false;
             break;
         case 'dev-server':
             runtimeConfig.isValidCommand = true;
             runtimeConfig.environment = 'dev';
+            runtimeConfig.verbose = true;
+
             runtimeConfig.useDevServer = true;
             runtimeConfig.devServerHttps = argv.https;
             runtimeConfig.useHotModuleReplacement = argv.hot || false;

diff --git a/lib/config/validator.js b/lib/config/validator.js
@@ -10,6 +10,7 @@
 'use strict';
 
 const pathUtil = require('./path-util');
+const logger = require('./../logger');
 
 class Validator {
     /**
@@ -46,9 +47,24 @@ class Validator {
     }
 
     _validateDevServer() {
-        if (this.webpackConfig.useVersioning && this.webpackConfig.useDevServer()) {
+        if (!this.webpackConfig.useDevServer()) {
+            return;
+        }
+
+        if (this.webpackConfig.useVersioning) {
             throw new Error('Don\'t enable versioning with the dev-server. A good setting is Encore.enableVersioning(Encore.isProduction()).');
         }
+
+        /*
+         * An absolute publicPath is incompatible with webpackDevServer.
+         * This is because we want to *change* the publicPath to point
+         * to the webpackDevServer URL (e.g. http://localhost:8080/).
+         * There are some valid use-cases for not wanting this behavior
+         * (see #59), but we want to warn the user.
+         */
+        if (this.webpackConfig.publicPath.includes('://')) {
+            logger.warning(`Passing an absolute URL to setPublicPath() *and* using the dev-server can cause issues. Your assets will load from the publicPath (${this.webpackConfig.publicPath}) instead of from the dev server URL (${this.webpackConfig.runtimeConfig.devServerUrl}).`);
+        }
     }
 }
 

diff --git a/lib/logger.js b/lib/logger.js
@@ -0,0 +1,59 @@
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+'use strict';
+
+const chalk = require('chalk');
+let isVerbose = false;
+let quiet = false;
+let messages = {
+    debug: [],
+    warning: [],
+};
+
+function log(message) {
+    if (quiet) {
+        return;
+    }
+
+    console.log(message);
+}
+
+module.exports = {
+    debug(message) {
+        messages.debug.push(message);
+
+        if (isVerbose) {
+            log(`${chalk.bgBlack.white(' DEBUG ')} ${message}`);
+        }
+    },
+
+    warning(message) {
+        messages.warning.push(message);
+
+        log(`${chalk.bgYellow.black(' WARNING ')} ${chalk.yellow(message)}`);
+    },
+
+    clearMessages() {
+        messages.debug = [];
+        messages.warning = [];
+    },
+
+    getMessages() {
+        return messages;
+    },
+
+    quiet() {
+        quiet = true;
+    },
+
+    verbose() {
+        isVerbose = true;
+    }
+};
diff --git a/test/WebpackConfig.js b/test/WebpackConfig.js
@@ -104,15 +104,6 @@ describe('WebpackConfig object', () => {
                 config.setPublicPath('foo/');
             }).to.throw('The value passed to setPublicPath() must start with "/"');
         });
-
-        it('Setting to a URL when using devServer throws an error', () => {
-            const config = createConfig();
-            config.runtimeConfig.useDevServer = true;
-
-            expect(() => {
-                config.setPublicPath('https://examplecdn.com');
-            }).to.throw('You cannot pass an absolute URL to setPublicPath() and use the dev-server');
-        });
     });
 
     describe('getRealPublicPath', () => {
@@ -142,11 +133,10 @@ describe('WebpackConfig object', () => {
             expect(config.getRealPublicPath()).to.equal('/public/');
         });
 
-        it('devServer & devServerKeepPublicPath option allows absolute publicPath', () => {
+        it('devServer does not prefix if publicPath is absolute', () => {
             const config = createConfig();
             config.runtimeConfig.useDevServer = true;
             config.runtimeConfig.devServerUrl = 'http://localhost:8080/';
-            config.runtimeConfig.devServerKeepPublicPath = true;
             config.setPublicPath('http://coolcdn.com/public');
             config.setManifestKeyPrefix('/public/');
 

diff --git a/test/config/validator.js b/test/config/validator.js
@@ -13,6 +13,9 @@ const expect = require('chai').expect;
 const WebpackConfig = require('../../lib/WebpackConfig');
 const RuntimeConfig = require('../../lib/config/RuntimeConfig');
 const validator = require('../../lib/config/validator');
+const logger = require('../../lib/logger');
+
+logger.quiet();
 
 function createConfig() {
     const runtimeConfig = new RuntimeConfig();
@@ -65,4 +68,19 @@ describe('The validator function', () => {
             validator(config);
         }).to.throw('Don\'t enable versioning with the dev-server');
     });
+
+    it('warning with dev-server and absolute publicPath', () => {
+        const config = createConfig();
+        config.outputPath = '/tmp/public/build';
+        config.setPublicPath('https://absoluteurl.com/build');
+        config.setManifestKeyPrefix('build/');
+        config.addEntry('main', './main');
+        config.runtimeConfig.useDevServer = true;
+
+        logger.clearMessages();
+        validator(config);
+
+        expect(logger.getMessages().warning).to.have.lengthOf(1);
+        expect(logger.getMessages().warning[0]).to.include('Passing an absolute URL to setPublicPath() *and* using the dev-server can cause issues');
+    });
 });