Skip to content
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

Fix circular peer dependencies blocking updates #1383

Merged
merged 5 commits into from
Mar 22, 2024
Merged
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
30 changes: 0 additions & 30 deletions src/lib/getPeerDependencies.ts

This file was deleted.

66 changes: 56 additions & 10 deletions src/lib/getPeerDependenciesFromRegistry.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,54 @@
import ProgressBar from 'progress'
import { Index } from '../types/IndexType'
import { Options } from '../types/Options'
import { VersionSpec } from '../types/VersionSpec'
import { Version } from '../types/Version'
import getPackageManager from './getPackageManager'

type CircularData =
| {
isCircular: true
offendingPackage: string
}
| {
isCircular: false
}

/**
* Checks if the specified package will create a loop of peer dependencies by traversing all paths to find a cycle
*
* If a cycle was found, the offending peer dependency of the specified package is returned
*/
function isCircularPeer(peerDependencies: Index<Index<string>>, packageName: string): CircularData {
let queue = [[packageName]]
while (queue.length > 0) {
const nextQueue: string[][] = []
for (const path of queue) {
const parents = Object.keys(peerDependencies[path[0]] ?? {})
for (const name of parents) {
if (name === path.at(-1)) {
return {
isCircular: true,
offendingPackage: path[0],
}
}
nextQueue.push([name, ...path])
}
}
queue = nextQueue
}
return {
isCircular: false,
}
}

