chore(deps): update chokidar to v4

[Fuzzyma]

This PR aims to close #5368

In order to replace chokidars removed glob functionality, we have to use some trickery.

• glob-parent is used to get the common ancenstor dir that needs to be watched.
• picomatch is used as ignore-filter to make sure that only files/dirs are watched that match the given glob
• is-glob is used to only use glob logic when needed

In total this adds 4 direct or transitive dependencies. However, the update to chokidar v4 removes 12. So its a net positive

I also added a test to ensure that creation of nested directories is detected properly.

• 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

NOTE: This PR needs to wait for paulmillr/chokidar#1394. If that issue is not resolved, we have to make sure to not let undefined values slip through for watcher options.

I didn't add this change to this PR because it would add more changed lined and it might be not needed after all.
That's why this PR is still draft

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: Fuzzyma / name: Ulrich-Matthias Schäfer (184588d, 5d42bac, 970c353, 9492a00, a34faf6, 3dd2b4b, b01a2c3, 7b113cf, d883a1a, c5539b0, 3374234)

[alexander-akait]

@Fuzzyma Can you rebase?

[Fuzzyma]

@alexander-akait i am still waiting for the chokidar issue to be resolved. So this isn't passing tests atm anyway.

That said, surely can rebase :)

[Fuzzyma]

@alexander-akait I saw that when I run the tests a lot of them fail - unrelated to my changes. What is that about?

[alexander-akait]

@Fuzzyma Can we simplify this and rewrite it into a more understandable logic, here one array is derived from the second and the second from the third, it is very difficult to read, I mean:

watchOptions.ignored = []
  .concat(watchPathArr.map(item => isGlob(item) ? generateIgnoreMatcher(item) : false))
  .concat(watchOptions.ignored.map(item => {  isGlob(item) ? generateIgnoreMatcher(item) :  item }))
  .filter(Boolean)

This is an approximate pseudocode, but it is easy to understand where everything comes from and how it is formed.

[alexander-akait]

I would like to finish this soon and make a release

[Fuzzyma]

Will do!

As long as chokidar is not handling the issue linked in the first post, this is blocked anyway. As alternative we have to filter out undefined options ourselves before passing them to chokidar. Wdyt?

[Fuzzyma]

@alexander-akait I tried it your way. But there is one detail that might end up being performance sensitive.
I dont want to generate multiple matchers (one for each glob) if one is sufficient for all globs. Therefore I need to filter out the glob strings twice.

I tried to simplify the code further and think the code that I have now is easy enough to understand. I made it so that everything ends up in the ignoreArr.
Also fixed a bug while I was at it.

We need tests for the ignore case though! I am not quite sure how to do the ignore case. I need to take a look.

Also I made the snapshots break because ignore is an array now when passied to chokidar. Need to fix that as well

[Fuzzyma]

@alexander-akait so there are 2 remaining issues:

• chokidar doesnt allow undefined as options anymore and crashes (even tho the types allow it... I raised an issue but they dont really wanna change it?). So we have to clean the options before we pass them to chokidar or wait for chokidar to come to the conclusion that its useful to follow their own types
• I dont know how to create tests for "ignored" files. How do you detect that nothing should happen? Maybe you can help me out here

[alexander-akait]

@Fuzzyma

chokidar doesnt allow undefined as options anymore and crashes (even tho the types allow it... I raised an issue but they dont really wanna change it?). So we have to clean the options before we pass them to chokidar or wait for chokidar to come to the conclusion that its useful to follow their own types

I.e. our logic is broken somewhere? Because I don't think someone write option: undefined, maybe you can provide an example?

I dont know how to create tests for "ignored" files. How do you detect that nothing should happen? Maybe you can help me out here

There is a test - https://github.com/webpack/webpack-dev-server/blob/master/test/e2e/watch-files.test.js#L160, just add a several cases where we have ignored from watchFiles - negative glob, nested glob (i.e. like test/**/a/**), just static value, and just when developer set directly.

In fact, this is one of the reasons why we moved away from chokidar inside webpack - it is very difficult to interact and make changes. Perhaps we should consider https://github.com/webpack/watchpack as chokidar replacer in future... we need to improve some things there.

Another solution that I am considering is the implementation of this function fundamentally in webpack (based on watchpack), so it will work not only for serve, but for watch too

[Fuzzyma]

@alexander-akait no your logic is not broken. It is usually totally acceptable to pass undefined options to functions because typescripts optional type is { foo?: string | undefined }. Chokidar breaks this because they pass the options through to the native node api and that api throws if you pass undefined.

However, yes, we do create undefineds when calling getInterval and no interval or poll is defined:

webpack-dev-server/lib/Server.js

Lines 899 to 911 in 55220a8

	const getInterval = () => {
	if (typeof watchOptions.interval !== "undefined") {
	return watchOptions.interval;
	}
	if (typeof watchOptions.poll === "number") {
	return watchOptions.poll;
	}
	if (typeof compilerWatchOptions.poll === "number") {
	return compilerWatchOptions.poll;
	}
	};

If they dont fix it, we have to fix it on our side.

For the tests: I already added tests there for the globs. But again: how do you test if something does not happen because it is ignored?

[alexander-akait]

If they dont fix it, we have to fix it on our side.

👍

Do they have a pr?

For the tests: I already added tests there for the globs. But again: how do you test if something does not happen because it is ignored?

maybe chokidar itself have a method that we can mock, well or we can try to check that the reload did not happen, but this will require timeouts, and it is slow

[Fuzzyma]

Yeah, mine: paulmillr/chokidar#1396

Well since chokidar doesnt support globbing anymore, everything that touches globs, we have to test on our side.
Or we just test how specific globs are correctly resolved into correct ignore functions or matchers? Maybe easier?

[alexander-akait]

Or we just test how specific globs are correctly resolved into correct ignore functions or matchers? Maybe easier?

Yeah, maybe

Let's first try to simplify everything in the code, maybe we can even use unit testing here and one or two integrations to understand that everything really works

[Fuzzyma]

Then I will pull out the creation of the ignore functions into own units. The we can simply test those.

[Fuzzyma]

@alexander-akait I added tests for the glob matching and also added wrapper code to support chokidars weird behavior with undefined values. It can be removed once they make up their mind. This should work now and is ready for review