Skip to content

Enable some eslint:recommended rules #15

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

Closed
edg2s opened this issue Oct 17, 2019 · 8 comments · Fixed by #16
Closed

Enable some eslint:recommended rules #15

edg2s opened this issue Oct 17, 2019 · 8 comments · Fixed by #16

Comments

@edg2s
Copy link

edg2s commented Oct 17, 2019

It would be nice to extend eslint:recommended, but to being with we should enable:

  • no-irregular-whitespace - triggers once in jquery/jquery, a valid error on 103:72 of build/release.dist.js (tab instead of space)
  • no-extra-semi - triggers in nodeName.js and isAutoPx.js. These extra semi colons can be removed.
@mgol
Copy link
Member

mgol commented Oct 19, 2019

I like the idea of adding no-extra-semi but I'm not sure of no-irregular-whitespace. As far as I understand, it detects spaces after tabs in an example like:

	/**
	 * Clone the distribution repo
	 */

where the space is used to align a JSDoc-style comment properly.

Thoughts about those rules, @jquery/core?

@SharakPL
Copy link

I've used eslint default config for my jquery projects so far and with this jquery config every .js file is filled with errors. I feel like authors of this config try to make it different from eslint:recommended in every possible way and it's really not a good idea. I believe it should only differ in jquery specific rules.

@mgol
Copy link
Member

mgol commented Oct 22, 2019

@SharakPL You can't use a default ESLint config for all projects that may have their own configs. If your ESLint does that, it's configured incorrectly.

Also, the jQuery code style is way older than the ESLint recommended preset and it's even a few years (!) older than ESLint itself.

@SharakPL
Copy link

@mgol yeah, just like jquery. Another reason to make it up to date, at least with coding standards ;)

@mgol
Copy link
Member

mgol commented Oct 22, 2019

Code formatting rules are a question of personal taste and we don't want to changed them just for the sake of changing; that would generate big diffs & would make it harder to track when particular changes were introduced.

Let's focus on the proposed changes to the config here.

@SharakPL
Copy link

You're right. Good luck.

@mgol
Copy link
Member

mgol commented Oct 28, 2019

@edg2s We decided we want both rules enabled (my initial comment was incorrect as those JSDoc rules are exempt from the check already). Would you like to submit a PR?

mgol added a commit to mgol/eslint-config-jquery that referenced this issue Apr 8, 2020
The no-extra-semi rule:
https://eslint.org/docs/rules/no-extra-semi
disallows unnecessary semicolons, e.g. it forbids the following:
```js
var x = 5;;

function foo() {
    // code
};
```

In jQuery we usually don't add a semicolon at the end of function declarations
but I found a few cases where we do, creating an inconsistency.

The no-irregular-whitespace rule:
https://eslint.org/docs/rules/no-irregular-whitespace
forbids most whitespaces that may be hard to notice in source. We have one such
occurrence in Core code that would be good to forbid.

Fixes jquery#15
@mgol
Copy link
Member

mgol commented Apr 8, 2020

PR: #16

@mgol mgol closed this as completed in #16 Apr 20, 2020
mgol added a commit that referenced this issue Apr 20, 2020
The no-extra-semi rule:
eslint.org/docs/rules/no-extra-semi
disallows unnecessary semicolons, e.g. it forbids the following:
```js
var x = 5;;

function foo() {
    // code
};
```

In jQuery we usually don't add a semicolon at the end of function declarations
but I found a few cases where we do, creating an inconsistency.

The no-irregular-whitespace rule:
eslint.org/docs/rules/no-irregular-whitespace
forbids most whitespaces that may be hard to notice in source. We have one such
occurrence in Core code that would be good to forbid.

Fixes #15
Closes #16
mgol added a commit to jquery/jquery that referenced this issue May 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants