Skip to content

Conversation

@stormslowly
Copy link
Contributor

@stormslowly stormslowly commented Oct 16, 2025

Why

for issue web-infra-dev/rspack#11239 (comment)
When rspack-resolver failed, the missing files were just recorded by symbol-linked paths.
In some cases, Watchpack will not emit FS change events when symbol-linked files are changed.

Solution

When a missing file is detected, add its "real" path to the missing file dependencies.
Since the file does not exist in the file system, we attempt to canonicalize it by its ancestors' path, then assemble a real path.

For example:
/root/app/node_module/lib/dist/esm/index.mjs is not existed.

/root/app/node_module/lib/ is symbol linked to real path /root/lib/

So the real path is /root/lib/dist/esm/index.mjs

We get the both symbolic path and real path in missing files in a failure resolving, so it makes watchpack to watch the right files.

Copilot AI review requested due to automatic review settings October 16, 2025 05:56
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR resolves an issue where missing files that are accessed through symbolic links are not properly tracked for file watching. When symlinks are enabled and a file resolution fails, the resolver now attempts to determine the real path of the missing file by traversing its parent directories to find an existing symlinked ancestor.

  • Adds logic to resolve symbolic link paths for missing files when symlinks option is enabled
  • Introduces a new is_file method that handles missing dependency tracking for symlinked files
  • Replaces direct is_file calls with the new method that handles symlink resolution

Reviewed Changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 4 comments.

File Description
src/lib.rs Implements symlink resolution for missing files and refactors file existence checking
src/tests/missing.rs Adds test case for missing files in symbol-linked folders
fixtures/pnpm-workspace/packages/missing/package.json Creates test fixture package with missing files
fixtures/pnpm-workspace/packages/app/package.json Adds dependency on missing package for testing
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

src/lib.rs Outdated
Comment on lines 719 to 721
let right_path = path.strip_prefix(ancestor).unwrap();

return Some(real_path.join(right_path));
Copy link

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

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

Using unwrap() can cause panics. Consider using proper error handling or a more descriptive error message if this operation is expected to always succeed.

Suggested change
let right_path = path.strip_prefix(ancestor).unwrap();
return Some(real_path.join(right_path));
if let Ok(right_path) = path.strip_prefix(ancestor) {
return Some(real_path.join(right_path));
}

Copilot uses AI. Check for mistakes.
@codspeed-hq
Copy link

codspeed-hq bot commented Oct 16, 2025

CodSpeed Performance Report

Merging #109 will degrade performances by 33.48%

Comparing fix/missng_files_resolve_to_real (457f0ed) with main (8124bf3)

Summary

❌ 2 regressions
✅ 3 untouched

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
resolver[resolve from symlinks multi thread] 138.3 ms 207.9 ms -33.48%
resolver[resolve from symlinks] 365.5 ms 439.9 ms -16.91%

@stormslowly stormslowly requested a review from hardfist October 16, 2025 09:33
@stormslowly stormslowly force-pushed the fix/missng_files_resolve_to_real branch from 201f25e to f6fac32 Compare October 17, 2025 07:18
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