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

Migrate eslint-plugin-react to PeerDependencies #720

Closed
kwelch opened this issue Feb 9, 2016 · 8 comments
Closed

Migrate eslint-plugin-react to PeerDependencies #720

kwelch opened this issue Feb 9, 2016 · 8 comments

Comments

@kwelch
Copy link

kwelch commented Feb 9, 2016

Shouldn't the react plugin be a Dependency or PeerDependency since it must be installed as a sibling of the config for it to properly list.

@ljharb
Copy link
Collaborator

ljharb commented Feb 9, 2016

That's a really good point. It's currently a devDependency.

I think I prefer it being a dep instead of a peer dep - anyone have any reason why a peer dep would be more appropriate?

@kwelch
Copy link
Author

kwelch commented Feb 10, 2016

I was not sure of the difference between the two, so I would agree it is a dep.

@tleunen
Copy link

tleunen commented Feb 10, 2016

I think a peerDep is more appropriate here to avoid having it twice (if different versions are used?).
And because a direct dependency is required, specify it as a dev dependency here is not really needed

@ljharb
Copy link
Collaborator

ljharb commented Feb 10, 2016

(if it becomes a dep, it wouldn't be a dev dep, but if it's a peer dep, it would definitely also be a dev dep to ensure tests can pass without explicitly installing the dep)

@tleunen
Copy link

tleunen commented Feb 10, 2016

Oh yep, you're right.

@kwelch
Copy link
Author

kwelch commented Feb 12, 2016

So just to confirm, it will be added as a peer dep. This is because as a peer dep it is installed properly for use as a dep and a devDep.

Is this correct?

kwelch added a commit to kwelch/javascript that referenced this issue Feb 12, 2016
By moving it to peerDep it will be properly installed for dev use and public
fixes: airbnb#720
@ljharb
Copy link
Collaborator

ljharb commented Feb 12, 2016

@kwelch as of npm 3, peer deps are not automatically installed anymore. I'm still not certain, but am leaning towards straight dep. If it is a peer dep, it would have to be both a peer dep and a dev dep.

@ljharb
Copy link
Collaborator

ljharb commented Feb 14, 2016

I ended up making this both a peer dep and a dev dep. I may make it a straight dep in the future but for now this will suffice.

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

No branches or pull requests

3 participants