lint JS with ESlint #951
lint JS with ESlint #951
Conversation
.eslintrc.json
Outdated
| "keyword-spacing": "error", | ||
| "linebreak-style": ["error", "unix"], | ||
| "no-multiple-empty-lines": "error", | ||
| "no-plusplus": "error", |
There was a problem hiding this comment.
I find this rule weird but that is probably just a habit so I do not oppose it. In either case i += 1 would be preferred replacement to i = i + 1.
gruntfile.js
Outdated
| @@ -157,4 +162,5 @@ module.exports = function(grunt) { | |||
| grunt.registerTask('default', ['install', 'versionupdater', 'compress']); | |||
| grunt.registerTask('version', ['versionupdater']); | |||
| grunt.registerTask('zip', ['compress']); | |||
| grunt.registerTask('lint_js', 'Check JS syntax', ['eslint']); | |||
There was a problem hiding this comment.
Few lines above, we use client:install. Not sure if it is recommended practice but we should probably be consistent.
There was a problem hiding this comment.
Do you mean I should rename the task client:lint?
There was a problem hiding this comment.
Maybe, I am not sure if the convention is correct.
package.json
Outdated
| "grunt-composer": "^0.4.5", | ||
| "grunt-contrib-compress": "^0.10.0", | ||
| "grunt-eslint": "", |
There was a problem hiding this comment.
Why is there an empty constraint?
| { | ||
| "root": true, | ||
| "extends": "eslint:recommended", | ||
| "rules": { |
There was a problem hiding this comment.
Going through the list, I would also add at least these rules:
{
"brace-style": "error",
"camelcase": "error",
"curly": "error",
"eol-last": "error",
"no-use-before-define": "error",
"quotes": ["error", "single"],
"unicode-bom": "error"
}
gruntfile.js
Outdated
| @@ -157,4 +162,5 @@ module.exports = function(grunt) { | |||
| grunt.registerTask('default', ['install', 'versionupdater', 'compress']); | |||
| grunt.registerTask('version', ['versionupdater']); | |||
| grunt.registerTask('zip', ['compress']); | |||
| grunt.registerTask('client:lint', 'Check JS syntax', ['eslint']); | |||
There was a problem hiding this comment.
Apparently, the convention is task:target, that means lint:client.
https://gruntjs.com/configuring-tasks#task-configuration-and-targets
public/js/selfoss-base.js
Outdated
| @@ -109,16 +109,17 @@ var selfoss = { | |||
| */ | |||
| getValues: function(element) { | |||
| var values = {}; | |||
|
|
|||
| $(element).find(':input').each(function (i, el) { | |||
There was a problem hiding this comment.
Add "space-before-function-paren": ["error", "never"]
public/js/selfoss-base.js
Outdated
| @@ -128,7 +129,7 @@ var selfoss = { | |||
|
|
|||
| setSession: function() { | |||
| Cookies.set('onlineSession', 'true', {expires: 10, | |||
There was a problem hiding this comment.
Add "object-curly-newline": ["error", {"multiline": true, "consistent": true}]
public/js/selfoss-base.js
Outdated
| @@ -202,46 +204,49 @@ var selfoss = { | |||
| showErrors: function(form, errors) { | |||
| $(form).find('span.error').remove(); | |||
| $.each(errors, function(key, val) { | |||
| form.find("[name='"+key+"']").addClass('error').parent('li').append('<span class="error">'+val+'</span>'); | |||
| form.find('[name=\'' + key + '\']').addClass('error').parent('li').append('<span class="error">' + val + '</span>'); | |||
There was a problem hiding this comment.
You can use double quotes instead of the escaped single quotes.
public/js/selfoss-events-entries.js
Outdated
|
|
||
| if (selfoss.isSmartphone()) { | ||
| $('.entry-icon, .entry-datetime').unbind('click').click(function(e) { | ||
| e.preventDefault(); return false; |
There was a problem hiding this comment.
Add max-statements-per-line: ["error", { "max": 1 }]
| var tagsCountEl = $('#nav-tags > li > span.tag').filter(function(i){ | ||
| return $(this).html()==tag; } | ||
|
|
||
| var tagsCountEl = $('#nav-tags > li > span.tag').filter(function(){ |
There was a problem hiding this comment.
Add "space-before-blocks": "error"
|
|
||
| var tagsCountEl = $('#nav-tags > li > span.tag').filter(function(){ | ||
| return $(this).html() == tag; | ||
| } | ||
| ).next(); |
There was a problem hiding this comment.
This should be on the same line as the closing bracket, though I could not find an eslint rule.
| } | ||
|
|
||
| if (unread) { | ||
| unreadstats = unreadstats - 1; |
There was a problem hiding this comment.
Do we still do the explicit (inc|dec)rements?
public/js/selfoss-events-sources.js
Outdated
| // hide input form | ||
| parent.find('.source-edit-form').hide(); | ||
|
|
||
| // update title | ||
| var title = $('<p>').html(response.title).text(); | ||
| parent.find('.source-title').text(title); | ||
| parent.find("input[name='title']").val(title) | ||
| parent.find('input[name=\'title\']').val(title); |
There was a problem hiding this comment.
Replace the escaped single quotes with double quotes.
No description provided.