allow explicitly setting the protocol from the public option (#1117)

robertaistleitner · shellscape · commit c490b245ad65 · 2017-10-08T19:53:09.000-04:00
* allow explicitly setting the protocol from the public option

* add feedback from PR + add example

 + found one other glitch that auto-reload doesn't work in iframes since it tries to reload the iframe itself (which has src `about:blank`) and therefore cannot be reloaded properly. I'm now checking for protocol `about:` and traverse upwards until a reloadable window can be found. Feedback on this fix is welcome!

* fix linting issues

* provide tests for createDomain helper function

 + found a bug due to tests: `url.parse` doesn't properly parse a provided `public` option since it counts everything before the port colon as the protocol which is just wrong. I replaced it with a protocol regex.

* increase timeout for https test which exceeds default 2000ms

* move info log to the place where the reload happens

* apply feedback

 * remove docker stuff from examples (minimize example)
 * use setInterval and clearInterval instead of do while loop when getting root window for reload

* increase test timeout for https to 60 seconds
diff --git a/client/index.js b/client/index.js
@@ -218,7 +218,25 @@ function reloadApp() {
       self.postMessage('webpackHotUpdate' + currentHash, '*');
     }
   } else {
+    let rootWindow = self;
+    // use parent window for reload (in case we're in an iframe with no valid src)
+    const intervalId = self.setInterval(function findRootWindow() {
+      if (rootWindow.location.protocol !== 'about:') {
+        // reload immediately if protocol is valid
+        applyReload(rootWindow, intervalId);
+      } else {
+        rootWindow = rootWindow.parent;
+        if (rootWindow.parent === rootWindow) {
+          // if parent equals current window we've reached the root which would continue forever, so trigger a reload anyways
+          applyReload(rootWindow, intervalId);
+        }
+      }
+    });
+  }
+
+  function applyReload(rootWindow, intervalId) {
+    clearInterval(intervalId);
     log.info('[WDS] App updated. Reloading...');
-    self.location.reload();
+    rootWindow.location.reload();
   }
 }
diff --git a/examples/cli-public-protocol/README.md b/examples/cli-public-protocol/README.md
@@ -0,0 +1,13 @@
+# CLI - public protocol
+
+NOTE: replace `<insert local ip>` with your local ip.
+
+```shell
+node ../../bin/webpack-dev-server.js
+```
+
+You're now able to explicitly define the protocol used with the `public` option (have a look to the config provided in `webpack.config.js`). 
+
+## What should happen
+
+If you open your browser at `http://localhost:8080` you'll see that dev-server tries to establish a connection to `/sockjs-node` via the explicitly defined `https://localhost:8080`. This fails of course since we're not hosting https.
diff --git a/examples/cli-public-protocol/app.js b/examples/cli-public-protocol/app.js
@@ -0,0 +1,5 @@
+'use strict';
+
+document.open();
+document.write('Please check the sockjs-info request in your dev-tools, it should try to connect to the protocol + server defined in the public setting.');
+document.close();
diff --git a/examples/cli-public-protocol/index.html b/examples/cli-public-protocol/index.html
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+	<head>
+		<script src="bundle.js" type="text/javascript" charset="utf-8"></script>
+	</head>
+	<body>
+	</body>
+</html>
diff --git a/examples/cli-public-protocol/webpack.config.js b/examples/cli-public-protocol/webpack.config.js
@@ -0,0 +1,11 @@
+'use strict';
+
+module.exports = {
+  context: __dirname,
+  entry: './app.js',
+  devServer: {
+    host: '0.0.0.0',
+    public: 'https://localhost:8080',
+    disableHostCheck: true
+  }
+};
diff --git a/lib/util/createDomain.js b/lib/util/createDomain.js
@@ -10,8 +10,12 @@ module.exports = function createDomain(options, listeningApp) {
   const port = options.socket ? 0 : appPort;
   const hostname = options.useLocalIp ? internalIp.v4() : options.host;
 
+  // use explicitly defined public url (prefix with protocol if not explicitly given)
+  if (options.public) {
+    return /^[a-zA-Z]+:\/\//.test(options.public) ? `${options.public}` : `${protocol}://${options.public}`;
+  }
   // the formatted domain (url without path) of the webpack server
-  return options.public ? `${protocol}://${options.public}` : url.format({
+  return url.format({
     protocol,
     hostname,
     port
diff --git a/test/Util.test.js b/test/Util.test.js
@@ -0,0 +1,94 @@
+'use strict';
+
+const webpack = require('webpack');
+const internalIp = require('internal-ip');
+const Server = require('../lib/Server');
+const createDomain = require('../lib/util/createDomain');
+const config = require('./fixtures/simple-config/webpack.config');
+
+describe('check utility funcitons', () => {
+  let compiler;
+  before(() => {
+    compiler = webpack(config);
+  });
+
+  const tests = [{
+    name: 'default',
+    options: {
+      host: 'localhost',
+      port: 8080
+    },
+    expected: 'http://localhost:8080'
+  }, {
+    name: 'https',
+    options: {
+      host: 'localhost',
+      port: 8080,
+      https: true
+    },
+    expected: 'https://localhost:8080',
+    timeout: 60000
+  }, {
+    name: 'override with public',
+    options: {
+      host: 'localhost',
+      port: 8080,
+      public: 'myhost.test'
+    },
+    expected: 'http://myhost.test'
+  }, {
+    name: 'override with public (port)',
+    options: {
+      host: 'localhost',
+      port: 8080,
+      public: 'myhost.test:9090'
+    },
+    expected: 'http://myhost.test:9090'
+  }, {
+    name: 'override with public (protocol)',
+    options: {
+      host: 'localhost',
+      port: 8080,
+      public: 'https://myhost.test'
+    },
+    expected: 'https://myhost.test'
+  }, {
+    name: 'override with public (protocol + port)',
+    options: {
+      host: 'localhost',
+      port: 8080,
+      public: 'https://myhost.test:9090'
+    },
+    expected: 'https://myhost.test:9090'
+  }, {
+    name: 'localIp',
+    options: {
+      useLocalIp: true,
+      port: 8080
+    },
+    expected: `http://${internalIp.v4()}:8080`
+  }];
+
+  tests.forEach((t) => {
+    const itInstance = it(`test createDomain '${t.name}'`, (done) => {
+      const options = t.options;
+      const server = new Server(compiler, options);
+      const expected = t.expected;
+      server.listen(options.port, options.host, (err) => {
+        if (err) {
+          done(err);
+        }
+        const generatedDomain = createDomain(options, server.listeningApp);
+        if (generatedDomain !== expected) {
+          done(`generated domain ${generatedDomain} doesn't match expected ${expected}`);
+        } else {
+          done();
+        }
+        server.close();
+      });
+    });
+    if (t.timeout) {
+      itInstance.timeout(t.timeout);
+    }
+  });
+});

-Original file line number
+Diff line change
@@ @@ -0,0 +1,8 @@ @@
 +<!DOCTYPE html>
 +<html>
 +	<head>
 +		<script src="bundle.js" type="text/javascript" charset="utf-8"></script>
 +	</head>
 +	<body>
 +	</body>
 +</html>