/**
* Get the latest or greatest versions from the NPM repository based on the version target.
*
* @param packageMap An object whose keys are package name and values are version
* @param [options={}] Options.
* @returns Promised {packageName: peer dependencies} collection
*/
async function getPeerDependenciesFromRegistry(packageMap: Index<VersionSpec>, options: Options) {
async function getPeerDependenciesFromRegistry(packageMap: Index<Version>, options: Options) {
const packageManager = getPackageManager(options, options.packageManager)
if (!packageManager.getPeerDependencies) return {}

Expand All @@ -22,14 +59,23 @@ async function getPeerDependenciesFromRegistry(packageMap: Index<VersionSpec>, o
bar.render()
}

return Object.entries(packageMap).reduce(async (accumPromise, [pkg, version]) => {
const dep = await packageManager.getPeerDependencies!(pkg, version)
if (bar) {
bar.tick()
}
const accum = await accumPromise
return { ...accum, [pkg]: dep }
}, {})
const peerDependencies: Index<Index<string>> = Object.entries(packageMap).reduce(
async (accumPromise, [pkg, version]) => {
const dep = await packageManager.getPeerDependencies!(pkg, version)
if (bar) {
bar.tick()
}
const accum = await accumPromise
const newAcc: Index<Index<string>> = { ...accum, [pkg]: dep }
const circularData = isCircularPeer(newAcc, pkg)
if (circularData.isCircular) {
delete newAcc[pkg][circularData.offendingPackage]
CreativeTechGuy marked this conversation as resolved.
Show resolved Hide resolved
}
return newAcc
},
{},
)
return peerDependencies
}

export default getPeerDependenciesFromRegistry
17 changes: 13 additions & 4 deletions src/lib/runLocal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import get from 'lodash/get'
import isEmpty from 'lodash/isEmpty'
import pick from 'lodash/pick'
import prompts from 'prompts-ncu'
import { satisfies } from 'semver'
import nodeSemver from 'semver'
import { Index } from '../types/IndexType'
import { Maybe } from '../types/Maybe'
import { Options } from '../types/Options'
Expand All @@ -15,7 +15,7 @@ import chalk from './chalk'
import getCurrentDependencies from './getCurrentDependencies'
import getIgnoredUpgrades from './getIgnoredUpgrades'
import getPackageManager from './getPackageManager'
import getPeerDependencies from './getPeerDependencies'
import getPeerDependenciesFromRegistry from './getPeerDependenciesFromRegistry'
import keyValueBy from './keyValueBy'
import { print, printIgnoredUpdates, printJson, printSorted, printUpgrades, toDependencyTable } from './logging'
import programError from './programError'
Expand Down Expand Up @@ -183,7 +183,14 @@ async function runLocal(
}

if (options.peer) {
options.peerDependencies = await getPeerDependencies(current, options)
options.peerDependencies = await getPeerDependenciesFromRegistry(
Object.fromEntries(
Object.entries(current).map(([packageName, versionSpec]) => {
return [packageName, nodeSemver.minVersion(versionSpec)?.version ?? versionSpec]
}),
),
options,
)
}

const [upgraded, latestResults, upgradedPeerDependencies] = await upgradePackageDefinitions(current, options)
Expand All @@ -210,7 +217,9 @@ async function runLocal(

// filter out satisfied deps when using --minimal
const filteredUpgraded = options.minimal
? keyValueBy(upgraded, (dep, version) => (!satisfies(latest[dep], current[dep]) ? { [dep]: version } : null))
? keyValueBy(upgraded, (dep, version) =>
!nodeSemver.satisfies(latest[dep], current[dep]) ? { [dep]: version } : null,
)
: upgraded

const ownersChangedDeps = (options.format || []).includes('ownerChanged')
Expand Down
58 changes: 22 additions & 36 deletions test/peer.test.ts
Original file line number Diff line number Diff line change
@@ -1,52 +1,38 @@
import fs from 'fs/promises'
import path from 'path'
import ncu from '../src/'
import spawnNpm from '../src/package-managers/npm'
import chaiSetup from './helpers/chaiSetup'

chaiSetup()

describe('peer dependencies', function () {
it('peer dependencies of installed packages are ignored by default', async () => {
it('peer dependencies are ignored by default', async () => {
const cwd = path.join(__dirname, 'test-data/peer/')
try {
await spawnNpm('install', {}, {}, { cwd })
const upgrades = await ncu({ cwd })
upgrades!.should.deep.equal({
'ncu-test-return-version': '2.0.0',
})
} finally {
await fs.rm(path.join(cwd, 'node_modules'), { recursive: true, force: true })
await fs.rm(path.join(cwd, 'package-lock.json'), { recursive: true, force: true })
}
const upgrades = await ncu({ cwd })
upgrades!.should.deep.equal({
'ncu-test-return-version': '2.0.0',
})
})

it('peer dependencies of installed packages are checked when using option peer', async () => {
it('peer dependencies are checked when using option peer', async () => {
const cwd = path.join(__dirname, 'test-data/peer/')
try {
await spawnNpm('install', {}, {}, { cwd })
const upgrades = await ncu({ cwd, peer: true })
upgrades!.should.deep.equal({
'ncu-test-return-version': '1.1.0',
})
} finally {
await fs.rm(path.join(cwd, 'node_modules'), { recursive: true, force: true })
await fs.rm(path.join(cwd, 'package-lock.json'), { recursive: true, force: true })
}
const upgrades = await ncu({ cwd, peer: true })
upgrades!.should.deep.equal({
'ncu-test-return-version': '1.1.0',
})
})

it('peer dependencies of installed packages are checked iteratively when using option peer', async () => {
it('peer dependencies are checked iteratively when using option peer', async () => {
const cwd = path.join(__dirname, 'test-data/peer-update/')
try {
await spawnNpm('install', {}, {}, { cwd })
const upgrades = await ncu({ cwd, peer: true })
upgrades!.should.deep.equal({
'ncu-test-return-version': '1.1.0',
'ncu-test-peer-update': '1.1.0',
})
} finally {
await fs.rm(path.join(cwd, 'node_modules'), { recursive: true, force: true })
await fs.rm(path.join(cwd, 'package-lock.json'), { recursive: true, force: true })
}
const upgrades = await ncu({ cwd, peer: true })
upgrades!.should.deep.equal({
'ncu-test-return-version': '1.1.0',
'ncu-test-peer-update': '1.1.0',
})
})

it('circular peer dependencies are ignored', async () => {
const cwd = path.join(__dirname, 'test-data/peer-lock/')
const upgrades = await ncu({ cwd, peer: true })
upgrades!.should.contain.keys('@vitest/ui', 'vitest')
})
})
7 changes: 7 additions & 0 deletions test/test-data/peer-lock/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"license": "MIT",
"dependencies": {
"@vitest/ui": "^1.3.1",
"vitest": "^1.3.1"
}
}
Loading