-
Notifications
You must be signed in to change notification settings - Fork 21
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
Comments
I like the idea of adding /**
* Clone the distribution repo
*/ where the space is used to align a JSDoc-style comment properly. Thoughts about those rules, @jquery/core? |
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 |
@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. |
@mgol yeah, just like jquery. Another reason to make it up to date, at least with coding standards ;) |
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. |
You're right. Good luck. |
@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? |
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
PR: #16 |
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
Closes gh-4696 Ref jquery/eslint-config-jquery#15 Ref jquery/eslint-config-jquery#16 (cherry picked from commit ef4d6ca)
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.The text was updated successfully, but these errors were encountered: