Skip to content

Commit 8042af3

Browse files
authored
fix: prune optional peer dependencies that are no longer explicitly depended on (#8431)
Currently optional peer dependencies are retained so long as at least one package in the tree has them as an optional peer dependency, meaning once added they become non-optional even though `npm` does actually seem to mark such dependencies. To resolve this, I've modified the pruning logic to check for nodes that are both `optional` and `peer`, in addition to `extraneous` nodes. Resolves #4737
1 parent ef3529e commit 8042af3

File tree

7 files changed

+818
-1
lines changed

7 files changed

+818
-1
lines changed

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1511,7 +1511,12 @@ This is a one-time fix-up, please be patient...
15111511

15121512
#idealTreePrune () {
15131513
for (const node of this.idealTree.inventory.values()) {
1514-
if (node.extraneous) {
1514+
// optional peer dependencies are meant to be added to the tree
1515+
// through an explicit required dependency (most commonly in the
1516+
// root package.json), at which point they won't be optional so
1517+
// any dependencies still marked as both optional and peer at
1518+
// this point can be pruned as a special kind of extraneous
1519+
if (node.extraneous || (node.peer && node.optional)) {
15151520
node.parent = null
15161521
}
15171522
}

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

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,82 @@ ArboristNode {
124124
}
125125
`
126126

127+
exports[`test/arborist/pruner.js TAP prune with lockfile with implicit optional peer dependencies > should remove all deps from reified tree 1`] = `
128+
ArboristNode {
129+
"children": Map {
130+
"dedent" => ArboristNode {
131+
"edgesIn": Set {
132+
EdgeIn {
133+
"from": "",
134+
"name": "dedent",
135+
"spec": "^1.6.0",
136+
"type": "prod",
137+
},
138+
},
139+
"edgesOut": Map {
140+
"babel-plugin-macros" => EdgeOut {
141+
"name": "babel-plugin-macros",
142+
"spec": "^3.1.0",
143+
"to": null,
144+
"type": "peerOptional",
145+
},
146+
},
147+
"location": "node_modules/dedent",
148+
"name": "dedent",
149+
"path": "{CWD}/test/arborist/tap-testdir-pruner-prune-with-lockfile-with-implicit-optional-peer-dependencies/node_modules/dedent",
150+
"resolved": "https://registry.npmjs.org/dedent/-/dedent-1.6.0.tgz",
151+
"version": "1.6.0",
152+
},
153+
},
154+
"edgesOut": Map {
155+
"dedent" => EdgeOut {
156+
"name": "dedent",
157+
"spec": "^1.6.0",
158+
"to": "node_modules/dedent",
159+
"type": "prod",
160+
},
161+
},
162+
"isProjectRoot": true,
163+
"location": "",
164+
"name": "tap-testdir-pruner-prune-with-lockfile-with-implicit-optional-peer-dependencies",
165+
"packageName": "prune-lockfile-optional-peer",
166+
"path": "{CWD}/test/arborist/tap-testdir-pruner-prune-with-lockfile-with-implicit-optional-peer-dependencies",
167+
"version": "1.0.0",
168+
}
169+
`
170+
171+
exports[`test/arborist/pruner.js TAP prune with lockfile with implicit optional peer dependencies > should remove optional peer dependencies in package-lock.json 1`] = `
172+
Object {
173+
"lockfileVersion": 3,
174+
"name": "prune-lockfile-optional-peer",
175+
"packages": Object {
176+
"": Object {
177+
"dependencies": Object {
178+
"dedent": "^1.6.0",
179+
},
180+
"name": "prune-lockfile-optional-peer",
181+
"version": "1.0.0",
182+
},
183+
"node_modules/dedent": Object {
184+
"integrity": "sha512-F1Z+5UCFpmQUzJa11agbyPVMbpgT/qA3/SKyJ1jyBgm7dUcUEa8v9JwDkerSQXfakBwFljIxhOJqGkjUwZ9FSA==",
185+
"license": "MIT",
186+
"peerDependencies": Object {
187+
"babel-plugin-macros": "^3.1.0",
188+
},
189+
"peerDependenciesMeta": Object {
190+
"babel-plugin-macros": Object {
191+
"optional": true,
192+
},
193+
},
194+
"resolved": "https://registry.npmjs.org/dedent/-/dedent-1.6.0.tgz",
195+
"version": "1.6.0",
196+
},
197+
},
198+
"requires": true,
199+
"version": "1.0.0",
200+
}
201+
`
202+
127203
exports[`test/arborist/pruner.js TAP prune workspaces > must match snapshot 1`] = `
128204
ArboristNode {
129205
"children": Map {

workspaces/arborist/test/arborist/pruner.js

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,36 @@ t.test('prune with lockfile', async t => {
3838
t.matchSnapshot(printTree(tree))
3939
})
4040

41+
t.test('prune with lockfile with implicit optional peer dependencies', async t => {
42+
registry.audit({})
43+
const opts = {}
44+
45+
// todo: for some reason on Windows when doing this test NPM looks for
46+
// the cache in the home directory, resulting in an unexpected real
47+
// call being made to the registry
48+
if (process.platform === 'win32') {
49+
opts.cache = 'C:\\npm\\cache\\_cacache'
50+
}
51+
52+
const path = fixture(t, 'prune-lockfile-optional-peer')
53+
const tree = await pruneTree(path, opts)
54+
55+
const dep = tree.children.get('dedent')
56+
t.ok(dep, 'required prod dep was pruned from tree')
57+
58+
const optionalPeerDep = tree.children.get('babel-plugin-macros')
59+
t.notOk(optionalPeerDep, 'all listed optional peer deps pruned from tree')
60+
61+
t.matchSnapshot(
62+
require(path + '/package-lock.json'),
63+
'should remove optional peer dependencies in package-lock.json'
64+
)
65+
t.matchSnapshot(
66+
printTree(tree),
67+
'should remove all deps from reified tree'
68+
)
69+
})
70+
4171
t.test('prune with actual tree omit dev', async t => {
4272
const path = fixture(t, 'prune-actual-omit-dev')
4373
const tree = await pruneTree(path, { omit: ['dev'] })

0 commit comments

Comments
 (0)