fix(arborist): prioritize valid workspace nodes (#4230)

closes #3637

Author: nlf

workspaces/arborist/lib/arborist/build-ideal-tree.js

@@ -1180,6 +1180,11 @@ This is a one-time fix-up, please be patient...
           return true
         }
 
+        // If the edge is a workspace, and it's valid, leave it alone
+        if (edge.to.isWorkspace) {
+          return false
+        }
+
         // user explicitly asked to update this package by name, problem
         if (this[_updateNames].includes(edge.name)) {
           return true

workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs

@@ -159209,6 +159209,106 @@ ArboristNode {
 }
 `
 
+exports[`test/arborist/build-ideal-tree.js TAP workspaces should allow adding a workspace as a dep to a workspace > must match snapshot 1`] = `
+ArboristNode {
+  "children": Map {
+    "workspace-a" => ArboristLink {
+      "edgesIn": Set {
+        EdgeIn {
+          "from": "",
+          "name": "workspace-a",
+          "spec": "file:{CWD}/test/arborist/tap-testdir-build-ideal-tree-workspaces-should-allow-adding-a-workspace-as-a-dep-to-a-workspace/workspace-a",
+          "type": "workspace",
+        },
+      },
+      "isWorkspace": true,
+      "location": "node_modules/workspace-a",
+      "name": "workspace-a",
+      "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-workspaces-should-allow-adding-a-workspace-as-a-dep-to-a-workspace/node_modules/workspace-a",
+      "realpath": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-workspaces-should-allow-adding-a-workspace-as-a-dep-to-a-workspace/workspace-a",
+      "resolved": "file:../workspace-a",
+      "target": ArboristNode {
+        "location": "workspace-a",
+      },
+      "version": "1.0.0",
+    },
+    "workspace-b" => ArboristLink {
+      "edgesIn": Set {
+        EdgeIn {
+          "from": "",
+          "name": "workspace-b",
+          "spec": "file:{CWD}/test/arborist/tap-testdir-build-ideal-tree-workspaces-should-allow-adding-a-workspace-as-a-dep-to-a-workspace/workspace-b",
+          "type": "workspace",
+        },
+        EdgeIn {
+          "from": "workspace-a",
+          "name": "workspace-b",
+          "spec": "*",
+          "type": "prod",
+        },
+      },
+      "isWorkspace": true,
+      "location": "node_modules/workspace-b",
+      "name": "workspace-b",
+      "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-workspaces-should-allow-adding-a-workspace-as-a-dep-to-a-workspace/node_modules/workspace-b",
+      "realpath": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-workspaces-should-allow-adding-a-workspace-as-a-dep-to-a-workspace/workspace-b",
+      "resolved": "file:../workspace-b",
+      "target": ArboristNode {
+        "location": "workspace-b",
+      },
+      "version": "1.0.0",
+    },
+  },
+  "edgesOut": Map {
+    "workspace-a" => EdgeOut {
+      "name": "workspace-a",
+      "spec": "file:{CWD}/test/arborist/tap-testdir-build-ideal-tree-workspaces-should-allow-adding-a-workspace-as-a-dep-to-a-workspace/workspace-a",
+      "to": "node_modules/workspace-a",
+      "type": "workspace",
+    },
+    "workspace-b" => EdgeOut {
+      "name": "workspace-b",
+      "spec": "file:{CWD}/test/arborist/tap-testdir-build-ideal-tree-workspaces-should-allow-adding-a-workspace-as-a-dep-to-a-workspace/workspace-b",
+      "to": "node_modules/workspace-b",
+      "type": "workspace",
+    },
+  },
+  "fsChildren": Set {
+    ArboristNode {
+      "edgesOut": Map {
+        "workspace-b" => EdgeOut {
+          "name": "workspace-b",
+          "spec": "*",
+          "to": "node_modules/workspace-b",
+          "type": "prod",
+        },
+      },
+      "isWorkspace": true,
+      "location": "workspace-a",
+      "name": "workspace-a",
+      "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-workspaces-should-allow-adding-a-workspace-as-a-dep-to-a-workspace/workspace-a",
+      "version": "1.0.0",
+    },
+    ArboristNode {
+      "isWorkspace": true,
+      "location": "workspace-b",
+      "name": "workspace-b",
+      "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-workspaces-should-allow-adding-a-workspace-as-a-dep-to-a-workspace/workspace-b",
+      "version": "1.0.0",
+    },
+  },
+  "isProjectRoot": true,
+  "location": "",
+  "name": "tap-testdir-build-ideal-tree-workspaces-should-allow-adding-a-workspace-as-a-dep-to-a-workspace",
+  "packageName": "root",
+  "path": "{CWD}/test/arborist/tap-testdir-build-ideal-tree-workspaces-should-allow-adding-a-workspace-as-a-dep-to-a-workspace",
+  "workspaces": Map {
+    "workspace-a" => "workspace-a",
+    "workspace-b" => "workspace-b",
+  },
+}
+`
+
 exports[`test/arborist/build-ideal-tree.js TAP workspaces should ignore nested node_modules folders > expect resolving Promise 1`] = `
 ArboristNode {
   "children": Map {

workspaces/arborist/test/arborist/build-ideal-tree.js

@@ -833,6 +833,50 @@ t.test('workspaces', t => {
     )
   })
 
+  t.test('should allow adding a workspace as a dep to a workspace', async t => {
+    // turn off networking, this should never make a registry request
+    nock.disableNetConnect()
+    t.teardown(() => nock.enableNetConnect())
+
+    const path = t.testdir({
+      'package.json': JSON.stringify({
+        name: 'root',
+        workspaces: ['workspace-a', 'workspace-b'],
+      }),
+      'workspace-a': {
+        'package.json': JSON.stringify({
+          name: 'workspace-a',
+          version: '1.0.0',
+        }),
+      },
+      'workspace-b': {
+        'package.json': JSON.stringify({
+          name: 'workspace-b',
+          version: '1.0.0',
+        }),
+      },
+    })
+
+    const arb = new Arborist({
+      ...OPT,
+      path,
+      workspaces: ['workspace-a'],
+    })
+
+    const tree = arb.buildIdealTree({
+      path,
+      add: [
+        'workspace-b',
+      ],
+    })
+
+    // just assert that the buildIdealTree call resolves, if there's a
+    // problem here it will reject because of nock disabling requests
+    await t.resolves(tree)
+
+    t.matchSnapshot(printTree(await tree))
+  })
+
   t.end()
 })