Repository navigation
Be more lax about peer dependencies in eslint-config-react-app #1110
Description
When using CRA with an additional linter I hit problems with matching peer dependency versions with eslint-config-react-app. It has absolute requirements for its peer dependencies.
{
"peerDependencies": {
"babel-eslint": "7.0.0",
"eslint": "3.8.1",
"eslint-plugin-flowtype": "2.21.0",
"eslint-plugin-import": "2.0.1",
"eslint-plugin-jsx-a11y": "2.2.3",
"eslint-plugin-react": "6.4.1"
}
}Specifically babel-eslint, eslint, and eslint-plugin-react is hard not to clash with. I had another eslint-config that listed all three of them with incompatible versions. As an example it listed eslint as ^3.11.0.
Had eslint-config-react-app used the caret-style dependencies for it's peer dependencies, it would have worked out fine.
In general, I understand why you might want to absolutify your linter dependencies - there's nothing more annoying than lint errors showing up on CI that you didn't catch locally. But in this case, where it's very common to add another linter setup I think it might be worth it, to be more lax about the requirements.
For my case, pinning the versions listed in peerDeps of eslint-config-react-app got rid of the peer dependency warnings, after I had downgraded the requirements in the other eslint-config I used.
Would you be open to widen the peer dependency version ranges in eslint-config-react-app?
Activity
[-]Be more lax about peer dependencies in eslint-config-[/-][+]Be more lax about peer dependencies in eslint-config-react-app[/+]gaearon commentedon Nov 29, 2016
@fson Any thoughts?
fson commentedon Nov 29, 2016
In the past I've had ESLint plugins misbehave after upgrading to a new minor version, and I don't want this to happen in CRA. However, we can still keep the strict versions for these dependencies in
react-scripts, so loosening the requirements for peer dependencies will only affect people usingeslint-config-react-appdirectly. The risks are also diminished by usingyarn.lockornpm-shrinkwrap.json, which advanced users should be doing already.I think it's probably worth doing for compatibility with other configurations. We can also keep the exact versions pinned in the installation command we have in the readme, so we'll still have a "sanctioned" set of versions all the time that we have tested.
gustavnikolaj commentedon Nov 29, 2016
@fson I completely understand your stance on this matter, and I agree wholeheartedly. But I think that it is better controlled from the projects themselves and not by third party library code. In my opinion, both CRA and eslint-config-react-app fall into the latter category.
As a user of CRA I'd expect that I could just run
npm install --save-dev standardand add a lint script to mypackage.jsonand be up and running. That will not work without triggering peerDependency warnings if standard relies on a package that is too newer than one defined ineslint-config-react-app. As of writing, this will actually work, as the most recent version ofeslint-config-standardhas a peer dependency oneslint >=3.8.1. If that was changed to>=3.8.2we would start seeing the issue I described.Most of the sharable eslint configurations that I have seen are using peerDependencies with either caret or greater-than-or-equal to, to avoid dependency hell. A few examples:
Doing the same here will allow people to get off the ground easily, with no friction, but still allowing them to freeze packages using shrinkwrap / yarn or manually adding absolute versions for the important packages.
fson commentedon Nov 29, 2016
That's a good point. My only concern is that a breaking change in an ESLint plugin is going to be published as a minor version and it's going to break new projects created with
create-react-app.Is there any way to be strict in what we install automatically when you run CRA, but at the same time allow users to upgrade if they need to? Should we start bundling a
yarn.lockfile in our project template?gustavnikolaj commentedon Nov 30, 2016
I'm not aware of any way to change how the version ranges are interpreted by npm. Unless I've got it all wrong, it will install the latest package that satisfies your spec.
As I see it, breaking changes can primarily be introduced into
eslint-config-react-appin two ways:One way to mitigate some of the problem is to not depend on other
eslint-config-*packages or presets, but instead list the rules directly ineslint-config-react-app. That way, it's only if a rule changes behavior or disappears completely from a given plugin.That seems less likely to happen, and it does constrain our vulnerability :-)
fson commentedon Nov 30, 2016
We already list all the rules in
eslint-config-react-app. But sometimes bugs are introduced in the plugins or ESLint itself, or they don't work with a certain version ofbabel-eslintetc.gustavnikolaj commentedon Nov 30, 2016
Okay... In that case, I don't think there's any thing we can do to mitigate the problem. It's an inherent problem to the node eco system and how npm works.
The open version ranges allow people to push out new versions that fix those bugs, without requiring a new release of CRA or other projects that depend on the given package.
My personal opinion is that the current version ranges are too conservative for a module, which is meant to be plugged into other projects.
As the eslint config is style-agnostic by choice, users are pretty much required to bring their own linter - and eslint being the defacto standard linter the absolute peer dependency versions will cause additional friction.
I would be more okay with this strategy if CRA also bundled an opinionated linter. Then users wouldn't be required to bring their own, and the problem would be irrelevant. But that seems to be a major change in the direction of the package. (Just for the record, I don't think that's a good idea - just trying to make a point wrt. different expectations for different circumstances).
I realize that this is a matter of personal opinion and preference (religion? 😄) so I'll get out of your way now that I presented my view. I trust that you'll find the optimal tradeoff for this project. :-)
EnoahNetzach commentedon Dec 5, 2016
Another way possible is to give an explicit range, so that the minimum is needed for CRA to work, and the maximum is granted to work well without having bugs.
This, on one other hand, is quite a lot of work on the maintainers' side, but on the other hand grants more stability to those who have to enforce a stricter lint.
gaearon commentedon Dec 7, 2016
Fixed in the latest version of the preset.