ESLint v4.0.0 breaks eslint-config-eyeo

[mjethani]

Environment

ESLint v4.0.0
adblockpluschrome repo at changeset 7aeb2fb83e11
eslint-config-eyeo 1.0.4

How to reproduce

1. Run the command eslint *.js lib/ qunit/ ext/ chrome/ from the repo

Observed behaviour

104 errors.

Expected behaviour

No errors.

Notes

It seems there have been major breaking changes in ESLint 4.0.0. Either we update the config and fix our code where appropriate, or we stick with the v3.x.x.

[mjethani]

I was able to fix many of the errors by adding the following to my .eslintrc.json:

"rules": {
  "indent": [
    "error", 2, {
      "SwitchCase": 1,
      "ArrayExpression": "first",
      "FunctionDeclaration": {"parameters": "first"},
      "FunctionExpression": {"parameters": "first"},
      "CallExpression": {"arguments": "off"},
      "MemberExpression": "off",
      "ObjectExpression": "first"
    }
  ],
  "no-multi-spaces": ["error", {"ignoreEOLComments": true}]
}

For the rest, I had to make changes to the code as there's no good way to disable those checks.

[mjethani]

In particular ESLint 4.0.0 doesn't like this:

const {Export1, Export2, Export3,
       Export4, Export5} = require("example");

We have to change it to one of the following two:

const {Export1, Export2, Export3, Export4, Export5} =
  require("example");

const {
  Export1, Export2, Export3,
  Export4, Export5
} = require("example");

Also it doesn't like this:

return value1 &&
       (flag ?
          value2 :
          value3);

We have to change it to:

return value1 &&
       (
         flag ?
           value2 :
           value3
       );

[mjethani]

As part of fixing this issue, we should probably update the value of "eslint" in "peerDependencies" to just "4". This will allow for only minor and patch level changes (i.e. ​non-breaking). With the latest version of NPM it's possible to install modules locally and ​run them easily from the command line.

[mjethani]

The workaround for development purposes for now is to use npm install -g eslint@3.19 eslint-config-eyeo to install version 3.19 of ESLint along with the config.

In addition, we should add "ObjectExpression": "first" to fix the one error that was introduced in adblockpluschrome with ​changeset 0259a924a1af.

[sebastian]

This seems like a regression bug in ESLint to me, in particular in those cases where there is no option we could add to our configuration in order to restore the previous behavior. We should file issues with upstream ESLint, and stick with ESLint 3.19 for the time being, IMO. I'd rather not perform random changes to our code, some of which might even degrade readability.

[mjethani]

I have filed two bugs in ESLint: ​#8977, ​#8979.

Meanwhile, we have the option of using ​indent-legacy if we want to upgrade to ESLint 4 without making any code changes. Alternatively, we could stick with ESLint 3.19. The third option, of course, is to update the configuration and make minor changes to the code.

[mjethani]

Well, it looks like the second bug is ​fixed in the ​next release, which happily is today. This leaves us with the option of configuration updates and ​one small change to our code.

Note that there is ​another change but that's a genuine error that we should fix.

[mjethani]

ESLint 4.3.0 is ​out.

I've confirmed that ObjectExpression (​#8979) is fixed in this release.

[abpbot]

A commit referencing this issue has landed:
​Issue 5429 - Upgrade to ESLint 4

[abpbot]

A commit referencing this issue has landed:
​Issue 5429 - Release eslint-config-eyeo 2.0.0

[kzar]

Since much of the team are away I have made the minimum configuration changes required to have ESLint 4 behave like ESLint 3 for now. (I've switched from indent to indent-legacy and added the ignoreEOLComments option to no-multi-spaces.)

I've gone with Manish's suggestion of specifying ESLint 4 now, so when ESLint 5 is released we won't be upgraded accidentally. I've also gone with his suggestion of updating the eslint-config-eyeo version to 2.0.0 since the indent-legacy option is not supported for older version of ESLint and so this is a breaking change.

I will now file an issue where we can continue the discussion about indentation for our ESLint configuration and our coding style guide.

[mjethani]

Confirmed, this works on adblockpluschrome, adblockplusui, and adblockpluscore master with eslint-config-eyeo 2.0.0 and ESLint 4.

[kzar]

I've created #5449 for the discussion and further work required regarding indentation.