fix(arborist): condition to include name field in package-lock fixed (#7602)

When metadata is committed for the first time when there is no
package-lock, when target node has the same name field value as target
package name and link node also share the same name field, name field is
omitted from lock file, in subsequent times when there is already a lock
file, it reads target node with name field derived from realpath value
of the node and included in lock file. this creates mismatch of lock
file between installs.

This PR adds additional condition to check if name derived from realpath
is the same name as package and adds the name property.

Fixes: #7166

Author: milaninfy

workspaces/arborist/lib/shrinkwrap.js

@@ -50,6 +50,7 @@ const versionFromTgz = require('./version-from-tgz.js')
 const npa = require('npm-package-arg')
 const pkgJson = require('@npmcli/package-json')
 const parseJSON = require('parse-conflict-json')
+const nameFromFolder = require('@npmcli/name-from-folder')
 
 const stringify = require('json-stringify-nice')
 const swKeyOrder = [
@@ -233,7 +234,8 @@ class Shrinkwrap {
     // root to help prevent churn based on the name of the directory the
     // project is in
     const pname = node.packageName
-    if (pname && (node === node.root || pname !== node.name)) {
+    // when Target package name and Target node share the same name, we include the name, target node should have name as per realpath.
+    if (pname && (node === node.root || pname !== node.name || nameFromFolder(node.realpath) !== pname)) {
       meta.name = pname
     }
 

workspaces/arborist/tap-snapshots/test/shrinkwrap.js.test.cjs

@@ -280,6 +280,7 @@ Object {
         "a": "",
         "link": "",
         "link2": "",
+        "link3": "",
       },
       "devDependencies": Object {
         "d": "",
@@ -349,6 +350,10 @@ Object {
       "link": true,
       "resolved": "target",
     },
+    "node_modules/link3": Object {
+      "link": true,
+      "resolved": "realPkg",
+    },
     "node_modules/nopkg": Object {
       "extraneous": true,
     },
@@ -401,6 +406,14 @@ Object {
       "resolved": "file:archives/tarball-pkg-resolved.tgz",
       "version": "1.2.3",
     },
+    "realPkg": Object {
+      "funding": Object {
+        "url": "https://example.com/payme",
+      },
+      "name": "link3",
+      "resolved": "git+ssh://[email protected]/isaacs/foobarbaz.git#aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+      "version": "1.2.3",
+    },
     "target": Object {
       "funding": Object {
         "url": "https://example.com/payme",
@@ -466,6 +479,13 @@ Object {
 }
 `
 
+exports[`test/shrinkwrap.js TAP construct metadata from node and package data > link metadata with same pkg name as link target 1`] = `
+Object {
+  "link": true,
+  "resolved": "realPkg",
+}
+`
+
 exports[`test/shrinkwrap.js TAP construct metadata from node and package data > link target metadata 1`] = `
 Object {
   "funding": Object {
@@ -477,6 +497,17 @@ Object {
 }
 `
 
+exports[`test/shrinkwrap.js TAP construct metadata from node and package data > link target metadata with same pkg name as link target 1`] = `
+Object {
+  "funding": Object {
+    "url": "https://example.com/payme",
+  },
+  "name": "link3",
+  "resolved": "git+ssh://[email protected]/isaacs/foobarbaz.git#aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
+  "version": "1.2.3",
+}
+`
+
 exports[`test/shrinkwrap.js TAP construct metadata from node and package data > meta for dev dep 1`] = `
 Object {
   "dependencies": Object {
@@ -549,6 +580,7 @@ Object {
     "a": "",
     "link": "",
     "link2": "",
+    "link3": "",
   },
   "devDependencies": Object {
     "d": "",
@@ -572,6 +604,7 @@ Object {
     "a": "",
     "link": "",
     "link2": "",
+    "link3": "",
   },
   "devDependencies": Object {
     "d": "",

workspaces/arborist/test/shrinkwrap.js

@@ -331,7 +331,7 @@ t.test('construct metadata from node and package data', t => {
   const root = new Node({
     pkg: {
       name: 'root',
-      dependencies: { a: '', link: '', link2: '' },
+      dependencies: { a: '', link: '', link2: '', link3: '' },
       devDependencies: { d: '', e: 'https://foo.com/e.tgz', devit: '' },
       optionalDependencies: { optin: '' },
       peerDependencies: { peer: '' },
@@ -430,6 +430,31 @@ t.test('construct metadata from node and package data', t => {
       },
     }),
   })
+  // special case when link alias is the same as target package name
+  const link3 = new Link({
+    name: 'link3',
+    path: '/home/user/projects/root/node_modules/link3',
+    realpath: '/home/user/projects/root/realPkg',
+    parent: root,
+    pkg: {
+      name: 'link3',
+      version: '1.2.3',
+      funding: 'https://example.com/payme',
+      _resolved: 'github:isaacs/foobarbaz#aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa',
+    },
+    target: new Node({
+      name: 'link3',
+      realpath: '/home/user/projects/root/realPkg',
+      path: '/home/user/projects/root/realPkg',
+      root,
+      pkg: {
+        name: 'link3',
+        version: '1.2.3',
+        funding: 'https://example.com/payme',
+        _resolved: 'github:isaacs/foobarbaz#aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa',
+      },
+    }),
+  })
 
   const a = new Node({
     resolved: 'https://example.com/a.tgz',
@@ -520,6 +545,10 @@ t.test('construct metadata from node and package data', t => {
     'metadata for tarball file pkg with _resolved value')
   t.matchSnapshot(meta.get(link.path), 'link metadata')
   t.matchSnapshot(meta.get(link.target.path), 'link target metadata')
+
+  t.matchSnapshot(meta.get(link3.path), 'link metadata with same pkg name as link target')
+  t.matchSnapshot(meta.get(link3.target.path), 'link target metadata with same pkg name as link target')
+
   t.matchSnapshot(meta.get(a.location), 'dep a metadata')
   t.matchSnapshot(meta.get(d.location), 'dep d metadata')
   t.matchSnapshot(meta.get(e.location), 'dep e metadata')