bug #917 Re-working dev-server and https detection (weaverryan)

This PR was merged into the main branch.

Discussion
----------

Re-working dev-server and https detection

Hi!

Fixes #903

If you pass `--https` at the command line, that overrides your `devServer.https` config. In webpack-dev-server v3, that wasn't a problem. But in v4, the certificate config was moved under `devServer.https`, which can now be an object. This meant that if you passed `--https` at the command line (which Encore previously required) but also set `devServer.https = { ... }`, that would be "re-set" back to `devServer.https = true` and your config would be lost.

The fix is to not require the `--https` flag and look at it or the user's config to determine if the dev-server is running in https.

Cheers!

Commits
-------

ee417f0 Re-working dev-server and https detection

lib/WebpackConfig.js

@@ -15,6 +15,7 @@ const crypto = require('crypto');
 const RuntimeConfig = require('./config/RuntimeConfig'); //eslint-disable-line no-unused-vars
 const logger = require('./logger');
 const regexpEscaper = require('./utils/regexp-escaper');
+const { calculateDevServerUrl } = require('./config/path-util');
 
 /**
  * @param {RuntimeConfig|null} runtimeConfig
@@ -338,8 +339,10 @@ class WebpackConfig {
             return this.publicPath;
         }
 
+        const devServerUrl = calculateDevServerUrl(this.runtimeConfig);
+
         // if using dev-server, prefix the publicPath with the dev server URL
-        return this.runtimeConfig.devServerUrl.replace(/\/$/,'') + this.publicPath;
+        return devServerUrl.replace(/\/$/,'') + this.publicPath;
     }
 
     addEntry(name, src) {
@@ -991,10 +994,6 @@ class WebpackConfig {
         return this.runtimeConfig.useDevServer;
     }
 
-    useDevServerInHttps() {
-        return this.runtimeConfig.devServerHttps;
-    }
-
     isProduction() {
         return this.runtimeConfig.environment === 'production';
     }

lib/config-generator.js

@@ -57,6 +57,20 @@ class ConfigGenerator {
     }
 
     getWebpackConfig() {
+        const devServerConfig = this.webpackConfig.useDevServer() ? this.buildDevServerConfig() : null;
+        /*
+         * An unfortunate situation where we need to configure the final runtime
+         * config later in the process. The problem is that devServer https can
+         * be activated with either a --https flag or by setting the devServer.https
+         * config to an object or true. So, only at this moment can we determine
+         * if https has been activated by either method.
+         */
+        if (this.webpackConfig.useDevServer() && (devServerConfig.https || this.webpackConfig.runtimeConfig.devServerHttps)) {
+            this.webpackConfig.runtimeConfig.devServerFinalIsHttps = true;
+        } else {
+            this.webpackConfig.runtimeConfig.devServerFinalIsHttps = false;
+        }
+
         const config = {
             context: this.webpackConfig.getContext(),
             entry: this.buildEntryConfig(),
@@ -85,8 +99,8 @@ class ConfigGenerator {
             }
         }
 
