Update parser options

[Lysholm]

Latest eslint version issues a deprecation warning for "parserOptions.ecmaFeatures.experimentalObjectRestSpread"
Removed it and changed ecmaVersion to 2018 as suggested on https://eslint.org/docs/user-guide/configuring#deprecated

[coveralls]

Coverage decreased (-1.0%) to 97.184% when pulling 4155458 on Lysholm:patch-1 into f0d0c4f on benmosher:master.

[coveralls]

Coverage remained the same at 98.177% when pulling 8eb6815 on Lysholm:patch-1 into f0d0c4f on benmosher:master.

[ljharb]

The latest version is v4; you might be referring to the v5 alpha, and this warning still isn't set in stone there.

This plugin needs to retain compatibility with older eslint versions, which might not support this ecmaVersion, so I don't think this is a change we should make.

[Lysholm]

@ljharb You are right the warning is only present on the alpha channel, i was confused since the option was deprecated as of the documentation starting in eslint 4.18.0.

Espree 3.5 added the support for es2018 and was introduced in eslint 4.4, so eslint versions prior to that would probably break. That makes this a breaking change.

Still think it should be done though, but maybe hold off until eslint 5 is released and add a recommended-legacy config file and update docs to reflect.

[Lysholm]

@ljharb At second look, it doesn't look like this object rest spread config is done at all in the documented way of setting up eslint-plugin-import (import/error + import/warning prebaked configs). Should it even be there at all? It's atleast a point of confusion that recommended is equal to error + warning except for this one thing.

[ljharb]

As the comment says, it's needed for parsing dependencies.

I don't think it's worth a breaking change, I think we need to find a backwards compatible way to do it.

[Lysholm]

@ljharb: I think there's something i don't understand here. Is the recommended config not meant to be used?

Most people probably set this up copying the settings off the readme:

extends: [
        "eslint:recommended",
        "plugin:import/warnings",
        "plugin:import/errors"
]

That seems to work and doesn't end up setting the parser options at all so this is a non issue for everyone who read the readme and not blindly configured from pattern matching setup of other plugins. ;)

Is there some special use case for

extends: [
    "eslint:recommended",
    "plugin:import/recommended"
]

or is the difference accidental?

[ljharb]

@benmosher, any context?

[benmosher]

Honestly, it's been ages since I've looked at that file. I am not sure if that block is even needed. I think it may be a vestige from a time when plugins didn't have access to parser or options. This particular block actually seems to stem from #416.

I would probably move to just drop that whole parserOptions bit, feels like an over-reach at this point to include it. is that a semver-major event, though?

[Lysholm]

Should be a semver-patch event. I tested eslint-plugin-import 2.11.0 against eslint 1.10.3 (the latest version without the changes you referred to) and it's not supported. So no semver-major event.

[ljharb]

So someone without those parser settings, but who uses the recommended config, but whose code relies on the parser settings - what happens to them with this change?

[benmosher]

I honestly only vaguely remember. I want to say that I was just trying to be helpful and cover bases because I ran into a situation where the linted code didn't use spread, but some ES-module dependency did.

So really, only that exact situation. Is a breaking change, I think, technically, but also feels like a pretty niche case.

unblocking. i think the change is fine, but it’s technically semver-major.

[Lysholm]

@ljharb Good catch, it would break for them so semver-major then.

[ljharb]

It seems worth holding off on a semver-major release, however, at least until eslint 5 is out - we can drop support for eslint v2 at that time - so perhaps we should wait to merge this until then (and right after the last non-major release prior to v3 is published)?

[Lysholm]

That makes sense to me