Skip to content

Fix crawling cloud placeholder files on Windows that readdir reports as symlinks - #2216

Open
LuisDulo wants to merge 1 commit into
react:mainfrom
LuisDulo:fix/windows-reparse-points-not-symlinks
Open

LuisDulo wants to merge 1 commit into
react:mainfrom
LuisDulo:fix/windows-reparse-points-not-symlinks

Conversation

@LuisDulo

Copy link
Copy Markdown

Summary

On Windows, fs.readdir(..., {withFileTypes: true}) reports every reparse point as a symbolic link. That includes cloud placeholders such as OneDrive and Dropbox files, which lstat correctly reports as regular files or directories. libuv's directory listing maps the reparse-point attribute to UV_DIRENT_LINK, while lstat only treats real symlinks and junctions as links.

The node crawler in metro-file-map took the dirent's word for it. With symlinks enabled, it marked those files with H.SYMLINK. #maybeReadLink then called readlink on them, which fails with EINVAL, and anything other than ENOENT/EACCESS there is fatal:

Error: EINVAL: invalid argument, readlink '...\node_modules\expo\node_modules\ci-info\package.json'
    at async DependencyGraph.ready (...\metro\src\node-haste\DependencyGraph.js:94:5)
  errno: -4071, code: 'EINVAL', syscall: 'readlink'

This is easy to hit when a project lives under OneDrive's Documents folder, the Windows default. In one Expo app's node_modules I found 57 files that readdir called symlinks; lstat said none were, and readlink failed on every one.

The crawler already calls lstat on every entry it records, so this change trusts that result:

  • an entry is recorded as a symlink only if lstat agrees,
  • a "link" that lstat says is a regular file is recorded as a file, but only if its extension is listed,
  • a "link" that lstat says is a directory is crawled like any other directory.

It adds no filesystem calls, so the existing tests' call counts and mtimes are unchanged.

Not changed here: with includeSymlinks off, entries that readdir misreports as links are still skipped before lstat runs, so such placeholders are silently left out of the map. Fixing that needs an extra lstat for each skipped link. I'm happy to follow up if you'd like that.

Possibly related: #1146 ("None of these files exist" on Windows, fixed by re-saving files, which hydrates placeholders), though I have not confirmed it.

Test plan

New test in crawlers/__tests__/node-test.js. It mocks readdir entries that all claim to be links:

  • a placeholder .js file, expected to be recorded as a plain file
  • a placeholder .md file, expected to be ignored because .md is not a listed extension
  • a placeholder directory, expected to be crawled
  • a real symlink, expected to still be recorded as a link

I wasn't able to run the full suite locally, so I'm relying on CI here.

@meta-cla

meta-cla Bot commented Nov 10, 2026

Copy link
Copy Markdown

Hi @LuisDulo!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

@LuisDulo

Copy link
Copy Markdown
Author

I have signed the CLA with this GitHub account (LuisDulo). Thanks for taking a look.

@github-actions

Copy link
Copy Markdown

Metro review: Looks good

Risk: low · Commit: 245e543 · 4 commands run, in a restricted sandbox after an automated safety assessment

On Windows, readdir reports every reparse point as a link, including cloud placeholders. With this change the node crawler records an entry as a symlink only if lstat also says it is one. Entries that lstat calls regular files are kept only when their extension is listed, and entries it calls directories are crawled. The change is small and uses the lstat call the crawler already makes, so it adds no filesystem calls. It also brings the crawler in line with FallbackWatcher and NativeWatcher, which already classify entries by lstat. The new test fails on the base and passes on the head.

Findings

Verified
  • ✅ yarn jest --no-watchman packages/metro-file-map/src/crawlers (head) — 51 tests passed, including the new one on both win32 and posix. The 2 failures are the watchman integration cases, which time out because the sandbox blocks Watchman's state directory. They are unrelated to this PR.
  • ✅ yarn jest --no-watchman packages/metro-file-map/src/crawlers/__tests__/node-test.js (base src with head's test file) (failed as expected) — Without the fix, the new test records cloud/directory and cloud/notes.md as symlinks (symlink=1) and never finds cloud/directory/inside.js.
  • ✅ yarn eslint packages/metro-file-map/src/crawlers/node/index.js packages/metro-file-map/src/crawlers/__tests__/node-test.js
  • ✅ yarn flow check — No errors.

Not run: No benchmark: the change adds no filesystem calls and only a few boolean checks per symlink entry in the crawler. No Windows run either. A hosted VM can't easily produce a cloud-filter reparse point that isn't a symlink (no OneDrive). The libuv behavior the PR relies on was checked against libuv's source instead. An early background run of the integration tests on head and base hit its time limit and was stopped. A later head run covered the same tests, but the integration tests were never run on base.

Automated review by Claude. Advisory only: it never approves or blocks, and a maintainer makes the call.

// including cloud placeholders (OneDrive, Dropbox) that lstat
// reports as regular files or directories, and that readlink
// rejects with EINVAL. Trust lstat over the dirent.
const isLink = isSymbolicLink && stat.isSymbolicLink();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit · PR description has no Changelog line

The description has a test plan but no Changelog: line. Something like Changelog: [Fix] metro-file-map: Don't treat Windows cloud placeholder files (OneDrive, Dropbox) as symlinks when crawling would do.

Evidence: Read pr.md: it has no Changelog: line. · id 65618bea

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant