Experimental ES6 Support

[andrewdeandrade]

This diff adds experimental ES6 support.

Since ES6 is for all intents and purposes a different language (until all runtimes, frontend and backend, support it), is experimental and has not yet been supported yet at uber until now, es6 mode will be used as a testing ground for trying out rules that may be backported into es5 mode if they provide utility in removing footguns and making code more consistent.

i.e. if you want greater rule stability, choose es5 mode. If you're open to try out ES6, you're also opting in to being tolerant of trying out new rules to see if they provide greater code consistency between teams.

In an effort to ship ES6 mode to those who have been waiting, this PR will remain open for 24 hours after the first LGTM. Please look over the proposed rules and raise concerns now.

The rules specific to ES6 that I am curious to get people's opinion on are:

• object-shorthand mode - There are cases where it makes things more readable and cases where it makes things less readable.
• accessor-pairs mode - I didn't turn this one are, but should we? This one has configuration options.

Rules specific to ES6 mode are:

• no-var (what's the point in having let and const if you aren't going to use them over var)
• prefer-const
• generator-star-spacing
• accessor-pairs
• constructor-super
• no-this-before-super

Rules that are likely candidates to be ported to ES5 mode for the next major release are:

• no-catch-shadow (probably a noop since there was already a no-shadow rule)
• no-continue
• no-div-regex
• no-else-return
• no-eq-null (probably a noop since we already enforce strict equality)
• no-extra-parens
• no-inline-comments (was already in lint-trap)
• no-mixed-requires (probably a noop since we do one var per line)
• no-throw-literal (long overdue, you should always generate an error object with a stack trace)
• no-unexpected-multiline
• no-unneeded-ternary
• array-bracket-spacing (was already in lint trap)
• block-scoped-var
• computed-property-spacing
• dot-location
• func-style - this rule I am unsure about and want to try out because it seems like this isn't a 100% rule and it looks like it allows function expressions in many cases where you really only want a function expression (like assigning a function expression to a property on this in the constructor or assigning prototype props to function expressions). Because of this, I want to see how this actually works in practice.
• newline-after-var
• object-curly-spacing (was already in lint-trap)
• operator-linebreak
• quote-props - This one is turned on with the configuration "as-needed". I'm curious to get feedback on whether or not this is useful or problematic. Hard to tell without seeing how it actually works in the wild.
• sort-vars - Given that we require one var per line, this one probably is a noop. However there is another rule, one-var which has an "uninitialized" and "initialized" option which is interesting that would make this rule relevant.
• space-before-function-paren - this one should be a noop because I believe this is already enforced by some other rule we already have turned on. This was already rule in lint-trap.
• spaced-comment - I need feedback from people on this one when it is used in practice. Most likely this is a useful rule, but will need to have the "exceptions" and "markers" options modified based on cases where it may not work.

Rules that remain turned off for both ES5 and ES6 mode:

• no-bitwise
• no-nested-ternary
• no-param-reassign
• no-plusplus
• no-reserved-keys
• no-sync
• no-ternary
• no-undefined
• no-void
• lines-around-comment - completely off, but I want others to look at whether some of the configurable options on this rule could be valuable.
• padded-blocks
• vars-on-top

The no-restricted-modules mode will be evaluated separately for both environments and will be added in in a separate pull request.

All the rules can be found here: https://github.com/eslint/eslint/tree/master/docs/rules

FWIW, I want to eventually look into getting "custom env" support in eslint so that these can be moved to something you could turn on and off per file with an inline directive like: /*eslint-env es6*/, but for now you turn this on with the --es6 command line flag.

cc: @raynos @lxe @johnmegahan @rtsao @mlmorg @marthakelly @jcorbin @freeqaz @vicapow @philogb @superbeefy @mishabosin @addisonedwardlee @kenzanboo @collenjones @cain-uber @partriv @shaohua @yuanzong @Amazeotron @danielheller @Jeloi @jschao-uber @MarkReeder @noamlerner @Matt-Esch @rajeshsegu @rhobot @sh1mmer @thegleb @kriskowal @Willyham @ross- @orbiteleven

[rtsao]

I personally like object shorthand in combination with destructuring in scenarios like this:

ES6:

function component({foo, bar}) {
  return foo + bar;
}

ES5:

function component(obj) {
  return obj.foo + obj.bar;
}

Object shorthand for methods is also convenient if func-names is enforced:

ES6:

React.createClass({
    render() {}
});

ES5:

React.createClass({
  render: function render() {}
});

Thus, I think I'd be okay with either turning off object-shorthand or setting it to "always".

Regarding accessor-pairs, the defaults seem sensible to me. setWithoutGet true and getWithoutSet false. I can't think of a reason to have a setter but no getter.

[andrewdeandrade]

@rtsao I haven't used the ES6 getters/setters yet, but I can think of cases where you would want to set something, but have no reason to get the value back. Internal configuration of a class for example, would involve setting, but you probably wouldn't need to get that value back out for use elsewhere. It would be nice if you could enforce no-param-reassign for setters that have getters, so you would be passing and getting out values instead of references.

Why wouldn't you write the first example as:

function component(foo, bar) {
  return foo + bar;
}

So between turning off object-shorthand or setting it to always, which one and why?

[shaohua]

minor thing, inline comments are helpful and I hope we can reconsider no-inline-comments for es5. my philosophy has been 'inline comments are better than no comments'

[marthakelly]

fwiw inline comments will break the postcss css parser... because it doesn't know where to end the comment block

[rtsao]

@malandrew It's not so bad with just two parameters, but I find named object properties more convenient because I don't have to remember the correct parameter order.

I'm not sure how I feel about heterogeneous object literals that mix shorthand and non-shorthand:

var obj = {
  foo: 'abc',
  bar: 4,
  blah,
  thing
}

versus:

var obj = {
  foo: 'abc',
  bar: 4,
  blah: blah,
  thing: thing
}

Should the former be enforced by turning on the rule?

[andrewdeandrade]

I'm not a fan of the mixing either, so I updated object-shorthand to "always".

I also turned accessor-pairs on for getWithoutSet.

[andrewdeandrade]

@marthakelly the inline-comments rule disables inline comments, so it's all good.

[andrewdeandrade]

@shaohua Furthermore, lint-trap always disallowed inline comments. Comments for a line should be put before the line being commented on. It's not that you can't comment (you most certainly should), it's where the comment goes.

[johnmegahan]

lgtm

[vicapow]

LGTM 👍

[cain-wang]

Thanks, can't wait!

[rajeshsegu]

:clap:

[ross-]

We've been doing object-shorthand. Mixing the two was weird for a bit but I quickly got used to it.

What's the status of https://github.com/yannickcr/eslint-plugin-react support? :grin:

[andrewdeandrade]

@ross- I will investigate anything react specific separately in a different issue/pull request. This is just a pull request for ES6 mode. I'll check out the module you linked to.