[rush] Install fails if multiple versions of peer dependency with strictPeerDependencies

[mikeharder]

• Bug

Rush fails to update or install dependencies when multiple packages attempt to use different versions of a peer dependency.

To repro, clone https://github.com/mikeharder/rush-peer-dependencies and run rush update, which fails with:

ERROR  @azure/[email protected] requires a peer of rhea-promise@^0.1.15 but none was installed.

Note that pnpmOptions.strictPeerDependencies is set to true. If this option is changed to false, a warning is printed during rush update --full, but the generated lockfile looks valid and rush install seems to work fine.

However, I believe the dependencies in this repo are valid. Both project1 and project2 specify matching versions of their dependency and peer dependency:

https://github.com/mikeharder/rush-peer-dependencies/blob/05203c08c2358e6fe7843226cf6cdeffe21eba1f/project1/package.json#L12-L13

https://github.com/mikeharder/rush-peer-dependencies/blob/05203c08c2358e6fe7843226cf6cdeffe21eba1f/project2/package.json#L12-L13

Note that project1 depends on core which is another project in this repo, while project2 depends on @azure/amqp-common which is a package from npmjs.com. core has a peer dependency on package rhea-promise@^1.0.0, while @azure/amqp-common has a peer dependency on package rhea-promise@^0.1.5.

So this issue might only repro if one package with the peer dependency is in the repo, while the other package is pulled from npmjs.com. I tried to repro with both dependencies in the repo, but this seemed to work fine:

https://github.com/mikeharder/rush-peer-dependencies/tree/peer-dependency-in-own-project

Possibly related discussion: pnpm/pnpm#1142

• Tool: rush
• Tool Version: 5.10.3 (latest)
• PNPM Version: 3.6.0 (latest)
• Node Version: 10.16.0 (latest LTS)
  • Is this a LTS version? yes
• OS: Windows 10

[octogonz]

The rush install operation creates common/temp/package.json and runs pnpm install in that folder. So to understand this problem, we need to look at that file:

common/temp/package.json

{
  "dependencies": {
    "@azure/amqp-common": "1.0.0-preview.6",
    "@rush-temp/core": "file:./projects/core.tgz",
    "@rush-temp/project1": "file:./projects/project1.tgz",
    "@rush-temp/project2": "file:./projects/project2.tgz"
  },
  "description": "Temporary file generated by the Rush tool",
  "name": "rush-common",
  "private": true,
  "version": "0.0.0"
}

As you say, @azure/[email protected] has a peer dependency on rhea-promise@^0.1.15. But above file is not satisfying this peer dependency. Thus the error.

Now, @azure/amqp-common is an external dependency, so why does it appear alongside the tarballs in this file? It is due to Rush's "implicitly preferred versions" feature. The workings of that feature are complex, but the gist is that adding these dependencies tunes the package manager's version selection heuristics, reducing annoying inconsistencies that otherwise arise in a monorepo install. This practice greatly improves NPM and Yarn (whose squashed node_modules folder has many possible topologies), whereas for PNPM the improvements are more mild (since its install is fairly deterministic).

The implicitly preferred @azure/amqp-common entry originates from this package.json:

project2/package.json

{
  "name": "project2",
  "version": "1.0.0",
  "description": "",
  "main": "index.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "@azure/amqp-common": "1.0.0-preview.6",
    "rhea-promise": "^0.1.15"
  }
}

That package.json has the right rhea-promise in it, so why didn't it get implicitly preferred as well? The answer is that in order to be implicitly preferred, there cannot be multiple versions of that dependency in the repo. So inconsistently specifying rhea-promise@^1.0.0 in project1 causes rhea-promise to be omitted from common/temp/package.json, and thus our problem.

Workaround

For your isolated repro at least, the workaround is pretty easy: We can simply EXPLICITLY prefer the version, by adding a setting like this:

common/config/rush/common-versions.json

{
  "$schema": "https://developer.microsoft.com/json-schemas/rush/v5/common-versions.schema.json",
  "preferredVersions": {
    "rhea-promise": "^0.1.15"
  },
   "allowedAlternativeVersions": {
  }
}

This tells Rush that ^0.1.15 is the version you want in common/temp/package.json, and so the peer is satisfied, and the install succeeds.

Root cause

