Add --host option to CLI #278
Conversation
bin/cli.js
Outdated
| .option( | ||
| '-p, --port <port>', | ||
| 'set the port to serve on. defaults to 1234', | ||
| parseInt |
There was a problem hiding this comment.
Added coercion to the port. Node handles it as-is, but it was resulting in a false error message on Windows:
bin/cli.js
Outdated
| require('opn')( | ||
| `http://${command.host || 'localhost'}:${server.address().port}` | ||
| ); | ||
| }; |
There was a problem hiding this comment.
The below fixes a bug in my original implementation. Timing wasn't an issue when using localhost but listening appears to be slower (at least on Windows) when providing a host. This results in an attempted server.address().port call before listening. Simply defer if not listening yet.
There was a problem hiding this comment.
You are setting the default host here, but not the default port ...
src/Bundler.js
Outdated
| return await Server.serve(this, port); | ||
| async serve(port = 1234, host) { | ||
| await this.bundle(); | ||
| return await Server.serve(this, port, host); |
There was a problem hiding this comment.
This was resulting in output like this:
Instead of like this:
Seems the persistent server logging should come at the end, and not be shown until everything is ready.
There was a problem hiding this comment.
Caused by #164, build output was on a single line prior to that. The build time will remain the same with this change, but there could be a minor delay added between end of build and dev server availability due to the blocking. Not a big deal IMO, it should be on the magnitude of milliseconds.
Alternatives:
- Revert this, leave the broken up logging
- Refactor logging of server messages until build is finished
There was a problem hiding this comment.
... whereas here you are setting the default port but not the default host. Can't the defaults be in the same place?
There was a problem hiding this comment.
@rondonjon - good call! Updated to handle defaults with commander. This also gives us nicer help commands.
| 'set the port to serve on. defaults to 1234', | ||
| parseInt | ||
| 'set the port to serve on', | ||
| x => parseInt(x, 10), |
There was a problem hiding this comment.
I've seen this fail without the radix:
Adding for safety. Fixes the issue on my end.
|
hmm, not sure I understand the usecase here... why is this needed? doesn't the default (i.e. no host passed to the server) listen on all interfaces already? |
|
Good question. I (incorrectly) thought the default was |
|
@devongovett @brandon93s the dev server itself works fine in its current form, but hot reloading does not work when viewing from hosts other than localhost (i.e. - when developing on a remote machine), as the WebSocket is hardcoded to look for localhost - see comment by @Fluidbyte on #276. |
|
I'm currently working on a project that needs this feature. We have a lot of sites running on subdomains including our local development sites. Cookies are often attached to the top domain (*.foo.com) and also CORS is also enabled for subdomains in the dev environment. We are basically running our local dev sites under a subdomain e.g local.foo.com etc (these domains point to 127.0.0.1 in the host file). So being able to set `-h local.foo.com -open' would be great to avoid manually changing the hostname in the address bar. |
Add a new option
-h --hostto override the host / IP of the dev server.Closes #276