Skip to content
This repository has been archived by the owner on Jan 20, 2022. It is now read-only.

fix: _findMissingEdges missing dependency due to inconsistent path separators #363

Closed
wants to merge 2 commits into from

Conversation

salvadorj
Copy link

When installing a package with bundled dependencies with npm it can end up missing some dependencies that were bundled and attempt to fetch from the remote registry (see npm/cli#2757). This issue only happens on Windows and it's caused by _findMissingEdges missing one of the bundled dependencies. The reason it only happens on Windows is because of this lookup in this[_cache] with a path that contains mixed path separators:

const depPath = `${p}/node_modules/${name}`
const cached = this[_cache].get(depPath)
if (!cached || cached.dummy) {

On macOS we get a result back and avoid creating a new node. On Windows we get no result since the key in the cache is using backslashes but the lookup key has both backslashes and forward slashes. This results in a new node being created with the path of an existing node which in turn removes the existing node and its children from the tree. The children are however not loaded from the file system again because of this check:

// impossible except in pathological ELOOP cases
/* istanbul ignore if */
if (did.has(node.realpath)) {
return Promise.resolve(node)
}

The end result is an unmet dependency and npm attempting to fetch it from the registry instead of using the bundled dependency.

I've changed the cache key by normalizing the path to avoid replacing existing nodes. I've setup a minimal test case where the difference in behavior (with and without normalization) shows up as an error on the outgoing edge of a node.

References

Fixes npm/cli#2757

Dependencies nested in node_modules triggers an
issue with a file path used as a cache key in the
_findMissingEdges function on Windows due to
paths with mixed path separtors.
Normalize path to avoid mixed path separators on
Windows causing a cache miss for paths that refer
to the same thing. This would otherwise cause a
new node with the path of an existing node to be
created which in turns deletes the existing node
and its children. However, the children will not
be loaded again from the file system due to the
_actualTreeLoaded check in _loadFSTree.
@fritzy
Copy link
Contributor

fritzy commented Jan 19, 2022

@salvadorj This repo is being archived as arborist is now maintained as a workspace in the CLI. I recreated this PR as npm/cli#4261, and the commits maintain you as the author.

@fritzy fritzy closed this Jan 19, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] npm install attempts to fetch already bundled indirect dependency from registry
2 participants