fix(arborist): node.target can be null when it is a file dep or s…

…ymlink (#7027) Co-authored-by: Luke Karrys <[email protected]>
npm · Dec 6, 2023 · ae2d982 · ae2d982 · lebedev05tmn · Jan 9, 2024
1 parent 5a2e334
commit ae2d982
Show file tree

Hide file tree

Showing 13 changed files with 2,231 additions and 2 deletions.
diff --git a/workspaces/arborist/lib/arborist/load-actual.js b/workspaces/arborist/lib/arborist/load-actual.js
@@ -333,7 +333,7 @@ module.exports = cls => class ActualLoader extends cls {
 
   async #loadFSTree (node) {
     const did = this.#actualTreeLoaded
-    if (!did.has(node.target.realpath)) {
+    if (!node.isLink && !did.has(node.target.realpath)) {
       did.add(node.target.realpath)
       await this.#loadFSChildren(node.target)
       return Promise.all(

diff --git a/workspaces/arborist/lib/tree-check.js b/workspaces/arborist/lib/tree-check.js
@@ -90,7 +90,7 @@ const checkTree = (tree, checkUnreachable = true) => {
       })
     }
 
-    if (node.path === tree.root.path && node !== tree.root) {
+    if (node.path === tree.root.path && node !== tree.root && !tree.root.isLink) {
       throw Object.assign(new Error('node with same path as root'), {
         node: node.path,
         tree: tree.path,

diff --git a/workspaces/arborist/tap-snapshots/test/arborist/reify.js.test.cjs b/workspaces/arborist/tap-snapshots/test/arborist/reify.js.test.cjs
@@ -2007,6 +2007,140 @@ ArboristNode {
 }
 `
 
+exports[`test/arborist/reify.js TAP bundled file dep with same name as other dep > must match snapshot 1`] = `
+ArboristNode {
+  "children": Map {
+    "@fixtures/has-package-exports" => ArboristLink {
+      "bundleDependencies": Array [
+        "abbrev",
+      ],
+      "edgesIn": Set {
+        EdgeIn {
+          "from": "",
+          "name": "@fixtures/has-package-exports",
+          "spec": "file:fixtures-has-package-exports",
+          "type": "prod",
+        },
+      },
+      "location": "node_modules/@fixtures/has-package-exports",
+      "name": "@fixtures/has-package-exports",
+      "path": "{CWD}/test/arborist/tap-testdir-reify-bundled-file-dep-with-same-name-as-other-dep/node_modules/@fixtures/has-package-exports",
+      "realpath": "{CWD}/test/arborist/tap-testdir-reify-bundled-file-dep-with-same-name-as-other-dep/fixtures-has-package-exports",
+      "resolved": "file:../../fixtures-has-package-exports",
+      "target": ArboristNode {
+        "location": "fixtures-has-package-exports",
+      },
+    },
+    "@ljharb/has-package-exports-patterns" => ArboristNode {
+      "edgesIn": Set {
+        EdgeIn {
+          "from": "node_modules/has-package-exports",
+          "name": "@ljharb/has-package-exports-patterns",
+          "spec": "^0.0.2",
+          "type": "prod",
+        },
+      },
+      "location": "node_modules/@ljharb/has-package-exports-patterns",
+      "name": "@ljharb/has-package-exports-patterns",
+      "path": "{CWD}/test/arborist/tap-testdir-reify-bundled-file-dep-with-same-name-as-other-dep/node_modules/@ljharb/has-package-exports-patterns",
+      "resolved": "https://registry.npmjs.org/@ljharb/has-package-exports-patterns/-/has-package-exports-patterns-0.0.2.tgz",
+      "version": "0.0.2",
+    },
+    "has-package-exports" => ArboristNode {
+      "edgesIn": Set {
+        EdgeIn {
+          "from": "",
+          "name": "has-package-exports",
+          "spec": "^1.3.0",
+          "type": "prod",
+        },
+      },
+      "edgesOut": Map {
+        "@ljharb/has-package-exports-patterns" => EdgeOut {
+          "name": "@ljharb/has-package-exports-patterns",
+          "spec": "^0.0.2",
+          "to": "node_modules/@ljharb/has-package-exports-patterns",
+          "type": "prod",
+        },
+      },
+      "location": "node_modules/has-package-exports",
+      "name": "has-package-exports",
+      "path": "{CWD}/test/arborist/tap-testdir-reify-bundled-file-dep-with-same-name-as-other-dep/node_modules/has-package-exports",
+      "resolved": "https://registry.npmjs.org/has-package-exports/-/has-package-exports-1.3.0.tgz",
+      "version": "1.3.0",
+    },
+  },
+  "edgesOut": Map {
+    "@fixtures/has-package-exports" => EdgeOut {
+      "name": "@fixtures/has-package-exports",
+      "spec": "file:fixtures-has-package-exports",
+      "to": "node_modules/@fixtures/has-package-exports",
+      "type": "prod",
+    },
+    "has-package-exports" => EdgeOut {
+      "name": "has-package-exports",
+      "spec": "^1.3.0",
+      "to": "node_modules/has-package-exports",
+      "type": "prod",
+    },
+  },
+  "fsChildren": Set {
+    ArboristNode {
+      "bundleDependencies": Array [
+        "abbrev",
+      ],
+      "children": Map {
+        "has-package-exports" => ArboristLink {
+          "bundleDependencies": Array [
+            "abbrev",
+          ],
+          "dev": true,
+          "edgesIn": Set {
+            EdgeIn {
+              "from": "fixtures-has-package-exports",
+              "name": "has-package-exports",
+              "spec": "file:.",
+              "type": "dev",
+            },
+          },
+          "location": "fixtures-has-package-exports/node_modules/has-package-exports",
+          "name": "has-package-exports",
+          "path": "{CWD}/test/arborist/tap-testdir-reify-bundled-file-dep-with-same-name-as-other-dep/fixtures-has-package-exports/node_modules/has-package-exports",
+          "realpath": "{CWD}/test/arborist/tap-testdir-reify-bundled-file-dep-with-same-name-as-other-dep/fixtures-has-package-exports",
+          "resolved": "file:..",
+          "target": ArboristNode {
+            "location": "fixtures-has-package-exports",
+          },
+        },
+      },
+      "dev": true,
+      "edgesOut": Map {
+        "abbrev" => EdgeOut {
+          "error": "MISSING",
+          "name": "abbrev",
+          "spec": "1.1.1",
+          "to": null,
+          "type": "prod",
+        },
+        "has-package-exports" => EdgeOut {
+          "name": "has-package-exports",
+          "spec": "file:.",
+          "to": "fixtures-has-package-exports/node_modules/has-package-exports",
+          "type": "dev",
+        },
+      },
+      "location": "fixtures-has-package-exports",
+      "name": "@fixtures/has-package-exports",
+      "path": "{CWD}/test/arborist/tap-testdir-reify-bundled-file-dep-with-same-name-as-other-dep/fixtures-has-package-exports",
+    },
+  },
+  "isProjectRoot": true,
+  "location": "",
+  "name": "tap-testdir-reify-bundled-file-dep-with-same-name-as-other-dep",
+  "path": "{CWD}/test/arborist/tap-testdir-reify-bundled-file-dep-with-same-name-as-other-dep",
+}
+`
+
 exports[`test/arborist/reify.js TAP collide case-variant dep names > tree 1 1`] = `
 ArboristNode {
   "children": Map {

diff --git a/workspaces/arborist/test/arborist/reify.js b/workspaces/arborist/test/arborist/reify.js
@@ -160,6 +160,11 @@ const newArb = (opt) => new Arborist({
 
 const reify = (path, opt) => newArb({ path, ...(opt || {}) }).reify(opt)
 
+t.test('bundled file dep with same name as other dep', async t => {
+  const tree = await printReified(fixture(t, 'conflict-bundle-file-dep'))
+  t.matchSnapshot(tree)
+})
+
 t.test('tarball deps with transitive tarball deps', t =>
   t.resolveMatchSnapshot(printReified(fixture(t, 'tarball-dependencies'))))
 

diff --git a/...arborist/test/fixtures/conflict-bundle-file-dep/fixtures-has-package-exports/package.json b/...arborist/test/fixtures/conflict-bundle-file-dep/fixtures-has-package-exports/package.json
@@ -0,0 +1,9 @@
+{
+  "bundleDependencies": true,
+  "dependencies": {
+    "abbrev": "1.1.1"
+  },
+  "devDependencies": {
+    "has-package-exports": "file:."
+  }
+}
diff --git a/workspaces/arborist/test/fixtures/conflict-bundle-file-dep/package.json b/workspaces/arborist/test/fixtures/conflict-bundle-file-dep/package.json
@@ -0,0 +1,6 @@
+{
+	"dependencies": {
+		"@fixtures/has-package-exports": "file:fixtures-has-package-exports",
+		"has-package-exports": "^1.3.0"
+	}
+}