Use Node-native recursive fs.watch on macOS and Windows, replace FSEventsWatcher and fsevents dependency #1420
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: Pull Request resolved: #1417 Refactor watcher backends so that they're not required to disambiguate "add" file events from "change" file events, instead emitting ambiguous "touch" events. This distinction isn't typically available from the host OS (`fsevents`, `ReadDirectoryChangesW`, `inotify`), so for the backends to supply it, they must track all files to know whether they existed prior to the OS event. Watchman does this by design anyway, and typically only once per session, but both our `FSEventsWatcher` and `NodeWatcher` currently crawl the whole directory tree and keep an in-memory list of files, and must do so on each Metro start. We don't need this information from the watchers anyway, because we can already infer new vs modified according to whether the file is present in `TreeFS` (ie, from the crawl, or subsequent event). Therefore we *don't* need to reduce the granularity of `metro-file-map`'s *public* events, only the internal ones. By simplifying this, we can follow up by taking a huge burden off the watcher backends. Changelog: Internal Differential Revision: D67579233
Summary: The include/exclude logic in `metro-file-map` is a bit of a mess, but roughly it's meant to work as follows: - Paths matching `ignorePattern`, a `?RegExp`, are not crawled (added to `TreeFS`) and do not result in emitted events. Note that `ignorePattern` may match directories, in order to ignore anything inside them. This is important to prevent crawling whole subtrees unnecessarily. - *Regular files* must match the given `globs` (`Array<string>`). Directories and symlinks are *not* filtered by glob. Metro uses `globs` to limit the file extensions crawled and watched, eg `**/*.js`. (Note that there are all sorts of caveats here - it's not consistent whether we match whole paths, project-relative paths, or watch-root relative paths, nor whether we normalise to posix separators before matching. This needs properly unpicking.. some other time.) Because `globs` are only applied to regular files, we must `lstat` a changed path to know whether we should filter it with `globs`. However, `ignorePattern` sometimes allows us to filter out a path early, before `lstat`. By splitting out `isIncluded` from `doIgnore` (and tweaking the naming a bit for clarity), we can avoid some unnecessary I/O. Changelog: ``` - **[Performance]**: Don't stat ignored files in fsevents watcher. ``` Differential Revision: D67593664
Summary: Following D67579233, we no longer make the distinction between new and changed files in events from watcher *backends*. For `FSEventsWatcher` (macOS), this straightforwardly means we can stop "tracking" files in the watcher, as the only purpose was to disambiguate new/changed, and to not emit deletion events for untracked files (which is already handled downstream for all watchers in `index.js`). Changelog ``` - **[Performance]**: Don't walk project on startup when using fsevents watcher on macOS ``` Differential Revision: D67579239
Summary: Some very light mostly-Flow tidying up to constrain inputs to watcher backends to match what `metro-file-map` actually uses. This allows us to fold the weird "assignOptions" function, that mutates backend public props, into backend constructors, without too much duplication (IMO, worth the duplication this adds to make things more explicit and easier to reason about - eg we can mark props read-only). Changelog: Internal Differential Revision: D67579909
Differential Revision: D67636697
…prove type safety Differential Revision: D67622921
Differential Revision: D67634394
…entsWatcher and fsevents dependency
Summary:
Replaces the `FSEventsWatcher` with `fs.watch(dir, {recursive: true})` ("`NativeWatcher`"), allowing us to remove a dependency and open this up to all macOS and Windows users.
Why now? Why is this better? That depends on the platform...
## macOS
When `FSEventsWatcher` was written, Node.js didn't support OS-native recursive watch on macOS (Node.js supported the `recursive` flag, but the implementation inefficiently opened a file descriptor for each watched file - see libuv/libuv#2082).
Therefore, we used the `fsevents` library to bind to the native macOS API, and achieved cheap recursive watching at the cost of a slightly awkward optional dependency.
Now, Node.js effectively does the same thing behind the scenes that we've always done, using the [OS FSEvents primitives](https://developer.apple.com/documentation/coreservices/file_system_events), and there's no need for us to use the `fsevents` npm package.
## Windows
Similarly, (this bit of Jest and) Metro pre-dates Node's support of Windows native recursive watching, introduced to libuv [as recently as 2015](libuv/libuv#421). We've never had a good alternative on Windows though, and simply fell back to walking the whole tree and watching each directory individually (`FallbackWatcher`).
## Linux
Unfortunately, linux has no native primitive for recursive watching, and Node.js's attempts to patch over that are still in flux, [with no great options]( nodejs/node#48437). Watchman, or our `FallbackWatcher`, remain our best options on Linux.
(That's likely to continue to be the case even if `Node.js` settles on a `'ready'` event or async API, because by controlling the walk we can avoid watching ignored subtrees.)
Changelog:
```
- **[Performance]**: Use fast, recursive watch on Windows, and on macOS regardless of optional dependency installation.
```
Differential Revision: D67636998
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D67636998 |
|
This pull request has been merged in 11a1f97. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Replaces the
FSEventsWatcherwithfs.watch(dir, {recursive: true})("NativeWatcher"), allowing us to remove a dependency and open this up to all macOS and Windows users.Why now? Why is this better? That depends on the platform...
macOS
When
FSEventsWatcherwas written, Node.js didn't support OS-native recursive watch on macOS (Node.js supported therecursiveflag, but the implementation inefficiently opened a file descriptor for each watched file - see libuv/libuv#2082).Therefore, we used the
fseventslibrary to bind to the native macOS API, and achieved cheap recursive watching at the cost of a slightly awkward optional dependency.Now, Node.js effectively does the same thing behind the scenes that we've always done, using the OS FSEvents primitives, and there's no need for us to use the
fseventsnpm package.Windows
Similarly, (this bit of Jest and) Metro pre-dates Node's support of Windows native recursive watching, introduced to libuv as recently as 2015. We've never had a good alternative on Windows though, and simply fell back to walking the whole tree and watching each directory individually (
FallbackWatcher).This change should mean watching starts up near-instantaneously and cheaply, using
ReadDirectoryChangesWvialibuv: https://github.com/libuv/libuv/blob/v1.49.2/src/win/fs-event.c#L46Linux
Unfortunately, linux has no native primitive for recursive watching, and Node.js's attempts to patch over that are still in flux, with no great options. Watchman, or our
FallbackWatcher, remain our best options on Linux.(That's likely to continue to be the case even if
Node.jssettles on a'ready'event or async API, because by controlling the walk we can avoid watching ignored subtrees.)Changelog:
Differential Revision: D67636998