is_dir misses missing_dependency when path exists as a non-directory (cold-path bug, diverges from enhanced-resolve)

Open
#239 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
76/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Quiet
Tech stack
rust
Domain
devtools

Research direction

Start in src/cache.rs at CachedPathImpl::is_dir, then inspect its call sites, especially the node_modules walk and find_package_json. Extend src/tests/dependencies.rs::warm_cache_missing_dependencies with an ancestor regular file named node_modules and run the dependency tests; done means cold and warm missing_dependencies both contain it and remain equal.

Written by the indexing model from the issue text.

Description

Summary

CachedPathImpl::is_dir (in src/cache.rs) only calls ctx.add_missing_dependency when fs.metadata() returns None (path absent). When the path exists but is not a directory (e.g., a regular file named node_modules), is_dir returns false without recording anything to missing_dependencies. This diverges from enhanced-resolve, which records the path in missingDependencies either way so that webpack/rspack watchers re-run resolution when the file/dir type flips.

Repro

Fixture:

/a/b/c/some.js                          (file)
/a/b/node_modules                       (REGULAR FILE — not a dir)
/a/node_modules/module/index.js         (file inside real node_modules)

Resolve module from /a/b/c.

enhanced-resolve (5.21.1) missingDependencies includes:

  • /a/b/c/node_modules (absent)
  • /a/b/node_modules (exists as file — present)

rspack-resolver cold path missing_dependencies includes:

  • /a/b/c/node_modules (absent)
  • (no entry for /a/b/node_modules)missing

Why this matters

  • Webpack/rspack file watchers won't be notified when the user later replaces /a/b/node_modules (a file) with a real directory, so resolution stays stale and incremental rebuilds don't pick up the newly-installed package.
  • It also leaves a cold/warm divergence in PR #236: that PR's warm path adds the entry (because the OnceLock folds "absent" and "non-dir" into the same None), while the cold path still doesn't.

Suggested fix

In src/cache.rs, is_dir:

pub async fn is_dir<Fs: Send + Sync + FileSystem>(&self, fs: &Fs, ctx: &mut Ctx) -> bool {
  match self.meta(fs).await {
    Some(meta) if meta.is_dir => true,
    _ => {
      ctx.add_missing_dependency(self);
      false
    }
  }
}

Needs auditing for over-tracking impact on other call sites (e.g., find_package_json's parent-walk loop), but the node_modules walk is the load-bearing case.

Test

Extend src/tests/dependencies.rs::warm_cache_missing_dependencies fixture with a regular file named node_modules at some ancestor, then assert both cold and warm missing_dependencies contain it (and remain equal).

Related: #236

Dominant language
Rust
Stars
42
Forks
10
Avg merge
3h
Merged PRs (30d)
1

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from rstackjs/rspack-resolver

All issues in rstackjs/rspack-resolver

Similar issues

More Rust issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.