Ckeditor5: Switch to ESLint

[Reinmar]

Part of https://github.com/ckeditor/ckeditor5/issues/366.

Requires: https://github.com/ckeditor/ckeditor5-dev/issues/191.

[Reinmar]

I pushed t/953 with a prototype.

It requires switching ckeditor5-dev to t/191 and running npm run switch-to-dev-dev in ckeditor5.

[pomek]

When I called gulp lint on a branch t/953 in this repository I have an error:

Corrected gulpfile.js for the future:

/**
 * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
 * For licensing, see LICENSE.md.
 */

'use strict';

const gulp = require( 'gulp' );
const lintConfig = {
    // Files ignored by `gulp lint` task.
    // Files from .gitignore will be added automatically during task execution.
    ignoredFiles: [
        'src/lib/**'
    ]
};

const ckeditor5Lint = require( '@ckeditor/ckeditor5-dev-lint' );

gulp.task( 'lint', () => ckeditor5Lint.lint( lintConfig ) );
gulp.task( 'lint-staged', () => ckeditor5Lint.lintStaged( lintConfig ) );
gulp.task( 'pre-commit', [ 'lint-staged' ] );

[Reinmar]

I have committed changes to ckeditor5 (symlinking) just a couple of mins ago. The same with ckeditor5-engine. Please try again.

[Reinmar]

The gulpfile.js is already updated. Config too. package.json too.

[pomek]

Now it works fine. Thx for speed helping :D

[pomek]

See you next year :D

[Reinmar]

Run eslint --fix src test and you'll have only 300 issues to fix manually. Make sure to review the automatic fixes and commit that separately.

[ma2ciek]

@pomek I had a similar situation when I created a validator for jsdoc output :smile:

[Reinmar]

I've been reviewing your changes, @pomek, in t/953 and just like I commented in ckeditor5-dev, issues should be fixed not muted. So:

• no-invalid-this should be disabled. It doesn't make sense if we're any call/appy() or assigning functions to objects.
• no-confusing-arrow should not be muted but the code should be cleaned.
• one-var should never need to be muted. I don't understand why you ever needed to mute it because lines like https://github.com/ckeditor/ckeditor5-engine/commit/900cc698c2bbad5a2690bdb00831458ff9aa62f5#diff-9678cc8d16efd8d3fd875965f70835dcR554 look totally valid – what's wrong there? Was ESLint seriously complaining?

[Reinmar]

The rule of thumb when aligning code to the linter is that it should not require breaking the code and it should not require muting the linter too often.

If such cases happen we need to reconsider a given rule. Linter must be a help, not an enemy.

[pomek]

Short update:

• no-invalid-this - we disabled this rule (https://github.com/ckeditor/ckeditor5-dev/pull/192/commits/a69bbc0e7bbb0d2d8a0795c54f376a48c8797239)
• no-confusing-arrow - I simplified the functions and the error does not occur
• one-var - fixed

[pomek]

@Reinmar, could you review my changes once again?

[Reinmar]

Could you make a PR from this branch?

[pomek]

Will do it.

[pomek]

@Reinmar, https://github.com/ckeditor/ckeditor5-engine/pull/956.