Resolve deprecation warning message from eslint while running eslint

[kabirbaidhya]

Issue

Running grunt eslint or ESLint CLI directly throws this deprecation warning since it was configured as per the older version of eslint:

(node:7146) DeprecationWarning: [eslint] The 'ecmaFeatures' config file property is deprecated, and has no effect. (found in /home/kabir/opensource/handlebars.js/.eslintrc.js)

Changes

I followed this Migrating to v2.0.0 to update the configuration as per the recommendation which fixed the deprecation message.

• The ecmaFeatures property is now under a top-level parserOptions property.
• Define "ecmaVersion": 6 since we're using ES6.
• Enable ES6 built-in global variables by adding "es6": true under env.
• Remove all the EcmaScript 6 flags that are now already available with ecmaVersion: 6. Check the ESLint migration guidelines for details - https://eslint.org/docs/user-guide/migrating-to-2.0.0#language-options.

Now

After these changes you shouldn't see the DeprecationWarning like before.

You can also check the travis build logs - for 4.x branch and for the PR 1586 and see that the warning isn' showing up now.

---

Before creating a pull-request, please check https://github.com/wycats/handlebars.js/blob/master/CONTRIBUTING.md first.

Generally we like to see pull requests that

• Please don't start pull requests for security issues. Instead, file a report at https://www.npmjs.com/advisories/report?package=handlebars
• Maintain the existing code style
• Are focused on a single change (i.e. avoid large refactoring or style adjustments in untouched code if not the primary goal of the pull request)
• Have good commit messages
• Have tests
• Have the typings (lib/handlebars.d.ts) updated on every API change. If you need help, updating those, please mention that in the PR description.
• Don't significantly decrease the current code coverage (see coverage/lcov-report/index.html)
• Currently, the 4.x-branch contains the latest version. Please target that branch in the PR.

[nknapp]

The comment in the eslint config indicates that not all es6 features should be enabled.

Object.assign for example does not work in IE and we don't use polyfills.

Its a good thing to fix the config. Could you do it so that the list of features is used again?

[nknapp]

It would also be helpful to have the disable es6-language at all for the files in the spec directory. Those are not compiled with babel. I am planning to refactor the tests, but until than, that would be helpful.

It has happend a couple of times that I wrote a test and everything was find when I ran the tests locally. But when they were executed in SauceLabs with IE11, there was an error.

It doesn't have to be in this PR, but it is something to keep in mind.

[kabirbaidhya]

Hi @nknapp,
Did you mean to undo these list of ecma script features?

Actually, they aren't being removed; but it's just that explicitly setting them under ecmaFeatures isn't required anymore in the current version of eslint - these are already included when you set ecmaVersion: 6.

So, that being said there isn't actually any change done in the config. These are default features of Ecma Script that eslint already knows once you set the version now. Check this:

Also, as per the deprecation warning - even if you do set them it has no effect now.

(node:7146) DeprecationWarning: [eslint] The 'ecmaFeatures' config file property is deprecated, and has no effect. (found in /home/kabir/opensource/handlebars.js/.eslintrc.js)

Let me know your thoughts on this.

[nknapp]

Are there any features that are allowed by eslint after your change that weren't allowed when the ecmaFeatures property was still used by the old eslint-version?

[nknapp]

I think I misunderstood what the "ecmaFeatures" was doing exactly. But still rules for avoiding "Object.assign" and other things that break in IE or Safari, would be great.

That's another matter however.

[nknapp]

I have added the eslint-plugin-compat, which checks for browse-compatibility and eslint-plugin-es5 which is now configured to ensure that the spec/ directory only contains es5 syntax. (The reason why the tests have failed after merging #1584)

[kabirbaidhya]

Hi @nknapp, Thanks!

Regarding your comment:

But still rules for avoiding "Object.assign" and other things that break in IE or Safari, would be great.

I'll check if there are rules available for these use cases and can do a PR if possible.

[nknapp]

Eslint-plugin-compat is doing that. Check for Object.assign etc.
I've added it already, because the last three builds already failed because of a "let" in the tests, which I had not seen in my review.