Move eslint to a peer dependency

[IanVS]

Fixes #40
Closes #50

This package has always bundled eslint as a dependency because I wanted it to be easy to get up and running. The more-proper thing to do is to specify eslint as a peer dependency. Even though peer dependencies can be a bit confusing, I think it's likely that users of this package will generally already have eslint installed, and it makes a lot of sense to use their version, instead of bundling our own.

[ljharb]

Note that you can make it both a dep and a peer dep, and it’s still easy for users, but the peer dep will prevent conflicts with a higher level eslint.

[IanVS]

Thanks for commenting. Honestly, it feels weird for it to be both. Is there any documentation for how exactly the resolution is determined in such a case? Is it simply that eslint will be conditionally installed depending on whether or not an eslint which satisfies the version in peerDependencies is found at the time of eslint-nibble installation? If so, does that work in both yarn and npm?

[ljharb]

It will be installed if it’s not already in the tree and requireable; if it results in a duplicate, the one up the tree will fail the peer dep requirement.

I’m not sure how it works in yarn, but how it works in npm is the standard, so if it doesn’t work in yarn, yarn is broken :-)

[ljharb]

You’ll want the dep and peer dep range to be identical - as is, eslint 4 users will always get a conflict.

[IanVS]

I took eslint back out of dependencies for a few reasons:

• I think it's reasonable to expect users to install their own eslint. After all, they're planning to use eslint, which is why they're turning to this tool, to help them out.
• Including a package in both dependencies and peerDependencies feels very non-standard and opens us up to bugs or inconsistencies in how package managers choose to behave, which brings us to:
• Yarn doesn't seem to behave nicely in this case. After having installed eslint with npm, trying to install eslint-nibble with yarn was throwing fatal errors. I wiped everything out, installed eslint@4 with yarn, and then installed eslint-nibble with yarn as well, but it ended up putting [email protected] into eslint-nibble's node_modules. Neither of which is a great outcome.

Hopefully the readme is clear enough for people to know they need eslint to be installed. And if they try to install and run eslint-nibble without having eslint, they'll get errors that pretty clearly tell them they need to install eslint.

All that to say, I'm going to stick with just peerDependencies for now, and if I get a bunch of issues from confused users, I'll re-evaluate.

[ljharb]

Up to you; we do it in almost every package we author at airbnb - it works perfectly, as long as any peerDep range always has an identical range in either deps, or devDeps.

If yarn doesn't work with it, I'd file a bug on yarn, but it's fair not to do it if yarn is something that matters to you for some reason.

[IanVS]

Thanks for the feedback. Do you have any particular reasons that you think including eslint in dependencies would be an advantage?

The fact that it doesn't work well on yarn is a bummer, but isn't a showstopper on its own. And I could open an issue with them, but it seems similar to yarnpkg/yarn#3951, and I'd wager my issue would be a wontfix for them as well (see also yarnpkg/yarn#4125 (comment)).

Aside from all that, using plain peer dependency has an advantage in simplicity. I'll be able to say, "'eslint-nibble' uses the version of eslint you have already installed in your project." without the complexity of figuring out whether this package installed its own version or not.

[ljharb]

npm already handles that complexity - by adding it as a dep, someone without eslint installed at all can use your package without any additional work, but for someone who already has a compatible eslint installed, eslint-nibble will happily use that one.

In the event of someone having eslint 3, or eslint 6, npm ls or npm ls -g will print out an error, and they'll hopefully be able to address it.

[IanVS]

someone without eslint installed at all can use your package without any additional work

That's true, but they'd also need to create an eslint config file for this tool to do any good, and for that, they'd need to know what version of eslint would be used, since config files have changed between many of the major versions. Yes they could use npm ls eslint to figure it out, but now we've taken something that was supposed to be easy, and made it more complicated again.

I'm sure that using the hybrid approach you suggested works great in a lot of cases. I'm just not convinced it's the right thing for this project. But thanks again for the suggestion and discussion!