Skip to content

Fix subpath resolution when package.json has no exports field#894

Closed
Copilot wants to merge 4 commits into
mainfrom
copilot/fix-subpath-resolution-error
Closed

Fix subpath resolution when package.json has no exports field#894
Copilot wants to merge 4 commits into
mainfrom
copilot/fix-subpath-resolution-error

Conversation

Copilot AI commented Dec 2, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

Resolving package subpaths like b/c/d fails with NotFound when the package has no exports field and both a file (d.js) and directory (d/) exist at the target path.

fixtures/abc/node_modules/
├── a/
│   └── index.js          # import x from 'b/c/d'
└── b/
    ├── package.json      # { "name": "b", "main": "index.js" }
    └── c/
        ├── d.js          # ← should resolve here
        └── d/
            └── e.js

Root cause: In load_node_modules, when is_dir returns true, the code tried directory resolution but used else if for file resolution—never falling back to load_as_file when the directory exists but load_as_directory returns None.

Fix: Changed else if to unconditional if so file resolution is attempted after directory resolution fails:

// Before: file resolution skipped when directory exists
if self.cache.is_dir(&cached_path, ctx) {
    // try directory...
} else if let Some(path) = self.load_as_file(...) { ... }

// After: always try file resolution as fallback
if self.cache.is_dir(&cached_path, ctx) {
    // try directory...
}
if let Some(path) = self.load_as_file(...) { ... }
Original prompt

This section details on the original issue you should resolve

<issue_title>Package subpath resolution fails when package.json has no exports field</issue_title>
<issue_description>### Problem Description

When resolving b/c/d from fixtures/abc/node_modules/a/index.js:

  1. Expected: Should resolve to fixtures/abc/node_modules/b/c/d.js
  2. Actual: Returns Err(NotFound("b/c/d"))

Directory Structure

fixtures/abc/node_modules/
├── a/
│   └── index.js          # import x from 'b/c/d'
└── b/
    ├── package.json      # { "name": "b", "main": "index.js" }
    └── c/
        ├── d.js          # expected resolution target
        └── d/
            └── e.js

Analysis

This is a subpath resolution issue. When resolving b/c/d:

  • b is the package name, should locate node_modules/b first
  • c/d is the subpath within the package, should resolve to b/c/d.js

The resolver fails to correctly resolve package subpaths (e.g., package-name/path/to/file) when there is no exports field in package.json. It should fall back to filesystem-based resolution within the package directory, but currently returns NotFound.</issue_description>

Comments on the Issue (you are @copilot in this section)

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@graphite-app

graphite-app Bot commented Dec 2, 2025

Copy link
Copy Markdown

How to use the Graphite Merge Queue

Add the label merge to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

Copilot AI and others added 3 commits December 2, 2025 07:19
@shulaoda
Add test case for subpath resolution without exports field
53de248 
Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
@shulaoda
Fix subpath resolution when package.json has no exports field
18f75a5 
Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
@shulaoda
Format package.json fixture with proper indentation
bec527f 
Co-authored-by: shulaoda <165626830+shulaoda@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix package subpath resolution error for missing exports Fix subpath resolution when package.json has no exports field Dec 2, 2025
Copilot AI requested a review from shulaoda December 2, 2025 07:28
Boshen added a commit that referenced this pull request Dec 2, 2025
@Boshen
fix: resolve node_modules/package/dir/foo.js if `node_modules/packa…
8a75edd 
…ge/dir/foo/` exists

fixes #893
closes #894

PR #839 incorrectly assumed the specifier is always a package name,
but a specifier can be `package/dir/foo` where both `node_modules/package/dir/foo.js` and `node_modules/package/dir/foo/` exists.
@Boshen Boshen mentioned this pull request Dec 2, 2025
Merged
@Boshen Boshen closed this in 121152f Dec 2, 2025
@Boshen Boshen deleted the copilot/fix-subpath-resolution-error branch December 18, 2025 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shulaoda shulaoda Awaiting requested review from shulaoda

Assignees

@shulaoda shulaoda

Copilot code review Copilot

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Package subpath resolution fails when package.json has no exports field

2 participants

@shulaoda