Skip to content
This repository was archived by the owner on Jan 20, 2022. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
do not take a link's target when removing link from tree
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.
  • Loading branch information
isaacs committed Mar 26, 2021
commit c5d1b9fbff10af8fb13eba714c55a90fc9b49c84
7 changes: 5 additions & 2 deletions lib/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -685,6 +685,7 @@ class Node {
...this.children.values(),
...this.inventory.values(),
].filter(n => n !== this))

for (const child of family) {
if (child.root !== root) {
child[_delistFromMeta]()
Expand All @@ -704,12 +705,14 @@ class Node {
}

// if we had a target, and didn't find one in the new root, then bring
// it over as well.
if (this.isLink && target && !this.target)
// it over as well, but only if we're setting the link into a new root,
// as we don't want to lose the target any time we remove a link.
if (this.isLink && target && !this.target && root !== this)
target.root = root

// tree should always be valid upon root setter completion.
treeCheck(this)
treeCheck(root)
}

get root () {
Expand Down
26 changes: 26 additions & 0 deletions tap-snapshots/test-arborist-reify.js-TAP.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1899,6 +1899,32 @@ ArboristNode {
},
},
"fsChildren": Set {
ArboristNode {
"dev": true,
"edgesOut": Map {
"once" => EdgeOut {
"error": "MISSING",
"name": "once",
"spec": "*",
"to": null,
"type": "prod",
},
"wrappy" => EdgeOut {
"error": "MISSING",
"name": "wrappy",
"spec": "1.0.2",
"to": null,
"type": "prod",
},
},
"extraneous": true,
"location": "packages/a",
"name": "a",
"optional": true,
"path": "{CWD}/test/arborist/reify-filtered-reification-in-workspaces/packages/a",
"peer": true,
"version": "1.2.3",
},
ArboristNode {
"children": Map {
"wrappy" => ArboristNode {
Expand Down
5 changes: 3 additions & 2 deletions test/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -591,8 +591,9 @@ t.test('load with a virtual filesystem parent', t => {
t.equal(link.root, link, 'removed from parent, removed from root')
t.equal(root.inventory.get(linkLoc), undefined, 'removed from root inventory')
t.equal(link.inventory.has(link), true, 'link added to own inventory')
t.equal(link.target, linkTarget, 'target taken along for the ride')
t.equal(linkTarget.root, link, 'target rooted on link')
t.equal(link.target, null, 'target left behind when setting root to null')
linkTarget.root = link
t.equal(link.target, linkTarget, 'target set once roots match')
t.equal(link.inventory.get(''), linkTarget)
t.equal(root.edgesOut.get('link').error, 'MISSING')

Expand Down