[0.73.x] Hotfix release 0.73.9 - sync watcher backends with 0.76.0 #949
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: Some lightweight modernising of `metro-file-map` watcher backends to allow the cleaner addition of an another async step (`readLink`), to follow. (Re `graceful-fs` -> `fs` in `FSEventsWatcher` - this only affects the call to `lstat`, which is [wrapped by `graceful-fs`](https://l.facebook.com/l.php?u=https%3A%2F%2Fgithub.com%2Fisaacs%2Fnode-graceful-fs%2Fblob%2F1f19b0b467e4144260b397343cd60f37c5bdcfda%2Fpolyfills.js%23L292&h=AT1P8P4IV8T7USaqoeaSrADFV0TYg4fSes1ACxBJTf1xD0thUYMKLdwbJ9DsI-jp3yG-j52_EJE7rec2VOa0H_0k5Prll3CAT747p8IGaxvZrscX40tFFtsWIB4OAB695JLank07cGR4vH57l13fVuxo) *only* to fix a very old `guid`/`uid` reporting issue irrelevant to us - in short, `graceful-fs` isn't adding anything here). Changelog: [Internal] Reviewed By: jacdebug Differential Revision: D41522808 fbshipit-source-id: 03819fc489710d3091505a83a3babfa5aceaf4a7
Summary: Refactor watcher integration tests to extract a lot of supporting code for clarity, and add some fixtures *before* creating the Watcher in order to test behaviour when pre-existing files are modified or deleted. Use those fixtures for two additional tests. (Motivation: Similar tests for symlinks have inconsistent results, fixed in the next diff) Changelog: [Internal] Reviewed By: jacdebug Differential Revision: D42110285 fbshipit-source-id: 0721cbc5a41ce3eddef7ff4c4aff0b7ffff34156
Summary: Currently, symlinks are reported (similarly to regular files) by all watcher backends when added or removed (if not ignored), *except* in some specific cases: - Symlinks discovered when adding a new subtree are not always reported by `NodeWatcher`. - Symlinks deleted are not reported by `FSEventsWatcher` or `NodeWatcher`. That's simply because we use `walker` to scan files, and it distinguishes symlinks from regular files - we were not listening to the `symlink` event. - In the case of symlinks in very new directories, that meant we didn't emit an `add` event unless we already had a watch on the parent - the scan is expected to race against the recursive watch, so this was non-deterministic. - In the case of symlink deletions, these were not reported by `NodeWatcher` or `FSEventsWatcher` because those rely on an internal set of "tracked" files provided by `walker`, and don't emit events for untracked files that no longer exist. This diff adds a listener for discovered symlinks and tests to verify pre-existing symlinks correctly have their deletion reported. Changelog: [Internal] *(symlink changes are not yet propagated out of `metro-file-map`, so this fix isn't technically observable)* Reviewed By: jacdebug Differential Revision: D41774723 fbshipit-source-id: 0e9e7d0ecd0bdfbb316af7657e7f2599b5dfb49d
Summary: While checking some `metro-file-map` behaviour on Windows I noticed the Watchman detection we use in integration tests was all wrong :) - `os.platform()` returns `win32` on Windows, not `windows`. - `where` works in a command prompt but not PowerShell - there it's usually an alias for `Where-Object`. `where.exe` works on both. - `where.exe` prints "INFO: Could not find files for the given pattern(s)" to stderr on no match. Use `/Q` for silent running - the exit code is all we need. Reviewed By: motiz88 Differential Revision: D42266748 fbshipit-source-id: 4494cd39dd94da7dc17442264a3828a35aaf9c30
Summary: The various "ignore" mechanisms in `metro-file-map` are in a bit of tangle - we have explicit and implicit glob ignores, a set of regex arrays, and a `dot` parameter to exclude dotfiles. This is a small step in the right direction by removing the `hasIgnore` variable that's always true for Metro, and has odd side effects when it's false. Changelog: [Internal] (There's no observable effect on Metro since we always set `ignorePattern`, and so `hasIgnore` is always `true`) Reviewed By: motiz88 Differential Revision: D43234562 fbshipit-source-id: 166dca72774f3d6c3fb8c6825bf1a8d429439999
… events Summary: Metro uses the `glob` filter of the Watcher backends to include: - `package.json` - Files with a watched extension - Health check files All of these only make sense to apply against regular files - all directory events are ignored downstream, and symlinks must be included because they may (at some point) point to a directory. Separately, we have an `ignorePattern` to exclude source control patterns and the user-provided `blockList`. It continues to make sense to apply `ignorePattern` to all events, and for some backends it helps avoid walking ignored subtrees unnecessarily. Currently, backends apply both the glob filter and ignore pattern to all files. This diff: - Modifies tests to more closely resemble the way Metro uses the backends - providing a glob filter to allowlist file extensions. - Adds a `type` parameter to `common.isFileIncluded` (-> `isIncluded`) so that glob patterns are only applied to known regular files. D43214089 follows by making similar changes downstream on the `Watcher`. Changelog: [Internal] Reviewed By: motiz88 Differential Revision: D43234543 fbshipit-source-id: f9f2289c74e7e0d4c119eee05e0ede76fe7230d4
facebook-github-bot
added
the
CLA Signed
robhogan
changed the title
motiz88
approved these changes
Mar 21, 2023
| @@ -207,6 +207,33 @@ describe.each(Object.keys(WATCHERS))( | |||
| }); | |||
| }); | |||
|
|
|||
| maybeTest('detects changes to files in a new directory', async () => { | |||
There was a problem hiding this comment.
I assume you're planning to add this test on main? Or do we have equivalent coverage there by now?
There was a problem hiding this comment.
Yep - diff incoming.
huntie
approved these changes
Mar 21, 2023
jacdebug
approved these changes
Mar 21, 2023
facebook-github-bot
pushed a commit
that referenced
this pull request
Mar 21, 2023
Summary: Cherry pick the new integration test covering facebook/react-native#36387 from #949 Pull Request resolved: #950 Reviewed By: huntie Differential Revision: D44254000 Pulled By: robhogan fbshipit-source-id: 10d202ec153b55e8ecbe282d0b907fabee0fe5b6
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
Cherry picked from https://github.com/facebook/metro/commits/v0.76.0/packages/metro-file-map/src/watchers :
New commits:
node ./scripts/updateVersion.js 0.73.9Test plan
resolutions) before proposing bringing it in as a CLI hotfix.