Skip to content

Commit

Permalink
Watcher backends: simplify ignore functions, remove anymatch dependen…
Browse files Browse the repository at this point in the history
…cy (#1407)

Summary:
We currently use the [`anymatch`](https://www.npmjs.com/package/anymatch) dependency to evaluate ignore patterns in watcher backends. `anymatch` handles various input types including globs and arrays of globs/regexps

However, since D67285745, it's become clear that the `ignored` param is only ever a `?RegExp`, so we hardly use any of `anymatch` and can simply `test()` paths, and remove the dependency.

One subtlety to note that this change makes explicit, is that `anymatch` converts all input paths to posix separators before testing against the input `RegExp` (see block comment). We do the same in this diff so that the change is behaviour-preserving - however, this isn't a convention in Metro and should be revisited (maybe in a breaking change).

Note that we don't need to handle other kinds of path normalisation (like `anymatch` does with [`normalize-path`](https://www.npmjs.com/package/normalize-path)) because watcher backends must already emit normal paths, with no trailing separators. 

Changelog: Internal


Differential Revision: D67259981
  • Loading branch information
robhogan authored and facebook-github-bot committed Dec 20, 2024
1 parent a3d9e71 commit a20d658
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 14 deletions.
1 change: 0 additions & 1 deletion packages/metro-file-map/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
},
"license": "MIT",
"dependencies": {
"anymatch": "^3.0.3",
"debug": "^2.2.0",
"fb-watchman": "^2.0.0",
"flow-enums-runtime": "^0.0.6",
Expand Down
20 changes: 13 additions & 7 deletions packages/metro-file-map/src/watchers/FSEventsWatcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ import type {ChangeEventMetadata} from '../flow-types';
import type {FSEvents} from 'fsevents';

import {isIncluded, recReaddir, typeFromStat} from './common';
// $FlowFixMe[untyped-import] - anymatch
import anymatch from 'anymatch';
import EventEmitter from 'events';
import {promises as fsPromises} from 'fs';
import * as path from 'path';
Expand Down Expand Up @@ -63,7 +61,11 @@ export default class FSEventsWatcher extends EventEmitter {

constructor(
dir: string,
opts: $ReadOnly<{
{
ignored,
glob,
dot,
}: $ReadOnly<{
ignored: ?RegExp,
glob: string | $ReadOnlyArray<string>,
dot: boolean,
Expand All @@ -78,10 +80,14 @@ export default class FSEventsWatcher extends EventEmitter {

super();

this.dot = opts.dot || false;
this.ignored = opts.ignored;
this.glob = Array.isArray(opts.glob) ? opts.glob : [opts.glob];
this.doIgnore = opts.ignored ? anymatch(opts.ignored) : () => false;
this.dot = dot || false;
this.ignored = ignored;
this.glob = Array.isArray(glob) ? glob : [glob];
this.doIgnore = ignored
? // No need to normalise Windows paths to posix because this backend
// only runs on macOS, and backends always emit system-native paths.
(filePath: string) => ignored.test(filePath)
: () => false;

this.root = path.resolve(dir);

Expand Down
32 changes: 26 additions & 6 deletions packages/metro-file-map/src/watchers/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@
import type {ChangeEventMetadata} from '../flow-types';
import type {Stats} from 'fs';

// $FlowFixMe[untyped-import] - Write libdefs for `anymatch`
const anymatch = require('anymatch');
// $FlowFixMe[untyped-import] - Write libdefs for `micromatch`
const micromatch = require('micromatch');
const platform = require('os').platform();
Expand Down Expand Up @@ -74,8 +72,10 @@ export const assignOptions = function (
if (!Array.isArray(watcher.globs)) {
watcher.globs = [watcher.globs];
}
watcher.doIgnore =
opts.ignored != null ? anymatch(opts.ignored) : () => false;
const ignored = watcher.ignored;
watcher.doIgnore = ignored
? filePath => posixPathMatchesPattern(ignored, filePath)
: () => false;

if (opts.watchman == true && opts.watchmanPath != null) {
watcher.watchmanPath = opts.watchmanPath;
Expand Down Expand Up @@ -105,6 +105,21 @@ export function isIncluded(
return micromatch.some(relativePath, globs, {dot});
}

/**
* Whether the given filePath matches the given RegExp, after converting
* (on Windows only) system separators to posix separators.
*
* Conversion to posix is for backwards compatibility with the previous
* anymatch matcher, which normlises all inputs[1]. This may not be consistent
* with other parts of metro-file-map.
*
* [1]: https://github.com/micromatch/anymatch/blob/3.1.1/index.js#L50
*/
const posixPathMatchesPattern: (pattern: RegExp, filePath: string) => boolean =
path.sep === '/'
? (pattern, filePath) => pattern.test(filePath)
: (pattern, filePath) => pattern.test(filePath.replaceAll(path.sep, '/'));

/**
* Traverse a directory recursively calling `callback` on every directory.
*/
Expand All @@ -117,8 +132,13 @@ export function recReaddir(
errorCallback: Error => void,
ignored: ?RegExp,
) {
walker(dir)
.filterDir(currentDir => !anymatch(ignored, currentDir))
const walk = walker(dir);
if (ignored) {
walk.filterDir(
(currentDir: string) => !posixPathMatchesPattern(ignored, currentDir),
);
}
walk
.on('dir', normalizeProxy(dirCallback))
.on('file', normalizeProxy(fileCallback))
.on('symlink', normalizeProxy(symlinkCallback))
Expand Down

0 comments on commit a20d658

Please sign in to comment.