feat: consider file URLs in WebSocket URL creation

[kroko]

• This is a bugfix
• This is a feature
• This is a code refactor
• This is a test update
• This is a docs update
• This is a metadata update

For Bugs and Features; did you add new tests?

No.

However here's a test repo https://github.com/kroko/webpacktest-devserver-v4-case03 which shows

• when index.html is opened via file system currently devserver fails
• when index.html is opened via file system proposed changes would allow for devserver to work (including HMR)

Motivation / Use-Case

The use case is not easy to explain in short, but

• we face need to run webpack-dev-server on DOOH stands for a remote validation and changes
• the client's infrastructure for those devices is set up so that stuff is run from file system, basically everything is done by Chrome and --allow-file-access-from-files, --disable-web-security,  --autoplay-policy=no-user-gesture-required, etc.
• This addition would allow to run webpack-dev-server when HTML is opened in browser from filesystem (file:// protocol, that is browser opens i.e. file:///some/path/to/public/index.html)
• I do not see any downsides in introducing replacement also for file:// protocol 😄

Breaking Changes

None

Additional Info

[codecov]

Codecov Report

Merging #2890 (f037beb) into master (cee170c) will decrease coverage by 1.84%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #2890      +/-   ##
==========================================
- Coverage   92.61%   90.77%   -1.85%     
==========================================
  Files          38       38              
  Lines        1246     1246              
  Branches      324      324              
==========================================
- Hits         1154     1131      -23     
- Misses         87      107      +20     
- Partials        5        8       +3     

Impacted Files	Coverage Δ
client-src/clients/WebsocketClient.js	58.13% <100.00%> (ø)
lib/utils/DevServerPlugin.js	76.31% <0.00%> (-23.69%)	⬇️
lib/utils/getCertificate.js	69.23% <0.00%> (-19.24%)	⬇️

---

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 cee170c...f037beb. Read the comment docs.

[alexander-akait]

Good fix, but we really need test for this

[kroko]

Good fix, but we really need test for this

Thanks.

I tried to check tests (as codecov is failing for this PR), but at the glance did not find what and where, so kind of skipped. 😕

• https://github.com/webpack/webpack-dev-server/blob/master/test/client/clients/WebsocketClient.test.js
• https://github.com/webpack/webpack-dev-server/blob/master/test/client/utils/createSocketUrl.test.js

Please advise.

[alexander-akait]

@ylemkimon maybe you can help with tests? I think we need one good e2e tests to we can improve more them in future

[ylemkimon]

Unit tests are currently done in:

webpack-dev-server/test/client/clients/WebsocketClient.test.js

Line 34 in 763cde1

describe('client', () => {

I think file URL conversion can be tested with:

-  describe('client', () => {
+  describe.each([['http'], ['file']])('client (%s)', (protocol) => {
     it('should open, receive message, and close', (done) => {
       socketServer.on('connection', (connection) => {
         connection.send('hello world');

         setTimeout(() => {
           connection.close();
         }, 1000);
       });

-      const client = new WebsocketClient(`http://localhost:${port}/ws-server`);
+      const client = new WebsocketClient(`${protocol}://localhost:${port}/ws-server`);

Note you can run tests and update snapshots via npm run test:only -- -u.

E2E tests are in test/e2e, and to test file protocol, runBrowser() should be able to run with custom arguments:

webpack-dev-server/test/helpers/run-browser.js

Line 6 in 05bdb0c

function runBrowser(config) {

-function runBrowser(config) {
+function runBrowser(config, args) {
   const options = {
     viewport: {
       width: 500,
       height: 500,
     },
     userAgent: '',
     ...config,
   };

   return new Promise((resolve, reject) => {
     let page;
     let browser;

     puppeteer
       .launch({
         headless: true,
         // args come from: https://github.com/alixaxel/chrome-aws-lambda/blob/master/source/index.js
-        args: puppeteerArgs,
+        args: args ? puppeteerArgs.concat(args) : puppeteerArgs,

and then you can run puppeteer with custom arguments, e.g., runBrowser(null, ['--allow-file-access-from-files', '--disable-web-security']).

[kroko]

@ylemkimon thanks for the direction.

I tried wrapping my head around your infrastructure. Failed to achieve passing tests.

Also, for the test to check all cases it should include 3 protocols, HTTP, HTTPS, FILE

describe.each([['http'], ['https'], ['file']])('client (%s)', (protocol) => { /* ... */});

I'm at a point, where I have the feeling (as I cannot pinpoint at all lines in the code I scrolled over 😄 ) that to achieve more than HTTP test would require quite a lot of work. However, as in some of the aspects I have limited experience I could be wrong.

Currently as you know it only checks HTTP.

I'm still stuck at

  ● WebsocketClient › client (https) › should open, receive message, and close
  ● WebsocketClient › client (file) › should open, receive message, and close

and tracing back through log.error.mock.calls stack ... I kind of give up, for me to solve this would probably take too much time.

[alexander-akait]

Sorry for delay, can we continue?

[ylemkimon]

Changes are implemented in #2954. E2E test needs to be added, though.

[alexander-akait]

Fixed