Unable to upgrade packages which are circular peer dependencies about npm-check-updates HOT 14 CLOSED

Yes, great catch.

I don't have time to work on this right now, but I approve the suggestion and am open to PR's.

from npm-check-updates.

Okay! I'll attempt this fix. If you have any tips/hints on where in the codebase I should look for this that'd be awesome. :)

from npm-check-updates.

Here are some relevant files:

• https://github.com/raineorshine/npm-check-updates/blob/main/src/package-managers/npm.ts#L640
• https://github.com/raineorshine/npm-check-updates/blob/main/src/lib/getPeerDependencies.ts
• https://github.com/raineorshine/npm-check-updates/blob/main/src/lib/getPeerDependenciesFromRegistry.ts

One challenge is that getPeerDependencies reads peer dependencies from installed packages (i.e. node_modules), so obviously that won't work for an upgraded package (which has not been installed yet). getPeerDependenciesFromRegistry doesn't have this problem, but I'm not sure why two methods of getting peer dependencies exist. I didn't build the --peer feature so I'm not super familiar with the implementation. It is possible that the peer dependencies were read from node_modules for performance reasons, to avoid additional http requests.

from npm-check-updates.

Thank you! I've been going through the code for the last few hours and am so far pretty confused. I keep thinking that something works but then it breaks something else. I'll keep pushing at it.

from npm-check-updates.

Yeah, sorry to hear that. The architecture has some debt. At least the tests are solid.

Let me know if you have any questions.

from npm-check-updates.

FYI getPeerDependencies reads the current peerDependencies from node_modules and injects them into options.peerDependencies, which ultimately gets read by satisfiesPeerDependencies. I think this is causing the vitest and @vitest/ui to get filtered out of the results.

I suspect that getPeerDependencies will need to be completely removed and getPeerDependenciesFromRegistry used in its place.

from npm-check-updates.

My proposed solution above doesn't seem possible since there's the iterative peer updating feature. It seems like it'll evaluate the current peers on disk, then attempt to install as far as possible, then evaluate peers based on those proposed installs, and repeat until there's no new packages to install.

I cannot seem to find a good place to change that without just breaking that whole thing.

So I'm now going down the path of seeing if I can "fix" the circular dependency by detecting a loop and deleting one chain of the loop so it is no longer in a deadlock. It feels bad, but so far it's looking more promising and much easier. 😬

from npm-check-updates.

Ah, I see. I didn't notice it was recursive. That certainly does make it a bit more complicated.

Try replacing getPeerDependencies with getPeerDependenciesFromRegistry. I don't think node_modules can safely be used, since those peerDependencies will be outdated for upgraded packages.

It will have to use the upgraded versions though (and stick with the current versions if a package is excluded).

from npm-check-updates.

Yeah that was my first attempt. Unfortunately the recursive checks only recurse to higher version numbers (stepping up through versions). If we instead got the peers of the latest versions to start with, then the recursion would need to be backwards (stepping down through to older versions).

I'm sure that's possible, but I don't understand what the current code is doing well enough to make that change without breaking something (trust, me I've tried for most of the day haha).

from npm-check-updates.

Eek... good point 😬

from npm-check-updates.

I have an attempted PR to fix this. I know it's not in your FP style, but I'm curious what you think of it before I polish it. (Or feel free to mess with it if you'd like)

from npm-check-updates.

Oh and I realized that my implementation is a bit too heavy-handed. I need to go back and surgically remove just the one field that is making the peer dependency cycle, rather than the entire object. But let me know if you like the overall idea.

from npm-check-updates.

Okay I think I finished it. It's not FP, but the algorithm is already difficult to understand so I don't really want to rewrite it if I can help it. 😁

from npm-check-updates.

Thank you! Reviewed.

Okay I think I finished it. It's not FP, but the algorithm is already difficult to understand so I don't really want to rewrite it if I can help it. 😁

No worries. I'm getting more flexible in my old age 😂

from npm-check-updates.