Currently we:
(a) Do not renovate peerDependencies by default, but it can be enabled by users and adopts the same behaviour as dependencies and devDependencies
(b) Do not have any special logic enforcing correct peerDependencies when we upgrade dependencies/devDependencies
(c) Do not support "range joins", e.g. "^14.0.0 || ^15.0.0", and then 16.0.0 is released. Would you want "^14.0.0 || ^15.0.0 || ^16.0.0" or "^15.0.0 || ^16.0.0"? It may be impossible to "guess" what one's policy is for peerDependencies, so should we require there to be a configured policy type so we know?
For example we upgraded to eslint here and it works, however some of our eslint add-ons specify 3.x. Should we have waited even though tests seem to pass?
I also have a suspicion that npm5 may return a non zero exit code if peer dependencies fail
npm ls returns non zero exit code if it detects unmet or invalid peer deps
We can do this after npm install but right now we're only installing when there's a lock file
Here's how I think things should work:
npm ls passes prior to the update, make sure npm ls passes after the update.npm ls does not pass prior to the update, file an issue warning the dev that their dep graph is invalid, and probably, pause all updating on that repo.const newRange = new semver.Range(existingRange).concat(newVersion).toMinimalRangeString() and then update it (and the matching dep/peerDep, ofc) with the newRange if it's different.I think that covers it, but I'm happy to clear up edge cases if I've forgotten any.
(i'm happy to help with a PR to semver if needed, and if this all seems reasonable)
Now that it's common to use lock files, it may also be easier to solve (b). Both npm and yarn seem to warn whenever peerDependencies are unmet. In theory it's possible for us to "reject' an upgrade if it fails the peerDependency check when we generate the updated lock file. But then - what action should we take? Should we just silently ignore that upgrade (i.e. suppress the branch/PR until one day peerDependencies are met), or should we raise the PR anyway but add a warning or perhaps failing status check to the PR? It comes down to whether people want visibility or not. Having it "fail" via a status check may be nice, as it should stop any automatic or accidental merging, yet the user can choose to ignore it.
A good example was when eslint upgraded to 4.x. When that happened, several other related eslint-dependencies then had outdated/incompatible peerDependencies stating 3.x or less.
I think neither - you should try to update multiple things at once until the deps are satisfied.
In other words, when eslint updates to v5, i'd expect that as soon as all of the plugins and configs I depended on supported v5, I'd get a single update PR with all of them updated simultaneously to v5-supporting versions.
Regarding (c), I think that the default should be "always widen peerDep support, never tighten it" - but it totally makes sense to allow people to opt in to the update PR tightening support (thus making it a semver-major change)
@ljharb thanks for the ideas and lending your expertise to this feature idea.
when a lockfile is present (any of the 3 formats), it should be appropriately updated to match the normal package.json updates
This is the default behaviour today, very reliable. Renovate generates its own lockfiles and resolves conflicts, etc - no need to add anything to CI.
does not pass prior to the update, file an issue warning the dev that their dep graph is invalid, and probably, pause all updating on that repo.
I can do this, but people can hate intrusive bots, so I have some hesitation unless I'm sure that's what they want. Are you sure there aren't cases where a failing npm ls due to peerDependencies isn't "totally ok"? i.e. it's not a matter of "meh, I'll ignore it, should be ok" - it's actually "that's a false negative - there is nothing wrong with this package.json"?
I guess this is related to:
everything that's a peerDep must always also be a dep or devDep (99% of the time, a devDep)
I was not aware this was a "must always" type of thing. I have definitely seen a lot of repositories in the past that don't do this. That said, I am definitely open to the approach of "make sure your peerDependencies/devDependencies follow these rules or this new feature won't be turned on for your repo". And it definitely sounds like a good thing to me.
Some miscellaneous feedback:
I like the idea that "if it wasn't broken previously, it's a big problem if a new update breaks it". That should definitely be communicated one way or another.
We have multiple ways we could act on problems, including:
The challenge with npm ls is that it requires a full node_modules to be installed, and this is something I've been able to avoid (unnecessarily) doing since npm added support for npm install --package-lock-only. Also, if the repository is using yarn to install, I think npm ls may not be reliable. In theory it is not necessary to do a full install of node_modules to know if there's a problem with peerDependencies, so I would like to avoid a full install.
Even if the eventual solution requires us to generate a lockfile to verify peer dependencies, that doesn't mean we need to insist that the project commits it. It can just be the most efficient way for us to achieve this, instead of npm ls.
Renovate already has great support for grouping, an we could leverage that here. Autodetecting which packages have related peers and then grouping them together is definitely possible in theory, although not trivial in practice. As a first step, it's pretty easy to add a rule that says "all packages starting with eslint are grouped together into one PR". I'd be tempted to make this default, but I have learned that "do nothing" is usually the best default and allow users to opt in, because there's always 10-20% who object to highly opinionated defaults. Nevertheless, it can be simplified to one line of config for those who choose to opt in.
I agree with you that widening peerDependency ranges is a good default, especially if we allow configuration to override that default to tighten instead.
The challenge with widening is understanding all the various semver range types enough to be abler to correctly extend them. e.g. it's not just "^1.2.0 || ^2.0.0", it can be ">= 1.0.0 < 3.0.0", etc. And then you might have someone do "^1.2.0 || ~2.0.0", etc. It's obviously not an impossible task, just not trivial.
I’ll respond to the rest tomorrow; but my hope for the semver thing is that the semver module could add the ability to produce a minimal normalized range string from a list of comparators.
@ljharb no problem, please excuse me if I talk to myself a little on here in the meantime. Feel free to pick up again when you're back.
We also might want to distinguish between two different use cases:
peerDependenciespeerDependenciese.g. for (1) we can consider the author of eslint-config-something. Hopefully, that person has eslint listed as both a devDependency as well as a peerDependency. Let's say in both cases it's defined as "^3.0.0". Now eslint 4.0.0 is released, what should Renovate do? Perhaps it should update the devDependencies to "^4.0.0" (this would be current behaviour - nothing to change) and then widen the peer dependency to "^3.0.0 || ^4.0.0".
At the same time, there is a webapp builder who uses eslint and eslint-config-foo as devDependencies, both of them pinned to a specific 3.x.y release - although it doesn't need to be pinned. As soon as eslint 4.0.0 is released, Renovate will be ready to raise a PR. It can update eslint in devDependencies to 4.0.0, but as we know - this "breaks"eslint-config-something's peerDependency range. Maybe it works regardless.. maybe the user will decide later to proceed regardless.. but it's still a technical break that should not be silenced.
How can Renovate know this is a problem? It could npm install and then npm ls. Or hopefully it could instead generate a lock file only and check stderr for peerDependency warnings. Whether or not the repository has a lock file committed is irrelevant.
So now Renovate knows there's an update to eslint, but also knows that there are unmet peerDependencies in this branch. How to inform the user? Right now I'm thinking the best bet is to add a renovate/peer-dependencies status check that fails. That way the user still gets the PR and is aware of eslint being upgraded to 4.x but also knows there is a peer dependency failure and can check the log that we attach to the status failure.
Now let's go back to the author of eslint-config-something. They've got a PR for eslint 4.x and maybe they're lucky - it all passes. They check release notes in case there's something non-obvious, and then they decide it's safe to merge. Because it's a backwards compatible change (the peer range was widened), they can push this out as a patch or minor release and they are done.
Back again to the webapp author. Now there is the update to eslint-config-something coming form Renovate. At this point, it could either:
(a) go into its own PR, or
(b) it could get combined with the eslint PR if the user has configured such a rule to group all eslint packages
For this case, it doesn't matter. If it's in its own PR with eslint 3.x from master then it should still pass tests and can be merged.
After that PR is updated, Renovate should then update existing PRs (although whether to do this or not is configurable). If so, then the eslint 4.x PR will now be rebased off the new master and hence with eslint-config-something and there will no longer be peer dependency warnings! The status check goes green and it can be merged.
Alternatively, if the user had configured eslint grouping, then eslint-config-something would have been added to the existing PR for eslint and that PR would go green, so they get merged together. This approach is actually superior for reasons I will describe:
Let's consider the case where eslint 4.x "broke" eslint-config-something. Let's say the author decides he/she does not want to support both 3.x and 4.x simultaneously, so they edit the Renovate PR for eslint to change "^3.0.0 || ^4.0.0" to just "^4.0.0", update their code, and then push out a new major release 2.0.0 for eslint-config-something.
The problem if this was in a separate PR like earlier is that now both the eslint major PR and the eslint-config-something PRs fail separately due to peer dependency checks. The only way to resolve is for a human to manually edit one of these branches to combine it, including updating a lock file if present.
Instead, if eslint packages were grouped together then the major update for eslint-config-something would have been added to the existing major update for eslint and passed tests + peer dependency checks, requiring no manual intervention other than merging the PR.
Conclusion: if packages that include peer dependencies are not grouped together with their peer then there are cases when you end up with separate - failing - PRs.
Could we magically/implicitly group eslint-config-something with eslint? Maybe, but not always. If eslint was the only peerDependency, then it's easy to say "group it together with eslint". But what if it had peerDependencies with both eslint and prettier - how can we know which is the "parent" to group with in that case? prettier does not list eslint and vice versa.
I would prefer - and certainly suggest this for the MVP - that any grouping is configured by the user and not implicit in the app. I can certainly create a preset that groups together any packages matching eslint* and add that as a rule of my default "config:base" preset meaning that the majority of users who accept this preset will automatically get grouping of eslint packages too.
I usually like to approach problems from the outside in, so in this case my instinct is to solve the problem for library authors first. i.e. supporting widening of peerDependency ranges by default.
It's worth noting that yarn currently warns about missing peer dependencies in cases where it shouldn't - eg yarnpkg/yarn#4850.
Thanks @edmorley - I have subscribed to that now to watch it. Like always, we should make Renovate's behaviour configurable in case something needs to be turned off!
Perhaps it should update the devDependencies to "^4.0.0" (this would be current behaviour - nothing to change) and then widen the peer dependency to "^3.0.0 || ^4.0.0".
If the peer dep is ^3 || ^4, then the dev dep should be too - so that if the module manually installs v3 for testing, npm ls will still pass.
Right now I'm thinking the best bet is to add a renovate/peer-dependencies status check that fails.
That sounds like a very ergonomic way of letting them know that npm ls failed due to peer deps.
The problem if this was in a separate PR like earlier is that now both the eslint major PR and the eslint-config-something PRs fail separately
Conclusion: if packages that include peer dependencies are not grouped together with their peer then there are cases when you end up with separate - failing - PRs.
Agreed, this is why separate PRs for a package that has peer deps, or is a peer dep, is incorrect.
I would prefer - and certainly suggest this for the MVP - that any grouping is configured by the user and not implicit in the app.
Certainly this might be fine for the MVP, but peer deps can be added or removed, and it doesn't make sense for the user to have to keep track of those. https://npmjs.com/install-peerdeps looks them up dynamically, for example - since it's just a field in JSON files, I'm not sure why renovate couldn't do so as well.
But what if it had peerDependencies with both eslint and prettier - how can we know which is the "parent" to group with in that case? prettier does not list eslint and vice versa.
I'm not sure why there needs to be a "parent" - there'd just be a "triggering package", ie, the one that published an update. The triggering package, combined with the project being updated, would implicitly create a dep graph in potentially multiple directions, and there's your group.
my instinct is to solve the problem for library authors first. i.e. supporting widening of peerDependency ranges by default.
This to me seems the much more important use case anyways, so that sounds great :-)
I was not aware this was a "must always" type of thing. I have definitely seen a lot of repositories in the past that don't do this.
I'm using "must" a bit strongly here; but the reasoning is that for anything that your lib declares as a peer dep, surely you'd want to install it during development and test with it. At all times, npm ls should be passing - when npm ls exits nonzero, nothing can ever be relied upon to work in any project. Thus, if the peer dep allows v1 - 5, then the dev dep must also allow that (even tho an npm install would always pick the latest, v5 in this case).
Thus, if the peer dep allows v1 - 5, then the dev dep must also allow that (even tho an npm install would always pick the latest, v5 in this case).
You have convinced me on the matter of "peer dependencies should always be present in dev dependencies", but I'm still chewing over this idea that they must be the exact same semver string as in peer. Because devDependencies are not propagated to downstream libs, their purpose to me is to align the project on "this is what we're all using - in dev and in tests - right now". Ideally, people who list more than one major version in a peer dependency should be testing both major versions - but most CI is not that sophisticated to add yet another dimension to matrix builds. You could argue that lock files help align everyone, but they are still rather a black box, there's no consensus on when/how they should be regularly kept up-to-date, etc. So I personally prefer to lock down devDependencies, Renovate itself is agnostic but defaults to pinning.
I don't personally think lockfiles should ever be used in non-apps, so that's not what I'm arguing.
"this is what we're all using" means "the latest version that satisfies the semver range". ^3 || ^4" in a devDep is equivalent to ^4 when using v4, it's only different when using v3 - which even if someone isn't yet testing v3, they should be able to.
Another angle: devDeps are only used within the project itself. And we both know what ^3 || ^4 means ^4 will always be installed. So in which practical scenarios would it make a difference whether the devDeps list both major ranges, or the latest major range, or the latest pinned version?
CI runs "within the project itself", and CI needs to be free to npm install package@^3 && npm ls as part of its testing.
I took a quick look around at some projects using peer dependencies and found too many who don't do as you think they should. In fact I gave up before I found one that did.
Example: https://github.com/ReactTraining/react-router/blob/master/packages/react-router/package.json has 27000 stars and lists "react": ">=15" for peer deps and "react": "^16.0.0" in dev Deps.
Personally, I'm all for enforcing good practices. But as a bot author, I've learnt not to force people to do things they don't want to. So in this case I definitely can't make "strict" behaviour the default, let alone enforce it, however I want to support "thought leaders" like yourself who are probably doing it the right way and want a tool to automate it a little.
In that case, what about this logic for libraries?
e.g. let's say someone had "^14.0.0 || ^15.0.0" for react in peerDependencies and ^15.0.0 in devDependencies. They'd end up with "^14.0.0 || ^15.0.0 || ^16.0.0" in peerDependencies and ^16.0.0 in devDependencies. By default we'd already have these two updates in the same PR, because it's the same package name. And if you had "^14.0.0 || ^15.0.0" for both then you'd end up with "^14.0.0 || ^15.0.0 || ^16.0.0" for both.
That sounds good. Regardless of the default, it'd be great to have a config option that forced the dep/devDep range to match the peerDep range.
fwiw, testing multiple versions of React is something most people don't do, because until very very recently (when we published enzyme v3 and enzyme-adapter-react-helper) it was very hard to do it right. With these tools, it's easy, so hopefully more people will start doing it :-)
Something to consider when actually implementing this: npm ls does _not_ check that peerDependencies are satisfied for the versions defined in package.json, it checks that the _currently installed_ versions satisfy the peerDependencies.
This can lead to situations where your minimum version is not actually satisfying a peerDependency, but a fresh install (e.g. in a CI build) reports everything is fine from a run of npm ls.
An example would be eslint-config-foo has a peerDependency on eslint@^4.9.0 while your devDependency is only [email protected]. If you just ran npm i; npm ls everything would be "fine", however somebody who has had an install running for a while and only updates eslint-config-foo will suddenly run into issues because their eslint install is still on say [email protected] since that was the newest at the time and it satisfies their devDependency.
I wrote check-peer-deps when I got tired of manually checking for this scenario and couldn't find anything else that did the job. Note: this doesn't require anything to actually be installed to check the peerDependencies, but I'm not sure I'd call it robust enough to use reliably as it is currently written 😛.
As for the semver range differences between peerDependencies and devDependencies, for _libraries_ I absolutely agree that they should be identical, however for project's that _use_ the library they shouldn't be required to be since that project may require some newer feature not in the minimum version defined in the peerDependency of its dependency.
Thanks @Arcanemagus for the feedback. I may have misunderstood, or maybe you typo'd.
eslint-config-foohas apeerDependencyoneslint@^4.9.0while yourdevDependencyis only[email protected].
So the package.json contains [email protected] as a devDependency?
their
eslintinstall is still on say[email protected]since that was the newest at the time and it satisfies theirdevDependency.
Why would that person have 4.6.0 if the package.json states 4.5.0?
Whoops, slight typo on my part causing confusion there. I meant that the devDependency was eslint@^4.5.0.
So let's say the user is working on package bar, which has this set of dependencies:
{
"... stuff": true,
"devDependencies": {
"eslint": "^4.5.0",
"eslint-config-foo": "^1.0.0"
}
}
The user runs npm install and gets [email protected] and [email protected] installed, since those were the latest versions available at the time.
_Time passes..._
eslint-config-foo updates to require a peerDependency of eslint@^4.9.0 in version 1.1.0.
The user (or in this case Renovate) updates the devDependency of bar to eslint-config-foo@^1.1.0 and starts a PR.
The CI build runs npm install, gets [email protected] and [email protected] installed. npm ls is happy because the installed versions of the packages satisfy all peerDependencies.
PR gets merged, user updates their local copy and runs npm install. eslint-config-foo is updated to 1.1.0... and suddenly the user gets a warning that the peerDependencies aren't satisfied because their local version of eslint is still 4.6.0 since it still satisfies the devDependency of bar of ^4.5.0.
Does that make sense?
Yes, it makes sense, although Renovate won't suggest an update from ^1.0.0 to ^1.1.0 (it satisfies the range, no point to update) so in your hypothetical scenario it must have been done manually.
What about in the scenario of lock files? Are npm or yarn smart enough to "mix" dev and peer dependencies in a way that they could determine that eslint 4.9.0 is now needed and then force the update of node_modules?
Anyway that's OK in this context because I'm hoping to solve this without needing to install a full node_modules every time -- in theory it can be checked via metadata only.
in theory it can be checked via metadata only.
That's what check-peer-deps does 😉.
Thanks @ljharb, @edmorley and @Arcanemagus for the suggestions and feedback. Here is how I'm planning to proceed:
First task is to start with functionality to benefit library authors (those whose packages have peerDependencies):
Add capability to "widen" peerDependencies by default, whenever a new version of a peer dependency is released that doesn't satisfy the existing range. Today we skip over peerDependencies by default, and if someone turns it on then we "replace" ranges, not widen.
Add logic to keep versions in devDependencies synchronised with the same package in peerDependencies, if they were already synchronised prior to us widening the peerDependency
A future functionality for a later implementation may be to encourage users to synchronise devDependencies values with peerDependencies during Renovate onboarding, similar to how we pin dependencies.
I need to think of a configuration option name for this new concept of widening or replacing. One possibility is to call it semverStrategy and with possible values of null (autodetect), "widen" and "replace". However another possibility if there are only two values is to use a boolean, e.g. call it widenRanges with values of null (autodetect), true and false.
Because semver ranges can be wickedly complicated (e.g. including greater than, less than, OR and AND logic, and hyphen (-) ranges, it may take some time to go through these use cases and perhaps some will be skipped in the first release.
If anyone has some "real life" examples - perhaps peerDependencies values you've already upgraded from/to in the past year or so - it would be appreciated if you can copy/paste them here for reference.
A future functionality for a later implementation may be to encourage users to synchronise
devDependenciesvalues with peerDependencies during Renovate onboarding, similar to how we pin dependencies.
Don't forget that this needs to be done any time a dependency updates its peerDependencies, not just during onboarding.
The real life example that caused me to write check-peer-deps is one of the packages @ljharb manages 😛: eslint-config-airbnb-base. Quite often the peerDepdencies for that package update to a very recent version of ESLint, which is great, but the packages I use that configuration in quite often have their minimum devDependency version of ESLint out of sync with the peerDepedencies version required by eslint-config-airbnb-base.
For example in linter-docker right now it is configured with:
{
"devDependencies": {
"eslint": "^4.6.0",
"eslint-config-airbnb-base": "^12.0.0"
}
}
A fresh install would get [email protected] and [email protected] installed, but I'm sure my local copy is probably old enough that its ESLint version might not satisfy the requirement of eslint@^4.9.0 since I haven't worked on that in a while.
I definitely agree that this is more important for library authors first, but just wanted to make sure the other half of the problem was clear 😉.
@ljharb I decided to start with eslint-config-airbnb-base as an example to study, as @Arcanemagus linked to it and that's what we use in this repo too.
Could you describe to me what "manual" logic you have been using for when to bump peerDependency versions of eslint? I had expected to see some infrequently changed "wide" ranges like we've been talking about but instead see quite "narrow" ranges there ("^4.16.0" == "4.16.0" right now, for example). Looking at the git blame for that file, I see this recent history for eslint:
https://github.com/airbnb/javascript/commit/7b1ced2bf79cfe119ab1b7becd096b88b9250ee7 - bumped eslint from ^4.15.0 to ^4.16.0 (latest). Was that because the changes required [email protected] and not supported in [email protected] ?
https://github.com/airbnb/javascript/commit/7b18d7c2155f52937b1d48a3c6b905943fc1e5b7 - bumped from ^4.14.0 to ^4.15.0. Was that because the ignoreComments change required an upgrade to 4.15.0?
https://github.com/airbnb/javascript/commit/9061044af05cce24cc322ce123b7d5c2f826d0e5 - this commit did nothing but bump the dependencies. Why "narrow" the range of peerDependencies if it was working fine with eslint@^4.9.0 previously? Or was it not working and only just discovered? I thought the goal should be to keep as wide of a range as possible for users, i.e. the minimum version that works.
I am wondering if you have actually been bumping the eslint version somewhat arbitrarily, instead of because of only when incompatibilities or features demand?
More importantly, if you were using Renovate when [email protected] is released, what would you want done? (a) Nothing, because it's an in-range update? Or (b) would you want it bumped, like you've been doing previously? e.g. change peer and dev versions to ^[email protected]? There is no possibility to "widen" it in this scenario.
And what if v5.0.0 was released instead? Would you want a PR replacing ^4.16.0 with ^5.0.0 or in this case would you want it to be ^4.16.0 || ^5.0.0 ?
@Arcanemagus wrote:
Quite often the
peerDepdenciesfor that package update to a very recent version of ESLint, which is great
Could you clarify why that is "great"? I would have thought it's great if a config like airbnb-base updates to very recent versions because it's utilising the latest features of eslint, but otherwise updating peerDependencies without a functional reason risks causing avoidable incompatibilities with other packages?
I would have thought it's great if a config like airbnb-base updates to very recent versions because it's utilising the latest features of
eslint
That's the reason why eslint-config-airbnb-base updates their peerDependency, and exactly why I called it "great" 😉. At least that's been the reason the few times I've looked into the changes between versions myself.
@ljharb @Arcanemagus the first step for library authors using peerDependencies is nearly complete:
1.x - 3.x, ^1.1.0 || ^2.0.0, etc.1.x - 4.x^1.1.0 || ^2.0.0 || ^3.0.0^15.0.0 -> ^16.0.0versionStrategy, which defaults to auto but can be set to widen or replaceversionStrategy can be configured at any level, e.g. for entire repo (unlikely), or for all peerDependencies (likely), or for individual packages or package listsSo far, Renovate does not keep the same package in devDependencies and peerDependencies in sync automatically *if you override versionStrategy for one and not the other. e.g. if you configuredversionStrategy=widenfor allpeerDependenciesandeslintinpeerDependenciesis widened in a PR, you won't see it widened in the correspondingdevDependencies` because it is still autodetecting 'replace', assuming it started as a simple semver range. For now, if you want that behaviour then you should configure a package-rule like so:
"packageRules": [
{
"packageNames": ["eslint", "eslint-config-import"],
"versionStrategy": "widen"
}
]
@ljharb could you clarify re: the airbnb-base peer dependency updates I mentioned above? I’m ready to move to the next step but what I’m seeing in that repo doesn’t seem to align with all the “widening” discussions here so I want to make sure I understand it first.
@rarkins sorry for the delay. With eslint-config-airbnb/eslint-config-airbnb-base in particular, whenever we use rule configurations that require a newer version of eslint, we do have to raise the lower threshold for eslint itself. In particular, in these configs, I'm not following the advice I normally follow elsewhere: for things that support multiple versions of React, I write code that checks for newer features, and falls back gracefully to older ones. With eslint configs, however, I'd have to change the declarative config format to something much more magical and dynamic if I wanted to retain support for older eslint versions in the current major line.
In other words, for an eslint shared config, config schema validation, and eslint's own semver policy, means that it's not practical to define semver-major the same way one would on other packages.
(yes, I tend to bump the eslint versions in those packages arbitrarily, even when it's not strictly necessary)
More importantly, if you were using Renovate when [email protected] is released, what would you want done?
Nothing indeed. What I'm most interested here isn't eslint; it's making sure that if, say, eslint-plugin-react had a peerDep that forced my eslint peer dep higher, that when updating eslint-plugin-react, renovate would also update eslint appropriately.
@ljharb thanks very much for the clarification. re: eslint vs react approach, I think you described the difference approaches clearly and I think it sounds logical.
What I'm most interested here isn't eslint; it's making sure that if, say, eslint-plugin-react had a peerDep that forced my eslint peer dep higher, that when updating eslint-plugin-react, renovate would also update eslint appropriately.
This is a great use case. Even narrowing it down to a use case like "webapps", I think it varies a little though depending on whether you (a) use ranges for devDependencies or not, and whether you use lock files or not.
In such a case, eslint is likely already upgraded to whatever eslint-plugin-react needs, or at least a PR exists, assuming the version of eslint necessary for the new eslint-plugin-react peerDependencies was released first. If an appropriate grouping existed in Renovate - explicit or a future implicit grouping - then they'd land into the same PR and pass all tests and should get merged. If no grouping, then eslint would be in a green PR and eslint-plugin-react in a red PR (assuming we validate peerDependencies somehow), but once eslint is merged then the eslint-plugin-react PR would be rebased off master and turn green.
Alternatively, let's say that eslint's new version "broke" the older plugin. We've discussed this situation before as it was applicable for when eslint released v4.0.0. In that case both PRs would be red, and neither would go green without manual intervention. The solution is that eslint and every package that depends on it as a peerDependency should be updated in one PR. Simple packageRules can help with that today but in future we may make the logic explicit in case there are any peers that don't follow the typical ^eslint syntax.
In this case eslint and eslint-plugin-react might be set to caret ranges in devDependencies, but a lock file exists that locks them down to a specific version. Let's say that a new eslint-plugin-react is released that requires eslint@^4.15.0 as its peer. It does not bump major version because it doesn't consider it "breaking" enough. Let's say that eslint had been locked to [email protected] in the lock file.
What happens: nothing. The new eslint-plugin-react version is still within the existing range defined in the app's devDependencies, so no PR from Renovate is necessary.
This is probably not best practice, considering that features or fixes in the eslint family of packages could break builds at any time, but let's consider it anyway.
Still nothing happens, for the same reason as with lock files. In dev or CI, some may have problems e.g. if they install the newest version of eslint-plugin-react while still having a cached version of eslint in node_modules, it might not work.
I appreciate this issue is old and likely already solved but I'm looking to understand if the following statement is expected to still be the case:
Currently we:
(a) Do not renovate peerDependencies by default, but it can be enabled by users and adopts the same behaviour as dependencies and devDependencies
I'm trying to configure renovate to not update peer dependencies but I can't seem to find a way to do this.
@andyrichardson please post in https://github.com/renovatebot/config-help instead
You need a package rule
"packageRules": [
{
"depTypeList": ["peerDependencies"],
"enabled": false
}
]
@rarkins I think we can close this, as peerDependencies are widen by default.
Most helpful comment
It's worth noting that yarn currently warns about missing peer dependencies in cases where it shouldn't - eg yarnpkg/yarn#4850.