search

raineorshine / npm-check-updates

Find newer versions of package dependencies than what your package.json allows
Other
9.23k stars 323 forks source link

Do not upgrade a package if its peer dependencies are not currently met #1418

Open rbnayax opened 2 months ago

rbnayax commented 2 months ago

Steps to Reproduce

.ncurc.js:

module.exports = {
  pre: false,
  interactive: true,
  enginesNode: true,
  cache: false,
  peer: true,
  upgrade: true,
  install: 'never',
  target: 'greatest'
}

Dependencies:

{
    "eslint": "^8.57.0",
    "eslint-plugin-import": "^2.29.1",
    "eslint-plugin-unused-imports": "^3.2.0"
}

Current Behavior

If you have the dependencies above, and you set -pre eslint package will not get upgraded to v9 because:

 eslint  ^8.57.0  →  ^9.4.0  reason: eslint-plugin-import requires ^2 || ^3 || ^4 || ^5 || ^6 || ^7.2.0 || ^8

But eslint-plugin-unused-imports will be upgraded to 4.0.0. Version 4.0.0 has eslint set in its peer dependencies as 9. This renders the generated package.json file unusable.

Expected Behavior

I would expect that with the pre flag set, if an upgraded package has unmet peer deps, it will not get upgraded

raineorshine commented 2 months ago

Hi, thanks for reporting. I can confirm that this is a bug in https://github.com/raineorshine/npm-check-updates/blob/main/src/lib/upgradePackageDefinitions.ts. It seems that when eslint-plugin-import is removed from the upgrade list due to its peerDependency on eslint, eslint-plugin-unused-imports accidentally gets added to the upgrades without being checked for peer compatibility.

Unfortunately I'm not super familiar with this feature, since I wasn't the original implementator. It's tricky because when any dependency is upgraded, the peerDependencies need to be checked again. I'm wondering if we should be upgrading dependencies one at a time and checking peerDependencies immediately after, rather than trying to upgrade them all and then iterating until peerDependencies do not change.

The next step would be to create a breaking unit test for this test case. Then we can attempt a solution. I don't have the bandwidth for this at the moment, but hopefully someone with some free time can take a stab at it.