npm 3: No warning when peer dependency also listed as regular dependency cannot be met

[papandreou]

Hi!

The unexpected.js team is currently looking into npm 3 in order to come up with a plan for how to ensure that only compatible plugins are installed into the library. We have some complex scenarios where some plugins depend on certain versions of other plugins being installed into the libary instance, and until now we have used peerDependencies to express those requirements. Also, we have been relying on the somewhat controversial feature that the mere presence of a peer dependency will cause the module to be installed.

All in all it seems like npm 3 can lead to a much better experience for our users. The only unsolved problem for us is that scenario where a plugin requires another plugin to be installed in a specific version (range). Reading through the discussion on #6565 we were led to believe that we could achieve that by listing the second level plugin under both dependencies and peerDependencies of the plugin that needs it.

As an experiment we tried publishing four packages to our private npm repo:

• bogus_a@1.0.0, lists bogus_b@1.0.0 and bogus_c@1.0.0 under dependencies
• bogus_b@1.0.0, lists bogus_d@1.0.0 under both dependencies and peerDependencies
• bogus_c@1.0.0, lists bogus_d@2.0.0 under both dependencies and peerDependencies
• bogus_d@1.0.0 and @2.0.0

We expected that installing bogus_a@1.0.0 would cause a warning because both of the bogus_d peer dependencies cannot be fulfilled at the same time. However, it seems like the peer dependency is ignored in this case:

 $ npm install bogus_a
/home/andreas/work/test
└─┬ bogus_a@1.0.0 
  ├─┬ bogus_b@1.0.0 
  │ └── bogus_d@1.0.0 
  └─┬ bogus_c@1.0.0 
    └── bogus_d@2.0.0 

The multistage/dedupe process does exactly the right thing, as it cannot install a bogus_d at the top level that would be satisfied by both 1.0.0 and 2.0.0 -- but there's no warning about the peer dependencies not being met.

Would it be possible to get that warning?

[sunesimonsen]

Would it be possible to only validate peer dependencies against top-level dependencies and issue a warning if the peer dependency is not satisfied? Then you could supply both a dependency and a peer dependency if dedupe hoist the right dependency to the top-level everything is good; otherwise you get a warning.

[iarna]

@papandreou Can you make your test module's sources publicly available? I can recreate them myself of course, but it would be easier if we were working from the same thing.

[papandreou]

Absolutely. I cleaned it up a bit, dropped the bogus_ prefixes and published the packages to the official repo as @papandreou/a, @papandreou/b, @papandreou/c, and @papandreou/d.

The sources are available here: https://github.com/papandreou/peerdep

Here's how I published the packages:

peerdep$ for dir in * ; do echo ${dir} && cd ${dir} && npm publish --access=public && cd .. ; done
a
+ @papandreou/a@1.0.0
b
+ @papandreou/b@1.0.0
c
+ @papandreou/c@1.0.0
d@1.0.0
+ @papandreou/d@1.0.0
d@2.0.0
+ @papandreou/d@2.0.0

Like in the original description of the issue, I don't get a warning when I try to install @papandreou/a:

myproject$ npm install @papandreou/a
/home/andreas/work/hey
└─┬ @papandreou/a@1.0.0 
  ├─┬ @papandreou/b@1.0.0 
  │ └── @papandreou/d@1.0.0 
  └─┬ @papandreou/c@1.0.0 
    └── @papandreou/d@2.0.0 

[iarna]

The current peerDependencies logic is:

Given that c peerDepends d@2.0.0 (as in your example), the peerDependencies validator ensures that c can load d@2.0.0– it doesn't care where from.

Because you've ALSO set a regular dependency on d@2.0.0 it will use the version installed inside d. In fact, it HAS to because node provides no facility for c loading any other version of d.

I'm not convinced that this is incorrect behavior. It's a bit confusing, but I think that both providing a dependency on something AND a peerDependency on the same thing isn't a use case that was considered when the feature was designed.

[papandreou]

I agree that it seems like a peerDependencies entry duplicated by a dependencies entry is at best underspecified. I got the idea from @getify's questions on #6565, although noone ever committed to making it work that way.

Given that c peerDepends d@2.0.0 (as in your example), the peerDependencies validator ensures that c can load d@2.0.0– it doesn't care where from.

Hmm... My intuition says that it should care where from now that the peerDependencies handling has been degraded to a check that emits warnings. I think the presence of a peer dependency should mean "If this module is installed anywhere in the tree, it should be at the root level, and its version should satisfy this spec".

[papandreou]

Or rather, "If this module is installed anywhere in the tree, it should be at least one level above me, and its version should satisfy this spec".

[iarna]

Well, I'm not opposed per se to your clarification.

[iarna]

I'm gonna schedule this for next week and see how that change feels.

[papandreou]

Thanks a lot for looking into it :)

[iarna]

Ok, I like this change. This is what output from my patch looks like:

(In my example a depends and peerdeps on call-limit@1.0.1 and b depends and peerdeps on 1.0.0)

rebecca@Caldina:~/code/npmtest/9050$ node ~/code/npm ls
/Users/rebecca/code/npmtest/9050
├─┬ a@1.0.0
│ └── call-limit@1.0.1
├─┬ b@1.0.0
│ └── call-limit@1.0.0
└── UNMET PEER DEPENDENCY call-limit@1.0.0

npm ERR! peer dep missing: call-limit@1.0.0, required by b@1.0.0

Plus, this let me add another enhancement– if more than one module needs a peerdep, we report all of them:

rebecca@Caldina:~/code/npmtest/9050$ node ~/code/npm ls
/Users/rebecca/code/npmtest/9050
├─┬ a@1.0.0
│ └── call-limit@1.0.1
├─┬ b@1.0.0
│ └── call-limit@1.0.0
├─┬ c@1.0.0
│ └── call-limit@1.0.0
└── UNMET PEER DEPENDENCY call-limit@1.0.0

npm ERR! peer dep missing: call-limit@1.0.0, required by b@1.0.0
npm ERR! peer dep missing: call-limit@1.0.0, required by c@1.0.0

[sunesimonsen]

That looks like a very good solution.

[papandreou]

Thanks a lot! This saves the day.

Wrt. me switching back and forth between "it should be at the root level" and "it should be at least one level above me", I've been thinking about it some more, and I'm no longer as sure as I was when I wrote it. Both interpretations will work fine in our own use case, and it's not that I think "one level above me" is wrong, even if it's the most liberal of the two. My intuition simply doesn't tell me what the right arena for settling the peer dependencies should be.

I think what I'm asking is -- can anyone come up with an example where the peer dependencies would have to be satisfied at the root level, ie. where the npm install is taking place? This is out of curiosity more than anything else.

[papandreou]

Never mind the last bit. My colleague @gustavnikolaj convinced me that "one level above me" is in fact the correct arena. Then the level above can specify its own peer dependency if it needs to escalate the resolution even further up the three.

[iarna]

So the changes described here landed in 3.2.2, I do think this is an improvement, but while I was thinking through this I got thinking and I think looking from the perspective of the parent in the physical tree (what we're doing now) isn't quite right. We should look from the perspective of the modules requiring the module with the peer dep. That is, we need to walk up the logical tree, not the physical tree.

I'm going to make another ticket for changing to that behavior and I'll link it here when I do.

[dlongley]

That is, we need to walk up the logical tree, not the physical tree.

See: nodejs/node#3402