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

Conversation

@isaacs
Copy link
Contributor

@isaacs isaacs commented Mar 25, 2021

Adds support for a workspaces array of names passed to the Arborist constructor, which will limit reification to only those workspace nodes and their graph of dependencies.

Also, uses the filtered Diff behavior to ensure that global installs only make changes within the specific paths indicated by the set of explicit requests (which should be happening anyway, but which has been a bit of a bug factory in the past).

Open to bike shedding on the option name, it is a bit confusing, since it isn't the map of nodes loaded by mapWorkspaces, but I wasn't sure what other way to make the reify call more clear.

@isaacs isaacs requested review from nlf and ruyadorno March 25, 2021 00:40
@isaacs isaacs marked this pull request as draft March 25, 2021 08:15
@isaacs
Copy link
Contributor Author

isaacs commented Mar 25, 2021

Found some bugs exploring the edge cases around this. Have fixes for them, but don't yet have tests for all the fixes.

@isaacs isaacs marked this pull request as ready for review March 25, 2021 19:01
isaacs added 7 commits March 26, 2021 09:33
This causes major problems when moving links and targets between trees.
When we move the link from one tree to another, it pulls its target,
out, meaning that the target ends up becoming orphaned.

Say there is a matching link and target in two different trees, A and B,
and we want to replace the link and target in one tree with the link and
target in another.

If we move the target A to tree B first, then it replaces the target B,
and link B is updated to point to it.

Then, we move link A to tree B, replacing link B.  To do that, we move
link B out of tree B, which _takes target B with it_, leaving the ported
link A without a target in tree B.
This also corrects an issue where manually updating the package.json
file would sometimes not be reified properly, if a new link target
matches the one that was formerly reified.
Fixes sorting incorrectly when a dependency is set to an empty string
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.

followed the changes well enough to see what's going on, tests told the rest of the story. i think this makes sense!

@isaacs isaacs closed this in 94c4d9c Apr 1, 2021
@wraithgar wraithgar deleted the isaacs/filtered-reification branch April 22, 2021 17:45
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.

2 participants