Release proposal: standard v8 #564

Closed
feross opened this Issue Jul 12, 2016 · 71 comments

Projects

None yet
@feross
Owner
feross commented Jul 12, 2016 edited

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

New features

New rules

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

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) (feross/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

@feross feross added the meta label Jul 12, 2016
@feross
Owner
feross commented Jul 13, 2016

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
Owner
feross commented Jul 22, 2016

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
Collaborator

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

@feross
Owner
feross commented Jul 22, 2016

Thanks @yoshuawuyts!

@feross
Owner
feross commented Jul 23, 2016 edited

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

@jprichardson
Collaborator
jprichardson commented Jul 23, 2016 edited

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
Owner
feross commented Jul 23, 2016

@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 feross added a commit to feross/eslint-config-standard that referenced this issue Jul 23, 2016
@feross Relax rule: Spacing around * in generator functions
There 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.

See feross/standard#564 (comment)
18ab64f
@feross
Owner
feross commented Jul 23, 2016 edited

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
Collaborator

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
Owner
feross commented Jul 23, 2016

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

@Flet
Collaborator
Flet commented Jul 25, 2016

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
Collaborator
Flet commented Jul 25, 2016

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

@tyrsius tyrsius referenced this issue in ricardofbarros/linter-js-standard Jul 25, 2016
Open

use standards new `--fix` tool #129

@feross
Owner
feross commented Jul 25, 2016 edited

@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
Owner
feross commented Jul 25, 2016

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

Released standard 8.0.0-beta.3.

@timoxley
Collaborator

@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
Owner
feross commented Jul 26, 2016

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

@timoxley
Collaborator
timoxley commented Jul 26, 2016 edited

what do you think of the jsx change?

@feross I do understand & accept the sentiment in feross/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
Collaborator
timoxley commented Jul 26, 2016 edited

@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 🎉

@tyrsius
tyrsius commented Jul 26, 2016 edited

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
Collaborator
@dcousens
Collaborator
dcousens commented Jul 27, 2016 edited

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

@timoxley
Collaborator
timoxley commented Jul 30, 2016 edited

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

@bentatum

I'm not a fan of double quotes in jsx. One of the things that turned me on to standard was its simplicity. I think this is a step backward in that respect.

@jprichardson
Collaborator

Fair points @timoxley. Seems like Facebook is inconsistent themselves...

https://facebook.github.io/react/docs/jsx-in-depth.html, https://facebook.github.io/react/docs/displaying-data.html, https://facebook.github.io/react/docs/multiple-components.html

Since JSX is really just JavaScript, maybe we do stick with the single quotes for everything. I'm assuming Facebook chose double quotes since they wanted to appeal to designers a bit? But I'm not sure.

This is a tough one since the Facebook ecosystem has chosen " for JSX, despite inconsistently using single and double elsewhere. Standard has chosen the route of consistency and "don't make me think bro", that's part of the principle of Standard.

On a personal level, I believe that operating on principles is a better way to operate - as at least when you operate on principles, the outcome is usually predictable to everyone.

If we want to attract those who value what we value: not bikeshedding on trivialities and getting work done, the answer is clear. If we want to attract more people from other ecosystems, the answer is clear - however, I'm not so sure this bodes well for the long-term viability of Standard though.

I asked my co-founder and lead designer of Exodus what his thoughts were (double vs single from a designer + HTML point of view)... he thought it was dumb that I even asked, he just wanted to get work done.

Maybe we should keep the single everywhere, it seems to align with the principles behind this great software. Over time, hopefully more and more will value these same principles.

What are others thoughts? I'd love to get some people from feross/eslint-config-standard#27 to comment. @pluma @tyrsius @dverbu @dcposch @javorosas

@xjamundx
Collaborator

Strong fan of double quotes in JSX! Makes it easier to copy/paste from HTML

On Sun, Jul 31, 2016 at 8:45 AM JP Richardson notifications@github.com
wrote:

Fair points @timoxley https://github.com/timoxley. Seems like Facebook
is inconsistent themselves...

https://facebook.github.io/react/docs/jsx-in-depth.html,
https://facebook.github.io/react/docs/displaying-data.html,
https://facebook.github.io/react/docs/multiple-components.html

Since JSX is really just JavaScript, maybe we do stick with the single
quotes for everything. I'm assuming Facebook chose double quotes since they
wanted to appeal to designers a bit? But I'm not sure.

This is a tough one since the Facebook ecosystem has chosen " for JSX,
despite inconsistently using single and double elsewhere. Standard has
chosen the route of consistency and "don't make me think bro", that's part
of the principle of Standard.

On a personal level, I believe that operating on principles is a better
way to operate - as at least when you operate on principles, the outcome is
usually predictable to everyone.

If we want to attract those who value what we value: not bikeshedding on
trivialities and getting work done, the answer is clear. If we want to
attract more people from other ecosystems, the answer is clear - however,
I'm not so sure this bodes well for the long-term viability of Standard
though.

I asked my co-founder and lead designer of Exodus what his thoughts were
(double vs single from a designer + HTML point of view)... he thought it
was dumb that I even asked, he just wanted to get work done.

Maybe we should keep the single everywhere, it seems to align with the
principles behind this great software. Over time, hopefully more and more
will value these same principles.

What are others thoughts? I'd love to get some people from
feross/eslint-config-standard#27
feross/eslint-config-standard#27 to comment.
@pluma https://github.com/pluma @tyrsius https://github.com/tyrsius
@dverbu @dcposch https://github.com/dcposch @javorosas
https://github.com/javorosas


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#564 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAPBf_qdbwgzMzYc_vnH8wwH0frPiQlOks5qbMMGgaJpZM4JK1_N
.

@jprichardson
Collaborator

@xjamundx while that may be true, what are your thoughts on my statements about principles? Also, does using standard --fix (easy to fix quoting) change your thoughts?

@bentatum

@jprichardson Agreee, fully. Don't make us think, please :)

@javorosas
javorosas commented Jul 31, 2016 edited

Since JSX is really just JavaScript, maybe we do stick with the single quotes for everything

I hear this argument over and over. For me it's the same as hearing "Let's not use XML tags, since JSX is really just JavaScript". It's not. Single quotes on XML-like syntax for me feel as weird as using another symbol for the opening and closing XML tags instead of < and >.

Yes, from the JavaScript perspective this is irrelevant because at the end everything is compiled to JavaScript. But the whole point of JSX is looking like XML, isn't it?

@rstacruz
Collaborator

Aren't single quotes valid to use in HTML?

On Mon, Aug 1, 2016, 2:29 AM Javier Rosas notifications@github.com wrote:

Since JSX is really just JavaScript, maybe we do stick with the single
quotes for everything

I hear this argument over and over. For me it's the same as hearing "Let's
not use XML tags, since JSX is really just JavaScript". It's not.
Single quotes on XML-like syntax for me feels as weird as using another
symbol for the opening and closing XML tags instead of < and >.

Yes, from the JavaScript perspective this is irrelevant because at the end
everything is compiled to JavaScript. But the whole point of JSX is looking
like XML, isn't it?


You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#564 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAEikep-AivPHmOcbP7VYBlbkpIp5Dz5ks5qbOmJgaJpZM4JK1_N
.

@javorosas

@rstacruz You're right. That was a bad analogy. But from a purist perspective, conventions are the only thing we should be using. Convention in HTML is double quotes. I'm not a purist, but I think this topic requires this level of discussion.

@joshmanders

+1 on keeping double quotes.

@bentatum
<If condition={status === 'pending'}>
  <Loading className="special" />
</If>

Gah! that looks so wrong 😭😭

@joshmanders
render () {
  const isPending = status === 'pending'

  return (
    <If condition={isPending}>
      <Loading className="special" />
    </If>
  )
}

Problem solved. Cleaner and easier to understand.

@dcousens
Collaborator

If we want to attract those who value what we value: not bikeshedding on trivialities and getting work done, the answer is clear. If we want to attract more people from other ecosystems, the answer is clear - however, I'm not so sure this bodes well for the long-term viability of Standard though.

I think this spells out the situation perfectly.
We may be appealing to the masses, but, I don't think it adheres to standard's core principles.

@Kovensky
Kovensky commented Jul 31, 2016 edited

I use eslint-config-standard and I, for one, will be using 'jsx-tags': ['error', 'prefer-single'] as a local override; which would become my
first break from standard.

Almost all places where you use a JSX string constant is not arbitrary text
but an enum value. The only standard attributes, off the top of my head,
that take arbitrary text where you'd run into the apostrophe problem are
title, placeholder and alt. value can take arbitrary strings but
you should be giving a variable to it.

It's more consistent with everything else in the JS files too. And you
really should not be copy&pasting arbitrary HTML into JSX; if you do, you
should definitely go over it by hand. Regardless of your lint settings,
HTML is not XML, let alone JSX. JSX is not XML either, it just looks like
XML.

It does help that the primary language of my app is Japanese, so we don't
have to deal with English's apostrophe problem at all.

@tyrsius
tyrsius commented Aug 1, 2016 edited

Yes, from the JavaScript perspective this is irrelevant because at the end everything is compiled to JavaScript. But the whole point of JSX is looking like XML, isn't it?

This is it exactly. The whole point of using it is to look like HTML, so picking a style option that removes that benefit is throwing out the baby with the bathwater. If your argument is that it doesn't look like the other Javascript code in the same file... well, JSX isn't supposed to look like javascript, its supposed to look like HTML. That's the whole point.

@pluma
pluma commented Aug 1, 2016

If you think "it doesn't look like HTML" is not a valid argument, you might as well exclude JSX altogether.

Here's my proposal: just don't enforce any special style restrictions at all for JSX. Don't enforce a quote style for prop values. Don't enforce the space before the closing slash. Then Standard is entirely about standard JS and doesn't have to worry about anything outside the spec.

@timoxley
Collaborator
timoxley commented Aug 3, 2016 edited

This is it exactly. The whole point of using it is to look like HTML, so picking a style option that removes that benefit is throwing out the baby with the bathwater.

single quoted html still looks like html.

@dcousens
Collaborator
dcousens commented Aug 3, 2016 edited

hmmm

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

/whisper pick prefer-single

@joshmanders

I agree with @dcousens

/whisper leave prefer-double

@feross feross was assigned by dcousens Aug 4, 2016
@feross feross removed their assignment Aug 4, 2016
@feross
Owner
feross commented Aug 4, 2016

@dcousens Looks like the babel-eslint bug report was deleted from this thread? Can't find it here, even though I got an email notification.

@casz
casz commented Aug 4, 2016 edited

@feross after doing 'npm list -g --depth=1' I noticed babel-eslint was not there. Even though moments before I did an install. Guess I missed the -g on the install. Funny though I did it twice before going here to write.
But third time is the charm.
Sorry about the email I hoped I had deleted it before it went out. My bad 😭

@feross
Owner
feross commented Aug 4, 2016

@casz I think you meant --depth=1 with no spaces 👍 No problem, glad you got it figured out.

@casz
casz commented Aug 4, 2016 edited

@feross silly phone reply.

@dcousens
Collaborator
dcousens commented Aug 4, 2016

@dcousens Looks like the babel-eslint bug report was deleted from this thread? Can't find it here, even though I got an email notification.

Never saw it?

@feross
Owner
feross commented Aug 10, 2016

New beta release to update standard-engine to v5. Here's the changelog:

  • BREAKING: Remove formatter support (replaced by ESLint's --fix)
  • BREAKING: Drop Node 0.12 and 0.10 support (because of ESLint 3)
  • Add {fix: true} option to .lintFiles and .lintText API
  • Support auto-fixing source text from stdin (standard --fix --stdin)

Released as 8.0.0-beta.4.

npm install -g standard@beta
@xjamundx
Collaborator

Just a heads up. One of the nicest things about the standard formatter was the tab/space fixes that it would provide. There's a pretty nasty bug with eslint --fix with regard to going from tabs to spaces. eslint/eslint#4274

@othiym23
Collaborator

FYI, dropping support for Nodes 0.10 and 0.12 means that npm won't be able to upgrade to standard@8 until the npm CLI team drops support for both those releases, which will happen around the time that maintenance support is ended for each one. As we're already a major version behind, this isn't a huge deal, but it's something to keep in mind.

@tyrsius
tyrsius commented Aug 12, 2016 edited

@othiym23 I don't understand why it would be the case that the npm team wouldn't be able to upgrade just because the CLI has to support older versions. The developer writing code for npm can have multiple node versions installed to test and run standard. And they probably should, if they are running tests for multiple node versions. If their is a test process as part of the PR, surely it would also need newer node versions to run tests tests with, if it also automatically runs standard.

Use of standard and style compliance should still be possible on code that needs to support older versions of node.

@dougwilson
dougwilson commented Aug 12, 2016 edited

I have been using standard against modules that support all the way down to Node.js 0.6. Yes, obviously I cannot run standard/eslint against the code when using an old Node.js version, but what I have been doing (since I use Travis CI) is just conditionally installing standard based on the Node.js version and then conditionally running standard based on if standard is installed. Typically if standard where to fail, it wouldn't matter which version of Node.js, so running it in at least one in Travis CI will at least still alert that there is an issue with the PR (even if it does look like the issue could be version-specific when it is not). I hope that helps.

@feross
Owner
feross commented Aug 12, 2016 edited

@xjamundx One of the nicest things about the standard formatter was the tab/space fixes that it would provide.

Good to know. We'll continue to mention standard-format in the readme, but I'm just happy to have standard --fix which will continue to get better for free (because of the eslint team's excellent work) and doesn't require maintaining a separate esformatter configuration that needs constant tweaking to be kept in sync.

@othiym23 Makes sense. Thanks for sharing that data point. v0.10 and v0.12 aren't long for this world because support for v0.10 ends in less than 50 days, and v0.12 in 140 days.

@tyrsius @dougwilson Thanks for sharing how you use standard on older node versions. v0.6, wow!

@timoxley
Collaborator
timoxley commented Aug 13, 2016 edited

@othiym23 standard@beta will install on 0.10, it just won't run. Perhaps you could conditionally run standard in the npm script, depending on the node version. This may seem a little gross but I think such handling for standard is acceptable since it isn't checking runtime logic.

@timoxley
Collaborator
timoxley commented Aug 13, 2016 edited

@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
Collaborator

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.

@tyrsius
tyrsius commented Aug 16, 2016 edited

@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
Collaborator

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

@pluma
pluma commented Aug 17, 2016 edited

@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
Collaborator

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
pluma commented Aug 17, 2016

@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
Collaborator
LinusU commented Aug 17, 2016

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
Owner
feross commented Aug 18, 2016

@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
Owner
feross commented Aug 18, 2016 edited

@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 feross modified the milestone: standard v8 Aug 19, 2016
@feross feross added a commit to feross/eslint-config-standard-jsx that referenced this issue Aug 19, 2016
@feross JSX: prefer single quotes
See rationale here:
feross/standard#564 (comment)
131aa22
@feross
Owner
feross commented Aug 19, 2016 edited

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

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
Owner
feross commented Aug 22, 2016

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 feross added a commit that referenced this issue Aug 24, 2016
@feross Changelog for standard v8
Fixes: #564
33670e9
@feross feross closed this in #603 Aug 24, 2016
@feross
Owner
feross commented Aug 24, 2016

standard v8.0.0 is released!

standard v8

@Kovensky

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

@feross
Owner
feross commented Aug 24, 2016

@Kovensky good call, done

@saadq saadq referenced this issue in nodejs/nodejs.org Aug 25, 2016
Merged

Update dependencies #873

@robinpokorny

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

@feross
Owner
feross commented Aug 26, 2016

@robinpokorny Done

@dcousens dcousens referenced this issue in feross/eslint-config-standard-jsx Sep 14, 2016
Closed

Double quotes are enforced #10

@gemmaleigh gemmaleigh added a commit to alphagov/govuk_elements that referenced this issue Oct 11, 2016
@gemmaleigh gemmaleigh Update standard to version 8.4.0
https://github.com/feross/standard/blob/master/CHANGELOG.md

8.4.0 - 2016-10-10

Update ESLint from 3.6.x to 3.7.x.
5 additional rules are now fixable with standard --fix!
Use more conservative semver ranges #654
8.3.0 - 2016-09-29

The last release (8.2.0) added ES7 support. This release (8.3.0) adds
ES8 support ...just 3 days later!

This release should eliminate the need to specify babel-eslint as a
custom parser, since standard can now parse ES8 (i.e. ES2017) syntax
out of the box. That means async and await will just work.

Support ES8 (i.e. ES2017) syntax.
8.2.0 - 2016-09-26

For many users, this release should eliminate the need to specify
babel-eslint as a custom parser, since standard can now parse ES7 (i.e.
ES2016) syntax out of the box.

Support ES7 (i.e. ES2016) syntax.
Update ESLint from 3.5.x to 3.6.x.
4 additional rules are now fixable with standard --fix!
8.1.0 - 2016-09-17

Update ESLint from 3.3.x to 3.5.x.
Around 10 additional rules are now fixable with standard --fix!
8.0.0 - 2016-08-23

This release contains a bunch of goodies, including new rules that
catch potential programmer errors (i.e. bugs) and enforce additional
code consistency.

However, the best feature is surely the new --fix command line flag to
automatically fix problems. If you ever used standard-format and ran
into issues with the lack of ES2015+ support, you'll be happy about
--fix.

standard --fix is built into standard v8.0.0 for maximum convenience,
it supports ES2015, and it's lightweight (no additional dependencies
since it's part of ESLint which powers standard). Lots of problems are
already fixable, and more are getting added with each ESLint release.

standard also outputs a message ("Run standard --fix to automatically
fix some problems.") when it detects problems that can be fixed
automatically so you can save time!

With standard v8.0.0, we are also dropping support for Node.js versions
prior to v4. Node.js 0.10 and 0.12 are in maintenance mode and will be
unsupported at the end of 2016. Node.js 4 is the current LTS version.
If you are using an older version of Node.js, we recommend upgrading to
at least Node.js 4 as soon as possible. If you are unable to upgrade to
Node.js 4 or higher, then we recommend continuing to use standard v7.x
until you are ready to upgrade Node.js.

Important: We will not be updating the standard v7.x versions going
forward. All bug fixes and enhancements will land in standard v8.x.

Full changelog below. Cheers!

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-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

Relax rule: Allow template literal strings (backtick strings) to avoid
escaping
 #421
Relax rule: Do not enforce spacing around * in generator functions
(feross/standard#564 (comment))
This is a temporary workaround for babel users who use async generator
functions.
7b2ed3f
@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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment