-
-
Notifications
You must be signed in to change notification settings - Fork 605
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
fix(serve): do not default host in webpack-dev-server v4 #2141
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2141 +/- ##
=======================================
Coverage 68.65% 68.65%
=======================================
Files 73 73
Lines 2386 2386
Branches 512 512
=======================================
Hits 1638 1638
Misses 748 748
Continue to review full report at Codecov.
|
@@ -36,9 +36,9 @@ export default function startDevServer(compiler, cliOptions): object[] { | |||
const usedPorts: number[] = []; | |||
devServerOptions.forEach((devServerOpts): void => { | |||
const options = mergeOptions(cliOptions, devServerOpts); | |||
options.host = options.host || 'localhost'; | |||
// devSever v4 handles the default port itself | |||
// devSever v4 handles the default host and port itself |
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.
I think we can break compatibility with webpack-dev-server@3, I think we need do it by default for else
branch
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.
isDevServer4
is negated in the condition.
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.
I definitely need a vacation
What kind of change does this PR introduce?
Bug fix
Did you add tests for your changes?
Integration tests will be possible once after webpack-dev-server v4 has been released.
If relevant, did you update the documentation?
N/A
Summary
#2126 (comment)
With webpack/webpack-dev-server#2869, it'd be possible to pass
undefined
as the host to letlisten()
handle the address to bind to.Depends on the PR above.
Does this PR introduce a breaking change?
N/A
Other information