Skip to content
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

Merged
merged 1 commit into from
Nov 27, 2020

Conversation

ylemkimon
Copy link
Contributor

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 let listen() handle the address to bind to.

Depends on the PR above.

Does this PR introduce a breaking change?
N/A

Other information

@codecov
Copy link

codecov bot commented Nov 27, 2020

Codecov Report

Merging #2141 (9d432a5) into master (1398295) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2141   +/-   ##
=======================================
  Coverage   68.65%   68.65%           
=======================================
  Files          73       73           
  Lines        2386     2386           
  Branches      512      512           
=======================================
  Hits         1638     1638           
  Misses        748      748           
Impacted Files Coverage Δ
packages/serve/src/startDevServer.ts 87.09% <100.00%> (ø)

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 1398295...9d432a5. Read the comment docs.

@@ -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
Copy link
Member

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

Copy link
Contributor Author

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.

Copy link
Member

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

@alexander-akait alexander-akait merged commit dbbe4d4 into webpack:master Nov 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants