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

jshint dependency #131

Closed
lundyio opened this issue Nov 20, 2015 · 20 comments

Comments

@lundyio
Copy link

commented Nov 20, 2015

What is the thought process behind making jshint a peer dependency? Just curious, had to update a couple builds this morning that were failing due to the dependency not being automatically installed.

@binarykitchen

This comment has been minimized.

Copy link

commented Nov 21, 2015

same here, it is breaking

@hutson

This comment has been minimized.

Copy link

commented Nov 21, 2015

@lundyio and @binarykitchen based on this commit ( 3e7ad84 ) it looks like the intent was to emphasize that any JSHint-compatible tool could be used with gulp-jshint just by installing that tool.

So, using gulp-jshint, you can install jshint if you want to use that linter, or jsxhint if you would prefer to use that linter. (Documented under options https://github.com/spalger/gulp-jshint#options )

Though, I think I would have preferred a dedicated gulp-jsxhint than the extra installation requirements for gulp-jshint.

@koolkt

This comment has been minimized.

Copy link

commented Nov 23, 2015

Same here, it tells me npm WARN EPEERINVALID gulp-jshint@2.0.0 requires a peer of jshint@2.x but none was installed. But jshint is already installed.

@thelostspore

This comment has been minimized.

Copy link

commented Nov 23, 2015

+1

2 similar comments
@mircoc

This comment has been minimized.

Copy link

commented Nov 24, 2015

+1

@jkphl

This comment has been minimized.

Copy link

commented Nov 24, 2015

+1

jkphl added a commit to jkphl/gulp-svg-sprite that referenced this issue Nov 24, 2015

@Preen

This comment has been minimized.

Copy link

commented Nov 24, 2015

+1

1 similar comment
@baires

This comment has been minimized.

Copy link

commented Nov 24, 2015

👍

@hutson

This comment has been minimized.

Copy link

commented Nov 25, 2015

This might seem like a silly question, but what does everyone on this issue want?

I ask because the issue was filed as a question and not a request to make any change to this package.

The commit, 3e7ad84, does a pretty good job explaining the reasoning. Do people agree with the reasoning?

@jkphl

This comment has been minimized.

Copy link

commented Nov 25, 2015

@hbetts While the reasoning behind might be good, at least for me the change completely breaks everything. It simply doesn't work. I'm facing the same situation as @KKfo described. Neither a globally nor a locally installed jshint does satisfy gulp-jshint's dependency. Maybe I'm doing something wrong, maybe it's a problem with Node 5, all I can say is that it worked like a charm prior to this change. Now it doesn't anymore, and I don't see what I can do about.

@spalger

This comment has been minimized.

Copy link
Owner

commented Nov 29, 2015

TLDR jshint is a dependency of gulp-jshint, it is listed in the package.json. There is not one version of jshint that would make everyone happy. The purpose of semver is to prevent new versions from breaking your builds, please use it.


The reason that I felt it was necessary to take a different approach with the jshint dependency is because people are regularly confused as to why gulp-jshint doesn't update it's jshint dependency (#46, #102, #122). Jshint has also demonstrated in the past that it is pretty liberal with semver (see jshint 2.5). Then jshint published a release candidate to npm as it's latest version so gulp-jshint users were unknowingly installing a broken/pre-release version of jshint.

I feel like peerDependencies (especially the way that they are handled by npm3) helps mitigate the pain of all of these issues by putting everyone in control of their own jshint.

It was clear that a good number of people are already using npm 3 with node 5, so in order to prevent their builds from breaking I bumped the version to 2.0. That is what major version numbers are for. If you are automatically installing major version bumps then you are asking for trouble.

As far as I can tell there is no issue installing peerDependencies, so please run the install command at the top of the readme if you are having a hard time getting gulp-jshint to install.

image

@spalger spalger closed this Nov 29, 2015

@jkphl

This comment has been minimized.

Copy link

commented Nov 29, 2015

@spalger Thanks for the explanation. Just to give you a short feedback: While I did understand all this before, it was the

npm install --save-dev jshint gulp-jshint

(taken from your screenshot) that made everything work now. This installed jshint 2.8.0 locally, obviously — although the very same version was already present globally (but didn't suffice gulp-jshint's needs). I think this is what was troubling people (including me).

Thanks for helping in the end, though. ;)

@Flobin

This comment has been minimized.

Copy link

commented Nov 30, 2015

I still have this issue, even after installing jshint:

(projenv) MacBook-Pro:posts robin$ gulp  
[21:57:21] Using gulpfile ~/learning/python/parrot/posts/static/posts/gulpfile.js
[21:57:21] Starting 'clean'...
[21:57:21] Finished 'clean' after 4.86 ms
[21:57:21] Starting 'default'...
[21:57:21] Starting 'styles'...
[21:57:21] Starting 'scripts'...
ERROR: Can't find config file: .jshintrc
(projenv) MacBook-Pro:posts robin$ sudo npm install jshint --save-dev
npm WARN gulp-jshint@2.0.0 requires a peer of jshint@2.x but none was installed.
(projenv) MacBook-Pro:posts robin$

Setting jshint to 2.8.0 and installing that fixed the issue.

@baires

This comment has been minimized.

Copy link

commented Nov 30, 2015

@Flobin
rm -rf node_modules and npm install

@Flobin

This comment has been minimized.

Copy link

commented Dec 1, 2015

@baires yep, I did that. It still tried to install 2.9.1-rc1, so when I removed node_modules, set it to 2.8.0, and reinstalled, it worked fine.

@RehanSaeed RehanSaeed referenced this issue Dec 8, 2015
@Chris-May

This comment has been minimized.

Copy link

commented Dec 8, 2015

@Flobin Thanks for the tip to downgrade jshint to 2.8.0. That's what got it "working" for me too.

@rshah9

This comment has been minimized.

Copy link

commented Feb 4, 2016

npm install --save-dev jshint@2.8.0 this worked for me.

@ihsanberahim

This comment has been minimized.

Copy link

commented Feb 19, 2016

Thx @rshah9 . that fix work for me.

@codemcu

This comment has been minimized.

Copy link

commented Mar 6, 2016

I'm starting to work with Gulp and was stuck with this error, thank you!!!! :)

@dylanh724

This comment has been minimized.

Copy link

commented Sep 17, 2016

The above seems to be a workaround, not a solution, unfortunately. Would love to see a more fluent install.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.