-
Notifications
You must be signed in to change notification settings - Fork 693
Conversation
## Summary
`metro-file-map` depends on `walker` for `FallbackWatcher`'s initial crawl and for nothing else - a single call site in `recReaddir`. It's ~120 lines of recursive `lstat`/`readdir`, unmaintained since 2021, untyped (we carry a `$FlowFixMe[untyped-import]` for it), and it pulls in `makeerror` and `tmpl` purely to construct one error class we never inspect.
This diff brings it in-tree at `packages/metro-file-map/src/third-party/walker.js`, following the `buck-worker-tool/src/third-party` convention - the Apache-2.0 notice retained as `LICENSE.APACHE2` alongside it, and a `Portions (c) Meta Platforms` header over Metro's own MIT notice. Behaviour is preserved: the same event set (`entry` followed by a type-specific event, plus `error` and `end`), the same `filterDir` semantics, the same pending-counter completion, and the same `lstat` dispatch order - which matters, because `isFile()` is tested last and only after the device types. It's modernised where Flow required it: an ES class composing an `EventEmitter` rather than `util.inherits`, `node:fs`/`node:path`, and a local `UnknownFileTypeError` subclass in place of `makeerror`.
`on()` is typed against a keyed listener map, so both the event name and the listener signature are checked at the call site. I checked that isn't vacuous - renaming `'dir'` to `'dirr'` in `recReaddir` gives `property 'dirr' (did you mean 'dir'?) is missing in EventListeners`.
Two consequences worth calling out:
- Because `walker` required the unprefixed `'fs'`, `resolver-test.js` carried a `jest.mock('fs', ...)` aliasing it to the same in-memory fs as `node:fs`. The vendored copy uses `node:fs`, so that goes.
- Apache-2.0 source is now in-tree in an MIT repo. It was already in the dependency graph and it ships either way, but this puts it in the tarball as our own source, so it's worth a look from that angle. `LICENSE.APACHE2` is copied to `build/third-party/` by `scripts/build.js`, which copies non-JS files under `src/` verbatim.
The file is under `third-party/` rather than `vendor/` deliberately. Both are in `.eslintignore`, but only `packages/**/third-party` is in `IGNORED_PATTERNS` in `generateTypeScriptDefinitions.js` - under `vendor/` the build fails at `generateTypeScriptDefinitions.js:164` reading `lintResult.output`, because ESLint returns no result at all for an ignored file. So `third-party/` is the name both configs already agree on, and this way needs no config change.
`walker` stays in `yarn.lock` under `jest-haste-map`, which is dev-only, so no published Metro package depends on it (or on `makeerror`/`tmpl`) any more.
Changelog: [Internal] Vendor the `walker` dependency into `metro-file-map`
## Test plan
```
yarn flow check
yarn typecheck-ts
yarn lint
yarn verify-api-snapshots
yarn build
yarn jest
```
All clean - full Jest run is 2662 passed, 1 skipped, across 147 suites.
`recReaddir` drives `FallbackWatcher`'s initial crawl, so `packages/metro-file-map/src/watchers/__tests__/integration-test.js` exercises the vendored code directly - 34 tests, including the `Fallback` matrix over new/changed/deleted files, symlinks (including to non-existent targets), pre-existing trees moved in and out of the watched root, and directory deletion. There's no dedicated unit test for `walker.js` itself.
One caveat: the first full run failed at `integration-test.js:92`, on the `NativeWatcher` directory-event assertion rather than `Fallback`. It passed in isolation and on a clean re-run of the whole suite, so it reads as a timing flake under parallel load, but I haven't bisected it against unmodified main to prove that.
@meta-cla
meta-cla
Bot
added
the
CLA Signed
This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
label
Sep 6, 2026
robhogan
commented
Sep 7, 2026
Collaborator
Author
Abandoning in favour of #1906
This file contains hidden or 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
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.
Uh oh!
There was an error while loading. Please reload this page.
Summary
metro-file-mapdepends onwalkerforFallbackWatcher's initial crawl only. It hasn't received updates since 2021 and pulls in two other dependencies, yet is only ~120 lines. For that alone, vendoring is preferable to the supply chain risk.There's also a known Metro bug we can't work around with
walkeras-is: directory events only fire after walker has statted their contents, so we can't attach a watch to the directory until after a short delay. Files added between the initial enumeration and the watch being attached are missed - and this is common when the directory is immediately populated eg by a package installation. This PR doesn't fix that, but it unblocks the follow up.This is a mostly verbatim port that just adds Flow typing, ESM syntax, and updates an import from
fsto Metro-conventionnode:fs.Changelog: [Internal] Vendor the
walkerdependency intometro-file-mapTest plan
All clean - full Jest run is 2662 passed, 1 skipped, across 147 suites.
recReaddirdrivesFallbackWatcher's initial crawl, sopackages/metro-file-map/src/watchers/__tests__/integration-test.jsexercises the vendored code directly - 34 tests, including theFallbackmatrix over new/changed/deleted files, symlinks (including to non-existent targets), pre-existing trees moved in and out of the watched root, and directory deletion. There's no dedicated unit test forwalker.jsitself.One caveat: the first full run failed at
integration-test.js:92, on theNativeWatcherdirectory-event assertion rather thanFallback. It passed in isolation and on a clean re-run of the whole suite, so it reads as a timing flake under parallel load, but I haven't bisected it against unmodified main to prove that.