Set addDevServerEntrypoints default to 'localhost'

[caryli]

What kind of change does this PR introduce?

Bug fix.

Did you add or update the examples/?

No.

Summary

The default host injected by addDevServerEntrypoints when using webpack-dev-server with the Node.js API is undefined instead of 'localhost' as specified at https://webpack.js.org/configuration/dev-server/#devserver-host where "by default this is localhost".

Reproducible from the code given up to the Hot Module Replacement step at the official guide at https://webpack.js.org/guides/getting-started/.

Does this PR introduce a breaking change?

No.

Other information

When using webpack-dev-server with the Node.js API in the following scenario:

1. using addDevServerEntrypoints to inject webpack-dev-server into the entry point in webpack.config.js instead of manually (i.e. entry: ['./app.js', '../../client/index.js?http://localhost:5000/'])

2. the options object passed as the argument to the devServerOptions parameter of addDevServerEntrypoints does not have a host property nor a useLocalIp property as in the below example used at https://webpack.js.org/guides/hot-module-replacement/#via-the-node-js-api:

   dev-server.js

   const webpackDevServer = require('webpack-dev-server');
   const webpack = require('webpack');
   
   const config = require('./webpack.config.js');
   const options = {
     contentBase: './dist',
     hot: true
   };
   
   webpackDevServer.addDevServerEntrypoints(config, options);
   

because of:

webpack-dev-server/lib/util/createDomain.js

11  const hostname = options.useLocalIp ? internalIp.v4() : options.host;

hostname will be set to options.host which is currently undefined.

This is not the case when using the webpack-dev-server CLI as defaults are set in webpack-dev-server/bin/webpack-dev-server.js. That step is skipped when calling addDevServerEntrypoints directly through the Node.js API.

Because of this, devClient:

webpack-dev-server/lib/util/addDevServerEntrypoints.js

18  const devClient = [`${require.resolve('../../client/')}?${domain}`];
19  console.log(devClient); // Added to print devClient.

will incorrectly resolve to and inject into webpack.config.js:

$ [ '/Users/someFolder/node_modules/webpack-dev-server/client/index.js?http:' ] // console.log(devClient)

instead of the correct:

$ [ '/Users/someFolder/node_modules/webpack-dev-server/client/index.js?http://localhost' ] // console.log(devClient)

This will eventually cause the error:

Uncaught SyntaxError: The URL 'http:/sockjs-node' is invalid

when starting the server:

$ node dev-server.js

and navigating to, in this case, localhost:5000 because socksjs-client is called with a url without a host property:

sockjs-client/lib/main.js

67  var parsedUrl = new URL(url);
68  if (!parsedUrl.host || !parsedUrl.protocol) {
69    throw new SyntaxError("The URL '" + url + "' is invalid");
70  }

The behavior of the webpack-dev-server CLI is different and correctly defaults to localhost:

$ ./node_modules/webpack-dev-server/bin/webpack-dev-server.js --content-base dist/
[ '/Users/someFolder/node_modules/webpack-dev-server/client/index.js?http://localhost:8080' ] // console.log(devClient)
Project is running at http://localhost:8080/
webpack output is served from /

because the options object passed as the argument to the devServerOptions parameter of addDevServerEntrypoints is given the defaults host: 'localhost' and port: 8080:

webpack-dev-server/bin/webpack-dev-server.js

 72  const DEFAULT_PORT = 8080;
...
 74  yargs.options({
...
210    host: {
211      type: 'string',
212      default: 'localhost',
213      describe: 'The hostname/ip address the server will bind to',
214      group: CONNECTION_GROUP
215    },
...
221  });
222
223  const argv = yargs.argv;
...
244  if (argv.host !== 'localhost' || !options.host) { options.host = argv.host; }
...
344  options.port = argv.port === DEFAULT_PORT ? defaultTo(options.port, argv.port) : defaultTo(argv.port, options.port);
345  if (options.port != null) {
346    startDevServer(webpackOptions, options);
...
359  function startDevServer(webpackOptions, options) {
360    addDevServerEntrypoints(webpackOptions, options);

Note

The example at https://webpack.js.org/guides/hot-module-replacement/ forgets to pass 'localhost' as the second argument of the server.listen call:

dev-server.js

server.listen(5000, () => {
  console.log('dev server listening on port 5000');
});

instead of:

dev-server.js

server.listen(5000, 'localhost', () => {
  console.log('dev server listening on port 5000');
});

But neither will fix the above as this 'localhost' is passed to the server, webpack-dev-server/lib/Server.js, and not the client, webpack-dev-server/client/index.js, that is injected into the entry point.

[codecov]

Codecov Report

Merging #1129 into master will not change coverage.
The diff coverage is 0%.

@@           Coverage Diff           @@
##           master    #1129   +/-   ##
=======================================
  Coverage   71.58%   71.58%           
=======================================
  Files           5        5           
  Lines         468      468           
  Branches      151      151           
=======================================
  Hits          335      335           
  Misses        133      133

Impacted Files	Coverage Δ
lib/util/createDomain.js	37.5% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a7f26b...852658d. Read the comment docs.

[shellscape]

@caryli this is probably the most thorough PR description I've run across, and for that I want to say thank you, it's really very much appreciated 🎉

Now for what's probably going to be a bummer; this isn't a change we're going to merge, mostly for a lot of the reasons you pointed out. We've got a functional API example here: https://github.com/webpack/webpack-dev-server/tree/master/examples/node-api-simple, which demonstrates how to properly use the API to start a server. Unfortunately if you're going to use the API method, you're going to have to specify host or useLocalIpin your config.

(Future versions may consolidate the options between the CLI and the API, however. see: #616)

This looks like a case of documentation confusion more than anything. Fortunately, the docs can be updated via a PR on this repo: https://github.com/webpack/webpack.js.org

Thanks again for taking the time to create this.