Don't update when it would break a peer dependency

[mythmon]

If I have two packages, A and B, and A has a peerdependency on B@^2.0, Greenkeper should not file a PR to update B to a version outside that range, unless A is being updated too.

This issue caused mozilla/normandy#235 to be created, when it should not have been.

[bjacobel]

Here's an example of a Greenkeeper PR (actually the second Greenkeeper has made for this dependency) that I cannot merge because it would cause an unsatisfied peerDependency. https://github.com/bjacobel/rak/pull/29

Edit: Travis CI actually notes this in the npm install log for that PR, I should probably find a way to make the build fail on unmet peerDependencies. https://travis-ci.org/bjacobel/rak/builds/166727590#L584

[janl]

This also applies to ecosystems that don’t use peerDependencies, but the alle-style monorepo like PouchDB

cc @gr2m, who’s been flagging this before.

[janl]

More user feedback:

• https://twitter.com/ljharb/status/875182940551921664
• https://twitter.com/ljharb/status/875365101632606208

[ljharb]

A suggestion: temporarily modify travis.yml with an after_install line that runs npm ls >/dev/null. If it fails, parse the failure output to detect peer dep conflicts. Might work?

[gr2m]

This also applies to ecosystems that don’t use peerDependencies, but the alle-style monorepo like PouchDB

cc @gr2m, who’s been flagging this before.

For monorepos I suggested to look at the "repo" value in the package.json. If there are multiple packages that point to the same repository I would consider it a monorepo and update dependencies from it all at once.

Did you mean that?

For peerDependencies I would suggest we update the defined range in peerDependencies as we update the same module in devDependencies. With the difference that I would turn peerDependencies into version ranges that can cross breaking version numbers. Say I have eslint-plugin-import as both dev & peer dependency. If it updates from 2.1.3 to 3.0.0 I would update in devDependency to ^3.0.0 and in peer dependency to >=2.1.3 < 4

Would that be helpful? I think that would addrees @mythmon’s problem

If I have two packages, A and B, and A has a peerdependency on B@^2.0, Greenkeper should not file a PR to update B to a version outside that range, unless A is being updated too.

[ljharb]

@gr2m just to clarify, peer deps are in the dep itself, not the package-being-greenkeepered.

[gr2m]

I think I got that, but isn’t it a problem with the dependency, which should update its peerDependency?

In the case of https://github.com/mozilla/normandy/pull/235, would you like Greenkeeper to not send a PR for the new version of eslint-plugin-jsx-a11y at all until eslint-preset-airbnb updated its peerDependencies?

I think there is a big opportunity here for Greenkeeper to help eco systems like react or eslint which use peerDependencies a lot. We are still figuring out how we could be of best help :) Thanks for all your input

[ljharb]

no - for example, eslint-config-airbnb requires eslint 3. Any repo that's using eslint-config-airbnb must remain on eslint 3, but greenkeeper is bothering all of them with failing eslint v4 builds.

eslint-config-airbnb, perhaps, should update its peer dep, but zero consumers of eslint-config-airbnb should be getting a single notification until that time.

[travi]

i'm not sure that i agree that consumers should get no notification.

as a consumer, i think i would rather get the PR with some obvious way to understand the situation. that way i could look for and subscribe to any issues related to the progress of updates and understand why greenkeeper didnt send me an update when i see it in the output of npm outdated

[ljharb]

If you're using eslint-config-airbnb, it's mostly irrelevant to you that eslint updated (when the config can't support it). If greenkeeper wanted to file an issue notifying them that a dep of theirs has a peer dep that doesn't allow updating to the latest, and encourages them to comment on an upstream issue on that dep (that greenkeeper filed), that's great! PRs that automatically fail and can't be merged, however, are utterly useless noise.

Note that this is a particularly huge problem in any sub-ecosystem that uses peer deps - React, eslint, Angular, Ember, moment.js… just to name a few. As a maintainer of multiple libs in some of these ecosystems (eslint-config-airbnb/eslint-config-airbnb-base, enzyme, react-dates), the only thing that happens when a new major version of a peer dep comes out is that I get a horde of angry people filing disparate issues and PRs to naively update the dep, without actually making all the required changes necessary to make it work - when I am always fully aware that there's a new major version out. In the case of eslint v4, I filed https://github.com/airbnb/javascript/issues/1447 to avoid the nightmare caused by eslint v2, which was much worse with v3 because greenkeeper was prevalent by then (with this same peer dep issue).

[travi]

filing an issue while avoiding the PR is a fair point. i agree about the noise from the PRs, but wouldn't want to be left in the dark in case i stumble upon the outdated dependency and then waste time trying to track down why it wasn't updated. having an issue logged would cover the point about keeping me informed, but i would also hope that the issue could be closed once the mismatch preventing the update has been resolved.

by the way, i'm a consumer of several of the libs you maintain and really appreciate the work you're doing, both with them and in conversations like these to help the community improve management of complex processes like this.

[gr2m]

thanks for sharing your experiences / pains, much appreciated! That will definitely help us moving forward