fix: watch mode using chokidar v4 (#5379)

* fix: watch mode using chokidar v4 (#5355)

Glob paths are no longer supported by chokidar starting at v4.

This update works around this by resolving `watchFiles` and `watchIgnore`
to valid paths and creating a list of matchers to determine if the files
should be allowed or ignored through the chokidar `ignored` match function.

This commit reverts changes from 8af0f1a
so that the watched file changes are not fixed to the current directory.

Additional note: when a `watchFile` path is removed while chokidar is
watching it, recreating the `watchFile` path does not trigger events
from chokidar to rerun the tests.

* refactor: fix watch mode using chokidar v4 (#5355)

This update fixes #5355 and refactors the previous watch mode fix attempt.

Instead of only filtering the paths in the chokidar `ignored` match
function, the chokidar `all` event handler now has a guard to ensure
that only file paths that match the allowed patterns from `watchFiles`
would trigger test reruns.

Doing this solves the issue where creating/deleting directories trigger
test reruns despite the watched path being a file (e.g., `**/*.xyz`)
as the allowed patterns would not match with just the directory paths.

* docs: fix jsdoc description for `normalizeGlob()`

* test: add tests for watch mode

- Test for watched files from outside the current working directory
- Test reruns with a thousand watched files

* test: fix test for multiple watched files

* test: remove test for multiple watched files

The test is dependent on how fast the machine could create 1000 watched
files, otherwise the test could fail due to a timeout error. Reducing
the number of watched files would make the test pass, but doing so would
make it more or less similar to already existing tests. At that point,
the test does not really test the limits of possibly huge projects with
thousands of watched files, so it has been removed instead.

* fix: update watch mode implementation

This update consolidates the function for retrieving the paths and globs
that will be used for the `watchFiles` and `watchIgnore` paths.
This should prevent path patterns from being duplicated and should now
also handle and watch paths under the provided glob paths.

Allowed and ignored file path results are now cached to a map (with a
size limit of 1000) to skip re-matching already matched file paths.

Additional tests for `watch.spec.js`:

- Test file and directory paths under `--watch-files` (no glob pattern)
- Test exact file path matches from `--watch-files`
- Test `--watch-files` starting and ending with a glob pattern
- Test `--watch-files` with a glob pattern in between

* Refactor, add debug

* Remove verbose debug

* Add debugs

* Remove castArray, capitalize Chokidar

* Replace for-of with Array.some

* Add test for false literal matches

* Document new test

* Remove unused function

* Remove createPathFilter and createPathMatcher from exports

* refactor: update watch mode

- Use array `flatMap` instead of `reduce` for glob path patterns
- Revert glob path matching check to a `for` loop (commit caca650)
- Update `MAX_CACHE_SIZE` from 1,000 to 10,000
- Update map cache to only delete the first key whenever the limit is reached
- Update debug logging and formatting

* chore: move watch mode callbacks and object typedefs to the new types.d.ts

* Apply suggestion from @JoshuaKGoldberg

Co-authored-by: Josh Goldberg ✨ <[email protected]>

---------

Co-authored-by: Josh Goldberg ✨ <[email protected]>
Co-authored-by: Mark Wiemer <[email protected]>
Co-authored-by: Mark Wiemer <[email protected]>

Author: 3 people

lib/cli/watch-run.js

@@ -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).
  * @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
  */
 
 /**
@@ -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) {
+    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
@@ -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
   });
 
   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();
     }
   });
@@ -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}
@@ -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
@@ -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
  */