Disallow quotes around object literal property names that are not strictly required (quote-props)

[tunnckoCore]

Digging more and more in ESLint rules, i found some good ones which enforces good style and more readable code. This issue is for quote-props rule - fixable.

Suggested configuration.

{
  "quote-props": ["error", "consistent-as-needed", { "keywords": true }]
}

Invalid

// NOT OK, should not have quotes
// autofix: quotes will be removed
var x = {
  'prop': 1,
  'bar': 'foo'
}

// NOT OK, should be consistent
// autofix: quotes will be removed
var x = {
  'prop': 1,
  bar: 'foo'
}

// NOT OK, should be consistent
// autofix: will set quotes for `prop`
var x = {
  prop: 1,
  'foo-bar': 'foo'
}

// NOT OK, all props should have quotes
// because `while` is keyword
var x = {
  while: 1,
  volatile: 'foo'
}

Valid

var x = {
  prop: 1,
  bar: 'foo'
}

var x = {
  prop: 1,
  bar: 'foo'
}

var x = {
  'prop': 1,
  'foo-bar': 'foo'
}

var x = {
  'while': 1,
  'volatile': 'foo'
}

[dcousens]

NACK. as-needed is the only approach I typically take.

var x = {
  'while': 2,
  prop: 1,
  'foo-bar': 'foo'
}

[tunnckoCore]

Me too, but it is messy for the eyes and we have autofix feature, so absolutely no problem.

[dcousens]

@tunnckoCore not true.

If I have an object with 100 properties that has no quotes, then suddenly introduce foo-bar, I'm not going to then re-quote each of those properties.
Even with --fix.

What a mess that would be for any diff too.

[tunnckoCore]

Kinda didn't thout for that case. But in any way

If I have an object with 100 properties that has no quotes

is some exotic case

[dcousens]

is some exotic case

If we're using objects this much, even an object of 3-5 properties would feel like 100 properties.
It's a pain.

NACK.

[timoxley]

The intention of this rule is good, however I agree with @dcousens. Requoting unrelated keys due to adding a single property that needs quoting is not great.

it is messy for the eyes and we have autofix feature, so absolutely no problem.

I think we should avoid any rules while lean heavily on --fix for day-to-day use, a typical standard user shouldn't need know anything about standard outside of the warnings it produces. IMO we should only rely on --fix as an escape-hatch for upgrades to standard.

[tunnckoCore]

So, maybe as-needed then?

I believe everyone should by default call standard with --fix flag. It just not make sense to see such errors while they can be fixed. Autofix implementations aren't dangerous, or i still didn't see such.

I even would suggest fix to be enabled by default. And standard officially to recommend using it at least.

[ghost]

👍 on catching reserved keywords, maybe. A big 👎 on forcing too much coding style (even if fixable). Standard provides coding bliss because it steps out of the way and allows Zen coding w/o interruption. Add too much opinion and you lose that zen. Related reading: http://ben.balter.com/2016/07/21/removing-a-feature-is-a-feature/

[feross]

@tunnckoCore I added a recommendation to use --fix at the very top of the readme. Thanks for the suggestion! See here: https://github.com/feross/standard#javascript-style-guide-with-linter--automatic-code-fixer

@timoxley I think we should avoid any rules while lean heavily on --fix for day-to-day use, a typical standard user shouldn't need know anything about standard outside of the warnings it produces

This is correct. As cool as --fix is, it's really important to me that I can write code that passes standard easily (from memory and without relying on --fix because something is too painful to do manually).

[feross]

The ecosystem impact of this is 7.5%.

# tests 238
# pass  220
# fail  18

However, this is mitigated by the fact that the rule is fixable. Most of the failing cases are places where folks were consistently using quoted props, even though they're not needed. Like this:

var yargKeys = {
  'boolean': { transform: null, error: null },
  'count': { transform: null, error: null },
  'string': { transform: null, error: null },
  'array': { transform: null, error: null },
  'required': { transform: null, error: null },
  'default:': { transform: null, error: null },
  'choices:': { transform: null, error: null },
}

We could try enabling this in a future beta version, so I'll leave this open for now.

[jaridmargolin]

Prettier implements as-needed and I believe it greatly affects readability:

I prefer consistent/consistent-as-needed, but understand the argument against:

If I have an object with 100 properties that has no quotes, then suddenly introduce foo-bar, I'm not going to then re-quote each of those properties.

I would prefer standard does not take a stance on this. I philosophically believe in what standardjs is doing in order avoid bikeshedding (or at least move it to a single repo 😂), but this rule greatly affects my ability to quickly scan/read code in my editor. The implementation of as-needed would most likely drive me away from standardjs, which would be very sad 😢

[jaridmargolin]

One additional note: Prettier is considering "Consistently add quotes to object keys" for v2.0. I would assume this change is due to overwhelming community feedback (although my assumption is probably based on my bias):

With the following comment coming from @vjeux who appears to want to adjust this rule regardless of a v2.0 release:

For consistent key quotes, I think we should just do it regardless, I’m not sure why we haven’t already.

prettier/prettier#3503

[mightyiam]

as-needed is my personal preference.

[feross]

I think we should go ahead with the following rule in standard 13.

"quote-props": ["error", "as-needed"]

Ecosystem breakage is only 4%. --fix can be used to automatically fix the issue. Furthermore, the rule is easy to remember: add quotes only where you are required to.