fix: fixes #86, client port bungle

lib/options.js

@@ -64,11 +64,11 @@ module.exports = (opts = {}) => {
       client: options.port,
       server: options.port,
     };
-  } else if (!options.port.server) {
+  } else if (isNaN(parseInt(options.port.server, 10))) {
     throw new HotClientError(
       '`port.server` must be defined when setting host to an Object'
     );
-  } else if (!options.port.client) {
+  } else if (isNaN(parseInt(options.port.client, 10))) {
     throw new HotClientError(
       '`port.client` must be defined when setting host to an Object'
     );
@@ -87,7 +87,8 @@ module.exports = (opts = {}) => {
   if (server && server.listening) {
     options.webSocket = {
       host: server.address().address,
-      port: server.address().port,
+      // a port.client value of 0 will be falsy, so it should pull the server port
+      port: options.port.client || server.address().port,
     };
   } else {
     options.webSocket = {

lib/socket-server.js

@@ -104,7 +104,8 @@ function onListening(server, options) {
       const { address, port } = server._server.address();
       server.host = address;
       server.port = port;
-      options.webSocket.port = port;
+      // a port.client value of 0 will be falsy, so it should pull the server port
+      options.webSocket.port = options.port.client || port;
 
       log.info(`WebSocket Server Listening on ${host.server}:${port}`);
     });

test/__snapshots__/index.test.js.snap

@@ -0,0 +1,164 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`api mismatched client/server options sanity check 1`] = `
+Object {
+  "host": "0.0.0.0",
+  "port": 7000,
+}
+`;
+
+exports[`api mismatched client/server options sanity check 2`] = `
+Object {
+  "allEntries": false,
+  "autoConfigure": true,
+  "hmr": true,
+  "host": Object {
+    "client": "localhost",
+    "server": "0.0.0.0",
+  },
+  "https": undefined,
+  "log": LogLevel {
+    "currentLevel": 5,
+    "debug": [Function],
+    "error": [Function],
+    "info": [Function],
+    "log": [Function],
+    "methodFactory": PrefixFactory {
+      "options": Object {
+        "level": [Function],
+        "name": [Function],
+        "template": "{{level}} ｢{{name}}｣: ",
+        "time": [Function],
+      },
+      Symbol(a log instance): [Circular],
+      Symbol(valid log levels): Object {
+        "DEBUG": 1,
+        "ERROR": 4,
+        "INFO": 2,
+        "SILENT": 5,
+        "TRACE": 0,
+        "WARN": 3,
+      },
+    },
+    "name": "hot",
+    "options": Object {
+      "factory": null,
+      "level": "silent",
+      "name": "hot",
+      "prefix": Object {
+        "level": [Function],
+        "template": "{{level}} ｢{{name}}｣: ",
+      },
+      "timestamp": false,
+      "unique": true,
+    },
+    "trace": [Function],
+    "type": "LogLevel",
+    "warn": [Function],
+  },
+  "logLevel": "silent",
+  "logTime": false,
+  "port": Object {
+    "client": 6000,
+    "server": 7000,
+  },
+  "reload": true,
+  "send": Object {
+    "errors": true,
+    "warnings": true,
+  },
+  "server": null,
+  "stats": Object {
+    "context": "<PROJECT_ROOT>",
+  },
+  "test": false,
+  "validTargets": Array [
+    "web",
+  ],
+  "webSocket": Object {
+    "host": "localhost",
+    "port": 6000,
+  },
+}
+`;
+
+exports[`api options sanity check 1`] = `
+Object {
+  "host": "127.0.0.1",
+  "port": Any<Number>,
+}
+`;
+
+exports[`api options sanity check 2`] = `
+Object {
+  "allEntries": false,
+  "autoConfigure": true,
+  "hmr": true,
+  "host": Object {
+    "client": "localhost",
+    "server": "localhost",
+  },
+  "https": undefined,
+  "log": LogLevel {
+    "currentLevel": 5,
+    "debug": [Function],
+    "error": [Function],
+    "info": [Function],
+    "log": [Function],
+    "methodFactory": PrefixFactory {
+      "options": Object {
+        "level": [Function],
+        "name": [Function],
+        "template": "{{level}} ｢{{name}}｣: ",
+        "time": [Function],
+      },
+      Symbol(a log instance): [Circular],
+      Symbol(valid log levels): Object {
+        "DEBUG": 1,
+        "ERROR": 4,
+        "INFO": 2,
+        "SILENT": 5,
+        "TRACE": 0,
+        "WARN": 3,
+      },
+    },
+    "name": "hot",
+    "options": Object {
+      "factory": null,
+      "level": "silent",
+      "name": "hot",
+      "prefix": Object {
+        "level": [Function],
+        "template": "{{level}} ｢{{name}}｣: ",
+      },
+      "timestamp": false,
+      "unique": true,
+    },
+    "trace": [Function],
+    "type": "LogLevel",
+    "warn": [Function],
+  },
+  "logLevel": "silent",
+  "logTime": false,
+  "port": Object {
+    "client": 0,
+    "server": 0,
+  },
+  "reload": true,
+  "send": Object {
+    "errors": true,
+    "warnings": true,
+  },
+  "server": null,
+  "stats": Object {
+    "context": "<PROJECT_ROOT>",
+  },
+  "test": false,
+  "validTargets": Array [
+    "web",
+  ],
+  "webSocket": Object {
+    "port": Any<Number>,
+  },
+}
+`;

test/__snapshots__/options.test.js.snap

@@ -300,3 +300,7 @@ Object {
   },
 }
 `;
+
+exports[`options throws if port.client is missing 1`] = `"webpack-hot-client: \`port.client\` must be defined when setting host to an Object"`;
+
+exports[`options throws if port.server is missing 1`] = `"webpack-hot-client: \`port.server\` must be defined when setting host to an Object"`;

test/fixtures/app.js

@@ -21,3 +21,14 @@ console.log('dirty');
 console.log('dirty');
 console.log('dirty');
 console.log('dirty');
+
+console.log('dirty');
+console.log('dirty');
+console.log('dirty');
+console.log('dirty');
+console.log('dirty');
+console.log('dirty');
+
+console.log('dirty');
+console.log('dirty');
+console.log('dirty');

test/index.test.js

@@ -113,4 +113,49 @@ describe('api', () => {
       });
     });
   });
+
+  test('options sanity check', (done) => {
+    const config = require('./fixtures/webpack.config-object.js');
+    const compiler = webpack(config);
+    const { options: opts, server } = client(compiler, options);
+
+    server.on('listening', () => {
+      const { host, port } = server;
+      expect({ host, port }).toMatchSnapshot({
+        port: expect.any(Number),
+      });
+      // this assign is necessary as Jest cannot manipulate a frozen object
+      expect(Object.assign({}, opts)).toMatchSnapshot({
+        webSocket: {
+          port: expect.any(Number),
+        },
+      });
+
+      setTimeout(() => server.close(done), 500);
+    });
+  });
+
+  test('mismatched client/server options sanity check', (done) => {
+    const config = require('./fixtures/webpack.config-object.js');
+    const compiler = webpack(config);
+    const clientOptions = Object.assign({}, options, {
+      host: {
+        client: 'localhost',
+        server: '0.0.0.0',
+      },
+      port: {
+        client: 6000,
+        server: 7000,
+      },
+    });
+    const { options: opts, server } = client(compiler, clientOptions);
+
+    server.on('listening', () => {
+      const { host, port } = server;
+      expect({ host, port }).toMatchSnapshot();
+      expect(opts).toMatchSnapshot();
+
+      setTimeout(() => server.close(done), 500);
+    });
+  });
 });

test/options.test.js

@@ -87,7 +87,7 @@ describe('options', () => {
     });
   });
 
-  test('throws on invalid options', () => {
+  test('throws on invalid host option', () => {
     const t = () => getOptions({ host: true });
     expect(t).toThrow();
   });
@@ -101,4 +101,14 @@ describe('options', () => {
     const t = () => getOptions({ host: { client: 'localhost' } });
     expect(t).toThrow();
   });
+
+  test('throws if port.client is missing', () => {
+    const t = () => getOptions({ port: { server: 0 } });
+    expect(t).toThrowErrorMatchingSnapshot();
+  });
+
+  test('throws if port.server is missing', () => {
+    const t = () => getOptions({ port: { client: 9000 } });
+    expect(t).toThrowErrorMatchingSnapshot();
+  });
 });