Skip to content

fix: prune optional peer dependencies that are no longer explicitly depended on #8449

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: release/v10
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions workspaces/arborist/lib/arborist/build-ideal-tree.js
Original file line number Diff line number Diff line change
Expand Up @@ -1465,19 +1465,19 @@ This is a one-time fix-up, please be patient...
const needPrune = metaFromDisk && (mutateTree || flagsSuspect)
if (this.#prune && needPrune) {
this.#idealTreePrune()
for (const node of this.idealTree.inventory.values()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: I snuck #8430 in because I feel otherwise technically I should apply the fix to this loop too as they're the same, and technically this loop has a performance cost so removing it is a little nicer

if (node.extraneous) {
node.parent = null
}
}
}

timeEnd()
}

#idealTreePrune () {
for (const node of this.idealTree.inventory.values()) {
if (node.extraneous) {
// optional peer dependencies are meant to be added to the tree
// through an explicit required dependency (most commonly in the
// root package.json), at which point they won't be optional so
// any dependencies still marked as both optional and peer at
// this point can be pruned as a special kind of extraneous
if (node.extraneous || (node.peer && node.optional)) {
node.parent = null
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,82 @@ ArboristNode {
}
`

exports[`test/arborist/pruner.js TAP prune with lockfile with implicit optional peer dependencies > should remove all deps from reified tree 1`] = `
ArboristNode {
"children": Map {
"dedent" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "",
"name": "dedent",
"spec": "^1.6.0",
"type": "prod",
},
},
"edgesOut": Map {
"babel-plugin-macros" => EdgeOut {
"name": "babel-plugin-macros",
"spec": "^3.1.0",
"to": null,
"type": "peerOptional",
},
},
"location": "node_modules/dedent",
"name": "dedent",
"path": "{CWD}/test/arborist/tap-testdir-pruner-prune-with-lockfile-with-implicit-optional-peer-dependencies/node_modules/dedent",
"resolved": "https://registry.npmjs.org/dedent/-/dedent-1.6.0.tgz",
"version": "1.6.0",
},
},
"edgesOut": Map {
"dedent" => EdgeOut {
"name": "dedent",
"spec": "^1.6.0",
"to": "node_modules/dedent",
"type": "prod",
},
},
"isProjectRoot": true,
"location": "",
"name": "tap-testdir-pruner-prune-with-lockfile-with-implicit-optional-peer-dependencies",
"packageName": "prune-lockfile-optional-peer",
"path": "{CWD}/test/arborist/tap-testdir-pruner-prune-with-lockfile-with-implicit-optional-peer-dependencies",
"version": "1.0.0",
}
`

exports[`test/arborist/pruner.js TAP prune with lockfile with implicit optional peer dependencies > should remove optional peer dependencies in package-lock.json 1`] = `
Object {
"lockfileVersion": 3,
"name": "prune-lockfile-optional-peer",
"packages": Object {
"": Object {
"dependencies": Object {
"dedent": "^1.6.0",
},
"name": "prune-lockfile-optional-peer",
"version": "1.0.0",
},
"node_modules/dedent": Object {
"integrity": "sha512-F1Z+5UCFpmQUzJa11agbyPVMbpgT/qA3/SKyJ1jyBgm7dUcUEa8v9JwDkerSQXfakBwFljIxhOJqGkjUwZ9FSA==",
"license": "MIT",
"peerDependencies": Object {
"babel-plugin-macros": "^3.1.0",
},
"peerDependenciesMeta": Object {
"babel-plugin-macros": Object {
"optional": true,
},
},
"resolved": "https://registry.npmjs.org/dedent/-/dedent-1.6.0.tgz",
"version": "1.6.0",
},
},
"requires": true,
"version": "1.0.0",
}
`

exports[`test/arborist/pruner.js TAP prune workspaces > must match snapshot 1`] = `
ArboristNode {
"children": Map {
Expand Down
29 changes: 29 additions & 0 deletions workspaces/arborist/test/arborist/pruner.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,35 @@ t.test('prune with lockfile', async t => {
t.matchSnapshot(printTree(tree))
})

t.test('prune with lockfile with implicit optional peer dependencies', async t => {
const opts = {}

// todo: for some reason on Windows when doing this test NPM looks for
// the cache in the home directory, resulting in an unexpected real
// call being made to the registry
if (process.platform === 'win32') {
opts.cache = 'C:\\npm\\cache\\_cacache'
}

const path = fixture(t, 'prune-lockfile-optional-peer')
const tree = await pruneTree(path, opts)

const dep = tree.children.get('dedent')
t.ok(dep, 'required prod dep was pruned from tree')

const optionalPeerDep = tree.children.get('babel-plugin-macros')
t.notOk(optionalPeerDep, 'all listed optional peer deps pruned from tree')

t.matchSnapshot(
require(path + '/package-lock.json'),
'should remove optional peer dependencies in package-lock.json'
)
t.matchSnapshot(
printTree(tree),
'should remove all deps from reified tree'
)
})

t.test('prune with actual tree omit dev', async t => {
const path = fixture(t, 'prune-actual-omit-dev')
const tree = await pruneTree(path, { omit: ['dev'] })
Expand Down
Loading
Loading