Remove depth limit from chokidar options

[marcofugaro]

• 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

Motivation / Use-Case

The watchContentBase option would only watch only the root folder, this PR removes that limitation.

Discussion in #1694

The watchContentBase option will now watch the folder recursively

[codecov]

Codecov Report

Merging #1697 into master will increase coverage by 2.22%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1697      +/-   ##
==========================================
+ Coverage   83.85%   86.08%   +2.22%     
==========================================
  Files           8        8              
  Lines         539      539              
  Branches      162      162              
==========================================
+ Hits          452      464      +12     
+ Misses         70       62       -8     
+ Partials       17       13       -4

Impacted Files	Coverage Δ
lib/Server.js	82.19% <ø> (+3.14%)	⬆️

---

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 09de43e...d179f45. Read the comment docs.

[alexander-akait]

Looks good, but need tests for this

[marcofugaro]

@evilebottnawi
For now I've made the test that edits a nested file, but I should check that chokidar has been triggered and the browser has been refreshed. How do i go about testing this?

Could you give me more directions?

[alexander-akait]

No need check is browser refreshed, anyway if you do this will be great (example e2e tests https://github.com/webpack/webpack-dev-server/blob/master/test/Client.test.js).

Main test is mocking chokidar and check event was handled (

webpack-dev-server/lib/Server.js

Line 954 in 21687c3

watcher.on('change', () => {

). More infromation about mocking https://jestjs.io/docs/en/mock-functions.html

[marcofugaro]

@evilebottnawi alright I wrote some code that should work, however I am having some troubles with connecting puppeteer to the server, am I doing something wrong?

[alexander-akait]

maybe you already have app on 9001 port? Or you we need run test in serial

[marcofugaro]

@evilebottnawi jest is already being run with the --runInBand, so it should run one at a time.

Also no, I don't have anything on the 9001 port, it still gives the error even if I change the port.

[hiroppy]

@marcofugaro This test code should be fixed like below.

diff --git a/test/ContentBase.test.js b/test/ContentBase.test.js
index 4813372..70b57eb 100644
--- a/test/ContentBase.test.js
+++ b/test/ContentBase.test.js
@@ -20,7 +20,6 @@ const contentBaseOther = path.join(
 describe('ContentBase', () => {
   let server;
   let req;
-  afterEach(helper.close);
 
   describe('to directory', () => {
     beforeAll((done) => {
@@ -32,15 +31,21 @@ describe('ContentBase', () => {
       addEntries(config, options);
       server = helper.start(
         config,
-        {
+        Object.assign({}, options, {
           contentBase: contentBasePublic,
           watchContentBase: true,
-        },
+        }),
         done
       );
       req = request(server.app);
     });
 
     it('Request to index', (done) => {
       req.get('/').expect(200, /Heyo/, done);
     });
@@ -54,7 +59,8 @@ describe('ContentBase', () => {
 
       runBrowser().then(({ page, browser }) => {
         // wait for first load
-        page.goto('http://localhost:9001').then(() => {
+        page.goto('http://0.0.0.0:9001').then(() => {
+          done(); // TODO: change this
           // page reloaded after the first load,
           // meaning it watched the file correctly
           page.on('load', () => {
@@ -82,6 +88,12 @@ describe('ContentBase', () => {
       req = request(server.app);
     });
 
+    afterAll((done) => {
+      helper.close(() => {
+        done();
+      });
+    });
+
     it('Request to first directory', (done) => {
       req.get('/').expect(200, /Heyo/, done);
     });
@@ -103,6 +115,12 @@ describe('ContentBase', () => {
       req = request(server.app);
     });
 
+    afterAll((done) => {
+      helper.close(() => {
+        done();
+      });
+    });
+
     it('Request to page', (done) => {
       req
         .get('/other.html')
@@ -123,6 +141,12 @@ describe('ContentBase', () => {
       req = request(server.app);
     });
 
+    afterAll((done) => {
+      helper.close(() => {
+        done();
+      });
+    });
+
     it('Request to page', (done) => {
       req
         .get('/foo.html')
@@ -147,6 +171,12 @@ describe('ContentBase', () => {
       req = request(server.app);
     });
 
+    afterAll((done) => {
+      helper.close(() => {
+        done();
+      });
+    });
+
     it('Request to page', (done) => {
       req.get('/other.html').expect(200, done);
     });
@@ -167,6 +197,12 @@ describe('ContentBase', () => {
       req = request(server.app);
     });
 
+    afterAll((done) => {
+      helper.close(() => {
+        done();
+      });
+    });
+
     it('Request to page', (done) => {
       req.get('/other.html').expect(404, done);
     });
@@ -184,6 +220,12 @@ describe('ContentBase', () => {
       req = request(server.app);
     });
 
+    afterAll((done) => {
+      helper.close(() => {
+        done();
+      });
+    });
+
     it('Request foo.wasm', (done) => {
       req.get('/foo.wasm').expect('Content-Type', 'application/wasm', done);
     });

[marcofugaro]

Thank you @hiroppy that fixed my problem, however i could not get socket.on('connection', to trigger during tests, so I ended up lisstening to chokidar's change event.

Now the test works @evilebottnawi!

[hiroppy]

/cc @evilebottnawi PTAL