Add default port as 8080 in options.js to match webpack-dev-server.js

[SirUppyPancakes]

• Operating System: Windows 10 x64
• Node Version: v11.4.0
• NPM Version: 6.4.1
• webpack Version: 4.29.6
• webpack-dev-server Version: 3.3.1

• This is a bug
• This is a modification request

Code

The minimal changes to webpack.config.js to introduce HMR are:

// webpack.config.js
// ....
devServer: {
    hot: true,
    port: 8080
}
// ...
plugins: [
    new HotModuleReplacementPlugin(),
    // other plugins...
]

Expected Behavior

The port option should default to 8080 (just like the server does).

Actual Behavior

The port option does not have a default, which results in the generated code trying to contact webpack-dev-server at the default port (for URLs) of 80.

Relevant webpack-dev-server code

In bin/webpack-dev-server.js

const DEFAULT_PORT = 8080;

In bin/options.js (missing a default key)

  port: {
    describe: 'The port',
    group: CONNECTION_GROUP,
  },

For Bugs; How can we reproduce the behavior?

Simply remove the port option above from a simple webpack-dev-server example project. There will be console errors on the loaded page, and if you check the generated bundle code, you'll find that it tries to contact webpack-dev-server at http://localhost instead of http://localhost:8080

For Features; What is the motivation and/or use-case for the feature?

Having less required options makes webpack much more approachable to beginners looking to "just make it work".

[alexander-akait]

We already use 8080 by default https://github.com/webpack/webpack-dev-server/blob/master/bin/webpack-dev-server.js#L242

[alexander-akait]

What is problem, how you run webpack-dev-server? CLI or Nodejs api?

[SirUppyPancakes]

I just run yarn webpack-dev-server with webpack-dev-server installed locally (so CLI), and withe the config as laid out above.

If you would like, I can put together a minimal reproduction that shows the issue.

[alexander-akait]

Please provide, default port is 8080, so something wrong in you configuration

[alexander-akait]

Also since 3.3.0 HotModuleReplacementPlugin is no need to be added (it is done automatically)

[SirUppyPancakes]

Ah okay I think this is actually related to my use of https://github.com/gajus/write-file-webpack-plugin

I use this plugin to force webpack-dev-server to write the files to a folder (which makes it easier to load with Electron in my case). Without this plugin, I would have to branch my Electron code to load from localhost:8080/index.html (during development) and ./index.html during production.

So, to revise the above issue, leaving out the port option only causes loading the local version (produced by the WriteFilePlugin) to fail to have HMR work, but loading localhost:8080/index.html still works fine. The bundled code (which I can see thanks to WriteFilePlugin), definitely isn't referencing port 8080 though...

Snippets from the bundle:

When setting port: 8080:

/***/ "./node_modules/webpack-dev-server/client/index.js?http://localhost:8080":
/*!*********************************************************!*\
  !*** (webpack)-dev-server/client?http://localhost:8080 ***!
  \*********************************************************/
