This repository has been archived by the owner on Jan 20, 2022. It is now read-only.
fix: if there are children but the diff itself has an action, make sure to add it to leaves #233
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
…re to add it to leaves
isaacs
added a commit
that referenced
this pull request
Feb 18, 2021
We loop over the diff.leaves to find all the shrinkwraps that _must_ be unpacked ahead of time in order to complete the idealTree. However, if the idealTree already contains the children of the shrinkwrap-containing module (because it's been previously installed and saved to a lockfile), then we saw the hasShrinkwrap flag, and assumed it had already been unpacked. This tracks a Set of all nodes unpacked for the purposes of reading their shrinkwraps, and only skips _those_ modules at unpackNewModules. Different approach than #233, without adding extra items into the diff.leaves, which can have the side effect of calling mkdirp more than necessary. Close: #233 Fix: npm/cli#2251 Reviewed-by: @nlf
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.
this problem is most easily highlighted with the dependency
aws-cdkwhich for some reason when younpm ciafter installing, never makes it to the leaves and so is never reified. we do reify its children, but not that node itself.this change does pass the hands on test, and the changes to the diff look correct to me (and do seem to show the exact bug that i'm trying to fix)
References
Fixes npm/cli#2251