Skip to content

Commit

Permalink
fix: deps of linked deps in package-lock
Browse files Browse the repository at this point in the history
When having optional dependencies of a dependency of
a top-level link dependency, e.g:

    root -> LINK(a)
    a -> (b)
    b -> OPTIONAL(c)

    * c marked as extraneous

Any optional dependency (and all its nested nodes) were marked
as extraneous in package-lock due to `calcDepFlags` missing
properly following the target of Link nodes.

This changeset fixes it by properly following the result of the
`treeverse.visit` method that will have already followed any
link targets at that point.

Fixes: npm/cli#2505
  • Loading branch information
ruyadorno committed Feb 12, 2021
1 parent 4f2c028 commit 54a2663
Show file tree
Hide file tree
Showing 5 changed files with 113 additions and 1 deletion.
2 changes: 1 addition & 1 deletion lib/calc-dep-flags.js
Expand Up @@ -11,7 +11,7 @@ const calcDepFlags = (tree, resetRoot = true) => {
tree,
visit: node => calcDepFlagsStep(node),
filter: node => node,
getChildren: node => [...node.edgesOut.values()].map(edge => edge.to),
getChildren: (node, tree) => [...tree.edgesOut.values()].map(edge => edge.to),
})
return ret
}
Expand Down
93 changes: 93 additions & 0 deletions tap-snapshots/test-arborist-build-ideal-tree.js-TAP.test.js
Expand Up @@ -5,6 +5,99 @@
* Make sure to inspect the output below. Do not ignore changes!
*/
'use strict'
exports[`test/arborist/build-ideal-tree.js TAP a direct link dep has a dep with optional dependencies > should not mark children of the optional dep as extraneous 1`] = `
ArboristNode {
"children": Map {
"a" => ArboristLink {
"edgesIn": Set {
EdgeIn {
"from": "",
"name": "a",
"spec": "./a",
"type": "prod",
},
},
"location": "node_modules/a",
"name": "a",
"path": "{CWD}/test/fixtures/link-dep-has-dep-with-optional-dep/node_modules/a",
"realpath": "{CWD}/test/fixtures/link-dep-has-dep-with-optional-dep/a",
"resolved": "file:../a",
"target": Object {
"location": "a",
},
"version": "1.0.0",
},
"graceful-fs" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "node_modules/rimraf",
"name": "graceful-fs",
"spec": "~2",
"type": "optional",
},
},
"location": "node_modules/graceful-fs",
"name": "graceful-fs",
"optional": true,
"path": "{CWD}/test/fixtures/link-dep-has-dep-with-optional-dep/node_modules/graceful-fs",
"resolved": "https://registry.npmjs.org/graceful-fs/-/graceful-fs-2.0.3.tgz",
"version": "2.0.3",
},
"rimraf" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "a",
"name": "rimraf",
"spec": "2.2.4",
"type": "prod",
},
},
"edgesOut": Map {
"graceful-fs" => EdgeOut {
"name": "graceful-fs",
"spec": "~2",
"to": "node_modules/graceful-fs",
"type": "optional",
},
},
"location": "node_modules/rimraf",
"name": "rimraf",
"path": "{CWD}/test/fixtures/link-dep-has-dep-with-optional-dep/node_modules/rimraf",
"resolved": "https://registry.npmjs.org/rimraf/-/rimraf-2.2.4.tgz",
"version": "2.2.4",
},
},
"edgesOut": Map {
"a" => EdgeOut {
"name": "a",
"spec": "./a",
"to": "node_modules/a",
"type": "prod",
},
},
"fsChildren": Set {
ArboristNode {
"edgesOut": Map {
"rimraf" => EdgeOut {
"name": "rimraf",
"spec": "2.2.4",
"to": "node_modules/rimraf",
"type": "prod",
},
},
"location": "a",
"name": "a",
"path": "{CWD}/test/fixtures/link-dep-has-dep-with-optional-dep/a",
"version": "1.0.0",
},
},
"location": "",
"name": "link-dep-has-dep-with-optional-dep",
"path": "{CWD}/test/fixtures/link-dep-has-dep-with-optional-dep",
"version": "1.0.0",
}
`

exports[`test/arborist/build-ideal-tree.js TAP a tree with an outdated dep, missing dep, no lockfile > should not update all 1`] = `
ArboristNode {
"children": Map {
Expand Down
5 changes: 5 additions & 0 deletions test/arborist/build-ideal-tree.js
Expand Up @@ -174,6 +174,11 @@ t.test('bad shrinkwrap file', t => {
return t.resolveMatchSnapshot(printIdeal(path), 'bad shrinkwrap')
})

t.test('a direct link dep has a dep with optional dependencies', t => {
const path = resolve(fixtures, 'link-dep-has-dep-with-optional-dep')
return t.resolveMatchSnapshot(printIdeal(path), 'should not mark children of the optional dep as extraneous')
})

t.test('cyclical peer deps', t => {
const paths = [
resolve(fixtures, 'peer-dep-cycle'),
Expand Down
@@ -0,0 +1,7 @@
{
"name": "a",
"version": "1.0.0",
"dependencies": {
"rimraf": "2.2.4"
}
}
7 changes: 7 additions & 0 deletions test/fixtures/link-dep-has-dep-with-optional-dep/package.json
@@ -0,0 +1,7 @@
{
"name": "link-dep-has-dep-with-optional-dep",
"version": "1.0.0",
"dependencies": {
"a": "./a"
}
}

0 comments on commit 54a2663

Please sign in to comment.