/*! no static exports found */
/***/ (function(module, exports, __webpack_require__) {

When leaving port option out:

/***/ "./node_modules/webpack-dev-server/client/index.js?http://localhost":
/*!****************************************************!*\
  !*** (webpack)-dev-server/client?http://localhost ***!
  \****************************************************/
/*! no static exports found */
/***/ (function(module, exports, __webpack_require__) {

If this is just an expected assumption made by webpack-dev-server that the index.html is located in a specific place (in this case localhost:8080), then that definitely makes sense 😁

[SirUppyPancakes]

Now that I think about it though, taking the minimalist approach to configuration, branching on where I load index.html from in my Electron code might be less configuration and more intuitive than using an entire plugin to write files to disk... lol

[alexander-akait]

You can replace WriteFilePlugin on writeToDisk option https://webpack.js.org/configuration/dev-server/#devserverwritetodisk-

[alexander-akait]

hm, maybe be we forget adding port in some cases 😕 Yes, need reproducible test repo

[SirUppyPancakes]

Alright, got together a minimal reproduction of the issue (with all the changes you suggested).

Steps to reproduce:

1. Download and unzip this example: wds_port_issue.zip
2. Run yarn or npm install in the root of the project.
3. Run yarn webpack-dev-server or node_modules/.bin/webpack-dev-server.
4. The webpack.config.js uses the options: devServer: { hot: true, writeToDisk: true, port: 8080 }, and regardless of whether or not you open app/index.html (produced by writeToDisk) or localhost:8080/index.html in a browser, HMR will work as expected, if you watch the console as you make changes to src/index.js.
5. Use ctrl-c or close the console that is running webpack-dev-server.
6. Open webpack.config.js and comment out the port: 8080 option.
7. Run yarn webpack-dev-server or node_modules/.bin/webpack-dev-server again.
8. Now, if you open localhost:8080/index.html in a browser and make changes to src/index.js, HMR still works; however, if you open app/index.html you'll see errors in the console.
9. You'll notice the errors are referencing a failed connection with URL: http://localhost/sockjs-node/info?t=1555097629687 or something similar.
10. Additionally, if you open up app/index.js and search for localhost there are lots of URLs with either localhost or localhost:80, where when you use port: 8080 in webpack.config.js, you'll find that many of these same URLs are localhost:8080.

One more thing to note however, is that I found another error, this time even when using port: 8080, but only when you open the local app/index.html generated by writeToDisk. After the first hot-update, the following error is printed to the console:
Access to XMLHttpRequest at 'file:///C:/Users/caleb/Desktop/wds_port_issue/wds_port_issue/app/088d5e7dcde32c3a4a2b.hot-update.json' from origin 'null' has been blocked by CORS policy: Cross origin requests are only supported for protocol schemes: http, data, chrome, chrome-extension, https.

Maybe this indicates that the files generated by writeToDisk are not actually meant to be used like this...

[knagaitsev]

I didn't test it, but from what you said, I suspect createDomain is not adding the port correctly, since http://localhost/sockjs-node/info?t=1555097629687 is missing the port. Just recently there was a problem where it was not inferring localhost as the hostname correctly.

[alexander-akait]

One more thing to note however, is that I found another error, this time even when using port: 8080, but only when you open the local app/index.html generated by writeToDisk. After the first hot-update, the following error is printed to the console:
Access to XMLHttpRequest at 'file:///C:/Users/caleb/Desktop/wds_port_issue/wds_port_issue/app/088d5e7dcde32c3a4a2b.hot-update.json' from origin 'null' has been blocked by CORS policy: Cross origin requests are only supported for protocol schemes: http, data, chrome, chrome-extension, https.

Don't use file, it is normal what you get CORS problem.

Anyway if you find better solution PR welcome. We investigate this in future.

[alexander-akait]

server.address().port

I think server.address().port should return 8080 (https://github.com/webpack/webpack-dev-server/blob/master/lib/utils/createDomain.js)

[SirUppyPancakes]

Just added some console logging to createDomain.js:

  const port = options.socket ? 0 : server ? server.address().port : 0;
  console.log("port:", port);
  console.log("server.address(): ", server ? server.address() : "<server is null>");

Here are the results:

When port: 8080 is present in webpack.config.js:

port: 8080
server.address():  { port: 8080 }
port: 8080
server.address():  { address: '127.0.0.1', family: 'IPv4', port: 8080 }

When port: 8080 is commented out in webpack.config.js:

port: undefined
server.address():  { port: undefined }
port: 8080
server.address():  { address: '127.0.0.1', family: 'IPv4', port: 8080 }

Looks like the first call to createDomain passes a server that has server.address() with some missing information and port: undefined, when port: 8080 is missing in webpack.config.js. And indeed, when I add a quick-and-dirty check to catch this scenario:

  let port = options.socket ? 0 : server ? server.address().port : 0;

  // if port ended up undefined and server.address() has port: undefined, patch it
  if (!port && !server.address().port)
    port = 8080;

  console.log("port:", port);
  console.log("server.address(): ", server ? server.address() : "<server is null>");

This resolves the error, and I can successfully use app/index.html with HMR and without port: 8080 present in webpack.config.js.

Regarding your other comment about the CORS problem: "Don't use file, it is normal what you get CORS problem.", I'm not sure what you mean. I don't create any URLs with file; that request was created by some code generated by webpack-dev-server in the bundle. It doesn't appear to prevent HMR from working however and only seems to happen on the first hot-update. This seems like a separate issue however, so I'll probably create another issue about this if needed.

[alexander-akait]

Let's fix problem with default port, PR welcome 👍

[SirUppyPancakes]

I'll be honest, I'm stumbling around trying to figure out where the change should be and why, but I don't know how to make it so that it doesn't break other things.

So far, it looks like we shouldn't change options.js to have port have a default, because it would cause the port finder to never get called if the port is already in use. I know that changing createDomain seems to work, but the result of that function is only ever passed (as uri) to status(uri, options, log, argv.color); which doesn't seem to do anything other than print to the console, let alone affect the generated bundle...

Maybe someone who has more experience with the source could take a look... sorry I can't be of more help, first time really looking at webpack-dev-server's source 😁

[SirUppyPancakes]

One more thing I just noticed from reading the source, I think the change should probably be to the lib/utils/addEntries file which seems to call createDomain in a way that makes more sense in the context of the generated bundle...

[SirUppyPancakes]

Ah yes looks like the change would be to here:

    const app = server || {
      address()
      {
        return { port: options.port };
      },
    };

The problem with just adding the default port 8080 as a fallback here would be that it wouldn't use the port finder result (from bin/webpack-dev-server.js) if it was needed....

[alexander-akait]

Yep, it should be easy

[SirUppyPancakes]

So findPort relies on an instance of Server in order to listen for EADDRINUSE and find a port that is available. However, when creating a Server you must pass the port, which eventually ends up in a call to addEntries, which is why addEntries is missing the port, because the Server is created before we find one.

It seems there would be two ways around this:

1. Rewrite the function that defaults the port, or finds a random open port to not use a Server instance (maybe it could use a temporary server on that port, though this would introduce a potential race condition). Then you could put this function call much earlier in the code (like in processOptions) so that all consumers of options get the accurate port value.
2. Move the function that defaults the port, or finds a random open port to the Server implementation itself and have Server set options.port to the right port. This would not have any race conditions (since the Server either ends up with a successful port or it doesn't).

I'll try and poke around with the second option and do a PR if I can get something to work.

[thesaltygerman]

@SirUppyPancakes came here to say that we were having of same problem. switched to https://www.npmjs.com/package/webpack-plugin-serve and haven't had issue since. it support Promise for port option so can use get-port or other npm module really easy.

[SirUppyPancakes]

Thanks for the link! @thesaltygerman That definitely looks like a decent alternative.

I think once I get a PR in (if I can manage to find a nice place to add in the default port value), webpack-dev-server should end up only needing devServer.hot = true, and should properly use the desired port (whether default, custom, or random) for both the server and generated code. I just need to track down the right spot in the code to add this in, so that it doesn't break anything else 😉

[alexander-akait]

I think it is bug:

1. we need findPort (https://github.com/webpack/webpack-dev-server/blob/master/bin/webpack-dev-server.js#L242) before run compilation https://github.com/webpack/webpack-dev-server/blob/master/bin/webpack-dev-server.js#L145 so you always have port
2. Add test on cli and node api usage

[SirUppyPancakes]

So I've nailed down the exact problem now, just need to come up with a way to minimally change the code to fix it...

When the server is created in bin/webpack-dev-server.js:

  try {
    server = new Server(compiler, options, log);
  } catch (err) {
    if (err.name === 'ValidationError') {
      log.error(colors.error(options.stats.colors, err.message));
      // eslint-disable-next-line no-process-exit
      process.exit(1);
    }

    throw err;
  }

it passes options and compiler. The Server constructor then handles all of the compiler-related things (and I'm guessing starts the compiler).

This all happens before we ever call findPort and actually listen to the server on that port. Unfortunately, the current findPort implementation seems to not only call the portfinder function (from the NPM package), but it also registers a handler on the server to watch for EADDRINUSE. This is useful, but unnecessarily requires that a call to findPort happen after the server is already created.

So we end up with a coupling/dependency problem:

1. The Server constructor starts the compiler (which immediately uses options.port).
2. The findPort function needs a Server instance (so must be called later), and then sets options.port.

So we can't easily move the findPort call up before the compiler stuff is run (in the Server constructor) because we need to pass the Server instance.

To me, it seems the only good solution to this would be to decouple all of the code that deals with the compiler from the Server constructor. That way, when we call findPort, we can restart the compiler and re-listen to the Server as needed.

I don't think I am confident enough to do a pull request for this, as this will require a lot of restructuring of the code, and I am not familiar enough to do that confidently.

[alexander-akait]

To me, it seems the only good solution to this would be to decouple all of the code that deals with the compiler from the Server constructor. That way, when we call findPort, we can restart the compiler and re-listen to the Server as needed.

Not sure it is solve the problem

[alsotang]

In v3.8.2, when using with program style, you also need to specify the port in options

	new WebpackDevServer(compiler, {
		port: 8080, // do not ignore this
	}).listen(8080, 'localhost', function(err) {
		console.log("[webpack-dev-server]", "http://localhost:8080/webpack-dev-server/index.html");
	});

[sumukhah]

Hii, @alexander-akait @SirUppyPancakes, Is this issue still exists?
I would like to work on this

[alexander-akait]

I think it is fixed, but need test

[alexander-akait]

Close in favor #3351, we will fix it in near future, also now we have DevServer.getFreePort utils, so getting to free port is not easy