metro icon indicating copy to clipboard operation
metro copied to clipboard

Fix `NativeWatcher` not ingesting hardlinked/cloned directories (Bun/pnpm installs) due to omitted events

Open kitten opened this issue 4 months ago • 3 comments

Summary

Hardlinking/cloning is increasingly common in package managers, when a file system allows for efficient copy-on-write hardlinks/clones. Bun and pnpm use this for dependency installations.

When installing a new package, package managers may create a hardlink of a package in node_modules (or an isolated dependencies sub-folder) that is a clone of a Node module directory in their cache folders. When this happens, since a copy-on-write clone operation may have occurred, only a single file-system event is emitted for the entire directory. This reproduces consistently on APFS.

The NativeWatcher does not recognise this and won't emit events for the cloned directory's contents, which means that the change events aren't successfully updating the FileMap.

Instead, we can:

  • batch change events in a macro-tick
  • detect hardlink directory candidates without any present contents
  • crawl these directories manually and emit events for them

Changelog: [Fix] Fix added directories not being detected properly by the NativeWatcher when they're cloned using a hardlink/copy-on-write operation (typically, as Bun or pnpm install new packages)

Test plan

  • Start up a project with metro (e.g. an Expo project) that misses a package or start using an uninstalled package (e.g. expo-image)
    • Setting DEBUG=Metro:NativeWatcher to observe watcher events
  • Bundling should observe that this package does not resolve
  • With Bun, bun add expo-image, or pnpm, pnpm add expo-image
  • Without the fix: We can observe that directory events are reported for the hardlinked directories without sub-directory/file events
  • With the fix: Traversal adds events for the added sub-directories/files

I've tested that removing the package again or modifying it works as expected. I didn't observe duplicate events, but I believe the parent classes filter out duplicates, if any occur.

kitten avatar Aug 17 '25 15:08 kitten

I've pushed a new implementation that attempts to avoid duplication by adding an input/output batch. This now skips hardlink directory candidates from being crawled, if the input batch contains any subpaths. This seems to behave much more predictably and the integration tests are passing with this change

kitten avatar Aug 17 '25 18:08 kitten

This looks awesome @kitten - thank you!

The NativeWatcher does not recognise this and won't emit events for the cloned directory's contents, which means that the change events aren't successfully updating the FileMap.

Do you happen to know if this is - a) A regression since https://github.com/facebook/metro/pull/1420 ? (Evan mentioned issues installing packages with bun around that time) b) a difference in behaviour from Watchman or the FallbackWatcher?

We could do with some integration tests using hard links with the different backends but you can leave that to me - thanks again for the PR and I'll make some time for a detailed review and testing this week.

robhogan avatar Aug 19 '25 16:08 robhogan

It's b). Specifically, for some syscalls it seems like fs.watch will only receive a directory change event

kitten avatar Aug 19 '25 16:08 kitten