Skip to content

fix(arborist): reuse a link target spelled with a different drive-letter case - #10133

Open
vsolano9 wants to merge 1 commit into
npm:latestfrom
vsolano9:fix/9079-link-target-alias
Open

vsolano9 wants to merge 1 commit into
npm:latestfrom
vsolano9:fix/9079-link-target-alias

Conversation

@vsolano9

Copy link
Copy Markdown

On Windows, links can spell the same target directory differently, most commonly with a different drive-letter case (D:\work\pkg and d:\work\pkg). Junctions keep the spelling they were created with, and fs.realpath/readlink return it unchanged.

loadActual caches link targets by the exact realpath string, but places nodes in the inventory by relpath(root.realpath, path), and path.relative is case-insensitive on Windows. So when a second spelling shows up:

  1. the cache misses, and new Link() builds a second target node for the same directory;
  2. that node takes the same inventory location, which evicts the first target and leaves the links that used it with target === null;
  3. the next link that is added with the first spelling finds one of those targetless links through inventory.query('realpath', ...) in set root, and reads null.package.

That is the TypeError: Cannot read properties of null (reading 'package') at set root / set parent / new Link / #newLink reported in #9079. Depending on the order Promise.all loads the children, the load either throws or finishes with detached, targetless links.

Reproduction (npm 12.2.0, Windows 11)

pkg/package.json              {"name":"pkg","version":"1.0.0"}
app/package.json              dependencies a, b, c: "file:../pkg"
app/node_modules/a  junction  D:\...\pkg
app/node_modules/b  junction  d:\...\pkg
app/node_modules/c  junction  D:\...\pkg

Calling new Arborist({ path: 'app' }).loadActual({ forceActual: true }) 40 times in one process gave:

Outcome Before After
a and c have target === null 32 0
b has target === null 6 0
TypeError: Cannot read properties of null (reading 'package') at set root / set parent / new Node / new Link 2 0
all three links share one target (linksIn.size === 3) 0 40

Six fresh processes of five loads each threw the same TypeError in two of them before the change; which outcome you get depends on the order the children finish loading.

I can't confirm that this spelling difference is how the reporter's tree got there: the issue describes nested file: dependencies and suspects load ordering, and this PR does not change ordering. It fixes one mechanism that produces exactly that TypeError, so I've referenced the issue instead of closing it.

One related case is left as it was: a workspace that is first reached through a link spelled differently from the workspace path still gets a second node when the workspace loop loads it. npm spells the workspace links it creates from the root path, so that needs a hand-made link. I kept this change small, and can extend it if you'd like it covered.

Change

#newLink first reuses the node already at the target's inventory location, and only then falls back to the realpath cache. The link takes that node's path spelling, so no duplicate target is created and nothing is evicted.

Tests

  • New Windows-only test in test/arborist/load-actual.js: three junctions with mixed drive-letter case must share one target that is in the tree and has all three linksIn. It fails without the change (links have a target, then Cannot read properties of null (reading 'root')) and passes with it.
  • node . test -w @npmcli/arborist on Windows 11 with Node 24: 58 of 59 test files pass and coverage stays at 100%. The one failure, linked strategy exposes store node_modules via NODE_PATH for lifecycle scripts in test/arborist/reify.js (spawn C:\WINDOWS\system32\cmd.exe ENOENT), fails the same way on unmodified latest on this machine, so it is environmental and unrelated.
  • eslint on both changed files: clean.

Only Windows was run locally. On other platforms the new test is skipped, and the changed lookup gives the same result as the cache did before.

Refs #9079

AI assistance: the investigation, the change and the checks above were done with Claude Code (an AI coding agent). The results quoted are from real runs on a Windows 11 machine.

🤖 Generated with Claude Code

…ter case

On Windows, junctions keep the target spelling they were created with, so
links can name one directory as D:\pkg and d:\pkg. loadActual cached link
targets by the exact realpath string but placed nodes by a case-insensitive
relative location, so a second spelling built a duplicate target that evicted
the first and left its links without a target. Adding the next link then read
null.package in set root.

Reuse the node already at the target's inventory location before falling back
to the realpath cache.

Refs: npm#9079

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@vsolano9
vsolano9 requested a review from a team as a code owner October 15, 2026 16:01
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