-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: improve host output and fix open #2892
Changes from 2 commits
a39d273
e922041
32d79d5
998bccc
884bf6e
a5a89c5
2846faa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ const url = require('url'); | |
const http = require('http'); | ||
const https = require('https'); | ||
const ip = require('ip'); | ||
const internalIp = require('internal-ip'); | ||
const killable = require('killable'); | ||
const chokidar = require('chokidar'); | ||
const express = require('express'); | ||
|
@@ -21,8 +22,8 @@ const { validate } = require('schema-utils'); | |
const normalizeOptions = require('./utils/normalizeOptions'); | ||
const updateCompiler = require('./utils/updateCompiler'); | ||
const getCertificate = require('./utils/getCertificate'); | ||
const status = require('./utils/status'); | ||
const createDomain = require('./utils/createDomain'); | ||
const colors = require('./utils/colors'); | ||
const runOpen = require('./utils/runOpen'); | ||
const runBonjour = require('./utils/runBonjour'); | ||
const routes = require('./utils/routes'); | ||
const getSocketServerImplementation = require('./utils/getSocketServerImplementation'); | ||
|
@@ -580,17 +581,93 @@ class Server { | |
} | ||
|
||
showStatus() { | ||
const suffix = '/'; | ||
const uri = `${createDomain(this.options, this.server)}${suffix}`; | ||
const useColor = getColorsOption(getCompilerConfigArray(this.compiler)); | ||
|
||
const configArr = getCompilerConfigArray(this.compiler); | ||
const colors = getColorsOption(configArr); | ||
const protocol = this.options.https ? 'https' : 'http'; | ||
const { hostname, port } = this; | ||
const prettyPrintUrl = (newHostname) => | ||
url.format({ protocol, hostname: newHostname, port, pathname: '/' }); | ||
|
||
let prettyHostname; | ||
let lanUrlForTerminal; | ||
|
||
if (hostname === '0.0.0.0' || hostname === '::') { | ||
prettyHostname = 'localhost'; | ||
|
||
const localIP = internalIp.v4.sync(); | ||
|
||
if ( | ||
/^10[.]|^172[.](1[6-9]|2[0-9]|3[0-1])[.]|^192[.]168[.]/.test(localIP) | ||
) { | ||
// Address is private, format it for later use | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this check is not needed, as There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can have local public IP (it is very rare edge case, but it is possible), for example I can run it on my server with public IP There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In that case, I think the public IP should be printed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this case we print two IP - local and network There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. IMHO, I see no reason not to print the internal IP (the IP address of the NIC) if it's a public one. It's still a network IP, just not a LAN. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't have strong opinion here, it was inspired by CRA and their feedback, but maybe you are right There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's simplify it, we will change it in future after feedback |
||
lanUrlForTerminal = prettyPrintUrl(localIP); | ||
} | ||
} else { | ||
prettyHostname = hostname; | ||
} | ||
|
||
status(uri, this.options, this.logger, colors); | ||
const localUrlForTerminal = prettyPrintUrl(prettyHostname); | ||
|
||
if (lanUrlForTerminal) { | ||
this.logger.info('Project is running at:'); | ||
this.logger.info(`Local: ${colors.info(useColor, localUrlForTerminal)}`); | ||
this.logger.info( | ||
`On Your Network: ${colors.info(useColor, lanUrlForTerminal)}` | ||
); | ||
} else { | ||
this.logger.info( | ||
`Project is running at ${colors.info(useColor, localUrlForTerminal)}` | ||
); | ||
} | ||
|
||
if ( | ||
this.options.dev && | ||
typeof this.options.dev.publicPath !== 'undefined' | ||
) { | ||
this.options.info( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @alexander-akait looks like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Already fixed in master |
||
`webpack output is served from '${colors.info( | ||
useColor, | ||
this.options.dev.publicPath === 'auto' | ||
? '/' | ||
: this.options.dev.publicPath | ||
)}' URL` | ||
); | ||
} | ||
|
||
if (this.options.static && this.options.static.length > 0) { | ||
this.logger.info( | ||
`Content not from webpack is served from '${colors.info( | ||
useColor, | ||
this.options.static | ||
.map((staticOption) => staticOption.directory) | ||
.join(', ') | ||
)}' directory` | ||
); | ||
} | ||
|
||
if (this.options.historyApiFallback) { | ||
this.logger.info( | ||
`404s will fallback to '${colors.info( | ||
useColor, | ||
this.options.historyApiFallback.index || '/index.html' | ||
)}'` | ||
); | ||
} | ||
|
||
if (this.options.bonjour) { | ||
this.logger.info( | ||
'Broadcasting "http" with subtype of "webpack" via ZeroConf DNS (Bonjour)' | ||
); | ||
} | ||
|
||
if (this.options.open) { | ||
runOpen(localUrlForTerminal, this.options, this.logger); | ||
} | ||
} | ||
|
||
listen(port, hostname, fn) { | ||
this.hostname = hostname; | ||
|
||
if (typeof port !== 'undefined' && port !== this.options.port) { | ||
this.logger.warn( | ||
'The port specified in options and the port passed as an argument is different.' | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,27 +8,27 @@ function createDomain(options, server) { | |
// use location hostname and port by default in createSocketUrl | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WIth There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think logic for terminal output and for entries is different, it is similar in some places, but it is for different purpose |
||
// ipv6 detection is not required as 0.0.0.0 is just used as a placeholder | ||
let hostname; | ||
|
||
if (options.useLocalIp) { | ||
hostname = ip.v4.sync() || '0.0.0.0'; | ||
} else if (server) { | ||
hostname = server.address().address; | ||
} else { | ||
hostname = '0.0.0.0'; | ||
} | ||
|
||
const port = server ? server.address().port : 0; | ||
|
||
// 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 url.format({ | ||
protocol, | ||
hostname, | ||
port, | ||
}); | ||
return url.format({ protocol, hostname, port }); | ||
} | ||
|
||
module.exports = createDomain; |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,7 +28,7 @@ exports[`CLI --hot webpack 4 1`] = ` | |
`; | ||
|
||
exports[`CLI --hot webpack 5 1`] = ` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note snapshots should be updated twice, each for webpack 4 and 5. |
||
"<i> [webpack-dev-server] Project is running at http://127.0.0.1:8080/ | ||
"<i> [webpack-dev-server] Project is running at http://localhost:8080/ | ||
<i> [webpack-dev-middleware] asset main.js X KiB [emitted] (name: main) | ||
<i> runtime modules X KiB 10 modules | ||
<i> cacheable modules X KiB | ||
|
@@ -73,7 +73,7 @@ exports[`CLI --no-hot webpack 4 1`] = ` | |
`; | ||
|
||
exports[`CLI --no-hot webpack 5 1`] = ` | ||
"<i> [webpack-dev-server] Project is running at http://127.0.0.1:8080/ | ||
"<i> [webpack-dev-server] Project is running at http://localhost:8080/ | ||
<i> [webpack-dev-middleware] asset main.js X KiB [emitted] (name: main) | ||
<i> runtime modules X bytes 3 modules | ||
<i> cacheable modules X KiB | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is for terminal output, developer should copy past it from terminal, IPv6 is not supported in many browsers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
url
automatically formats IPv6 hostname with brackets, which most browsers support including IE 11, e.g.,http://[::]:8080
. I think IPv6 should take precendence over IPv4, if available.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sense, I hope this does not scare developers