Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change peerDependencies #185

Closed
wants to merge 1 commit into from
Closed

Change peerDependencies #185

wants to merge 1 commit into from

Conversation

Keats
Copy link

@Keats Keats commented Aug 9, 2015

It currently prevents installation of eslint 1.1.0

It currently prevents installation of eslint 1.1.0
@nkbt
Copy link

nkbt commented Aug 9, 2015

👍

@tleunen
Copy link

tleunen commented Aug 9, 2015

Instead of updating the peerDependencies, you should just remove it and put eslint as a simple dependency

@nkbt
Copy link

nkbt commented Aug 9, 2015

@tleunen why?

@AlexKVal
Copy link
Contributor

AlexKVal commented Aug 9, 2015

package.json from react-bootstrap project

"eslint": "1.0.0",
"eslint-plugin-react": "^3.1.0",

https://github.com/react-bootstrap/react-bootstrap/blob/master/package.json#L58
https://github.com/react-bootstrap/react-bootstrap/blob/master/package.json#L63

As you can see - all works fine without this PR 😉
I hope it helps 🍒

@tleunen
Copy link

tleunen commented Aug 9, 2015

@nkbt nvm. Actually, I was saying that because soon the dependencies set as peer would be installed, but in this case, the end user will have eslint installed already so that's fine

@yannickcr
Copy link
Member

The current >=0.8.0 || ~1.0.0-rc-0 already match [email protected] but during my tests an error is still triggered when installing eslint + eslint-plugin-react in global (see #178), looks like an npm bug.

I think I will remove the peerDependency instead.

@yannickcr yannickcr closed this Aug 9, 2015
@lamuertepeluda
Copy link

UNMET PEER DEPENDENCY eslint@^2.0.0 || ^3.0.0

with eslint v3.3.1

Shouldn't peerDependency have been removed or is this plugin not supposed to work with eslint 3.3.x?

@yannickcr
Copy link
Member

What version of npm do you use? How is installed eslint/eslint-plugin-react, globally or locally?

@lamuertepeluda
Copy link

Npm v3.10.3
Node v6.4.0

(latest)

Tried both local (as devDependency) and global installation of eslint-plugin-react. Eslint installed globally.

@ljharb
Copy link
Member

ljharb commented Aug 23, 2016

You don't want anything installed globally, including eslint itself.

@lamuertepeluda
Copy link

@ljharb do you mean is global installation that causes troubles?
I use to install eslint and plugins globally because I have several projects managed in the same way, and I don't want to have node_modules replicated 100 times in my dev directory. Plus, I use nvm to manage node versions, I do eslint --init, ...

Probably this is more a npm issue, then, because semver also returns correctly.

@ljharb
Copy link
Member

ljharb commented Aug 23, 2016

Each project should have its own copy of eslint installed. Everything a given project needs should be installable with npm install.

@yannickcr
Copy link
Member

yannickcr commented Aug 23, 2016

Probably this is more a npm issue, then, because semver also returns correctly.

Yeah, seems the bug that makes us remove peerDependencies one year ago (and that I had completely forgotten the existence) is still there 😐

@ljharb
Copy link
Member

ljharb commented Aug 23, 2016

I'm not sure what bug?

@yannickcr
Copy link
Member

@ljharb

$ npm list -g
/home/yannick/.npm/lib
└── (empty)

$ npm install eslint -g
/home/yannick/.npm/bin/eslint -> /home/yannick/.npm/lib/node_modules/eslint/bin/eslint.js
/home/yannick/.npm/lib
└── [email protected]

$ npm install eslint-plugin-react -g
/home/yannick/.npm/lib
├── UNMET PEER DEPENDENCY eslint@^2.0.0 || ^3.0.0
└── [email protected]

npm WARN [email protected] requires a peer of eslint@^2.0.0 || ^3.0.0 but none was installed.

$ npm list -g
/home/yannick/.npm/lib
├── [email protected]
└── [email protected]

Everything is working at the end, there is only a false UNMET PEER DEPENDENCY warning when installing eslint-plugin-react. No warning at all if eslint is installed locally.

@ljharb
Copy link
Member

ljharb commented Aug 23, 2016

That's an easily ignore able warning when following the bad practice of having eslint installed globally - I think the value of the peer dep far outweighs that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

7 participants