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

False Positives with gulp-load-plugins && gulp-jshint #18

Closed
staxmanade opened this issue Dec 17, 2014 · 12 comments
Closed

False Positives with gulp-load-plugins && gulp-jshint #18

staxmanade opened this issue Dec 17, 2014 · 12 comments
Labels
Milestone

Comments

@staxmanade
Copy link

Hello,

Was looking around at how to determine un-used npm packages and found your project.

When I ran it the following report was generated. However, due to the way I'm loading gulp plugins (using gulp-load-plugins, my guess is this tool is not picking up anything that is dynamically loaded. Similarly, I'm asking it to use the jshint-stylish reporter for my jshint, which I presume is another dynamically loaded thing...

Unused Dependencies
* gulp-debug

Unused devDependencies
* gulp-markdown
* gulp-mocha
* gulp-plumber
* gulp-jshint
* gulp-jscs
* jshint-stylish
* gulp-istanbul

I can't say I have any bright ideas on how you can solve these problems, but in case you hadn't heard of them, they might be interesting ones to look into.

@SgtPooki
Copy link

Can confirm. I am using a yeoman project along with the common

$ = require('gulp-load-plugins')();

syntax, and each of those gulp plugins are firing as unused deps. I may not be using all of them, but I am using more than enough to know that this is breaking.

To reproduce, use the https://github.com/yeoman/generator-gulp-webapp, start a new project, and simply run depcheck on it. It should fire even though many (if not all) of the gulp-plugin packages are being used.

@rumpl
Copy link
Member

rumpl commented Jan 27, 2015

Hey guys, thanks for the interest in depcheck :)

Internally, depcheck handles this kind of things with its ignoreMatches option but I couldn't find a way to put it on the command line.

I started a new repo for a grunt plugin, as soon as it's released I will start working on a gulp plugin too, with these plugins you will be able to set the ignoreMatches option.

At one time I was thinking about an rc file, something like .depcheckrc where you could put all the options you want. What do you guys thinks about that ?

@rumpl rumpl closed this as completed Jan 27, 2015
@rumpl rumpl reopened this Jan 27, 2015
@SgtPooki
Copy link

SgtPooki commented Mar 1, 2015

@rumpl I think rc files are good, but I also think building a command line tool that can be passed all the options is the most versatile and future proof solution. If you add a .depcheckrc file that would be nice solution for some I am sure. I am starting to dislike rc files, because the last project I inherited had about 10 rc files in the project root.

dylang pushed a commit to dylang/depcheck that referenced this issue Oct 14, 2015
@lijunle
Copy link
Member

lijunle commented Nov 18, 2015

Hi, all.

I am taking care of this project. Could anybody tell me if he still has this problem with version 0.5.9? Is it worth to worth on this?

@lijunle
Copy link
Member

lijunle commented Jan 28, 2016

I looked into this problem. The false alerts are caused by gulp-load-plugins loading dependencies with its own way.

@eckdanny
Copy link

@lijunle I don't think dynamic loaders (e.g.; gulp-load-plugins, require-dir, ...) should be in scope for depcheck. Includes silliness like:

var pkg = 'f' + 'o' + 'o';
var foo = require(pkg);

Maybe just me, but I don't expect this behavior to be supported in a tool like this.

@eckdanny
Copy link

@staxmanade re jshint-stylish: sol'n that made most sense to me was to explitize dependency with a require:

var gulp = require('gulp');
var jshint = require('gulp-jshint');
var styllish = require('jshint-stylish'); // <== 

gulp.task('default', function () {
   gulp.src(['file.js'])
      .pipe(jshint('.jshintrc'))
      // .pipe(jshint.reporter('jshint-stylish'));
      .pipe(jshint.reporter(stylish));
});

@lijunle
Copy link
Member

lijunle commented Feb 19, 2016

@eckdanny I understand that, it is very hard to detect all dynamic dependencies. However, our principle is to try to enable one more, and one more detector.

For this particular issue (gulp-load-plugins), there are three levels of supports:

From my estimate, the first feature will cover 80% user cases - although it will miss some unused dependencies. Besides the second feature, it will cover ~90% cases.

The first one should be easy, the second is hard - I will make a try. We won't consider the third feature until someone comes back and request a third feature.

@lijunle lijunle added this to the 0.6.1 milestone Feb 27, 2016
@lijunle lijunle modified the milestones: backlog, 0.6.1 Mar 10, 2016
@lijunle lijunle modified the milestones: 0.6.2, backlog Mar 21, 2016
@nfantone
Copy link

Came here for this.

depcheck on my project lists a lot of false positives (mostly, gulp related):

# nfantone at thelonious in ~/Development/js/2pv/floating-bar on git:bug/8106-yargs-missing ✖︎ [20:26:57]
→ depcheck
Unused devDependencies
* eslint
* gulp-autoprefixer
* gulp-changed
* gulp-clean-css
* gulp-concat
* gulp-debug
* gulp-declare
* gulp-eslint
* gulp-gzip
* gulp-handlebars
* gulp-htmlmin
* gulp-if
* gulp-iife
* gulp-imagemin
* gulp-inject
* gulp-install
* gulp-istanbul
* gulp-mocha
* gulp-plumber
* gulp-print
* gulp-replace-task
* gulp-rev
* gulp-rev-replace
* gulp-robots
* gulp-sequence
* gulp-sonar
* gulp-strip-debug
* gulp-tar
* gulp-uglify
* gulp-useref
* gulp-util
* gulp-webserver
* pre-commit
  • All gulp-* are used through gulp-load-plugins.
  • pre-commit has no explicit require and is used through a declaration in package.json itself, like:
  "pre-commit": [
    "validate"
  ]
  • eslint is used as the project's linter and installing locally avoids the need for a global dep and ensures equal versions all around.

@lijunle
Copy link
Member

lijunle commented Mar 24, 2016

Hi, @nfantone thanks for reporting this. Could you please open a issue to track you requirements.

Besides, for the gulp-load-plugins, I have ideas and actively develop it here. Hope it can resolve most of your cases. :)

@lijunle
Copy link
Member

lijunle commented Mar 27, 2016

Version 0.6.2 will be released soon to recognize the lazy loaded gulp plugins via gulp-load-plugins package. (Check this list)

From my offline testing, a yoeman generated web-app has 99% support. The only exception is babel-core, which I am not sure it is a false alert or really useless dependency.

I am closing this issue. If you get new false alerts, please open a new issue. Thanks for reporting!


Update: Version 0.6.2 is out.

@lijunle lijunle closed this as completed Mar 27, 2016
@nfantone
Copy link

Nice!

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

No branches or pull requests

6 participants