Skip to content

Commit

Permalink
fix(arborist): condition to include name field in package-lock fixed (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
milaninfy authored Jun 27, 2024
1 parent 55639ef commit a8e666e
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 2 deletions.
4 changes: 3 additions & 1 deletion workspaces/arborist/lib/shrinkwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [
Expand Down Expand Up @@ -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
}

Expand Down
33 changes: 33 additions & 0 deletions workspaces/arborist/tap-snapshots/test/shrinkwrap.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ Object {
"a": "",
"link": "",
"link2": "",
"link3": "",
},
"devDependencies": Object {
"d": "",
Expand Down Expand Up @@ -349,6 +350,10 @@ Object {
"link": true,
"resolved": "target",
},
"node_modules/link3": Object {
"link": true,
"resolved": "realPkg",
},
"node_modules/nopkg": Object {
"extraneous": true,
},
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -549,6 +580,7 @@ Object {
"a": "",
"link": "",
"link2": "",
"link3": "",
},
"devDependencies": Object {
"d": "",
Expand All @@ -572,6 +604,7 @@ Object {
"a": "",
"link": "",
"link2": "",
"link3": "",
},
"devDependencies": Object {
"d": "",
Expand Down
31 changes: 30 additions & 1 deletion workspaces/arborist/test/shrinkwrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: '' },
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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')
Expand Down

0 comments on commit a8e666e

Please sign in to comment.