This repository has been archived by the owner on Jan 20, 2022. It is now read-only.
Respect link deps when calculating peerDep sets #249
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
isaacs
force-pushed
the
isaacs/fix-peer-set-with-link-deps
branch
from
962214b
to
62d6fd2
Compare
* Show useful debug printing for virtual root nodes * Respect hidden lockfiles when symlink deps present. As long as the target of the link is in the shrinkwrap, and not newer. * Respect link deps when calculating peerDep sets Previously, we were not including link targets in the virtual trees where peer dependency sets are calculated. Additionally, we were still using the path `/virtual-root` for the virtual node, even though this is no longer load-bearing. (And, as of the recent change to the Node printable output, no longer necessary or particularly helpful for debugging.) As a result, a link dependency from the root node like `file:../../foo` would get resolved against `/virtual-root`, resulting in `/foo`, which of course does not match any Node in the virtual tree. The outcome was an ERESOLVE error where the `current` Node is shown as having no name or version (because it is an unsatisfied Link). The solution is two-part. First, the path of the virtual tree root now matches the path of the Node that it is sourced from. Second, any Link children of the source node have their targets mirrored in the virtual tree, resulting in them being matched appropriately. The result is that a Link dependency can now properly satisfy a peerDependency. Test shows an example of using a Link to a local Node as a workaround for a peerSet that otherwise would not be resolveable. This can of course be abused to get around valid peerDep contracts, but if they user takes it on themselves to use a local fork of a dependency, we should respect that in buildIdealTree as we do elsewhere. Fix: npm/cli#2199 PR-URL: #249 Credit: @isaacs Close: #249 Reviewed-by: @ruyadorno
ruyadorno
force-pushed
the
isaacs/fix-peer-set-with-link-deps
branch
from
62d6fd2
to
fa16e09
Compare
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, we were not including link targets in the virtual trees
where peer dependency sets are calculated. Additionally, we were still
using the path
/virtual-rootfor the virtual node, even though this isno longer load-bearing. (And, as of the other change in this PR to the Node
printable output, no longer necessary or particularly helpful for
debugging.)
As a result, a link dependency from the root node like
file:../../foowould get resolved against
/virtual-root, resulting in/foo, whichof course does not match any Node in the virtual tree. The outcome was
an ERESOLVE error where the
currentNode is shown as having no nameor version (because it is an unsatisfied Link).
The solution is two-part.
First, the path of the virtual tree root now matches the path of the
Node that it is sourced from.
Second, any Link children of the source node have their targets mirrored
in the virtual tree, resulting in them being matched appropriately.
The result is that a Link dependency can now properly satisfy a
peerDependency. Test shows an example of using a Link to a local Node
as a workaround for a peerSet that otherwise would not be resolveable.
This can of course be abused to get around valid peerDep contracts, but
if they user takes it on themselves to use a local fork of a dependency,
we should respect that in buildIdealTree as we do elsewhere.
Fix: npm/cli#2199