fix(deps): update chokidar to v4 #5256
Conversation
There was a problem hiding this comment.
Hey thanks for getting this started! Sorry it took so long to review - since Chokidar didn't end up needing to be bumped with v11, this fell by the wayside. But I'm excited to help push this forward along now! 🚀
Requesting changes primarily on a performance point, as well as a design point. Happy to talk if you want. 🙂
|
Hey @JoshuaKGoldberg, there is no way I remember the details after a month :). All I know it's been a struggle to get all tests working. In the meantime we switched to ava to resolve our issues, so I can't do much. |
|
@JoshuaKGoldberg is there any blocker to this PR besides merge conflicts of a lockfile? |
|
Just a re-request for review. I'll add it to the queue for review :) |
There was a problem hiding this comment.
🔥 This is fantastic, thanks a bunch @ggodlewski! I really appreciate you sticking through & working with me on the reviews.
I've played with this a bunch locally on relatively small and large repros, and haven't found any issues. I'd really prefer to get another review from someone on @mochajs/maintenance-crew just to be safe - but otherwise ✅ on my end.
Nice job!
|
I'll take a look at this tonight :) |
|
@ggodlewski are you able to resolve the merge conflict? Otherwise looks great :) |
Co-authored-by: Josh Goldberg ✨ <git@joshuakgoldberg.com>
|
@mark-wiemer I rebased against main and updated lock file. |
There was a problem hiding this comment.
Pull Request Overview
This PR updates the chokidar dependency to v4 to reduce unnecessary bundled dependencies and adjusts the file watching mechanism accordingly.
- Upgrades chokidar version in package.json
- Introduces a GlobFilesTracker class in lib/cli/watch-run.js for enhanced file pattern matching using glob
- Modifies the chokidar watcher initialization and event handling to integrate the new tracking logic
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| package.json | Upgraded chokidar from v3.5.3 to v4.0.1 |
| lib/cli/watch-run.js | Added GlobFilesTracker and refactored watcher logic using glob |
| const globOpts = { | ||
| dot: true, | ||
| ignore: { | ||
| childrenIgnored: pathToCheck => watchIgnoreSet.has(pathToCheck.relative()) |
There was a problem hiding this comment.
The call to 'pathToCheck.relative()' is likely incorrect since glob.sync returns strings, not path objects. Consider removing the 'relative()' call or converting the string to a path object using a standard Node.js path function if relative paths are required.
| childrenIgnored: pathToCheck => watchIgnoreSet.has(pathToCheck.relative()) | |
| childrenIgnored: pathToCheck => watchIgnoreSet.has(path.relative(process.cwd(), pathToCheck)) |
Copilot uses AI. Check for mistakes.
PR Checklist
status: accepting prsOverview
Current chokidar is bundled with tons of spam deps including
braceswhich trigger some audit alert.All test are working. It supports globs unlike #5215