diff --git a/lib/commands/ci.js b/lib/commands/ci.js index 17307badede98..207e4ee12ec54 100644 --- a/lib/commands/ci.js +++ b/lib/commands/ci.js @@ -6,7 +6,11 @@ const fs = require('node:fs/promises') const path = require('node:path') const { log, time } = require('proc-log') const validateLockfile = require('../utils/validate-lockfile.js') -const { validatePackageExtensions, validateNpmExtension } = require('../utils/validate-lockfile.js') +const { + validateAutoInstalledPeers, + validatePackageExtensions, + validateNpmExtension, +} = require('../utils/validate-lockfile.js') const ArboristWorkspaceCmd = require('../arborist-cmd.js') const getWorkspaces = require('../utils/get-workspaces.js') @@ -22,6 +26,7 @@ class CI extends ArboristWorkspaceCmd { 'omit', 'include', 'strict-peer-deps', + 'auto-install-peers', 'foreground-scripts', 'ignore-scripts', 'allow-directory', @@ -105,6 +110,9 @@ class CI extends ArboristWorkspaceCmd { errors.push(`Invalid: ${err.message}`) } errors.push(...validateNpmExtension(virtualArb.virtualTree, fileHash)) + if (opts.autoInstallPeers === false) { + errors.push(...validateAutoInstalledPeers(virtualArb.virtualTree)) + } if (errors.length) { throw this.usageError( '`npm ci` can only install packages when your package.json and package-lock.json are in sync. ' + diff --git a/lib/commands/dedupe.js b/lib/commands/dedupe.js index 0b3fee45bf256..fefb585591a29 100644 --- a/lib/commands/dedupe.js +++ b/lib/commands/dedupe.js @@ -12,6 +12,7 @@ class Dedupe extends ArboristWorkspaceCmd { 'legacy-bundling', 'global-style', 'strict-peer-deps', + 'auto-install-peers', 'package-lock', 'omit', 'include', diff --git a/lib/commands/find-dupes.js b/lib/commands/find-dupes.js index 15b312e6a7aec..e5c05cd8caab3 100644 --- a/lib/commands/find-dupes.js +++ b/lib/commands/find-dupes.js @@ -8,6 +8,7 @@ class FindDupes extends ArboristWorkspaceCmd { 'legacy-bundling', 'global-style', 'strict-peer-deps', + 'auto-install-peers', 'package-lock', 'omit', 'include', diff --git a/lib/commands/install.js b/lib/commands/install.js index 2fd9bc8d5cd7a..c732a64c2f260 100644 --- a/lib/commands/install.js +++ b/lib/commands/install.js @@ -25,6 +25,7 @@ class Install extends ArboristWorkspaceCmd { 'omit', 'include', 'strict-peer-deps', + 'auto-install-peers', 'prefer-dedupe', 'package-lock', 'package-lock-only', diff --git a/lib/commands/link.js b/lib/commands/link.js index cd6782a1f9836..f26cf1ee70107 100644 --- a/lib/commands/link.js +++ b/lib/commands/link.js @@ -24,6 +24,7 @@ class Link extends ArboristWorkspaceCmd { 'legacy-bundling', 'global-style', 'strict-peer-deps', + 'auto-install-peers', 'package-lock', 'omit', 'include', diff --git a/lib/commands/update.js b/lib/commands/update.js index 64c2c5128bb04..6e659a980eb15 100644 --- a/lib/commands/update.js +++ b/lib/commands/update.js @@ -19,6 +19,7 @@ class Update extends ArboristWorkspaceCmd { 'omit', 'include', 'strict-peer-deps', + 'auto-install-peers', 'package-lock', 'foreground-scripts', 'ignore-scripts', diff --git a/lib/utils/validate-lockfile.js b/lib/utils/validate-lockfile.js index 9039a797c03dd..9ae78268af293 100644 --- a/lib/utils/validate-lockfile.js +++ b/lib/utils/validate-lockfile.js @@ -125,6 +125,14 @@ function validateNpmExtension (virtualTree, fileHash) { return [] } +// with `auto-install-peers=false`, a lockfile must not keep packages that were only added to satisfy a dependency's peer +function validateAutoInstalledPeers (virtualTree) { + const { autoInstalledPeerNodes } = require('@npmcli/arborist') + return autoInstalledPeerNodes(virtualTree).map(node => + `Invalid: lock file's ${node.name}@${node.version} is only an auto-installed peer dependency`) +} + module.exports = validateLockfile module.exports.validatePackageExtensions = validatePackageExtensions module.exports.validateNpmExtension = validateNpmExtension +module.exports.validateAutoInstalledPeers = validateAutoInstalledPeers diff --git a/smoke-tests/tap-snapshots/test/index.js.test.cjs b/smoke-tests/tap-snapshots/test/index.js.test.cjs index 4d825ff73a96b..bf8aff1e6c59a 100644 --- a/smoke-tests/tap-snapshots/test/index.js.test.cjs +++ b/smoke-tests/tap-snapshots/test/index.js.test.cjs @@ -58,9 +58,10 @@ npm error Options: npm error [--install-strategy ] [--legacy-bundling] npm error [--global-style] [--omit [--omit ...]] npm error [--include [--include ...]] -npm error [--strict-peer-deps] [--foreground-scripts] [--ignore-scripts] -npm error [--allow-directory ] [--allow-file ] -npm error [--allow-git ] [--allow-remote ] +npm error [--strict-peer-deps] [--no-auto-install-peers] [--foreground-scripts] +npm error [--ignore-scripts] [--allow-directory ] +npm error [--allow-file ] [--allow-git ] +npm error [--allow-remote ] npm error [--allow-scripts [--allow-scripts ...]] npm error [--strict-allow-scripts] [--dangerously-allow-all-scripts] [--no-audit] npm error [--no-bin-links] [--no-fund] [--dry-run] @@ -85,6 +86,9 @@ npm error npm error --strict-peer-deps npm error If set to \`true\`, and \`--legacy-peer-deps\` is not set, then _any_ npm error +npm error --auto-install-peers +npm error When set to \`false\`, npm does not add packages to the tree only to +npm error npm error --foreground-scripts npm error Run all build scripts (ie, \`preinstall\`, \`install\`, and npm error diff --git a/tap-snapshots/test/lib/commands/config.js.test.cjs b/tap-snapshots/test/lib/commands/config.js.test.cjs index c92e2d942e34d..4995e758525fc 100644 --- a/tap-snapshots/test/lib/commands/config.js.test.cjs +++ b/tap-snapshots/test/lib/commands/config.js.test.cjs @@ -26,6 +26,7 @@ exports[`test/lib/commands/config.js TAP config list --json > output matches sna "also": null, "audit": true, "audit-level": null, + "auto-install-peers": true, "auth-type": "web", "before": null, "bin-links": true, @@ -223,6 +224,7 @@ also = null audit = true audit-level = null auth-type = "web" +auto-install-peers = true before = null bin-links = true browser = null diff --git a/tap-snapshots/test/lib/docs.js.test.cjs b/tap-snapshots/test/lib/docs.js.test.cjs index 3908dedbcf181..394d1f84aaeb4 100644 --- a/tap-snapshots/test/lib/docs.js.test.cjs +++ b/tap-snapshots/test/lib/docs.js.test.cjs @@ -404,6 +404,30 @@ config is given, this value will always be set to \`legacy\`. +#### \`auto-install-peers\` + +* Default: true +* Type: Boolean + +When set to \`false\`, npm does not add packages to the tree only to satisfy a +required \`peerDependencies\` entry of a dependency. Instead, it warns about +every package that depends on a package with a required peer without +declaring that peer itself. + +Each package must declare the peer itself: a provider elsewhere in the tree, +such as the project root for a workspace, does not count. A +\`devDependencies\` entry only counts when the package that requires the peer +is a \`devDependencies\` or \`peerDependencies\` entry of the project root or a +workspace, and not also one of its \`dependencies\` or \`optionalDependencies\`. + +With \`--strict-peer-deps\`, such a warning for the project root or a +workspace fails the install. + +This setting changes the tree and is not recorded in the lockfile, so set it +in the project \`.npmrc\`. Ignored when \`--legacy-peer-deps\` is set. + + + #### \`before\` * Default: null @@ -2125,6 +2149,10 @@ When such an override is performed, a warning is printed, explaining the conflict and the packages involved. If \`--strict-peer-deps\` is set, then this warning is treated as a failure. +With \`--auto-install-peers=false\`, it also fails the install when the +project root or a workspace does not declare a peer that one of its +dependencies requires. + #### \`strict-ssl\` @@ -2616,6 +2644,7 @@ Array [ "also", "audit", "audit-level", + "auto-install-peers", "auth-type", "before", "bin-links", @@ -2812,6 +2841,7 @@ Array [ "also", "audit", "audit-level", + "auto-install-peers", "auth-type", "before", "bin-links", @@ -3015,6 +3045,7 @@ Object { "audit": true, "auditLevel": null, "authType": "web", + "autoInstallPeers": true, "before": null, "binLinks": true, "browser": null, @@ -3455,9 +3486,10 @@ Options: [--install-strategy ] [--legacy-bundling] [--global-style] [--omit [--omit ...]] [--include [--include ...]] -[--strict-peer-deps] [--foreground-scripts] [--ignore-scripts] -[--allow-directory ] [--allow-file ] -[--allow-git ] [--allow-remote ] +[--strict-peer-deps] [--no-auto-install-peers] [--foreground-scripts] +[--ignore-scripts] [--allow-directory ] +[--allow-file ] [--allow-git ] +[--allow-remote ] [--allow-scripts [--allow-scripts ...]] [--strict-allow-scripts] [--dangerously-allow-all-scripts] [--no-audit] [--no-bin-links] [--no-fund] [--dry-run] @@ -3482,6 +3514,9 @@ Options: --strict-peer-deps If set to \`true\`, and \`--legacy-peer-deps\` is not set, then _any_ + --auto-install-peers + When set to \`false\`, npm does not add packages to the tree only to + --foreground-scripts Run all build scripts (ie, \`preinstall\`, \`install\`, and @@ -3550,6 +3585,7 @@ aliases: clean-install, ic, install-clean, isntall-clean #### \`omit\` #### \`include\` #### \`strict-peer-deps\` +#### \`auto-install-peers\` #### \`foreground-scripts\` #### \`ignore-scripts\` #### \`allow-directory\` @@ -3652,8 +3688,8 @@ npm dedupe Options: [--install-strategy ] [--legacy-bundling] -[--global-style] [--strict-peer-deps] [--no-package-lock] -[--omit [--omit ...]] +[--global-style] [--strict-peer-deps] [--no-auto-install-peers] +[--no-package-lock] [--omit [--omit ...]] [--include [--include ...]] [--ignore-scripts] [--allow-directory ] [--allow-file ] [--allow-git ] @@ -3674,6 +3710,9 @@ Options: --strict-peer-deps If set to \`true\`, and \`--legacy-peer-deps\` is not set, then _any_ + --auto-install-peers + When set to \`false\`, npm does not add packages to the tree only to + --package-lock If set to false, then ignore \`package-lock.json\` files when installing. @@ -3737,6 +3776,7 @@ alias: ddp #### \`legacy-bundling\` #### \`global-style\` #### \`strict-peer-deps\` +#### \`auto-install-peers\` #### \`package-lock\` #### \`omit\` #### \`include\` @@ -4157,8 +4197,8 @@ npm find-dupes Options: [--install-strategy ] [--legacy-bundling] -[--global-style] [--strict-peer-deps] [--no-package-lock] -[--omit [--omit ...]] +[--global-style] [--strict-peer-deps] [--no-auto-install-peers] +[--no-package-lock] [--omit [--omit ...]] [--include [--include ...]] [--ignore-scripts] [--no-audit] [--no-bin-links] [--no-fund] [-w|--workspace [-w|--workspace ...]] @@ -4176,6 +4216,9 @@ Options: --strict-peer-deps If set to \`true\`, and \`--legacy-peer-deps\` is not set, then _any_ + --auto-install-peers + When set to \`false\`, npm does not add packages to the tree only to + --package-lock If set to false, then ignore \`package-lock.json\` files when installing. @@ -4220,6 +4263,7 @@ npm find-dupes #### \`legacy-bundling\` #### \`global-style\` #### \`strict-peer-deps\` +#### \`auto-install-peers\` #### \`package-lock\` #### \`omit\` #### \`include\` @@ -4445,8 +4489,9 @@ Options: [--install-strategy ] [--legacy-bundling] [--global-style] [--omit [--omit ...]] [--include [--include ...]] -[--strict-peer-deps] [--prefer-dedupe] [--no-package-lock] [--package-lock-only] -[--foreground-scripts] [--ignore-scripts] [--allow-directory ] +[--strict-peer-deps] [--no-auto-install-peers] [--prefer-dedupe] +[--no-package-lock] [--package-lock-only] [--foreground-scripts] +[--ignore-scripts] [--allow-directory ] [--allow-file ] [--allow-git ] [--allow-remote ] [--allow-scripts [--allow-scripts ...]] @@ -4485,6 +4530,9 @@ Options: --strict-peer-deps If set to \`true\`, and \`--legacy-peer-deps\` is not set, then _any_ + --auto-install-peers + When set to \`false\`, npm does not add packages to the tree only to + --prefer-dedupe Prefer to deduplicate packages if possible, rather than @@ -4583,6 +4631,7 @@ aliases: add, i, in, ins, inst, insta, instal, isnt, isnta, isntal, isntall #### \`omit\` #### \`include\` #### \`strict-peer-deps\` +#### \`auto-install-peers\` #### \`prefer-dedupe\` #### \`package-lock\` #### \`package-lock-only\` @@ -4621,9 +4670,10 @@ Options: [--install-strategy ] [--legacy-bundling] [--global-style] [--omit [--omit ...]] [--include [--include ...]] -[--strict-peer-deps] [--foreground-scripts] [--ignore-scripts] -[--allow-directory ] [--allow-file ] -[--allow-git ] [--allow-remote ] +[--strict-peer-deps] [--no-auto-install-peers] [--foreground-scripts] +[--ignore-scripts] [--allow-directory ] +[--allow-file ] [--allow-git ] +[--allow-remote ] [--allow-scripts [--allow-scripts ...]] [--strict-allow-scripts] [--dangerously-allow-all-scripts] [--no-audit] [--no-bin-links] [--no-fund] [--dry-run] @@ -4648,6 +4698,9 @@ Options: --strict-peer-deps If set to \`true\`, and \`--legacy-peer-deps\` is not set, then _any_ + --auto-install-peers + When set to \`false\`, npm does not add packages to the tree only to + --foreground-scripts Run all build scripts (ie, \`preinstall\`, \`install\`, and @@ -4716,6 +4769,7 @@ aliases: cit, clean-install-test, sit #### \`omit\` #### \`include\` #### \`strict-peer-deps\` +#### \`auto-install-peers\` #### \`foreground-scripts\` #### \`ignore-scripts\` #### \`allow-directory\` @@ -4793,8 +4847,9 @@ Options: [--install-strategy ] [--legacy-bundling] [--global-style] [--omit [--omit ...]] [--include [--include ...]] -[--strict-peer-deps] [--prefer-dedupe] [--no-package-lock] [--package-lock-only] -[--foreground-scripts] [--ignore-scripts] [--allow-directory ] +[--strict-peer-deps] [--no-auto-install-peers] [--prefer-dedupe] +[--no-package-lock] [--package-lock-only] [--foreground-scripts] +[--ignore-scripts] [--allow-directory ] [--allow-file ] [--allow-git ] [--allow-remote ] [--allow-scripts [--allow-scripts ...]] @@ -4833,6 +4888,9 @@ Options: --strict-peer-deps If set to \`true\`, and \`--legacy-peer-deps\` is not set, then _any_ + --auto-install-peers + When set to \`false\`, npm does not add packages to the tree only to + --prefer-dedupe Prefer to deduplicate packages if possible, rather than @@ -4931,6 +4989,7 @@ alias: it #### \`omit\` #### \`include\` #### \`strict-peer-deps\` +#### \`auto-install-peers\` #### \`prefer-dedupe\` #### \`package-lock\` #### \`package-lock-only\` @@ -4969,8 +5028,8 @@ Options: [-S|--save|--no-save|--save-prod|--save-dev|--save-optional|--save-peer|--save-bundle] [-E|--save-exact] [-g|--global] [--install-strategy ] [--legacy-bundling] -[--global-style] [--strict-peer-deps] [--no-package-lock] -[--omit [--omit ...]] +[--global-style] [--strict-peer-deps] [--no-auto-install-peers] +[--no-package-lock] [--omit [--omit ...]] [--include [--include ...]] [--ignore-scripts] [--allow-directory ] [--allow-file ] [--allow-git ] @@ -5000,6 +5059,9 @@ Options: --strict-peer-deps If set to \`true\`, and \`--legacy-peer-deps\` is not set, then _any_ + --auto-install-peers + When set to \`false\`, npm does not add packages to the tree only to + --package-lock If set to false, then ignore \`package-lock.json\` files when installing. @@ -5066,6 +5128,7 @@ alias: ln #### \`legacy-bundling\` #### \`global-style\` #### \`strict-peer-deps\` +#### \`auto-install-peers\` #### \`package-lock\` #### \`omit\` #### \`include\` @@ -6708,8 +6771,8 @@ Options: [--legacy-bundling] [--global-style] [--omit [--omit ...]] [--include [--include ...]] -[--strict-peer-deps] [--no-package-lock] [--foreground-scripts] -[--ignore-scripts] +[--strict-peer-deps] [--no-auto-install-peers] [--no-package-lock] +[--foreground-scripts] [--ignore-scripts] [--allow-scripts [--allow-scripts ...]] [--strict-allow-scripts] [--dangerously-allow-all-scripts] [--no-audit] [--before ] [--min-release-age ] @@ -6742,6 +6805,9 @@ Options: --strict-peer-deps If set to \`true\`, and \`--legacy-peer-deps\` is not set, then _any_ + --auto-install-peers + When set to \`false\`, npm does not add packages to the tree only to + --package-lock If set to false, then ignore \`package-lock.json\` files when installing. @@ -6812,6 +6878,7 @@ aliases: u, up, upgrade, udpate #### \`omit\` #### \`include\` #### \`strict-peer-deps\` +#### \`auto-install-peers\` #### \`package-lock\` #### \`foreground-scripts\` #### \`ignore-scripts\` diff --git a/test/lib/commands/ci.js b/test/lib/commands/ci.js index a90ca4b0cffe6..2604e8d01c07c 100644 --- a/test/lib/commands/ci.js +++ b/test/lib/commands/ci.js @@ -139,6 +139,28 @@ t.test('fails when packageExtensions are out of sync with the lock file', async ) }) +t.test('auto-install-peers=false fails on a lock file with an auto-installed peer', async t => { + const lock = structuredClone(packageLock) + lock.packages['node_modules/abbrev'].peerDependencies = { peer: '1' } + lock.packages['node_modules/peer'] = { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/peer/-/peer-1.0.0.tgz', + peer: true, + } + const { npm } = await loadMockNpm(t, { + config: { audit: false, 'auto-install-peers': false }, + prefixDir: { + abbrev, + 'package.json': JSON.stringify(packageJson), + 'package-lock.json': JSON.stringify(lock), + }, + }) + await t.rejects( + npm.exec('ci', []), + /Invalid: lock file's peer@1.0.0 is only an auto-installed peer dependency/ + ) +}) + t.test('fails when both .npm-extension files are present', async t => { const { npm } = await loadMockNpm(t, { config: { audit: false }, diff --git a/workspaces/arborist/lib/arborist/build-ideal-tree.js b/workspaces/arborist/lib/arborist/build-ideal-tree.js index 536b0f8a48f91..673bc591d159a 100644 --- a/workspaces/arborist/lib/arborist/build-ideal-tree.js +++ b/workspaces/arborist/lib/arborist/build-ideal-tree.js @@ -26,6 +26,7 @@ const fromPath = require('../from-path.js') const calcDepFlags = require('../calc-dep-flags.js') const { isReleaseAgeExcluded, trustedSpecName } = require('../release-age-exclude.js') const { resolvePatchedDependencies } = require('../patched-dependencies.js') +const { autoInstalledPeerNodes, isAutoInstalledPeer, isMine, undeclaredPeers } = require('../peer-declarations.js') const PackageExtensions = require('../package-extensions.js') const NpmExtension = require('../npm-extension.js') const { hasExtensionFile } = require('../npm-extension.js') @@ -90,6 +91,7 @@ class DepsQueue { } module.exports = cls => class IdealTreeBuilder extends cls { + #autoInstallPeers #complete #currentDep = null #depsQueue = new DepsQueue() @@ -117,6 +119,7 @@ module.exports = cls => class IdealTreeBuilder extends cls { super(options) const { + autoInstallPeers = true, follow = false, installStrategy = 'hoisted', strictPeerDeps = false, @@ -124,6 +127,8 @@ module.exports = cls => class IdealTreeBuilder extends cls { } = options this.#strictPeerDeps = !!strictPeerDeps + // legacy-peer-deps creates no peer edges, so there is nothing to skip or check + this.#autoInstallPeers = autoInstallPeers !== false || this.legacyPeerDeps this.#installStrategy = global ? 'shallow' : installStrategy this.#follow = !!follow @@ -186,9 +191,11 @@ module.exports = cls => class IdealTreeBuilder extends cls { await this.#inflateAncientLockfile() await this.#applyUserRequests(options) await this.#buildDeps() + this.#pruneAutoInstalledPeers() await this.#fixDepFlags() await this.#pruneFailedOptional() await this.#checkEngineAndPlatform() + this.#checkPeerDeclarations() await resolvePatchedDependencies(this.idealTree, { path: this.path, allowUnusedPatches: this.options.allowUnusedPatches, @@ -378,6 +385,10 @@ module.exports = cls => class IdealTreeBuilder extends cls { } this.#complete = !!options.complete + + if (options.autoInstallPeers === false && this.legacyPeerDeps) { + log.warn('config', '`auto-install-peers` has no effect when `legacy-peer-deps` is set') + } this.#preferDedupe = !!options.preferDedupe // validates list of update names, they must @@ -1404,8 +1415,8 @@ This is a one-time fix-up, please be patient... // If the edge has no destination, that's a problem, unless // if it's peerOptional and not explicitly requested. if (!edge.to) { - if (edge.type !== 'peerOptional' || - this.#explicitRequests.has(edge)) { + if ((edge.type !== 'peerOptional' || this.#explicitRequests.has(edge)) && + !this.#skipPeer(edge, edge)) { problems.push(edge) } continue @@ -1622,6 +1633,10 @@ This is a one-time fix-up, please be patient... const conflictOK = this.options.force || !isMine && !this.#strictPeerDeps if (!edge.to) { + // leave a peer that the dependent does not provide to the declaration check + if (this.#skipPeer(edge, node.parent.sourceReference.edgesOut.get(edge.name))) { + continue + } if (!parentEdge) { // the peer is missing from the virtual root; check the real tree before skipping. // we can avoid a fetch for an optional peer, or a compatible provider, though @@ -1704,6 +1719,41 @@ This is a one-time fix-up, please be patient... return node } + // With `auto-install-peers=false`, whether to leave the required peer `edge` unplaced. + #skipPeer (edge, declaring) { + return !this.#autoInstallPeers && isAutoInstalledPeer(edge, declaring) + } + + #pruneAutoInstalledPeers () { + if (this.#autoInstallPeers) { + return + } + for (const node of autoInstalledPeerNodes(this.idealTree)) { + node.parent = null + this.#mutateTree = true + } + } + + #checkPeerDeclarations () { + if (this.#autoInstallPeers) { + return + } + const errors = [] + for (const { node, dep, peer } of undeclaredPeers(this.idealTree)) { + const name = node.packageName || node.name + const msg = `${name} depends on ${dep.packageName}, which requires peer ${peer.name}@${peer.spec}\n` + + `Add ${peer.name} to ${name}'s dependencies or peerDependencies` + if (this.#strictPeerDeps && !this.options.force && isMine(node)) { + errors.push(msg) + } else { + log.warn('EPEERUNDECLARED', msg) + } + } + if (errors.length) { + throw Object.assign(new Error(errors.join('\n')), { code: 'EPEERUNDECLARED' }) + } + } + #failPeerConflict (edge, currentEdge) { const expl = this.#explainPeerConflict(edge, currentEdge) throw Object.assign(new Error('unable to resolve dependency tree'), expl) diff --git a/workspaces/arborist/lib/index.js b/workspaces/arborist/lib/index.js index 4236427d6c4e3..7b525acc6e170 100644 --- a/workspaces/arborist/lib/index.js +++ b/workspaces/arborist/lib/index.js @@ -6,3 +6,4 @@ module.exports.Edge = require('./edge.js') module.exports.Shrinkwrap = require('./shrinkwrap.js') module.exports.PackageExtensions = require('./package-extensions.js') module.exports.NpmExtension = require('./npm-extension.js') +module.exports.autoInstalledPeerNodes = require('./peer-declarations.js').autoInstalledPeerNodes diff --git a/workspaces/arborist/lib/peer-declarations.js b/workspaces/arborist/lib/peer-declarations.js new file mode 100644 index 0000000000000..b2102477d1f02 --- /dev/null +++ b/workspaces/arborist/lib/peer-declarations.js @@ -0,0 +1,86 @@ +// Helpers for `auto-install-peers=false`, where only the project root and workspaces get their required peers installed. + +const RUNTIME_FIELDS = ['dependencies', 'optionalDependencies'] +const DECLARING_FIELDS = [...RUNTIME_FIELDS, 'peerDependencies'] +const DEV_DECLARING_FIELDS = [...DECLARING_FIELDS, 'devDependencies'] + +const isMine = node => node.isProjectRoot || node.isWorkspace + +// A peer edge that installs nothing on its own: an optional peer, or any peer of a dependency. +const isPassivePeer = edge => edge.type === 'peerOptional' || edge.peer && !isMine(edge.from) + +// Whether the required peer `edge` would be placed only for a dependency, given the `declaring` edge of the same name in the dependent's context. +const isAutoInstalledPeer = (edge, declaring) => edge.type === 'peer' && + (!declaring || isPassivePeer(declaring) && !declaring.to) + +// Nodes and links reached from `tree` through non-optional edges that pass `follow`. +const reachable = (tree, follow) => { + const seen = new Set([tree]) + for (const node of seen) { + for (const edge of node.target.edgesOut.values()) { + if (edge.to && edge.type !== 'peerOptional' && follow(edge)) { + seen.add(edge.to) + } + } + } + return seen +} + +// Nodes that are in the tree only because a dependency's required peer edge reaches them, e.g. loaded from a lockfile written with auto-installed peers. +const autoInstalledPeerNodes = tree => { + const kept = reachable(tree, edge => !isPassivePeer(edge)) + return [...reachable(tree, () => true)].filter(node => !kept.has(node)) +} + +const declares = (pkg, name, fields) => fields.some(field => Object.hasOwn(pkg[field] || {}, name)) + +// An optional peer does not make the dependent's consumers provide it. +const declaresRequired = (pkg, name, fields) => fields.some(field => declares(pkg, name, [field]) && + (field !== 'peerDependencies' || !pkg.peerDependenciesMeta?.[name]?.optional)) + +// Every package that depends on a package with a required peer without declaring that peer itself, once per package and peer. +const undeclaredPeers = tree => { + const found = [] + for (const node of tree.inventory.values()) { + if (node.isLink || node.inDepBundle || node.inert) { + continue + } + const { package: pkg } = node + const seen = new Set() + for (const edge of node.edgesOut.values()) { + if (!edge.to) { + continue + } + // Implicit workspace edges are not declarations, and devDependencies are only installed for the project root and workspaces. + const mine = isMine(node) + const runtime = declares(pkg, edge.name, RUNTIME_FIELDS) + const peer = declares(pkg, edge.name, ['peerDependencies']) + const dev = mine && declares(pkg, edge.name, ['devDependencies']) + if (!runtime && !peer && !dev) { + continue + } + const fields = runtime || !mine ? DECLARING_FIELDS : DEV_DECLARING_FIELDS + // A peer the dependent does not install is provided by its consumer, which must provide its peers too, so any declaration forwards them. + const installs = runtime || dev || mine && declaresRequired(pkg, edge.name, ['peerDependencies']) + const satisfied = installs ? declaresRequired : declares + const dep = edge.to.target + for (const peer of dep.edgesOut.values()) { + if (peer.type !== 'peer' || seen.has(peer.name) || peer.name === pkg.name || + satisfied(pkg, peer.name, fields)) { + continue + } + seen.add(peer.name) + found.push({ node, dep, peer }) + } + } + } + return found.sort((a, b) => a.node.location.localeCompare(b.node.location, 'en') || + a.peer.name.localeCompare(b.peer.name, 'en')) +} + +module.exports = { + autoInstalledPeerNodes, + isAutoInstalledPeer, + isMine, + undeclaredPeers, +} diff --git a/workspaces/arborist/test/arborist/build-ideal-tree.js b/workspaces/arborist/test/arborist/build-ideal-tree.js index 758c43df94bb9..b65fc00749ed0 100644 --- a/workspaces/arborist/test/arborist/build-ideal-tree.js +++ b/workspaces/arborist/test/arborist/build-ideal-tree.js @@ -5506,3 +5506,259 @@ t.test('unresolvable optional dep drops its optional peer dependent instead of E t.ok(tree.children.get('react').inert, 'failed react is inert') t.ok(tree.children.get('react-dom').inert, 'react-dom is inert with its missing peer') }) + +t.test('autoInstallPeers: false', async t => { + // strict registry mocks fail on any fetch of a skipped peer + const mock = async (t, pkgs, times = 1) => { + const registry = createRegistry(t, false) + for (const [name, pkg] of Object.entries(pkgs)) { + const manifest = registry.manifest({ name, packuments: [{ version: '1.0.0', ...pkg }] }) + await registry.package({ manifest, times }) + } + } + const project = (t, pkg, extra = {}) => t.testdir({ + 'package.json': JSON.stringify({ name: 'root', version: '1.0.0', ...pkg }), + ...extra, + }) + const undeclared = warnings => warnings.filter(w => w[1] === 'EPEERUNDECLARED').map(w => w[2]) + const opts = { autoInstallPeers: false } + + t.test('does not install a peer the root does not declare', async t => { + await mock(t, { dom: { peerDependencies: { react: '1' } } }) + const warnings = warningTracker(t) + const tree = await buildIdeal(project(t, { dependencies: { dom: '1' } }), opts) + t.notOk(tree.children.get('react'), 'react is not placed') + t.equal(tree.children.get('dom').edgesOut.get('react').missing, true) + t.strictSame(undeclared(warnings), [ + 'root depends on dom, which requires peer react@1\nAdd react to root\'s dependencies or peerDependencies', + ]) + }) + + t.test('strict-peer-deps fails for the root, force downgrades to a warning', async t => { + await mock(t, { dom: { peerDependencies: { react: '1' } } }, 2) + const path = project(t, { name: undefined, dependencies: { dom: '1' } }) + await t.rejects(buildIdeal(path, { ...opts, strictPeerDeps: true }), { + code: 'EPEERUNDECLARED', + message: `${basename(path)} depends on dom, which requires peer react@1\nAdd react to ${basename(path)}'s dependencies or peerDependencies`, + }, 'names a nameless root by its folder') + const warnings = warningTracker(t) + await buildIdeal(path, { ...opts, strictPeerDeps: true, force: true }) + t.equal(undeclared(warnings).length, 1) + }) + + t.test('third-party dependents only warn under strict-peer-deps', async t => { + await mock(t, { + lib: { dependencies: { dom: '1' } }, + dom: { peerDependencies: { react: '1' } }, + }) + const warnings = warningTracker(t) + const tree = await buildIdeal(project(t, { dependencies: { lib: '1' } }), { ...opts, strictPeerDeps: true }) + t.notOk(tree.children.get('react')) + t.match(undeclared(warnings), [/^lib depends on dom, which requires peer react@1\n/]) + }) + + t.test('a peer declared by a dependency must be declared by its dependent', async t => { + await mock(t, { + lib: { dependencies: { dom: '1' }, peerDependencies: { react: '1' } }, + dom: { peerDependencies: { react: '1' } }, + }) + const warnings = warningTracker(t) + const tree = await buildIdeal(project(t, { dependencies: { lib: '1' } }), opts) + t.notOk(tree.children.get('react')) + t.match(undeclared(warnings), [/^root depends on lib, which requires peer react@1\n/]) + }) + + t.test('installs peers declared by the root or provided by a dependency', async t => { + await mock(t, { + lib: { dependencies: { dom: '1', react: '1' } }, + dom: { peerDependencies: { react: '1' } }, + react: {}, + }) + const warnings = warningTracker(t) + const tree = await buildIdeal(project(t, { + dependencies: { lib: '1', dom: '1' }, + peerDependencies: { react: '1' }, + }), { ...opts, strictPeerDeps: true }) + t.equal(tree.children.get('react').version, '1.0.0') + t.equal(tree.children.get('dom').edgesOut.get('react').valid, true) + t.strictSame(undeclared(warnings), []) + }) + + t.test('devDependencies satisfy only a dev-only dependent', async t => { + await mock(t, { dom: { peerDependencies: { react: '1' } }, react: {} }) + let warnings = warningTracker(t) + await buildIdeal(project(t, { devDependencies: { dom: '1', react: '1' } }), opts) + t.strictSame(undeclared(warnings), [], 'dev dependent, dev peer') + + await mock(t, { dom: { peerDependencies: { react: '1' } }, react: {} }) + warnings = warningTracker(t) + await buildIdeal(project(t, { dependencies: { dom: '1' }, devDependencies: { react: '1' } }), opts) + t.equal(undeclared(warnings).length, 1, 'prod dependent, dev peer') + }) + + t.test('optional peers, self peers, and bundled dependents are not checked', async t => { + await mock(t, { + dom: { + peerDependencies: { react: '1', root: '1' }, + peerDependenciesMeta: { react: { optional: true } }, + }, + }) + const warnings = warningTracker(t) + const tree = await buildIdeal(project(t, { dependencies: { dom: '1' } }), opts) + t.notOk(tree.children.get('react')) + t.strictSame(undeclared(warnings), []) + }) + + t.test('a root declaration does not satisfy a workspace', async t => { + await mock(t, { dom: { peerDependencies: { react: '1', vue: '1' } }, react: {} }) + const warnings = warningTracker(t) + const path = project(t, { workspaces: ['a'], dependencies: { dom: '1', react: '1' } }, { + a: { 'package.json': JSON.stringify({ name: 'a', version: '1.0.0', dependencies: { dom: '1' } }) }, + }) + const tree = await buildIdeal(path, opts) + t.equal(tree.children.get('dom').edgesOut.get('react').valid, true, 'resolves from the root') + t.match(undeclared(warnings), [ + /^root depends on dom, which requires peer vue@1\n/, + /^a depends on dom, which requires peer react@1\n/, + /^a depends on dom, which requires peer vue@1\n/, + ]) + }) + + t.test('devDependencies of a linked dependency are not checked', async t => { + await mock(t, { dom: { peerDependencies: { react: '1' } } }) + const warnings = warningTracker(t) + const path = project(t, { name: undefined, dependencies: { local: 'file:local' } }, { + local: { 'package.json': JSON.stringify({ name: 'local', version: '1.0.0', devDependencies: { dom: '1' } }) }, + }) + const tree = await buildIdeal(path, opts) + t.ok(tree.children.get('dom'), 'dev dependency of the link is installed') + t.strictSame(undeclared(warnings), []) + }) + + t.test('prunes auto-installed peers loaded from a lockfile', async t => { + const path = project(t, { dependencies: { dom: '1' } }, { + 'package-lock.json': JSON.stringify({ + name: 'root', + version: '1.0.0', + lockfileVersion: 3, + requires: true, + packages: { + '': { name: 'root', version: '1.0.0', dependencies: { dom: '1' } }, + 'node_modules/dom': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/dom/-/dom-1.0.0.tgz', + peerDependencies: { react: '1' }, + }, + 'node_modules/react': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/react/-/react-1.0.0.tgz', + dependencies: { tokens: '1' }, + peer: true, + }, + 'node_modules/tokens': { + version: '1.0.0', + resolved: 'https://registry.npmjs.org/tokens/-/tokens-1.0.0.tgz', + peer: true, + }, + }, + }), + }) + const kept = await buildIdeal(path) + t.ok(kept.children.get('react'), 'kept by default') + const tree = await buildIdeal(path, opts) + t.notOk(tree.children.get('react'), 'peer pruned') + t.notOk(tree.children.get('tokens'), 'its dependencies pruned') + t.ok(tree.children.get('dom')) + }) + + t.test('an explicitly declared workspace is checked', async t => { + await mock(t, { react: {} }) + const warnings = warningTracker(t) + const path = project(t, { workspaces: ['p'], dependencies: { p: 'file:p' } }, { + p: { 'package.json': JSON.stringify({ name: 'p', version: '1.0.0', peerDependencies: { react: '1' } }) }, + }) + await buildIdeal(path, opts) + t.match(undeclared(warnings), [/^root depends on p, which requires peer react@1\n/]) + }) + + t.test('an optional peer of the root does not install a required peer', async t => { + await mock(t, { dom: { peerDependencies: { react: '1' } } }) + const warnings = warningTracker(t) + const tree = await buildIdeal(project(t, { + dependencies: { dom: '1' }, + peerDependencies: { react: '1' }, + peerDependenciesMeta: { react: { optional: true } }, + }), opts) + t.notOk(tree.children.get('react')) + t.match(undeclared(warnings), [/^root depends on dom, which requires peer react@1\n/]) + }) + + t.test('prunes an auto-installed peer link', async t => { + const path = project(t, { dependencies: { local: 'file:local' } }, { + local: { 'package.json': JSON.stringify({ name: 'local', version: '1.0.0', peerDependencies: { x: 'file:../x' } }) }, + x: { 'package.json': JSON.stringify({ name: 'x', version: '1.0.0' }) }, + }) + await newArb(path).reify() + const kept = await buildIdeal(path) + t.equal(kept.children.get('x')?.isLink, true, 'linked by default') + const tree = await buildIdeal(path, opts) + t.notOk(tree.children.get('x'), 'link pruned') + }) + + t.test('an optional peer that is present is checked', async t => { + await mock(t, { + lib: { dependencies: { dom: '1' } }, + dom: { peerDependencies: { react: '1' } }, + }) + const warnings = warningTracker(t) + await buildIdeal(project(t, { + dependencies: { lib: '1' }, + peerDependencies: { dom: '1' }, + peerDependenciesMeta: { dom: { optional: true } }, + }), opts) + t.match(undeclared(warnings), [ + /^root depends on dom, which requires peer react@1\n/, + /^lib depends on dom, which requires peer react@1\n/, + ]) + }) + + t.test('a peer reached through a peer can be declared as an optional peer or a devDependency', async t => { + await mock(t, { dom: { peerDependencies: { react: '1' } }, react: {} }) + const warnings = warningTracker(t) + const path = project(t, { workspaces: ['a'] }, { + a: { + 'package.json': JSON.stringify({ + name: 'a', + version: '1.0.0', + peerDependencies: { dom: '1', react: '1' }, + peerDependenciesMeta: { dom: { optional: true }, react: { optional: true } }, + devDependencies: { dom: '1', react: '1' }, + }), + }, + }) + await buildIdeal(path, { ...opts, strictPeerDeps: true }) + t.strictSame(undeclared(warnings), []) + }) + + t.test('a required peer of the root needs its own peers installed', async t => { + await mock(t, { p: { peerDependencies: { x: '1' } }, x: {} }) + let warnings = warningTracker(t) + await buildIdeal(project(t, { + peerDependencies: { p: '1', x: '1' }, + peerDependenciesMeta: { x: { optional: true } }, + }), opts) + t.match(undeclared(warnings), [/^root depends on p, which requires peer x@1\n/], 'optional peer does not install it') + + await mock(t, { p: { peerDependencies: { x: '1' } }, x: {} }) + warnings = warningTracker(t) + await buildIdeal(project(t, { peerDependencies: { p: '1' }, devDependencies: { x: '1' } }), opts) + t.strictSame(undeclared(warnings), [], 'devDependency installs it') + }) + + t.test('has no effect with legacy-peer-deps', async t => { + await mock(t, { dom: { peerDependencies: { react: '1' } } }) + const warnings = warningTracker(t) + await buildIdeal(project(t, { dependencies: { dom: '1' } }), { ...opts, legacyPeerDeps: true }) + t.strictSame(warnings, [['warn', 'config', '`auto-install-peers` has no effect when `legacy-peer-deps` is set']]) + }) +}) diff --git a/workspaces/config/lib/definitions/definitions.js b/workspaces/config/lib/definitions/definitions.js index bd5c82363937c..98262975c2cb2 100644 --- a/workspaces/config/lib/definitions/definitions.js +++ b/workspaces/config/lib/definitions/definitions.js @@ -325,6 +325,31 @@ const definitions = { `, flatten, }), + 'auto-install-peers': new Definition('auto-install-peers', { + default: true, + type: Boolean, + description: ` + When set to \`false\`, npm does not add packages to the tree only to + satisfy a required \`peerDependencies\` entry of a dependency. + Instead, it warns about every package that depends on a package with a + required peer without declaring that peer itself. + + Each package must declare the peer itself: a provider elsewhere in the + tree, such as the project root for a workspace, does not count. A + \`devDependencies\` entry only counts when the package that requires + the peer is a \`devDependencies\` or \`peerDependencies\` entry of the + project root or a workspace, and not also one of its \`dependencies\` + or \`optionalDependencies\`. + + With \`--strict-peer-deps\`, such a warning for the project root or a + workspace fails the install. + + This setting changes the tree and is not recorded in the lockfile, so + set it in the project \`.npmrc\`. Ignored when \`--legacy-peer-deps\` + is set. + `, + flatten, + }), 'auth-type': new Definition('auth-type', { default: 'web', type: ['legacy', 'web'], @@ -2491,6 +2516,10 @@ const definitions = { When such an override is performed, a warning is printed, explaining the conflict and the packages involved. If \`--strict-peer-deps\` is set, then this warning is treated as a failure. + + With \`--auto-install-peers=false\`, it also fails the install when the + project root or a workspace does not declare a peer that one of its + dependencies requires. `, flatten, }), diff --git a/workspaces/config/tap-snapshots/test/type-description.js.test.cjs b/workspaces/config/tap-snapshots/test/type-description.js.test.cjs index c923f6e7a6cb4..9011c8fa7dc72 100644 --- a/workspaces/config/tap-snapshots/test/type-description.js.test.cjs +++ b/workspaces/config/tap-snapshots/test/type-description.js.test.cjs @@ -77,6 +77,9 @@ Object { "legacy", "web", ], + "auto-install-peers": Array [ + "boolean value (true or false)", + ], "before": Array [ null, "valid Date string",