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 3 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.

27 changes: 19 additions & 8 deletions src/lib/getPeerDependenciesFromRegistry.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import ProgressBar from 'progress'
import nodeSemver from 'semver'
import { Index } from '../types/IndexType'
import { Options } from '../types/Options'
import { VersionSpec } from '../types/VersionSpec'
import getPackageManager from './getPackageManager'
import { isCircularPeer } from './isCircularPeer'

/**
* Get the latest or greatest versions from the NPM repository based on the version target.
Expand All @@ -22,14 +24,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, nodeSemver.coerce(version)?.version ?? version)
CreativeTechGuy marked this conversation as resolved.
Show resolved Hide resolved
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
38 changes: 38 additions & 0 deletions src/lib/isCircularPeer.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { Index } from '../types/IndexType'
CreativeTechGuy marked this conversation as resolved.
Show resolved Hide resolved

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
*/
export 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,
}
}
4 changes: 2 additions & 2 deletions src/lib/runLocal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,7 @@ async function runLocal(
}

if (options.peer) {
options.peerDependencies = await getPeerDependencies(current, options)
options.peerDependencies = await getPeerDependenciesFromRegistry(current, options)
}

const [upgraded, latestResults, upgradedPeerDependencies] = await upgradePackageDefinitions(current, options)
Expand Down
52 changes: 19 additions & 33 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 () => {
const cwd = path.join(__dirname, 'test-data/peer/')
CreativeTechGuy marked this conversation as resolved.
Show resolved Hide resolved
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 () => {
const cwd = path.join(__dirname, 'test-data/peer/')
CreativeTechGuy marked this conversation as resolved.
Show resolved Hide resolved
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 () => {
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