This repository has been archived by the owner on Jan 20, 2022. It is now read-only.
fix: move pathological nest fixing link to PlaceDep #309
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.
When a dependency graph cycles back on itself incompatibly like this:
We would find ourselves unable to handle the conflict via nesting. For
example:
In order to address this, we create a link when such a cycle is
detected.
Prior to the recent refactor to move much of the dependency placement
logic out of Arborist.buildIdealTree and into the PlaceDep class, this
link was created right at the moment when a new dependency was created
in a temp tree.
However, if we feed that Link object into the PlaceDep flow, it will
(correctly) see that the Link does not match the Node further up the
tree, and attempt to replace it.
Compounding the problem (and why it appeared in
npm dedupe
and notnpm install
) is the fact that explicitly named updates are alwaystreated as a "problem edge", so that they can be re-evaluated.
So, rather than creating a Node to be tested against the tree, it was
creating a Link object, and then attempting to replace the Link's target
with the Link itself, which caused some havoc.
This patch moves the loop detection and remediating Link creation into
the PlaceDep class, which is the more proper place for it, as that class
owns the "put deps into the tree" logic, and this is clearly a "put deps
into the tree" type of situation.
Via: @ParadoxInfinite
Close: npm/cli#3632
Close: #308
Fix: npm/cli#3565
References