Problem/Motivation
With the release of eslint 1.0.0, the .eslintrc file in Drupal core no longer works.
$ eslint .
/usr/local/lib/node_modules/eslint/lib/config-validator.js:88
throw new Error(message.join(""));
^
Error: ~/Sites/drupal/8.0.x/.eslintrc:
Configuration for rule "indent" is invalid:
Value "data["2"].indentSwitchCase" has additional properties.
Proposed resolution
With eslint 0.24.1, we were inheriting a bunch of rules that were enabled by default; that was good .eslintrc design because we didn't need to duplicate those rules in our file. Unfortunately, most of those rules are now disabled by default in 1.0.0.
That means in order to create a 1.0.0-compatible .eslintrc file that has linting parity with our existing .eslintrc file and eslint 0.24.1, we have to add all the rules that used to be enabled by default and are now disabled by default.
We can figure out this new config by following these steps:
- Add all the missing default enabled rules from eslint 0.24.1. (We will pair down the list in later steps.) The default rule configuration in eslint 0.24.1 can be seen in this JSON file: https://github.com/eslint/eslint/blob/v0.24.1/conf/eslint.json
- Eslint 1.0.0 comes with a new config initialization wizard that does a very basic setup. We should add the two rules from eslint 1.0.0's `--init` command option:
"extends": "eslint:recommended"
and"linebreak-style": [2, "unix"]
.
- Replace deprecated 0.24.1 rules with updated 1.0.0 ones. Removed rules are listed on the eslint docs page: http://eslint.org/docs/rules/#removed
- global-strict -> strict
- indent isn't deprecated, but has new syntax
- no-extra-strict -> strict
- no-reserved-keys -> quote-props
- no-wrap-func -> no-extra-parens
- space-in-brackets -> array-bracket-spacing, computed-property-spacing, and object-curly-spacing
- spaced-line-comment -> spaced-comment
- Fix lint errors from new quote-props rule (which replaced the no-reserved-keys rule we used).
- Remove disabled rules that are disabled by default in 1.0.0. Default rule configuration in eslint 1.0.0 can be seen in this JSON file: https://github.com/eslint/eslint/blob/v1.0.0/conf/eslint.json
- Remove enabled rules that are enabled by default in 1.0.0.
- Tighten no-unused-vars rule, since Drupal already passes this lint test. (Arguably scope-creep, but it's a one word change.)
Comment | File | Size | Author |
---|---|---|---|
#8 | eslintrc1-2545826-7.patch | 28.78 KB | JohnAlbin |
Comments
Comment #2
JohnAlbinI'm working on a patch for this.
Comment #3
nod_had a patch laying around for this. Let's see if we end up with the same thing.
Comment #4
JohnAlbinVery close!
The big problem is that the default enabled rules in 0.24.1 ( https://github.com/eslint/eslint/blob/v0.24.1/conf/eslint.json ) are very different from the ones in 1.0.0 ( https://github.com/eslint/eslint/blob/v1.0.0/conf/eslint.json ).
So we need to:
Now that I've typed this out, I realize my patch was starting from step #2, so I'll have to start over. But here it is anyway.
Comment #5
JohnAlbinHere's step #1 completed.
Comment #6
JohnAlbinHere's steps 1-2 completed.
Comment #7
nod_Starting to seem like scope creep, there are rules we weren't using before no? I don't have a problem with stricter rules but we should limit the patch to changing .eslintrc, otherwise it's another discussion.
And if we're being explicit we shouldn't need
+ "extends": "eslint:recommended",
.Comment #8
JohnAlbinAnd… here's steps 1-4 completed.
Comment #9
JohnAlbinWe were using them before. They were enabled by default in eslint 0.24.1, so we didn't have them in our .eslintrc file. But in eslint 1.0.0, those same rules are now disabled by default. So we need explicitly add the ones from 0.24.1 that we were inheriting.
We're not being explicit with 1.0.0's default rules. In fact, I removed all the rules from our .eslintrc file that are the same as 1.0.0's defaults.
So, to reiterate, we are:
Comment #10
JohnAlbinI've updated the issue description with the minimal steps needed to reproduce what eslint 0.24.1 + our existing .eslintrc file was doing.
Please double check I don't have any logic flaws there.
Comment #11
JohnAlbinComment #12
klausiThanks, using this eslint config now for pareview.sh: https://www.drupal.org/project/pareviewsh
Works for me!
I don't feel competent to RTBC this, so leaving at needs review.
Comment #13
nod_All right, read carefully the IS and verified the added configuration options. Their default values changed in the new version of eslint and it does mean we need to explicitly set them in our file.
Agreed with the change from "local" to "all" for no-unused-vars (there is no JS code change needed for this one).
Agreed with using the improved quote-props rule with the proposed config (and agreed with all the JS file changes).
Thanks for doing going through that, I do recognize my patch was lazy compared to yours.
Comment #14
alexpottCommitted fad555c and pushed to 8.0.x. Thanks!