How use fix and cache flags?

[ekaragodin]

ESLint 1.4 introduce new command line arguments: --fix, --cache. http://eslint.org/blog/2015/09/eslint-v1.4.0-released/
How use it with gulp-eslint?

[adametry]

The --cache/cache-file flag isn't likely be implemented in gulp-eslint since it packages like gulp-newer or gulp-changed already provide similar control for gulp file streams.

I've been looking into support for the --fix flag, and should have an update/beta soon. This will likely transform the virtual file and not the original source file directly; however, the "fixed" source may be piped to gulp.dest later in the stream. I'll update this issue as this concept evolves.

[oliviertassinari]

If this might help, this is how fix support was added to grunt-eslint sindresorhus/grunt-eslint#87.

[bekos]

@adametry --cache flag will skip over any files that had no problems in the previous run unless they have been modified. In contrast, gulp plugins will skip all unchanged files, even if they had problems. No? So implementing also cache flag is equally important :-)

[seiyria]

I definitely agree that implementing the CLI options for eslint are important. Also, much easier to configure and understand than gulp-newer and gulp-changed.

[hieutdo]

Agree with @seiyria as I believe that ESLint's caching mechanism is better than gulp-newer and gulp-changed.

[monzonj]

If you implement the --fix flag, it should indeed modify the sources. If anyone is going to use that flag they want the sources to be modified, so they are all nice and compliant. What's the use of fixing the virtual files? We might as well just remove the rules

[seiyria]

@adametry I've spent some time today trying to implement gulp-newer or gulp-changed in combination with gulp-eslint but I could not figure something out. How exactly does one do that?

[seiyria]

Seems for this change to take effect, all that needs to be done is to use executeOnFile instead of executeOnText (ref). Otherwise, as it stands, the CLI engine already receives the cache flag if you pass it into gulp-eslint().

[mradionov]

+1 for --fix support. Mostly agree with @monzonj . Not sure if piping fixed files is a good idea though. It makes sense from the Gulp point of view, when you can modify a file and then continue processing it. One of the problems is that in current implementation you already have a pipe for results. Do you want to make a second pipe for fixed files? I guess it won't be easy to use in a gulp-way. Moreover, I do not think one will need to pipe fixed files, developers wants to fix what they already have: I am not going to keep bad code in sources and gulp build+push clean and pretty code for whatever reason. I do not consider it a build tool, which should be piped with something else, it's an utility for developer process likely included into watch task or pre-commit hook.

[adametry]

I appreciate the feedback and perspective, and I see how gulp-changed and gulp-newer aren't up to par.

@seiyria, you've uncovered part of the issue I have with the --cache flag. It's only used by Eslint when processing files directly from the file system. Another concern is that gulp-eslint doesn't control where files are read from. It merely adapts the gulp file stream to eslint's CLIEngine.

While gulp-changed and gulp-newer were insufficient, gulp-cached lands much closer. Consider the following example:

var cache = require('gulp-cached');
var eslint = require('gulp-eslint');
var ifFile = require('gulp-if');
var through2 = require('through2');

function isLinty(file) {
    // check if a file has any errors or warnings
    return file.eslint != null
        && (file.eslint.warningCount > 0 || file.eslint.errorCount > 0);
}

function uncache(cacheName) {
    // create a stream that removes files from cache
    return through2.obj(function (file, enc, done) {
        if (gulpCached.caches[cacheName]) {
            delete gulpCached.caches[cacheName][file.path];
        }
        done(null, file);
    });
}

gulp.task('lint', function () {
    // Read all js files within ./src
    return gulp.src('src/**/*.js')
        .pipe(cache('lint-cache'))
        // Only uncached and changed files past this point
        .pipe(eslint())
        .pipe(eslint.format())
        // If a file has errors/warnings ("linty") uncache it
        .pipe(ifFile(isLinty, uncache('lint-cache')));
});

gulp.task('lint-watch', function () {
    // Any file that changes within ./src will trigger the linting task
    return gulp.watch('src/**/*.js', ['lint']);
});

[adametry]

Regarding the "fix" option, I'm indeed on board with it. In fact, you can try it out in the release candidate currently available (npm install gulp-eslint@rc).

I spent some time considering and vetting the recommendations (including a PR #109). Ultimately, I found more flexibility and stability in applying the fix like a transform. This approach allows Eslint to perform the fix at the time it filters fixable messages, yet leaves it up to the task author whether the file should be transformed, deployed, updated in place, or any other option.

Fortunately, the "fix" feature makes Gulp + Eslint an even better combo! Eslint is now a rule-based linting and transformation tool, and Gulp rocks at facilitating transformation tasks (note: I didn't say "builds").

Applying eslint fixes isn't much of a change:

var gulp = require('gulp');
var eslint = require('gulp-eslint');
gulp.task('lint-fix', function () {
    return gulp.src('js/**/*.js')
        .pipe(eslint({ fix: true }))
        .pipe(eslint.format())
        .pipe(gulp.dest('.'));// <-- update original files
});

If that's too much file-saving for you, here's a version that writes only writes files that Eslint fixed, using gulp-if:

var gulp = require('gulp');
var eslint = require('gulp-eslint');
var gulpIf = require('gulp-if');
function isFixed(file) {
    return file.eslint && typeof file.eslint.output === 'string';
}
gulp.task('lint-fix', function () {
    return gulp.src('js/**/*.js')
        .pipe(eslint({ fix: true }))
        .pipe(eslint.format())
        .pipe(gulpIf(isFixed, gulp.dest('./')));
});

How about something completely different? Sure! Let's try a dev server that lints and fixes files at request time:

var gulp = require('gulp');
var eslint = require('gulp-eslint');
var through2 = require('through2');
var express = require('express');
var localDev = express();
localDev.get('/scripts/index', function (req, res, next) {
    gulp.src('src/index.js')
        .pipe(eslint({fix: true}))
        .pipe(eslint.formatEach())
        .pipe(through2.obj(function (file, enc, done) {
            if (file.eslint && file.eslint.errorCount > 0) {
                res.status(500);
            }
            res.header('Content-Type', 'text/javascript; charset=UTF-8');
            file.pipe(res);
            done(null, file);
        }));
});

Ok... that last one was nuts, but the point is that there's flexibility in how you use it.

Anyway, take the release candidate for a test spin and report back.

npm install gulp-eslint@rc

[seiyria]

@adametry Thanks. For now, I'll just use that.

[adametry]

[email protected] has been released, and support for the "fix" option along with it. An example of using gulp-cached is included in the example directory. I'm closing this bug since this release addresses both topics. If further discussion is needed regarding a "cache" option, create a new issue specifically to that feature.