Skip to content

fix: corrects peer dependency flag propagation #8579

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 19, 2025
Merged

fix: corrects peer dependency flag propagation #8579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
222 changes: 30 additions & 192 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

809 changes: 809 additions & 0 deletions tap-snapshots/workspaces/arborist/test/calc-dep-flags.js.test.cjs
View file
Open in desktop

Large diffs are not rendered by default.

43 changes: 31 additions & 12 deletions workspaces/arborist/lib/calc-dep-flags.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const calcDepFlagsStep = (node) => {
// or normal dependency graphs overlap deep in the dep graph.
// Since we're only walking through deps that are not already flagged
// as non-dev/non-optional, it's typically a very shallow traversal

node.extraneous = false
resetParents(node, 'extraneous')
resetParents(node, 'dev')
Expand All @@ -47,10 +48,16 @@ const calcDepFlagsStep = (node) => {
if (!to) {
return
}

// everything with any kind of edge into it is not extraneous
to.extraneous = false

// If this is a peer edge, mark the target as peer
if (peer) {
to.peer = true
} else if (to.peer && !hasIncomingPeerEdge(to)) {
unsetFlag(to, 'peer')
}

// devOptional is the *overlap* of the dev and optional tree.
// however, for convenience and to save an extra rewalk, we leave
// it set when we are in *either* tree, and then omit it from the
Expand All @@ -61,11 +68,6 @@ const calcDepFlagsStep = (node) => {
// either the dev or opt trees
const unsetDev = unsetDevOpt || !node.dev && !dev
const unsetOpt = unsetDevOpt || !node.optional && !optional
const unsetPeer = !node.peer && !peer

if (unsetPeer) {
unsetFlag(to, 'peer')
}

if (unsetDevOpt) {
unsetFlag(to, 'devOptional')
Expand All @@ -83,6 +85,16 @@ const calcDepFlagsStep = (node) => {
return node
}

const hasIncomingPeerEdge = (node) => {
const target = node.isLink && node.target ? node.target : node
for (const edge of target.edgesIn) {
if (edge.type === 'peer') {
return true
}
}
return false
}

const resetParents = (node, flag) => {
if (node[flag]) {
return
Expand All @@ -109,12 +121,19 @@ const unsetFlag = (node, flag) => {
const children = []
const targetNode = node.isLink && node.target ? node.target : node
for (const edge of targetNode.edgesOut.values()) {
if (
edge.to &&
edge.to[flag] &&
((flag !== 'peer' && edge.type === 'peer') || edge.type === 'prod')
) {
children.push(edge.to)
if (edge.to?.[flag]) {
// For the peer flag, only follow peer edges to unset the flag
// Don't propagate peer flag through prod/dev/optional edges
if (flag === 'peer') {
if (edge.type === 'peer') {
children.push(edge.to)
}
} else {
// For other flags, follow prod edges (and peer edges for non-peer flags)
if (edge.type === 'prod' || edge.type === 'peer') {
children.push(edge.to)
}
}
}
}
return children
Expand Down
418 changes: 168 additions & 250 deletions workspaces/arborist/tap-snapshots/test/arborist/build-ideal-tree.js.test.cjs
View file
Open in desktop

Large diffs are not rendered by default.

5 changes: 5 additions & 0 deletions workspaces/arborist/tap-snapshots/test/arborist/load-actual.js.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ ArboristNode {
"location": "node_modules/@scope/y",
"name": "@scope/y",
"path": "root/node_modules/@scope/y",
"peer": true,
"version": "1.2.3",
},
"foo" => ArboristNode {
Expand Down Expand Up @@ -869,6 +870,7 @@ ArboristLink {
"location": "node_modules/@scope/y",
"name": "@scope/y",
"path": "root/node_modules/@scope/y",
"peer": true,
"version": "1.2.3",
},
"foo" => ArboristNode {
Expand Down Expand Up @@ -2699,6 +2701,7 @@ ArboristLink {
"location": "node_modules/@scope/y",
"name": "@scope/y",
"path": "root/node_modules/@scope/y",
"peer": true,
"version": "1.2.3",
},
"foo" => ArboristNode {
Expand Down Expand Up @@ -4428,6 +4431,7 @@ ArboristNode {
"location": "node_modules/@scope/y",
"name": "@scope/y",
"path": "root/node_modules/@scope/y",
"peer": true,
"version": "1.2.3",
},
"foo" => ArboristNode {
Expand Down Expand Up @@ -6072,6 +6076,7 @@ ArboristNode {
"location": "node_modules/@scope/y",
"name": "@scope/y",
"path": "root/node_modules/@scope/y",
"peer": true,
"version": "1.2.3",
},
"foo" => ArboristNode {
Expand Down
1 change: 0 additions & 1 deletion workspaces/arborist/tap-snapshots/test/arborist/load-virtual.js.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,6 @@ ArboristNode {
"location": "node_modules/wrappy",
"name": "wrappy",
"path": "{CWD}/test/fixtures/edit-package-json/changed/node_modules/wrappy",
"peer": true,
"resolved": "https://registry.npmjs.org/wrappy/-/wrappy-1.0.2.tgz",
"version": "1.0.2",
},
Expand Down
93 changes: 93 additions & 0 deletions workspaces/arborist/tap-snapshots/test/arborist/pruner.js.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,99 @@
* Make sure to inspect the output below. Do not ignore changes!
*/
'use strict'
exports[`test/arborist/pruner.js TAP do not prune dependencies that are optional but not peer > must match snapshot 1`] = `
ArboristNode {
"children": Map {
"optional-dep" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "node_modules/peer-pkg",
"name": "optional-dep",
"spec": "1.0.0",
"type": "optional",
},
},
"location": "node_modules/optional-dep",
"name": "optional-dep",
"optional": true,
"path": "{CWD}/test/arborist/tap-testdir-pruner-do-not-prune-dependencies-that-are-optional-but-not-peer/node_modules/optional-dep",
"version": "1.0.0",
},
"peer-pkg" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "",
"name": "peer-pkg",
"spec": "1.0.0",
"type": "peer",
},
EdgeIn {
"from": "node_modules/pkg-a",
"name": "peer-pkg",
"spec": "1.0.0",
"type": "peer",
},
},
"edgesOut": Map {
"optional-dep" => EdgeOut {
"name": "optional-dep",
"spec": "1.0.0",
"to": "node_modules/optional-dep",
"type": "optional",
},
},
"location": "node_modules/peer-pkg",
"name": "peer-pkg",
"path": "{CWD}/test/arborist/tap-testdir-pruner-do-not-prune-dependencies-that-are-optional-but-not-peer/node_modules/peer-pkg",
"peer": true,
"version": "1.0.0",
},
"pkg-a" => ArboristNode {
"edgesIn": Set {
EdgeIn {
"from": "",
"name": "pkg-a",
"spec": "1.0.0",
"type": "prod",
},
},
"edgesOut": Map {
"peer-pkg" => EdgeOut {
"name": "peer-pkg",
"spec": "1.0.0",
"to": "node_modules/peer-pkg",
"type": "peer",
},
},
"location": "node_modules/pkg-a",
"name": "pkg-a",
"path": "{CWD}/test/arborist/tap-testdir-pruner-do-not-prune-dependencies-that-are-optional-but-not-peer/node_modules/pkg-a",
"version": "1.0.0",
},
},
"edgesOut": Map {
"peer-pkg" => EdgeOut {
"name": "peer-pkg",
"spec": "1.0.0",
"to": "node_modules/peer-pkg",
"type": "peer",
},
"pkg-a" => EdgeOut {
"name": "pkg-a",
"spec": "1.0.0",
"to": "node_modules/pkg-a",
"type": "prod",
},
},
"isProjectRoot": true,
"location": "",
"name": "tap-testdir-pruner-do-not-prune-dependencies-that-are-optional-but-not-peer",
"packageName": "peer-optional-test",
"path": "{CWD}/test/arborist/tap-testdir-pruner-do-not-prune-dependencies-that-are-optional-but-not-peer",
"version": "1.0.0",
}
`

exports[`test/arborist/pruner.js TAP prune with actual tree > must match snapshot 1`] = `
ArboristNode {
"isProjectRoot": true,
Expand Down
14 changes: 14 additions & 0 deletions workspaces/arborist/tap-snapshots/test/arborist/reify.js.test.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10483,6 +10483,7 @@ ArboristNode {
"location": "node_modules/react",
"name": "react",
"path": "{CWD}/test/arborist/tap-testdir-reify-multiple-bundles-at-the-same-level/node_modules/react",
"peer": true,
"resolved": "https://registry.npmjs.org/react/-/react-16.12.0.tgz",
"version": "16.12.0",
},
Expand Down Expand Up @@ -11876,6 +11877,7 @@ ArboristNode {
"location": "node_modules/tap/node_modules/@babel/core",
"name": "@babel/core",
"path": "{CWD}/test/arborist/tap-testdir-reify-multiple-bundles-at-the-same-level/node_modules/tap/node_modules/@babel/core",
"peer": true,
"version": "7.7.5",
},
"@babel/generator" => ArboristNode {
Expand Down Expand Up @@ -12770,6 +12772,7 @@ ArboristNode {
"location": "node_modules/tap/node_modules/@types/react",
"name": "@types/react",
"path": "{CWD}/test/arborist/tap-testdir-reify-multiple-bundles-at-the-same-level/node_modules/tap/node_modules/@types/react",
"peer": true,
"version": "16.9.16",
},
"ansi-escapes" => ArboristNode {
Expand Down Expand Up @@ -18724,6 +18727,7 @@ ArboristNode {
"location": "node_modules/ajv",
"name": "ajv",
"path": "{CWD}/test/arborist/tap-testdir-reify-reify-properly-with-all-deps-when-lockfile-is-ancient/node_modules/ajv",
"peer": true,
"resolved": "https://registry.npmjs.org/ajv/-/ajv-4.11.2.tgz",
"version": "4.11.2",
},
Expand Down Expand Up @@ -21590,6 +21594,7 @@ ArboristNode {
"location": "node_modules/eslint",
"name": "eslint",
"path": "{CWD}/test/arborist/tap-testdir-reify-reify-properly-with-all-deps-when-lockfile-is-ancient/node_modules/eslint",
"peer": true,
"resolved": "https://registry.npmjs.org/eslint/-/eslint-3.10.2.tgz",
"version": "3.10.2",
},
Expand Down Expand Up @@ -21678,6 +21683,7 @@ ArboristNode {
"location": "node_modules/eslint-plugin-promise",
"name": "eslint-plugin-promise",
"path": "{CWD}/test/arborist/tap-testdir-reify-reify-properly-with-all-deps-when-lockfile-is-ancient/node_modules/eslint-plugin-promise",
"peer": true,
"resolved": "https://registry.npmjs.org/eslint-plugin-promise/-/eslint-plugin-promise-3.4.1.tgz",
"version": "3.4.1",
},
Expand Down Expand Up @@ -21720,6 +21726,7 @@ ArboristNode {
"location": "node_modules/eslint-plugin-react",
"name": "eslint-plugin-react",
"path": "{CWD}/test/arborist/tap-testdir-reify-reify-properly-with-all-deps-when-lockfile-is-ancient/node_modules/eslint-plugin-react",
"peer": true,
"resolved": "https://registry.npmjs.org/eslint-plugin-react/-/eslint-plugin-react-6.7.1.tgz",
"version": "6.7.1",
},
Expand Down Expand Up @@ -21750,6 +21757,7 @@ ArboristNode {
"location": "node_modules/eslint-plugin-standard",
"name": "eslint-plugin-standard",
"path": "{CWD}/test/arborist/tap-testdir-reify-reify-properly-with-all-deps-when-lockfile-is-ancient/node_modules/eslint-plugin-standard",
"peer": true,
"resolved": "https://registry.npmjs.org/eslint-plugin-standard/-/eslint-plugin-standard-2.0.1.tgz",
"version": "2.0.1",
},
Expand Down Expand Up @@ -34342,6 +34350,7 @@ ArboristNode {
"location": "node_modules/@isaacs/testing-peer-deps-b",
"name": "@isaacs/testing-peer-deps-b",
"path": "{CWD}/test/arborist/tap-testdir-reify-testing-peer-deps-nested-with-update/node_modules/@isaacs/testing-peer-deps-b",
"peer": true,
"resolved": "https://registry.npmjs.org/@isaacs/testing-peer-deps-b/-/testing-peer-deps-b-2.0.1.tgz",
"version": "2.0.1",
},
Expand Down Expand Up @@ -40618,6 +40627,7 @@ ArboristNode {
"location": "node_modules/react",
"name": "react",
"path": "{CWD}/test/arborist/tap-testdir-reify-update-a-bundling-node-without-updating-all-of-its-deps/node_modules/react",
"peer": true,
"resolved": "https://registry.npmjs.org/react/-/react-15.6.2.tgz",
"version": "15.6.2",
},
Expand Down Expand Up @@ -41818,6 +41828,7 @@ ArboristNode {
"location": "node_modules/tap/node_modules/@babel/core",
"name": "@babel/core",
"path": "{CWD}/test/arborist/tap-testdir-reify-update-a-bundling-node-without-updating-all-of-its-deps/node_modules/tap/node_modules/@babel/core",
"peer": true,
"version": "7.7.5",
},
"@babel/generator" => ArboristNode {
Expand Down Expand Up @@ -42712,6 +42723,7 @@ ArboristNode {
"location": "node_modules/tap/node_modules/@types/react",
"name": "@types/react",
"path": "{CWD}/test/arborist/tap-testdir-reify-update-a-bundling-node-without-updating-all-of-its-deps/node_modules/tap/node_modules/@types/react",
"peer": true,
"version": "16.9.16",
},
"ansi-escapes" => ArboristNode {
Expand Down Expand Up @@ -44438,6 +44450,7 @@ ArboristNode {
"location": "node_modules/tap/node_modules/react",
"name": "react",
"path": "{CWD}/test/arborist/tap-testdir-reify-update-a-bundling-node-without-updating-all-of-its-deps/node_modules/tap/node_modules/react",
"peer": true,
"resolved": "https://registry.npmjs.org/react/-/react-16.12.0.tgz",
"version": "16.12.0",
},
Expand Down Expand Up @@ -46277,6 +46290,7 @@ ArboristNode {
"location": "node_modules/typescript",
"name": "typescript",
"path": "{CWD}/test/arborist/tap-testdir-reify-update-a-bundling-node-without-updating-all-of-its-deps/node_modules/typescript",
"peer": true,
"resolved": "https://registry.npmjs.org/typescript/-/typescript-3.7.4.tgz",
"version": "3.7.4",
},
Expand Down
Loading
Loading