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

Ensure correct flags on shrinkwrapped module deps #225

Merged
merged 1 commit into from
Feb 12, 2021

Conversation

isaacs
Copy link
Contributor

@isaacs isaacs commented Feb 11, 2021

When running Arborist.loadVirtual() to read the tree from the point of
a tree that contains a shrinkwrap, we were inappropriately setting all
dep flags to false, as we would when reading the virtual tree in a
project root.

This patch updates loadVirtual to ensure that it always sets the proper
flags on nodes within the subtree, if the root supplied is not actually
the project root.

As a result, changes had to be made to the process that inflates
old/ancient lockfiles, and reify()'s handling of garbage data needed to
be updated as well (since the cases being tested previously would now
typically prune the invalid data out as extraneous before getting to the
reify step). If a node DOES make it through to reify() in the
idealTree, but lacks a version or resolved field, then something is very
wrong, and so we now print a warning to the user asking them to re-try
the install or delete their lockfile (which will typically fix the
problem, since it either is a root dep that's being removed and will be
re-added properly, or a metadep problem that can only be fixed with a
full tree rebuild anyway).

References

Copy link
Contributor

@nlf nlf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks to resolve the issue in my reproduction case 👍

When running `Arborist.loadVirtual()` to read the tree from the point of
a tree that contains a shrinkwrap, we were inappropriately setting all
dep flags to `false`, as we would when reading the virtual tree in a
project root.

This patch updates loadVirtual to ensure that it always sets the proper
flags on nodes within the subtree, if the root supplied is not actually
the project root.

As a result, changes had to be made to the process that inflates
old/ancient lockfiles, and reify()'s handling of garbage data needed to
be updated as well (since the cases being tested previously would now
typically prune the invalid data out as extraneous before getting to the
reify step).  If a node DOES make it through to reify() in the
idealTree, but lacks a version or resolved field, then something is very
wrong, and so we now print a warning to the user asking them to re-try
the install or delete their lockfile (which will typically fix the
problem, since it either is a root dep that's being removed and will be
re-added properly, or a metadep problem that can only be fixed with a
full tree rebuild anyway).

PR-URL: #225
Credit: @isaacs
Close: #225
Reviewed-by: @ruyadorno, @nlf
@isaacs isaacs force-pushed the isaacs/fix-shrinkwrap-deps-losing-flags branch from 1bf12bb to 30e6d7e Compare February 12, 2021 16:37
@isaacs isaacs closed this in 30e6d7e Feb 12, 2021
@isaacs isaacs merged commit 30e6d7e into main Feb 12, 2021
@wraithgar wraithgar deleted the isaacs/fix-shrinkwrap-deps-losing-flags branch April 22, 2021 17:46
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.

3 participants