Respect peer dependencies #59

korelstar · 2021-04-08T18:58:25Z

This PR improves the upgrade of dependencies which are peer dependencies of other dependencies.

Scenario

Your app has a dependency to the package ncu-test-peer. ncu-test-peer has a peer dependency to the package ncu-test-return-version and requires version 1.x of this package. I.e., ncu-test-peer has the following package.json:

{
  "peerDependencies": {
    "ncu-test-return-version": "1.x"
  }
}

Hence, your app has the following package.json:

{
  "dependencies": {
    "ncu-test-peer": "1.0.0",
    "ncu-test-return-version": "1.0.0"
  }
}

Let us assume, that ncu-test-return-version has published versions 1.0.0, 1.1.0 and 2.0.0.

Current Situation (without this PR)

If you run the current release of npm-upgrade (3.0.0) for your app, you get an upgrade suggestion for ncu-test-return-version version 2.0.0. If you follow this suggestion, your app is broken, since ncu-test-peer's peer dependency requirement is not met (2.0.0 does not satisfy 1.x). If you deny this suggestion, you will stay at version 1.0.0, although there is a newer version 1.1.0 that satisfies the requirement 1.x.

Situation with this PR

This PR adds the new peer dependency check of npm-check-updates which was introduced in version 11.4.0 (see raineorshine/npm-check-updates#869). Now, npm-upgrade provides an upgrade suggestion for ncu-test-return-version version 1.1.0 -- which is the expected result for this scenario.

package-lock.json

th0r · 2021-04-09T09:56:48Z

src/commands/check.js

@@ -76,7 +76,8 @@ export const handler = catchAsyncError(async opts => {
    .map(({ncuValue}) => ncuValue)
    .join(',');
  const currentVersions = ncu.getCurrentDependencies(packageJson, {dep: ncuDepGroups});
-  const latestVersions = await ncu.queryVersions(currentVersions, {versionTarget: 'latest'});
+  const peerDependencies = ncu.getPeerDependencies(currentVersions, {});
+  const latestVersions = await ncu.queryVersions(currentVersions, {versionTarget: 'latest', peerDependencies});


Is there a way to show some explanation to the user e.g. that latest released version of the dependency is v2 but the latest version that satisfies peerDependencies is v1.2?

Currently, with npm-check-updates, you can provide only one strategy at time. Hence, we would have to call ncu.queryVersions two times (one call with peerDependencies and one call without them). However, the package data will then be fetched two times from npm servers which doubles the process time. I don't think that this would be a good solution.

which doubles the process time

We can fetch them in parallel. My concern here is it may introduce confusion when you're absolutely sure the latest version of the package is v2 but npm-upgrade tells you that the latest version is v1.2 without any explanation.

By the way, what does ncu.getPeerDependencies(currentVersions, {}) method actually do?

We can fetch them in parallel. My concern here is it may introduce confusion when you're absolutely sure the latest version of the package is v2 but npm-upgrade tells you that the latest version is v1.2 without any explanation.

I understand that and I just had a deeper insight in the code. It looks like npm-check-updates caches the network responses, so a second call of ncu.queryVersions() doesn't trigger any new network requests and hence runs quite fast. Therefore, we can call it indeed twice, without any problems.

I just pushed a new commit which adds a hint, if the suggested version is not the latest version. It prints (ignored incompatible latest version X.Y.Z) in the output table.

For now, this is just a proof of concept. I think it would be better to show incompatible versions in a separate table just like the manually ignored versions. How about that?

By the way, what does ncu.getPeerDependencies(currentVersions, {}) method actually do?

For every package in currentVersions, it reads the local package.json (from the directory node_modules/package/) and extracts the section peerDependencies. All results are merged and sorted by package. See https://github.com/raineorshine/npm-check-updates/blob/v11.4.1/lib/index.js#L232

The result is added to the options, so that the filterPredicate can filter all versions that satisfy the peer dependencies.

Okay, I've changed the output for incompatible updates. Here are two examples:

First example

Checking for outdated dependencies for ".../package.json"... [====================] 2/2 100% New versions of active modules available: ncu-test-return-version 1.0.0 → 1.1.0 Ignored incompatible updates (peer dependencies): ncu-test-return-version 1.0.0 → 2.0.0

Second example

Checking for outdated dependencies for ".../package.json"... [====================] 50/50 100% New versions of active modules available: @nextcloud/dialogs ^3.1.1 → ^3.1.2 @nextcloud/router ^1.2.0 → ^2.0.0 @nextcloud/vue ^3.8.0 → ^3.9.0 @nextcloud/webpack-vue-config ^4.0.1 → ^4.0.2 @babel/core ^7.13.14 → ^7.13.15 @babel/preset-env ^7.13.12 → ^7.13.15 core-js ^3.10.0 → ^3.10.1 eslint ^7.23.0 → ^7.24.0 eslint-plugin-vue ^7.8.0 → ^7.9.0 webpack ^5.28.0 → ^5.32.0 Ignored incompatible updates (peer dependencies): css-loader ^4.3.0 → ^5.2.1 eslint-plugin-promise ^4.3.1 → ^5.1.0 sass-loader ^10.1.1 → ^11.0.1 Ignored updates: From To Ignored versions Reason vue-fragment 1.5.1 → 1.5.2 1.5.2 buggy version

th0r · 2021-04-18T14:22:18Z

It's much better now, but it's still not clear what package is actually blocking other packages from updating. E.g. in my case I see the following:

Ignored incompatible updates (peer dependencies):

  @angular/animations         11.2.7   →   11.2.10
  @angular/common             11.2.7   →   11.2.10
  @angular/core               11.2.7   →   11.2.10
  @angular/platform-browser   11.2.7   →   11.2.10
  tslib                        2.1.0   →     2.2.0
  @angular/compiler           11.2.7   →   11.2.10
  typescript                   4.1.5   →     4.2.4
  webpack                     4.44.1   →    5.33.2

So I want to know what package prevents @angular packages from updating? Is it possible to get this information somehow?

th0r · 2021-04-18T15:13:58Z

Another issue is I had to run npm-upgrade '@angular/*' 3 times in my project in order to make a minor Angular upgrade.
First time I was shown the following:

New versions of active modules available:

  @angular/cdk                        11.2.6   →    11.2.9
  @angular/forms                      11.2.7   →   11.2.10
  @angular/platform-browser-dynamic   11.2.7   →   11.2.10
  @angular/router                     11.2.7   →   11.2.10
  @angular/cli                        11.2.6   →    11.2.9
  @angular/compiler-cli               11.2.7   →   11.2.10

Ignored incompatible updates (peer dependencies):

  @angular/animations         11.2.7   →   11.2.10
  @angular/common             11.2.7   →   11.2.10
  @angular/core               11.2.7   →   11.2.10
  @angular/platform-browser   11.2.7   →   11.2.10
  @angular/compiler           11.2.7   →   11.2.10

After I have agreed with all updates and ran npm install and npm-upgrade again it showed the following:

New versions of active modules available:

  @angular/platform-browser   11.2.7   →   11.2.10
  @angular/compiler           11.2.7   →   11.2.10

Ignored incompatible updates (peer dependencies):

  @angular/animations   11.2.7   →   11.2.10
  @angular/common       11.2.7   →   11.2.10
  @angular/core         11.2.7   →   11.2.10

After doing same steps again:

New versions of active modules available:

  @angular/animations   11.2.7   →   11.2.10
  @angular/common       11.2.7   →   11.2.10

Ignored incompatible updates (peer dependencies):

  @angular/core   11.2.7   →   11.2.10

And only after 3rd time I've upgraded all the Angular packages.

It happens because some packages has other packages in their peer dependencies e.g. @angular/platform-browser@11.2.7 has @angular/animation@11.2.7 and @angular/common@11.2.7 in its peer dependencies list so these packages just can't be updated until platform-browser is updated. On the next step updated platform-browser@11.2.10 has animation@11.2.10 and common@11.2.10 in its peer deps so they can be updated now.

So the issue is current logic doesn't take into account that version of the peer dependency may change after the actual upgrade - it only takes current peer versions into account.

Another issue is only immediate peer deps are tracked which means an update to C@2 will be improperly suggested in the following situation:

<project>
 |-- C@^1 (update to v2 will be suggested but it's invalid as A -> B has C@^1 in its peer deps)
 |-- A
 |   |-- B
 |   |   |---C@^1 (peer)

korelstar · 2021-04-18T19:56:59Z

You mentioned three aspects:

1. Show source of peer incompatibility

npm-check-updates doesn't provide this information, currently.

2. Multiple calls of `npm-upgrade` are necessary for complex dependencies

This is caused by your approach of splitting npm-upgrade and npm install. The only solution I can see is to combine both actions and run them repeatedly.

3. Consider deep peer dependencies

I'm not sure if this is required, because I think that A should have declared C as it's own peer dependency. In my point of view, A is broken if it does not do this. And as far as I understand the npm 7 approach correctly, the npm people have changed their mind and now have this view, too (see --strict-peer-deps flag). If you nevertheless want to consider deep peer dependencies, this should be fixed in npm-check-updates.

th0r · 2021-04-19T10:04:39Z

This is caused by your approach of splitting npm-upgrade and npm install.

So what approach would work in your opinion?

korelstar · 2021-04-23T05:18:28Z

An approach would be to automatically call npm install after updating package.json and then automatically check for updates again.

Another approach would be to execute npm view with the updated package version and get the new list of peer dependencies. Then, a check for updates can be done again without installing the packages. But there are two ways for user interaction:

Approach 1

The above actions are executed before the list of upgradable packages are shown to the user. This means, the user gets the following list in your example:

New versions of active modules available:

  @angular/cdk                        11.2.6   →    11.2.9
  @angular/forms                      11.2.7   →   11.2.10
  @angular/platform-browser-dynamic   11.2.7   →   11.2.10
  @angular/router                     11.2.7   →   11.2.10
  @angular/cli                        11.2.6   →    11.2.9
  @angular/compiler-cli               11.2.7   →   11.2.10
  @angular/platform-browser           11.2.7   →   11.2.10
  @angular/compiler                   11.2.7   →   11.2.10
  @angular/animations                 11.2.7   →   11.2.10
  @angular/common                     11.2.7   →   11.2.10

However, if the user says no to @angular/platform-browser but yes to @angular/animation, the resulting list of upgradable packages will be broken (i.e. not all peer dependencies are satisfied).

Approach 2

In this approach, we do this iteratively. When executing npm-upgrade, the following list is shown:

New versions of active modules available:

  @angular/cdk                        11.2.6   →    11.2.9
  @angular/forms                      11.2.7   →   11.2.10
  @angular/platform-browser-dynamic   11.2.7   →   11.2.10
  @angular/router                     11.2.7   →   11.2.10
  @angular/cli                        11.2.6   →    11.2.9
  @angular/compiler-cli               11.2.7   →   11.2.10

Ignored incompatible updates (peer dependencies):

  @angular/animations         11.2.7   →   11.2.10
  @angular/common             11.2.7   →   11.2.10
  @angular/core               11.2.7   →   11.2.10
  @angular/platform-browser   11.2.7   →   11.2.10
  @angular/compiler           11.2.7   →   11.2.10

But after the user has answered all questions, but before asking for updating package.json, npm view is executed for getting the new list of peer dependencies and the check for updates is done again with this list. This means, after answering the last update question, there will appear a status bar (every updated package has to be checked with npm view) and the user sees the next upgrade iteration and the next question appears. But the user just have to wait and does not need to call npm-upgrade multiple times.

korelstar · 2021-04-25T11:22:10Z

I've made a PR for npm-check-updates in order to implement approach 1 directly over there: raineorshine/npm-check-updates#874

With that PR, it will be possible to add the reason to the list of incompatible updates. I will update this PR after the above PR has been merged and released.

Teaser 1:

New versions of active modules available:

  ncu-test-peer-update      1.0.0   →   1.1.0 
  ncu-test-return-version   1.0.0   →   1.1.0 

Ignored incompatible updates (peer dependencies):

  ncu-test-return-version   1.0.0   →   2.0.0   reason: ncu-test-peer-update requires 1.1.x 

? Update "ncu-test-peer-update" in package.json from 1.0.0 to 1.1.0? (Use arrow keys)
[...]

Teaser 2:

New versions of active modules available:

  @nextcloud/dialogs                ^3.1.1   →     ^3.1.2 
  @nextcloud/router                 ^1.2.0   →     ^2.0.0 
  @nextcloud/vue                    ^3.8.0   →     ^3.9.0 
  @nextcloud/webpack-vue-config     ^4.0.1   →     ^4.0.3 
  easymde                          ^2.14.0   →    ^2.15.0 
  markdown-it                      ^12.0.4   →    ^12.0.6 
  @babel/core                     ^7.13.14   →   ^7.13.16 
  @babel/preset-env               ^7.13.12   →   ^7.13.15 
  core-js                          ^3.10.0   →    ^3.11.0 
  eslint                           ^7.23.0   →    ^7.25.0 
  eslint-plugin-vue                 ^7.8.0   →     ^7.9.0 
  eslint-webpack-plugin             ^2.5.3   →     ^2.5.4 
  sass                             ^1.32.8   →   ^1.32.11 
  stylelint                       ^13.12.0   →   ^13.13.0 
  webpack                          ^5.28.0   →    ^5.35.1 

Ignored incompatible updates (peer dependencies):

  css-loader               ^4.3.0   →    ^5.2.4   reason: @nextcloud/webpack-vue-config requires ^4.3.0                                    
  eslint-plugin-promise    ^4.3.1   →    ^5.1.0   reason: @nextcloud/eslint-config requires ^4.2.1, eslint-config-standard requires ^4.2.1 
  sass-loader             ^10.1.1   →   ^11.0.1   reason: @nextcloud/webpack-vue-config requires ^10.0.5                                   

Ignored updates:

                  From          To   Ignored versions   Reason        
  vue-fragment   1.5.1   →   1.5.2        1.5.2         buggy version 

? Update "@nextcloud/dialogs" in package.json from ^3.1.1 to ^3.1.2? (Use arrow keys)
[...]

korelstar · 2021-04-27T18:39:21Z

I'm moving some more code to npm-check-updates: raineorshine/npm-check-updates#876
After that was merged, we can call getIgnoredUpgrades from ncu and reduce the code in npm-upgrade.

korelstar · 2021-04-29T13:07:34Z

Final update with much simplification was done. Ready for review and for merge 😁

korelstar · 2021-05-15T18:40:21Z

@th0r Is there anything open that I should do?

th0r requested changes Apr 9, 2021

View reviewed changes

korelstar force-pushed the peer branch 3 times, most recently from 87b4f9f to 9778689 Compare April 14, 2021 07:59

korelstar requested a review from th0r April 18, 2021 12:07

korelstar marked this pull request as draft April 25, 2021 11:22

korelstar force-pushed the peer branch from a61b7f5 to c0ac1bb Compare April 26, 2021 15:32

korelstar mentioned this pull request Apr 26, 2021

update peer dependencies on upgrade and retry raineorshine/npm-check-updates#874

Merged

korelstar marked this pull request as ready for review April 26, 2021 15:38

respect peer dependencies

3b16aa7

korelstar force-pushed the peer branch from c0ac1bb to 3b16aa7 Compare April 29, 2021 13:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Respect peer dependencies #59

Respect peer dependencies #59

korelstar commented Apr 8, 2021

th0r Apr 9, 2021

korelstar Apr 10, 2021

th0r Apr 13, 2021

korelstar Apr 13, 2021 •

edited

korelstar Apr 14, 2021

th0r commented Apr 18, 2021 •

edited

th0r commented Apr 18, 2021 •

edited

korelstar commented Apr 18, 2021

th0r commented Apr 19, 2021

korelstar commented Apr 23, 2021 •

edited

korelstar commented Apr 25, 2021 •

edited

korelstar commented Apr 27, 2021

korelstar commented Apr 29, 2021

korelstar commented May 15, 2021

Respect peer dependencies #59

Are you sure you want to change the base?

Respect peer dependencies #59

Conversation

korelstar commented Apr 8, 2021

Scenario

Current Situation (without this PR)

Situation with this PR

th0r Apr 9, 2021

Choose a reason for hiding this comment

korelstar Apr 10, 2021

Choose a reason for hiding this comment

th0r Apr 13, 2021

Choose a reason for hiding this comment

korelstar Apr 13, 2021 • edited

Choose a reason for hiding this comment

korelstar Apr 14, 2021

Choose a reason for hiding this comment

First example

Second example

th0r commented Apr 18, 2021 • edited

th0r commented Apr 18, 2021 • edited

korelstar commented Apr 18, 2021

1. Show source of peer incompatibility

2. Multiple calls of npm-upgrade are necessary for complex dependencies

3. Consider deep peer dependencies

th0r commented Apr 19, 2021

korelstar commented Apr 23, 2021 • edited

Approach 1

Approach 2

korelstar commented Apr 25, 2021 • edited

Teaser 1:

Teaser 2:

korelstar commented Apr 27, 2021

korelstar commented Apr 29, 2021

korelstar commented May 15, 2021

korelstar Apr 13, 2021 •

edited

th0r commented Apr 18, 2021 •

edited

th0r commented Apr 18, 2021 •

edited

2. Multiple calls of `npm-upgrade` are necessary for complex dependencies

korelstar commented Apr 23, 2021 •

edited

korelstar commented Apr 25, 2021 •

edited