This is arguably a Rush bug, however. (It's probably specific to Rush+PNPM; the NPM/Yarn node_modules semantics are arguably too weak to say that Rush did anything wrong here, nor is strictPeerDependencies validation even realistic for their model.)

Anyway, ideally Rush+PNPM should not implicitly prefer a dependency that would create an unsatisfied peer. The fix is to improve the implicitly preferred versions logic:

• One idea would be check whether @azure/amqp-common has any peers, and eliminate it if so.

• A more sophisticated fix would be to eliminate @azure/amqp-common ONLY if the peers would be unsatisfied in common/temp/package.json.

This sounds easy, but there's a hitch: The @azure/amqp-common/package.json comes from the NPM registry, so its "peerDependencies" field is not directly available to Rush at the time when it is constructing common/temp/package.json. If we wanted to query it, we need to fetch @azure/[email protected] which in general may be a SemVer range @azure/amqp-common@^1.2.3 whose peer relationship may depend on knowing how PNPM would solve the SemVer range. PNPM now supports monorepo installs, so longer term we'd like pnpm install to support enterprise features like implicitly preferred versions and get Rush out of this business. If I was going to work on this, I think I'd try to implement it in PNPM itself, and then we'd have the package.json's handy.

But there's also a simple fix:

• Add a rush.json option to disable implicitly preferred versions for specific packages, or entirely for the whole repo.

I'd be interested to try such an option and see how it affects the pnpm-lock.yaml for serious monorepos. I know that in large monorepos people definitely use the explicitly preferred versions feature, and implicitly preferred versions reduced the need for that, but we didn't study it very formally, especially with the latest PNPM.

If someone wants to work on this, the relevant Rush code is in InstallManager.collectImplicitlyPreferredVersions().

[mikeharder]

@octogonz: We have hit another issue with peerDependencies, and the preferredVersions workaround doesn't prevent the issue. Repro steps:

$ git clone -b acorn https://github.com/mikeharder/rush-peer-dependencies
$ cd rush-peer-dependencies
$ rush update
ERROR  eslint > espree: [email protected] requires a peer of acorn@^6.0.0 but version 7.0.0 was installed.

Attempted workaround in common-versions.json:

"preferredVersions": {
  "acorn": "^6.0.0"
},

[mikeharder]

I think another workaround should be to pin espree to 6.0.0, but this doesn't work either:

"preferredVersions": {
  "espree": "6.0.0"
},

[mikeharder]

The web-build-tools repo appears to be blocked by this issue as well:

https://dev.azure.com/RushStack/Gearbox%20GitHub%20Projects/_build/results?buildId=1842&view=logs&jobId=5d9f2291-cb88-5e73-8056-0cddc8368ed3&taskId=f063cadf-8e43-5e80-e120-767b136285e8&lineStart=228&lineEnd=229&colStart=1&colEnd=1

@octogonz, @iclanton, @apostolisms

[mikeharder]

This is fixed in [email protected] (acornjs/acorn-jsx#101), however I'm still concerned that the workarounds couldn't address this before the package was updated.

[octogonz]

The implicitlyPreferredVersions feature was introduced back when nobody used peer dependencies (because NPM didn't handle them very well). When I thought about it earlier in this thread, it seems difficult to make this feature work correctly with peer dependencies. Also, I suspect that the same problem is mostly solved by PNPM's resolutionStrategy=fewer-dependencies (introduced with Rush 5.7.0). Thus, I'm wondering if simply disabling implicit preferred versions would solve your problem.

Unfortunately I was sick this week and am a little swamped catching up, so I don't have time to investigate it right away. @mikeharder Maybe you could try an experiment? Go into this file:

https://github.com/microsoft/web-build-tools/blob/c81518dae04b7eb770cc3305c8181ae920001566/apps/rush-lib/src/logic/InstallManager.ts#L550-L553

And right after those lines, add allPreferredVersions.clear();. This will disable the implicitly preferred versions (while still allowing the explicit ones to get added).

Also make sure you have resolutionStrategy=fewer-dependencies in your rush.json, and PNPM 3.1 or newer.

Then redo your rush update --full and see if it improves things at all. Maybe we'll get lucky! :-P

If not I promise to investigate this more, since we really want these edge cases to work correctly.

[mikeharder]

@octogonz: We have one more repro which might be yet another variation of this issue: https://github.com/mikeharder/rush-peer-dependencies/tree/amqp

In short, there are two projects with these dependencies:

"dependencies": {
  "@azure/amqp-common": "1.0.0-preview.7",
  "rhea-promise": "^0.1.15"
}

"dependencies": {
  "@azure/core-amqp": "1.0.0-preview.5",
  "rhea-promise": "^1.0.0"
}

@azure/amqp-common has a peer dependency on rhea-promise@^0.1.15, while @azure/core-amqp has a peer dependency on rhea-promise@^1.0.0.

Running rush update gives the following error:

 ERROR  @azure/[email protected] requires a peer of rhea-promise@^0.1.15 but none was installed.

If you try to use the preferredVersions workaround:

"preferredVersions": {
  "rhea-promise": "^0.1.15"
},

Then rush update fails for the other package:

 ERROR  @azure/[email protected] requires a peer of rhea-promise@^1.0.0 but version 0.1.15 was installed.

Do you think there is any fix or workaround for this, other than setting strictPeerDependencies=false? I'm not sure if there is, since common\temp\package.json can contain a maximum of one version of rhea-promise, so I don't know how both peer dependencies can be satisfied.

CC: @KarishmaGhiya

[mikeharder]

@octogonz: Adding allPreferredVersions.clear() fixes the issue and allows our repo to use conflicting peer dependencies. It seems to work with both resolutionStrategy=fast (which we are currently using) and resolutionStrategy=fewer-dependencies.

Can you please add an option to Rush to trigger the allPreferredVersions.clear() codepath?

[octogonz]

Awesome, yep I'll start a PR asap

[octogonz]

@mikeharder Here's a PR: #1618

[mikeharder]

@octogonz: I am trying to verify this PR in our repo but currently hitting an error related to a missing peer dependency. Investigating...