Update to Chokidar v3

[paulmillr]

Hey! I wonder which steps would it take to:

1. Update to chokidar v3
2. Drop vscode-chokidar and vscode-fsevents packages.

Chokidar v3 incorporates a lot of improvements. We've decreased RAM consumption on large directories by a huge amount; and reduced dependency count by a factor of 13. All releases are signed now. See my blog post: Chokidar 3: How to save 32TB of traffic every week with one NPM package

Chokidar v3 requires nodejs 8 and higher. Unsure if that's good with vscode?

BTW, i'm keeping an eye on the Linux issue Ben mentioned.

cc @bpasero

[bpasero]

Hi @paulmillr, yes we totally want to update and drop our custom forks.

I do not think there is something blocking for us to move to version 3. However, one thing I was hoping gets resolved, still seems to be an issue: paulmillr/chokidar#860 (and the predecessor paulmillr/chokidar#447).

Any thoughts? This is the cause of one of our most upvoted watcher issues: #3998

PS: nice blog post, and great outcome 👍

[paulmillr]

For mac, I think the issue has been resolved if a user has fsevents.

For linux, i’ll dive into it.

[bpasero]

@paulmillr I tested specifically on macOS (version 3.0), were there any changes that came in recently that could have an impact? Maybe you check both OS. This would also be a concern for Windows: up until today we use our own custom file watcher on Windows (C# based), but we would eventually want to change to a single solution.

[paulmillr]

@bpasero check out the chokidar v2 vs v3 benchmarks — there were definitely some massive improvements across all platforms.

[ashthespy]

paulmillr/readdirp#99 (comment) is a possible blocker for this, as VS Code also runs on node v10.11.0 currently.

[paulmillr]

Bigints are supported since 10.2 or so, I don't think this is a root cause of the issue.

[ashthespy]

Cross linking - electron/electron#15323 is the actual blocker.

[bpasero]

@ashthespy your comment in paulmillr/readdirp#99 (comment) seems to suggest that this is fixed in Electron 4? VSCode is using Electron 4 since 2 months.

[ashthespy]

@ashthespy your comment in paulmillr/readdirp#99 (comment) seems to suggest that this is fixed in Electron 4? VSCode is using Electron 4 since 2 months.

Initially paulmillr/readdirp#99 (comment) seemed to suggest that it was a Node v10 vs v12 issue, but we did indeed figure out it's an electron issue.
So this isn't a blocker any more!

[paulmillr]

readdirp-3.1.2 is out with the fix for Electron.

[bpasero]

@paulmillr if we can get micromatch/anymatch#34 in, that would reduce our needed forks from 2 to 1. In order to get to 0 though we need to allow to set the options from chokidar too, or just hardcode the { dot: true } option.

[paulmillr]

Easy

[paulmillr]

@bpasero chokidar@master now uses [email protected] with dot: true by default.

[bpasero]

@paulmillr awesome, thanks a ton!! once that is out on NPM we can drop all our forks 🚀

[paulmillr]

@bpasero could you try how master works meanwhile?

[bpasero]

@paulmillr my first testing with master looks good in terms of respecting ignore patterns with a leading "." so I think from that point of view, this change looks good to us.

Still have to do some regression testing to see if events are coming in as before. If you know anything I should watch out for, let me know. We currently only use the watcher on macOS and Linux.

[bpasero]

Looks like paulmillr/chokidar#855 (comment) needs to be reopened, I see it in our tests too.

[bpasero]

...I am not happy with the new performance on Linux. It seems better on macOS, but not on Linux in my testing. Will follow up in an issue.

Update: paulmillr/chokidar#882

[paulmillr]

chokidar v3.1 is up!

[bpasero]

via #81028

[paulmillr]

@bpasero let's update to 3.2, just released this

[bpasero]

Will do next week, we close for endgame.