rstackjs / rstackjs/rspack-resolver

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.

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

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

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.

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.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.