-        if (this.webpackConfig.useDevServer()) {
-            config.devServer = this.buildDevServerConfig();
+        if (null !== devServerConfig) {
+            config.devServer = devServerConfig;
         }
 
         config.performance = {
@@ -571,14 +585,11 @@ class ConfigGenerator {
         const devServerOptions = {
             static: {
                 directory: contentBase,
-                // this doesn't appear to be necessary, but here in case
-                publicPath: this.webpackConfig.getRealPublicPath(),
             },
             // avoid CORS concerns trying to load things like fonts from the dev server
             headers: { 'Access-Control-Allow-Origin': '*' },
             compress: true,
             historyApiFallback: true,
-            https: this.webpackConfig.useDevServerInHttps()
         };
 
         return applyOptionsCallback(

lib/config/RuntimeConfig.js

@@ -17,8 +17,12 @@ class RuntimeConfig {
         this.environment = process.env.NODE_ENV ? process.env.NODE_ENV : 'dev';
 
         this.useDevServer = false;
-        this.devServerUrl = null;
         this.devServerHttps = null;
+        // see config-generator - getWebpackConfig()
+        this.devServerFinalIsHttps = null;
+        this.devServerHost = null;
+        this.devServerPort = null;
+        this.devServerPublic = null;
         this.devServerKeepPublicPath = false;
         this.outputJson = false;
         this.profile = false;

lib/config/parse-runtime.js

@@ -45,19 +45,12 @@ module.exports = function(argv, cwd) {
             runtimeConfig.devServerKeepPublicPath = argv.keepPublicPath || false;
 
             if (typeof argv.public === 'string') {
-                if (argv.public.includes('://')) {
-                    runtimeConfig.devServerUrl = argv.public;
-                } else if (runtimeConfig.devServerHttps) {
-                    runtimeConfig.devServerUrl = `https://${argv.public}`;
-                } else {
-                    runtimeConfig.devServerUrl = `http://${argv.public}`;
-                }
-            } else {
-                var host = argv.host ? argv.host : 'localhost';
-                var port = argv.port ? argv.port : '8080';
-                runtimeConfig.devServerUrl = `http${runtimeConfig.devServerHttps ? 's' : ''}://${host}:${port}/`;
+                runtimeConfig.devServerPublic = argv.public;
             }
 
+            runtimeConfig.devServerHost = argv.host ? argv.host : 'localhost';
+            runtimeConfig.devServerPort = argv.port ? argv.port : '8080';
+
             break;
     }
 

lib/config/path-util.js

@@ -11,6 +11,8 @@
 
 const path = require('path');
 const WebpackConfig = require('../WebpackConfig'); //eslint-disable-line no-unused-vars
+const RuntimeConfig = require('./RuntimeConfig'); //eslint-disable-line no-unused-vars
+const logger = require('../logger');
 
 module.exports = {
     /**
@@ -114,5 +116,29 @@ 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.`);
         }
+    },
+
+    /**
+     * @param {RuntimeConfig} runtimeConfig
+     * @return {string|null|Object.public|*}
+     */
+    calculateDevServerUrl(runtimeConfig) {
+        if (runtimeConfig.devServerFinalIsHttps === null) {
+            logger.warning('The final devServerFinalHttpsConfig was never calculated. This may cause some paths to incorrectly use or not use https and could be a bug.');
+        }
+
+        if (runtimeConfig.devServerPublic) {
+            if (runtimeConfig.devServerPublic.includes('://')) {
+                return runtimeConfig.devServerPublic;
+            }
+
+            if (runtimeConfig.devServerFinalIsHttps) {
+                return `https://${runtimeConfig.devServerPublic}`;
+            }
+
+            return `http://${runtimeConfig.devServerPublic}`;
+        }
+
+        return `http${runtimeConfig.devServerFinalIsHttps ? 's' : ''}://${runtimeConfig.devServerHost}:${runtimeConfig.devServerPort}`;
     }
 };

lib/config/validator.js

@@ -69,7 +69,7 @@ class Validator {
          * (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}).`);
+            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.`);
         }
     }
 

test/WebpackConfig.js

@@ -158,7 +158,9 @@ describe('WebpackConfig object', () => {
         it('Prefix when using devServer', () => {
             const config = createConfig();
             config.runtimeConfig.useDevServer = true;
-            config.runtimeConfig.devServerUrl = 'http://localhost:8080/';
+            config.runtimeConfig.devServerHost = 'localhost';
+            config.runtimeConfig.devServerPort = 8080;
+            config.runtimeConfig.devServerFinalIsHttps = false;
             config.setPublicPath('/public');
 
             expect(config.getRealPublicPath()).to.equal('http://localhost:8080/public/');
@@ -167,7 +169,9 @@ describe('WebpackConfig object', () => {
         it('No prefix with devServer & devServerKeepPublicPath option', () => {
             const config = createConfig();
             config.runtimeConfig.useDevServer = true;
-            config.runtimeConfig.devServerUrl = 'http://localhost:8080/';
+            config.runtimeConfig.devServerHost = 'localhost';
+            config.runtimeConfig.devServerPort = 8080;
+            config.runtimeConfig.devServerFinalIsHttps = false;
             config.runtimeConfig.devServerKeepPublicPath = true;
             config.setPublicPath('/public');
 
@@ -177,7 +181,9 @@ describe('WebpackConfig object', () => {
         it('devServer does not prefix if publicPath is absolute', () => {
             const config = createConfig();
             config.runtimeConfig.useDevServer = true;
-            config.runtimeConfig.devServerUrl = 'http://localhost:8080/';
+            config.runtimeConfig.devServerHost = 'localhost';
+            config.runtimeConfig.devServerPort = 8080;
+            config.runtimeConfig.devServerFinalIsHttps = false;
             config.setPublicPath('http://coolcdn.com/public');
             config.setManifestKeyPrefix('/public/');
 

test/config-generator.js

@@ -616,11 +616,42 @@ describe('The config-generator function', () => {
         it('devServer with custom options', () => {
             const config = createConfig();
             config.runtimeConfig.useDevServer = true;
-            config.runtimeConfig.devServerUrl = 'http://localhost:8080/';
+            config.runtimeConfig.devServerPort = 9090;
             config.outputPath = isWindows ? 'C:\\tmp\\public' : '/tmp/public';
             config.setPublicPath('/');
             config.addEntry('main', './main');
 
+            const actualConfig = configGenerator(config);
+
+            expect(actualConfig.devServer).to.containSubset({
+                static: {
+                    directory: isWindows ? 'C:\\tmp\\public' : '/tmp/public',
+                },
+            });
+
+            // this should be set when running the config generator
+            expect(config.runtimeConfig.devServerFinalIsHttps).is.false;
+        });
+
+        it('devServer enabled only at the command line', () => {
+            const config = createConfig();
+            config.runtimeConfig.useDevServer = true;
+            config.runtimeConfig.devServerHttps = true;
+            config.outputPath = isWindows ? 'C:\\tmp\\public' : '/tmp/public';
+            config.setPublicPath('/');
+            config.addEntry('main', './main');
+
+            configGenerator(config);
+            // this should be set when running the config generator
+            expect(config.runtimeConfig.devServerFinalIsHttps).is.true;
+        });
+
+        it('devServer enabled only via config', () => {
+            const config = createConfig();
+            config.runtimeConfig.useDevServer = true;
+            config.outputPath = isWindows ? 'C:\\tmp\\public' : '/tmp/public';
+            config.setPublicPath('/');
+            config.addEntry('main', './main');
             config.configureDevServerOptions(options => {
                 options.https = {
                     key: 'https.key',
@@ -636,6 +667,9 @@ describe('The config-generator function', () => {
                     cert: 'https.cert',
                 },
             });
+
+            // this should be set when running the config generator
+            expect(config.runtimeConfig.devServerFinalIsHttps).is.true;
         });
     });
 

test/config/parse-runtime.js

@@ -67,25 +67,37 @@ describe('parse-runtime', () => {
 
         expect(config.environment).to.equal('dev');
         expect(config.useDevServer).to.be.true;
-        expect(config.devServerUrl).to.equal('http://localhost:8080/');
+        expect(config.devServerHost).to.equal('localhost');
+        expect(config.devServerPort).to.equal('8080');
         expect(config.devServerKeepPublicPath).to.be.false;
+        expect(config.devServerPublic).to.be.null;
     });
 
     it('dev-server command with options', () => {
         const testDir = createTestDirectory();
         const config = parseArgv(createArgv(['dev-server', '--bar', '--host', 'foohost.l', '--port', '9999']), testDir);
 
         expect(config.environment).to.equal('dev');
-        expect(config.useDevServer).to.be.true;
-        expect(config.devServerUrl).to.equal('http://foohost.l:9999/');
+        expect(config.devServerHost).to.equal('foohost.l');
+        expect(config.devServerPort).to.equal(9999);
+        expect(config.devServerHttps).to.be.undefined;
     });
 
     it('dev-server command https', () => {
         const testDir = createTestDirectory();
         const config = parseArgv(createArgv(['dev-server', '--https', '--host', 'foohost.l', '--port', '9999']), testDir);
 
         expect(config.useDevServer).to.be.true;
-        expect(config.devServerUrl).to.equal('https://foohost.l:9999/');
+        expect(config.devServerHost).to.equal('foohost.l');
+        expect(config.devServerPort).to.equal(9999);
+        expect(config.devServerHttps).to.equal(true);
+    });
+
+    it('dev-server command public', () => {
+        const testDir = createTestDirectory();
+        const config = parseArgv(createArgv(['dev-server', '--public', 'https://my-domain:8080']), testDir);
+
+        expect(config.devServerPublic).to.equal('https://my-domain:8080');
     });
 
     it('--context is parsed correctly', () => {

test/config/path-util.js

@@ -31,7 +31,6 @@ describe('path-util getContentBase()', () => {
         it('contentBase is calculated correctly', function() {
             const config = createConfig();
             config.runtimeConfig.useDevServer = true;
-            config.runtimeConfig.devServerUrl = 'http://localhost:8080/';
             config.outputPath = isWindows ? 'C:\\tmp\\public\\build' : '/tmp/public/build';
             config.setPublicPath('/build/');
             config.addEntry('main', './main');
@@ -45,7 +44,6 @@ describe('path-util getContentBase()', () => {
         it('contentBase works ok with manifestKeyPrefix', function() {
             const config = createConfig();
             config.runtimeConfig.useDevServer = true;
-            config.runtimeConfig.devServerUrl = 'http://localhost:8080/';
             config.outputPath = isWindows ? 'C:\\tmp\\public\\build' : '/tmp/public/build';
             config.setPublicPath('/subdirectory/build');
             // this "fixes" the incompatibility between outputPath and publicPath
@@ -59,7 +57,6 @@ describe('path-util getContentBase()', () => {
         it('contentBase is calculated correctly with no public path', function() {
             const config = createConfig();
             config.runtimeConfig.useDevServer = true;
-            config.runtimeConfig.devServerUrl = 'http://localhost:8080/';
             config.outputPath = isWindows ? 'C:\\tmp\\public' : '/tmp/public';
             config.setPublicPath('/');
             config.addEntry('main', './main');
@@ -123,4 +120,50 @@ describe('path-util getContentBase()', () => {
             expect(actualPath).to.equal(isWindows ? 'public\\build' : 'public/build');
         });
     });
+
+    describe('calculateDevServerUrl', () => {
+        it('no https, no public', function() {
+            const runtimeConfig = new RuntimeConfig();
+            runtimeConfig.devServerFinalIsHttps = false;
+            runtimeConfig.devServerPublic = false;
+            runtimeConfig.devServerHost = 'localhost';
+            runtimeConfig.devServerPort = '8080';
+
+            expect(pathUtil.calculateDevServerUrl(runtimeConfig)).to.equal('http://localhost:8080');
+        });
+
+        it('yes https, no public', function() {
+            const runtimeConfig = new RuntimeConfig();
+            runtimeConfig.devServerFinalIsHttps = true;
+            runtimeConfig.devServerPublic = false;
+            runtimeConfig.devServerHost = 'localhost';
+            runtimeConfig.devServerPort = '8080';
+
+            expect(pathUtil.calculateDevServerUrl(runtimeConfig)).to.equal('https://localhost:8080');
+        });
+
+        it('no https, yes public not absolute', function() {
+            const runtimeConfig = new RuntimeConfig();
+            runtimeConfig.devServerFinalIsHttps = false;
+            runtimeConfig.devServerPublic = 'myhost.local:9090';
+
+            expect(pathUtil.calculateDevServerUrl(runtimeConfig)).to.equal('http://myhost.local:9090');
+        });
+
+        it('yes https, yes public not absolute', function() {
+            const runtimeConfig = new RuntimeConfig();
+            runtimeConfig.devServerFinalIsHttps = true;
+            runtimeConfig.devServerPublic = 'myhost.local:9090';
+
+            expect(pathUtil.calculateDevServerUrl(runtimeConfig)).to.equal('https://myhost.local:9090');
+        });
+
+        it('yes public and is absolute', function() {
+            const runtimeConfig = new RuntimeConfig();
+            runtimeConfig.devServerFinalIsHttps = false;
+            runtimeConfig.devServerPublic = 'https://myhost.local:9090';
+
+            expect(pathUtil.calculateDevServerUrl(runtimeConfig)).to.equal('https://myhost.local:9090');
+        });
+    });
 });