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:

  1. 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
  2. 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"].
  3. 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
  4. Fix lint errors from new quote-props rule (which replaced the no-reserved-keys rule we used).
  5. 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
  6. Remove enabled rules that are enabled by default in 1.0.0.
  7. Tighten no-unused-vars rule, since Drupal already passes this lint test. (Arguably scope-creep, but it's a one word change.)
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

JohnAlbin created an issue. See original summary.

JohnAlbin’s picture

I'm working on a patch for this.

nod_’s picture

had a patch laying around for this. Let's see if we end up with the same thing.

JohnAlbin’s picture

FileSize
1.9 KB

Very 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:

  1. Find any enabled rules in 0.24.1 and see if they are missing from our existing .eslintrc file and add them. (So that we have a full list of all rules that we were running with eslint 0.24.1.
  2. Find any deprecated rules in our .eslintrc file and replace them with the new rules in 1.0.0.
  3. Find any disabled rules in our old .eslintrc file that are disabled by default in 1.0.0 and remove them.
  4. Find any enabled rules in our old .eslintrc file that are enabled by default in 1.0.0 and remove them.

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.

JohnAlbin’s picture

Status: Active » Needs work
FileSize
3.09 KB

Here's step #1 completed.

JohnAlbin’s picture

FileSize
20.25 KB

Here's steps 1-2 completed.

nod_’s picture

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",.

JohnAlbin’s picture

Status: Needs work » Needs review
FileSize
28.78 KB

And… here's steps 1-4 completed.

JohnAlbin’s picture

Starting to seem like scope creep, there are rules we weren't using before no?

We 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.

And if we're being explicit we shouldn't need + "extends": "eslint:recommended",.

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:

  • adding the rules from 0.24.1 that were enabled by default, but are no longer enabled by default in 1.0.0
  • removing enabled rules that are enabled by default in 1.0.0.
  • removing disabled rules that are disabled by default in 1.0.0.
JohnAlbin’s picture

Issue summary: View changes

I'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.

JohnAlbin’s picture

Issue summary: View changes
klausi’s picture

Thanks, 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.

nod_’s picture

Status: Needs review » Reviewed & tested by the community

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.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed fad555c and pushed to 8.0.x. Thanks!

  • alexpott committed fad555c on 8.0.x
    Issue #2545826 by JohnAlbin, nod_: Update.eslintrc to work with eslint 1...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.