fix(serve): set client port when using default port

[jolyndenning]

What kind of change does this PR introduce?

Resolves webpack/webpack-dev-server#2873. This fixes a regression where webpack serve is not equivalent to the older webpack-dev-server command.

Did you add tests for your changes?

No, since the serve plugin's tests only run on webpack-dev-server@3 and my changes only apply to webpack-dev-server@4. If there is a way to run certain tests against different versions of webpack-dev-server, I'm happy to add tests.

If relevant, did you update the documentation?

N/A

Summary

webpack-dev-server adds a new entry point which loads the webpack-dev-server webpack client. The entry has a resource query which determines the host, port, and protocol of the websocket connection. That resource query must be created synchronously, since modifying the entry points must be done synchronously before webpack starts compilation. The problem is that findPort() (used within webpack-dev-server to find a default server port) is asynchronous.

webpack-dev-server@3 worked around this issue by having the webpack-dev-server CLI delay starting webpack until findPort() finishes. The following code shows where this used to happen:

https://github.com/webpack/webpack-dev-server/blob/4ab1f21bc85cc1695255c739160ad00dc14375f1/bin/webpack-dev-server.js#L165-L167

https://github.com/webpack/webpack-dev-server/blob/4ab1f21bc85cc1695255c739160ad00dc14375f1/lib/utils/processOptions.js#L30-L34

When the webpack-dev-server CLI was abandoned in favor of webpack serve, this behavior was lost. As a result, the webpack-dev-server socket client attempts to connect to the wrong port when you let webpack-dev-server find a default port for you.

I made the code change here in webpack-cli because it is the equivalent to the previous webpack-dev-server CLI code. I do not believe that any code change to webpack-dev-server could solve this issue, since the entries must be modified synchronously but findPort() is asynchronous.

Does this PR introduce a breaking change?

I don't think so. The startDevServer function within @webpack-cli/serve has changed to return a promise, but its use seems to be entirely internal to webpack-cli and so I don't consider it part of the package's public API.

Other information

[alexander-akait]

We need to fix CI here 😞

[jolyndenning]

Hmm the tests didn't fail for me locally, but the CI logs indicate it's likely related to my code changes to the types. I have pushed a new commit that (hopefully) fixes the tests in CI - they all work locally for me.

[codecov]

Codecov Report

Merging #2147 (043e5c9) into master (ae78411) will decrease coverage by 0.07%.
The diff coverage is 57.14%.

@@            Coverage Diff             @@
##           master    #2147      +/-   ##
==========================================
- Coverage   68.41%   68.34%   -0.08%     
==========================================
  Files          72       72              
  Lines        2340     2344       +4     
  Branches      516      518       +2     
==========================================
+ Hits         1601     1602       +1     
- Misses        739      742       +3     

Impacted Files	Coverage Δ
packages/serve/src/startDevServer.ts	80.00% <57.14%> (-7.10%)	⬇️

---

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 ae78411...043e5c9. Read the comment docs.

[jolyndenning]

The tests now passed! The coverage checks failed because I didn't add tests, since the tests only run on webpack-dev-server@3 and my code changes only apply to webpack-dev-server@4

[snitin315]

Yes we are enabling tests for v4 in #2155