Release proposal: standard v8

[feross]

Planned release date: September 1, 2016. (approx. 45 days from the date of this issue)

New features

• Upgrade to ESLint v3 (http://eslint.org/docs/user-guide/migrating-to-3.0.0) (#565)
  • BREAKING: Drop support for node < 4 (this was a decision made by the ESLint team)
• Expose ESLint's --fix command line flag (#540) (standard/standard-engine#107)
  • Lightweight, no additional dependencies, fixes dozens of rules automatically

New rules

(Estimated % of affected standard users, based on test suite in parens)

• Enforce placing object properties on separate lines (object-property-newline) (#524) (2%)
• Require block comments to be balanced (spaced-comment "balanced") (#572) (2%)
• Disallow constant expressions in conditions (no-constant-condition) (#563) (1%)
• Disallow renaming import, export, and destructured assignments to the same name (no-useless-rename) (#537) (0%)
• Disallow spacing between rest and spread operators and their expressions (rest-spread-spacing) (#567) (0%)
• Disallow the Unicode Byte Order Mark (BOM) (unicode-bom) (#538) (0%)
• Disallow assignment to native objects/global variables (no-global-assign) (#596) (0%)
• Disallow negating the left operand of relational operators (no-unsafe-negation) (#595) (0%)
• Disallow template literal placeholder syntax in regular strings (no-template-curly-in-string) (#594) (0%)
• Disallow tabs in file (no-tabs) (#593) (0%)

Changed rules

• Enforce use of double quotes in JSX attributes (jsx-quotes) (1%, but likely more in private repos)
  • BREAKING: This rule was changed to reflect the way the vast majority of developers quote HTML attributes (both in plain HTML and in the React community) (standard/eslint-config-standard#27)
  • This rule was re-evaluated. See #564 (comment)
• Relax rule: Allow template literal strings (backtick strings) to avoid escaping
 (#421)
• Relax rule: Do not enforce spacing around * in generator functions (#564 (comment))
  • This is a temporary workaround for babel users who use async generator functions.

Internal changes

• Migrate to new ESLint rule format for plugins (standard/eslint-plugin-standard#11) (xjamundx/eslint-plugin-promise#19)

[feross]

I'm happy to announce the immediate availability of a standard v8 release candidate. The version is 8.0.0-beta.0. The final version will be released around September 1, 2016 to allow ample time for community feedback.

You try it out today by installing it: npm install -g standard@beta

Cheers! 🎉

[feross]

Hey collaborators! I have a favor to ask: Can you test out the standard v8 beta on one or two of your repos?

npm install -g standard@beta

Feedback would be great!

@maxogden @mafintosh @othiym23 @dcposch @Flet @dcousens @jprichardson @xjamundx @watson @rstacruz @reggi @yoshuawuyts @bcomnes @jb55

[yoshuawuyts]

Tested it on 4 or so (ES6 heavy) repos, all seems good 🎉

[feross]

Thanks @yoshuawuyts!

[feross]

standard 8.0.0-beta.1 is out, which just updates eslint from ~3.0.0 to ~3.1.0.

[jprichardson]

Just tried it on a very large project I'm working on, got this error:

standard         
standard: Unexpected linter output:

TypeError: Cannot read property 'value' of undefined
    at getStarToken (/Users/jp/.nvm/versions/node/v6.1.0/lib/node_modules/standard/node_modules/eslint/lib/rules/generator-star-spacing.js:68:25)
    at EventEmitter.checkFunction (/Users/jp/.nvm/versions/node/v6.1.0/lib/node_modules/standard/node_modules/eslint/lib/rules/generator-star-spacing.js:123:29)
    at emitOne (events.js:101:20)
    at EventEmitter.emit (events.js:188:7)
    at NodeEventGenerator.enterNode (/Users/jp/.nvm/versions/node/v6.1.0/lib/node_modules/standard/node_modules/eslint/lib/util/node-event-generator.js:40:22)
    at CodePathAnalyzer.enterNode (/Users/jp/.nvm/versions/node/v6.1.0/lib/node_modules/standard/node_modules/eslint/lib/code-path-analysis/code-path-analyzer.js:607:23)
    at CommentEventGenerator.enterNode (/Users/jp/.nvm/versions/node/v6.1.0/lib/node_modules/standard/node_modules/eslint/lib/util/comment-event-generator.js:97:23)
    at Controller.traverser.traverse.enter (/Users/jp/.nvm/versions/node/v6.1.0/lib/node_modules/standard/node_modules/eslint/lib/eslint.js:902:36)
    at Controller.__execute (/Users/jp/.nvm/versions/node/v6.1.0/lib/node_modules/standard/node_modules/estraverse/estraverse.js:397:31)
    at Controller.traverse (/Users/jp/.nvm/versions/node/v6.1.0/lib/node_modules/standard/node_modules/estraverse/estraverse.js:501:28)

If you think this is a bug in `standard`, open an issue: https://github.com/feross/standard/issues

My config in package.json:

  "standard": {
    "ignore": [
      "static/"
    ],
    "parser": "babel-eslint",
  }

If I remove babel-eslint, it outputs expected standard errors, however then standard --fix barfs on decorators (legacy).

[feross]

@jprichardson Thanks for testing.

This is an issue with the interaction between babel-eslint and eslint. See eslint/eslint#6274

I think we'll just have to disable the generator-star-spacing rule until ESLint has support for the async keyword. Bummer.

[feross]

standard 8.0.0-beta.2 is out, which removes the generator-star-spacing rule so projects that use babel and have an async generator function will work.

@jprichardson Can you test it out now?

[jprichardson]

Wow. standard --fix feels like magic compared to what I use to have to do - I figured it would take me forever to migrate. Haha. So far it looks like everything is working. Gotta fix a lot of pesky object-property-newline, but that's ok. At first glance, the React changes don't bother me like I thought that they might. I'll keep playing with it.

Nice work @feross!

[feross]

@jprichardson Good to hear. Thanks for helping to test it out!

[Flet]

Thanks for kicking this off!

Any opposition to bumping to eslint-plugin-react@5.2.2?

There have been quite a few fixes and additions since 5.0.1:
https://github.com/yannickcr/eslint-plugin-react/blob/master/CHANGELOG.md

Seems like a good time to update?

[Flet]

Released semistandard@beta with updated standard config... its strangely satisfying to run --fix to toggle between standard and semistandard 😈

[feross]

@Flet eslint-plugin-react@5.2.2 is already used because of the semver range we're using ^5.0.1. But I just pushed a commit that forces the latest version of all deps, just to be sure an older version is never used. f6b92de

[feross]

Just merged one new rule: Require block comments to be balanced (#572)

Released standard 8.0.0-beta.3.

[timoxley]

@feross 👍 👍 Tried out 8.0.0-beta.3. No dramas. The JSX changes looked like a real PITA since it wasn't going to be a simple regex/find/replace but standard --fix did all the hard work. LGTM ✔️

[feross]

@timoxley 👍 👍 👍 great to hear! what do you think of the jsx change?

[timoxley]

what do you think of the jsx change?

@feross I do understand & accept the sentiment in standard/eslint-config-standard#27, though it probably wouldn't be my preference.

• One quoting rule is better than two quoting rules, especially within the one file. Find/replace for cleaning mixed js/jsx no longer trivial.
• Single quotes are a little tidier. Fewer unnecessary pixels is better IMO.
• Double quotes in html does make it easier to copy/paste html (though I did like the act of purifying html of its double quote filth).
• Creates a bit of a reliance on --fix since layman's find/replace isn't js/jsx sensitive. This is probably fine so long as --fix doesn't go the way of standard-format i.e. writing broken/mangled code.
• Since it's much of a muchness, I'm not sure the benefits outweigh the downsides of requiring sweeping changes across every codebase using JSX. Perhaps this is ok though, given that --fix exists & works.

[timoxley]

@feross I think you could head-off many complaints if standard promoted the use of standard --fix whenever it finds issues. As a part of the tagline perhaps?

Created an issue for this: #576

Update: Message promoting --fix has been added https://github.com/Flet/standard-engine/pull/115/files 🎉

[kyeotic]

I agree with @timoxley that fix should have more awareness. I manually updated all my code (though it was easier with "replace in selection" and only two JSX projects) because I didn't know about fix.

Maybe it could go into the error output?

[timoxley]

@tyrsius see #576

[dcousens]

I fully agree with #564 (comment), in that I'm not sure the benefits out weigh the extra rule.

[timoxley]

So the JSX double quoting rule requires mixing of double/single quotes when doing any kind of inline JS in JSX:

<div
  doubleQuotes="just because this is static"
  butForDynamicValues={singleQuotes === 'are' ? 'needed' : 'now'}
/>

Or a more realistic example:

<div title="Help Text" className={selected === item.id ? 'selected' : 'unselected'}>
  {item.content}
</div>

This converted me to a mild 👎 about this change; needing to use two quoting styles on the same line is not very "don't make me think" IMO.

One rule > Two rules

[timoxley]

@feross Another option could be for standard itself to check the node version and error appropriately rather than just waiting for it to fail on some syntax error. Perhaps then could add a flag like --ignore-unsupported so standard always exits 0 for users who need to run standard as a part of the test suite on older nodes.

For reference, this is the current error when running the latest standard beta (8.0.0-beta.4) on node 0.10.46:

> standard

/usr/local/lib/node_modules/standard/node_modules/eslint/node_modules/strip-bom/index.js:2
module.exports = x => {
                    ^
SyntaxError: Unexpected token >
    at Module._compile (module.js:439:25)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at Object.<anonymous> (/usr/local/lib/node_modules/standard/node_modules/eslint/lib/config/config-file.js:23:16)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)

[jprichardson]

Standard 8 is gonna land soon. We gotta figure out the JSX double quotes situation...

Given that all production versions of Standard before this don't allow JSX double quotes AND we don't have consensus, it should be removed from the Standard v8 release. Not too mention though other reasons like 1) Facebook themselves are inconsistent 2) it's confusing for new users (different rules (when/where) 3) JSX is not HTML. 4) --fix makes it crazy easy to copy and paste HTML.

[kyeotic]

@jprichardson

1. They have been consistent for a while now, examples of inconsistency are old
2. JSX Is not JavaScript either
3. --fix also makes it crazy easy to fix all the production versions

You might disagree with the decision, but saying we have to "figure it out" ignores the multiple threads over which this has already been discussed. The fact that its in the release is it being "figured out".

[dcousens]

@tyrsius I think what @jprichardson is saying, is perhaps the final decision should be delayed rather than delaying v8.

[pluma]

@jprichardson the logical conclusion if there is no consensus isn't to enforce your taste but to remove the validation until there is a consensus. JSX is neither HTML nor JS so I'm not even sure it belongs in standard at all. In fact, your entire "JSX isn't HTML" argument is already violated by the enforcement of boolean properties being passed with the shorthand syntax (i.e. <MyComponent booleanProp /> rather than <MyComponent booleanProp={true} />) and the enforcement of the XHTML-style space before the closing slash in empty components.

That said, there was a consensus. Just one you happen to disagree with.

[timoxley]

That said, there was a consensus. Just one you happen to disagree with.

There's also some consensus that the current consensus is disagreeable.

[pluma]

@timoxley which indicates this really shouldn't be part of standard to begin with.

[joshmanders]

Agreed with @pluma. It should just be removed. I really enjoy using standard but I can't stand single quotes in my html, OCD about it, and whether you see it as html or not, JSX to me is html-like, so that OCD triggers there. I'd hate to have to deviate away from the ease of just using standard all because of this.

[LinusU]

I don't think it should be removed. Standard is all about just picking one way and sticking with it. I don't see any great advantage with either one, but having it not enforce any of them would be worse.

My vote goes to @feross just picking one of them. It's not like it going to affect anyones day to day life. Both ways are easy to fix with the new --fix flag.

[feross]

@timoxley Another option could be for standard itself to check the node version and error appropriately rather than just waiting for it to fail on some syntax error

Good idea. Let's skip running standard on Node <4 and print a warning to stderr. I also added an install-time warning using the package.json "engines" field to warn on Node <4.

PR here: #590

[feross]

@dcousens This doesn't look like it has an easy answer @feross, I think we need a decision?

Yeah, tell me about it. Either decision we make, there are going to be some folks who aren't happy. That's just the way a curated style guide like this works.

There's no clear winner, IMO. Quoting some of the arguments from this thread, here's how I would sum this up:

Pros for JSX double quotes:

• Makes copy/pasting from HTML examples easier.
• Matches the majority of the React/HTML community's style.
• Makes JSX "look like HTML" more than when single quotes are used.
• Forces the developer to understand what is JSX and what is JS.
• Avoids escaping in attributes that contain an apostrophe (e.g. <img alt="I'm happy">

Pros for JSX single quotes:

• "Always use single quotes" is easier to remember. Less cognitive overhead.
• Friendlier to beginners. Doesn't require mixing double/single quotes when inline JS is used in JSX.
• Single quote attributes are totally valid in HTML.
• Arguably tidier. Fewer unnecessary pixels on screen.
• Avoids huge "upgrade to standard v8" commit in existing codebases using standard v7.
• Easier to find/replace double quotes with single using text editor tools, whereas to switch to double requires requires reliance on --fix.

The standard philosophy is to enforce consistency to maximize clarity and reduce programmer error. The decision to switch to double quotes in JSX was made to increase clarity (so it looks more like HTML and distinguishes it from the surrounding JS).

But in attempting to switch some of my own code to the new v8 style, I'm running into situations where the intermixing of single and double quotes feels like it's actually reducing clarity. This is a typical example:

<img className="poster" alt={this.getAltText('poster')} />

In this situation, the rule is not reducing the likelihood of errors or increasing the clarity of the code. There's added cognitive overhead to remember when to use each type of quote.

I worry that beginners won't be able to figure out when to use each type of quote, because we've taken a simple rule ("always use single quotes") and made it more complex ("use single quotes for JS, use double quotes for JSX").

There's no perfect decision here. But I think we should revert to standard v7 behavior ("always use single quotes") since that's the simpler choice.

Note that allowing any type of quote isn't acceptable. Whenever there's two ways to do something without a clear winner, standard tries to "just pick something".

[feross]

Just published standard v8.0.0-beta.5 with the following 4 new rules:

• Disallow assignment to native objects/global variables (no-global-assign) (#596) (0%)
• Disallow negating the left operand of relational operators (no-unsafe-negation) (#595) (0%)
• Disallow template literal placeholder syntax in regular strings (no-template-curly-in-string) (#594) (0%)
• Disallow tabs in file (no-tabs) (#593) (0%)

And, changed rule:

• Changed the jsx-quotes rule to enforce single quotes.

I promoted this release to the "latest" npm dist-tag so it gets installed with npm install standard, to increase exposure. The "beta" dist-tag is no more.

If there are no issues, this release will become the official standard v8.0.0 release on September 1, 2016.

[feross]

I just realized that my trip to China on August 26 might interfere with the planned September 1 release date.

So, new release date: August 23 (one week early).

That way I have a couple days to fix any issues that might come up. Also, there are plenty of contributors who have github/npm permission who can step up if some urgent issue surfaces while I'm gone.

[feross]

standard v8.0.0 is released!

[Jessidhia]

@feross you probably should make a non-beta release of eslint-config-standard (and depend on that instead) :)

[feross]

@Kovensky good call, done

[robinpokorny]

@feross Should eslint-config-standard-react v4 be released with eslint-config-standard-jsx@3.0.0 dependency?

[feross]

@robinpokorny Done

[yantakus]

@feross You forgot to mention one Pros for JSX double quotes and it is the most important of all: JSX is treated as HTML in most editors, so they use double quotes by default. I mean when you start typing an attribute and then choose from autocomplete options. Editors insert double quotes in this case. And this behavior is even not configurable for some editors, eg. Intellij Idea. It is possible to disable auto-insertion of quotes at all, but not auto-insert single quotes: http://stackoverflow.com/questions/37635871/how-can-i-disable-syntax-autocomplete-of-jsx-properties.

At this moment this is the only rule in Standard that forces me to overwrite rules. Kinda frustrating...