Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
24988ef
fix: watch mode using chokidar v4 (#5355)
Arnesfield Jun 18, 2025
4f3150d
refactor: fix watch mode using chokidar v4 (#5355)
Arnesfield Jun 19, 2025
5e39fae
docs: fix jsdoc description for `normalizeGlob()`
Arnesfield Jun 19, 2025
7d716db
Merge branch 'main' into fix-5355-watch-mode
JoshuaKGoldberg Jul 3, 2025
3660ed2
test: add tests for watch mode
Arnesfield Jul 3, 2025
bad7b36
test: fix test for multiple watched files
Arnesfield Jul 3, 2025
539d4de
Merge branch 'main' into fix-5355-watch-mode
Arnesfield Jul 12, 2025
fcc839f
test: remove test for multiple watched files
Arnesfield Jul 12, 2025
4860486
Merge branch 'main' into fix-5355-watch-mode
Arnesfield Jul 20, 2025
0b1babc
fix: update watch mode implementation
Arnesfield Jul 17, 2025
00bb46a
Merge branch 'main' into fix-5355-watch-mode
mark-wiemer Jul 22, 2025
5514fe2
Refactor, add debug
mark-wiemer Jul 27, 2025
eb2792c
Remove verbose debug
mark-wiemer Jul 27, 2025
2a9c88c
Add debugs
mark-wiemer Jul 27, 2025
9e2a41e
Remove castArray, capitalize Chokidar
mark-wiemer Jul 27, 2025
caca650
Replace for-of with Array.some
mark-wiemer Jul 27, 2025
f897e65
Add test for false literal matches
mark-wiemer Jul 27, 2025
fea5476
Document new test
mark-wiemer Jul 27, 2025
84b27a9
Merge branch 'main' into fix-5355-watch-mode
mark-wiemer Jul 27, 2025
ca7d630
Remove unused function
mark-wiemer Jul 27, 2025
c3dd839
Merge remote-tracking branch 'arnes/fix-5355-watch-mode' into arnes-f…
mark-wiemer Jul 27, 2025
8af5d0b
Remove createPathFilter and createPathMatcher from exports
mark-wiemer Jul 28, 2025
9bad30f
Merge branch 'main' into fix-5355-watch-mode
Arnesfield Aug 17, 2025
7bbc945
refactor: update watch mode
Arnesfield Aug 17, 2025
53d95b0
Merge branch 'main' into fix-5355-watch-mode
Arnesfield Sep 2, 2025
7d8c908
chore: move watch mode callbacks and object typedefs to the new types…
Arnesfield Sep 2, 2025
3f555c5
Merge branch 'main' into fix-5355-watch-mode
JoshuaKGoldberg Sep 30, 2025
bbfa8d8
Apply suggestion from @JoshuaKGoldberg
mark-wiemer Oct 1, 2025
89141df
Merge branch 'main' into fix-5355-watch-mode
mark-wiemer Oct 1, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
296 changes: 255 additions & 41 deletions lib/cli/watch-run.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,24 @@ const logSymbols = require('log-symbols');
const debug = require('debug')('mocha:cli:watch');
const path = require('node:path');
const chokidar = require('chokidar');
const glob = require('glob');
const isPathInside = require('is-path-inside');
const {minimatch} = require('minimatch');
const Context = require('../context');
const collectFiles = require('./collect-files');
const glob = require('glob');

/**
* @typedef {import('chokidar').FSWatcher} FSWatcher
* @typedef {import('glob').Glob['patterns'][number]} Pattern
* The `Pattern` class is not exported by the `glob` package.
* Ref [link](../../node_modules/glob/dist/commonjs/pattern.d.ts).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we file an issue on glob to export it?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They probably consider this internal API. I think we should ask them to export a public API like Glob.test(file: string): boolean that we can use instead of copying so much of its internal logic into createPathFilter here.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I filed this issue, let's see what they say: isaacs/node-glob#632

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment here talks about exporting the Pattern class, but the issue talks about exporting a glob.test function, I think there was a disconnect here. Regardless, not a blocker to this PR :)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was saying above, the Pattern class seems internal (I doubt they would be willing to export it). You're welcome to ask them if they'll export it though

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I posted a comment on the issue (isaacs/node-glob#632 (comment)) and it looks like the logic that uses the Pattern class for path parsing could be improved by using minimatch directly. Not sure when I'll have time to refactor it (I'll probably create a separate issue and PR), but I think for now the current implementation should still work as intended.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thing to consider: the logic copied from glob for dealing with patterns is a bit nontrivial, and glob has hopefully tested all possible cases that can happen with it, but we probably aren't testing them all here.

This is part of why I think the logic deserves to live in a reusable package.

If I have time soon I may make a proper package for working with multiple patterns, that puts them into a tree for more efficient matching and gives us a getTopDirs method, and has thorough unit tests.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made some changes to the path parsing logic to use Minimatch instead of Glob Pattern and while the tests seem to pass (after handling only a few of the differences in behavior), I came to the conclusion that using Glob Patterns is better as it produces more accurate path segments, especially for Windows paths.

It adds slashes for drives (e.g., C: to C:/). Getting the path segments of C:\Users\user for example:

  • Minimatch path segments: [ 'C:', 'Users', 'user' ]
  • Glob Pattern path segments: [ 'C:/', 'Users', 'user' ]

Small note: Glob uses slashes / even for Windows.

path.resolve() results:

// via NodeJS Windows

// using path segments from minimatch
console.log(path.resolve("C:\\a\\b\\c", "C:", "Users", "user"));
// output: C:\a\b\c\Users\user

// using path segments from glob pattern (more accurate)
console.log(path.resolve("C:\\a\\b\\c", "C:/", "Users", "user"));
// output: C:\Users\user

UNC paths also produce different path segments. Using \\host\share\a\b\c for example:

  • Minimatch path segments: [ '', '', 'host', 'share', 'a', 'b', 'c' ]
  • Glob Pattern path segments: [ '//host/share/', 'a', 'b', 'c' ]

Glob Pattern treats //host/share/ as one segment. path.resolve() seems to handle this case as well:

// via NodeJS Windows

// using path segments from minimatch
console.log(path.resolve('C:', '', '', 'host', 'share', 'a', 'b', 'c'));
// output: C:\host\share\a\b\c

// using path segments from glob pattern (more accurate)
console.log(path.resolve('C:', '//host/share/', 'a', 'b', 'c'));
// output: \\host\share\a\b\c

Refactoring to Minimatch as I previously suggested isn't the right call. Using UNC paths as watch paths probably won't make sense, but I guess the option is there so long as Chokidar is capable of watching those paths.

There may be more cases that Glob Pattern handles that I haven't looked into. In my opinion, we can keep the current way Pattern is referenced. It's hard to say if Pattern needs to be exported from the Glob package since we really only need a reference to the type and not the class itself just for explicitly typing a few variables.

* @typedef {import('../mocha.js')} Mocha
* @typedef {import('../types.d.ts').BeforeWatchRun} BeforeWatchRun
* @typedef {import('../types.d.ts').FileCollectionOptions} FileCollectionOptions
* @typedef {import('../types.d.ts').Rerunner} Rerunner
* @typedef {import('../types.d.ts').PathPattern} PathPattern
* @typedef {import('../types.d.ts').PathFilter} PathFilter
* @typedef {import('../types.d.ts').PathMatcher} PathMatcher
*/

/**
Expand Down Expand Up @@ -145,44 +153,240 @@ exports.watchRun = (mocha, {watchFiles, watchIgnore}, fileCollectParams) => {
});
};

class GlobFilesTracker {
constructor(watchFiles, watchIgnore) {
this.watchFilesSet = new Set();
this.watchFiles = watchFiles;
this.watchIgnore = watchIgnore;
}
/**
* Extracts out paths without the glob part, the directory paths,
* and the paths for matching from the provided glob paths.
* @param {string[]} globPaths The list of glob paths to create a filter for.
* @param {string} basePath The path where mocha is run (e.g., current working directory).
* @returns {PathFilter} Object to filter paths.
* @ignore
* @private
*/
function createPathFilter(globPaths, basePath) {
debug('creating path filter from glob paths: %s', globPaths);

/**
* The resulting object to filter paths.
* @type {PathFilter}
*/
const res = {
dir: {paths: new Set(), globs: new Set()},
match: {paths: new Set(), globs: new Set()}
};

regenerate() {
const watchIgnoreSet = new Set();
for (const pattern of this.watchIgnore) {
glob.sync(pattern, { dot: true }).forEach(filePath => watchIgnoreSet.add(filePath));
}
// for checking if a path ends with `/**/*`
const globEnd = path.join(path.sep, '**', '*');

const globOpts = {
/**
* The current glob pattern to check.
* @type {Pattern[]}
*/
const patterns = globPaths.flatMap(globPath => {
return new glob.Glob(globPath, {
dot: true,
ignore: {
childrenIgnored: pathToCheck => watchIgnoreSet.has(pathToCheck.relative())
magicalBraces: true,
windowsPathsNoEscape: true
}).patterns;
}, []);

// each pattern will have its own path because of the `magicalBraces` option
for (const pattern of patterns) {
debug('processing glob pattern: %s', pattern.globString());

/**
* Path segments before the glob pattern.
* @type {string[]}
*/
const segments = [];

/**
* The current glob pattern to check.
* @type {Pattern | null}
*/
let currentPattern = pattern;
let isGlob = false;

do {
// save string patterns until a non-string (glob or regexp) is matched
const entry = currentPattern.pattern();
const isString = typeof entry === 'string';
debug(
'found %s pattern: %s',
isString ? 'string' : 'glob or regexp',
entry
);
if (!isString) {
// if the entry is a glob
isGlob = true;
break;
}
};

this.watchFilesSet.clear();
for (const pattern of this.watchFiles) {
glob.sync(pattern, globOpts).forEach(pathToCheck => {
if (watchIgnoreSet.has(pathToCheck)) {
return;
}
this.watchFilesSet.add(pathToCheck);
});
segments.push(entry);

// go to next pattern
} while ((currentPattern = currentPattern.rest()));
if (!isGlob) {
debug('all subpatterns of %j processed', pattern.globString());
}

// match `cleanPath` (path without the glob part) and its subdirectories
const cleanPath = path.resolve(basePath, ...segments);
debug('clean path: %s', cleanPath);
res.dir.paths.add(cleanPath);
res.dir.globs.add(path.resolve(cleanPath, '**', '*'));

// match `absPath` and all of its contents
const absPath = path.resolve(basePath, pattern.globString());
debug('absolute path: %s', absPath);
(isGlob ? res.match.globs : res.match.paths).add(absPath);

// always include `/**/*` to the full pattern for matching
// since it's possible for the last path segment to be a directory
if (!absPath.endsWith(globEnd)) {
res.match.globs.add(path.resolve(absPath, '**', '*'));
}
}

has(filePath) {
return this.watchFilesSet.has(filePath)
debug('returning path filter: %o', res);
return res;
}

/**
* Checks if the provided path matches with the path pattern.
* @param {string} filePath The path to match.
* @param {PathPattern} pattern The path pattern for matching.
* @param {boolean} [matchParent] Treats the provided path as a match if it's a valid parent directory from the list of paths.
* @returns {boolean} Determines if the provided path matches the pattern.
* @ignore
* @private
*/
function matchPattern(filePath, pattern, matchParent) {
if (pattern.paths.has(filePath)) {
return true;
}

if (matchParent) {
for (const childPath of pattern.paths) {
if (isPathInside(childPath, filePath)) {
return true;
}
}
}

// loop through the set of glob paths instead of converting it into an array
for (const globPath of pattern.globs) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (const globPath of pattern.globs) {
// provides minor perf increase
for (const globPath of pattern.globs) {

if (
minimatch(filePath, globPath, {dot: true, windowsPathsNoEscape: true})
) {
return true;
}
}

return false;
}

/**
* Creates an object for matching allowed or ignored file paths.
* @param {PathFilter} allowed The filter for allowed paths.
* @param {PathFilter} ignored The filter for ignored paths.
* @param {string} basePath The path where mocha is run (e.g., current working directory).
* @returns {PathMatcher} The object for matching paths.
* @ignore
* @private
*/
function createPathMatcher(allowed, ignored, basePath) {
debug(
'creating path matcher from allowed: %o, ignored: %o',
allowed,
ignored
);

/**
* Cache of known file paths processed by `matcher.allow()`.
* @type {Map<string, boolean>}
*/
const allowCache = new Map();

/**
* Cache of known file paths processed by `matcher.ignore()`.
* @type {Map<string, boolean>}
*/
const ignoreCache = new Map();

const MAX_CACHE_SIZE = 10000;

/**
* Performs a `map.set()` but will delete the first key
* for new key-value pairs whenever the limit is reached.
* @param {Map<string, boolean>} map The map to use.
* @param {string} key The key to use.
* @param {boolean} value The value to set.
*/
function cache(map, key, value) {
// only delete the first key if the key doesn't exist in the map
if (map.size >= MAX_CACHE_SIZE && !map.has(key)) {
map.delete(map.keys().next().value);
}
map.set(key, value);
}

/**
* @type {PathMatcher}
*/
const matcher = {
allow(filePath) {
let allow = allowCache.get(filePath);
if (allow !== undefined) {
return allow;
}

allow = matchPattern(filePath, allowed.match);
cache(allowCache, filePath, allow);
return allow;
},

ignore(filePath, stats) {
// Chokidar calls the ignore match function twice:
// once without `stats` and again with `stats`
// see `ignored` under https://github.com/paulmillr/chokidar?tab=readme-ov-file#path-filtering
// note that the second call can also have no `stats` if the `filePath` does not exist
// in which case, allow the nonexistent path since it may be created later
if (!stats) {
return false;
}

// resolve to ensure correct absolute path since, for some reason,
// Chokidar paths for the ignore match function use slashes `/` even for Windows
filePath = path.resolve(basePath, filePath);

let ignore = ignoreCache.get(filePath);
if (ignore !== undefined) {
return ignore;
}

// `filePath` ignore conditions:
// - check if it's ignored from the `ignored` path patterns
// - otherwise, check if it's not ignored via `matcher.allow()` to also cache the result
// - if no match was found and `filePath` is a directory,
// check from the allowed directory paths if it's a valid
// parent directory or if it matches any of the allowed patterns
// since ignoring directories will have Chokidar ignore their contents
// which we may need to watch changes for
ignore =
matchPattern(filePath, ignored.match) ||
(!matcher.allow(filePath) &&
(!stats.isDirectory() || !matchPattern(filePath, allowed.dir, true)));

cache(ignoreCache, filePath, ignore);
return ignore;
}
};

return matcher;
}

/**
* Bootstraps a chokidar watcher. Handles keyboard input & signals
* Bootstraps a Chokidar watcher. Handles keyboard input & signals
* @param {Mocha} mocha - Mocha instance
* @param {Object} opts
* @param {BeforeWatchRun} [opts.beforeRun] - Function to call before
Expand All @@ -206,36 +410,46 @@ const createWatcher = (
watchFiles = fileCollectParams.extension.map(ext => `**/*.${ext}`);
}

debug('watching files: %s', watchFiles);
debug('ignoring files matching: %s', watchIgnore);
let globalFixtureContext;

// we handle global fixtures manually
mocha.enableGlobalSetup(false).enableGlobalTeardown(false);

const tracker = new GlobFilesTracker(watchFiles, watchIgnore);
tracker.regenerate();

const watcher = chokidar.watch('.', {
ignoreInitial: true
// glob file paths are no longer supported by Chokidar since v4
// first, strip the glob paths from `watchFiles` for Chokidar to watch
// then, create path patterns from `watchFiles` and `watchIgnore`
// to determine if the files should be allowed or ignored
// by the Chokidar `ignored` match function

const basePath = process.cwd();
const allowed = createPathFilter(watchFiles, basePath);
const ignored = createPathFilter(watchIgnore, basePath);
const matcher = createPathMatcher(allowed, ignored, basePath);

// Chokidar has to watch the directory paths in case new files are created
const watcher = chokidar.watch(Array.from(allowed.dir.paths), {
ignoreInitial: true,
ignored: matcher.ignore
Comment thread
mark-wiemer marked this conversation as resolved.
Resolved
});

const rerunner = createRerunner(mocha, watcher, {
beforeRun
});

watcher.on('ready', async () => {
debug('watcher ready');
if (!globalFixtureContext) {
debug('triggering global setup');
globalFixtureContext = await mocha.runGlobalSetup();
}
rerunner.run();
});

watcher.on('all', (event, filePath) => {
if (event === 'add') {
tracker.regenerate();
}
if (tracker.has(filePath)) {
watcher.on('all', (_event, filePath) => {
// only allow file paths that match the allowed patterns
if (matcher.allow(filePath)) {
rerunner.scheduleRun();
}
});
Expand Down Expand Up @@ -294,7 +508,7 @@ const createWatcher = (
* Create an object that allows you to rerun tests on the mocha instance.
*
* @param {Mocha} mocha - Mocha instance
* @param {FSWatcher} watcher - chokidar `FSWatcher` instance
* @param {FSWatcher} watcher - Chokidar `FSWatcher` instance
* @param {Object} [opts] - Options!
* @param {BeforeWatchRun} [opts.beforeRun] - Function to call before `mocha.run()`
* @returns {Rerunner}
Expand Down Expand Up @@ -353,9 +567,9 @@ const createRerunner = (mocha, watcher, {beforeRun} = {}) => {
};

/**
* Return the list of absolute paths watched by a chokidar watcher.
* Return the list of absolute paths watched by a Chokidar watcher.
*
* @param watcher - Instance of a chokidar watcher
* @param watcher - Instance of a Chokidar watcher
* @return {string[]} - List of absolute paths
* @ignore
* @private
Expand Down Expand Up @@ -399,7 +613,7 @@ const eraseLine = () => {

/**
* Blast all of the watched files out of `require.cache`
* @param {FSWatcher} watcher - chokidar FSWatcher
* @param {FSWatcher} watcher - Chokidar FSWatcher
* @ignore
* @private
*/
Expand Down
Loading
Loading