tools: add quote-props eslint rule

[BridgeAR]

This adds the quote-props rule and uses the consistent style. I personally would like to change that to consistent-as-needed but this would create quite some churn and therefore using consistent right now is probably best.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

benchmark,test,tools

[tniessen]

Linter CI: https://ci.nodejs.org/job/node-test-linter/10235/

[refack]

I personally would like to change that to consistent-as-needed but this would create quite some churn and therefore using consistent right now is probably best.

Personally I'd rather have as-needed if we're going to churn (392 problems in 71 files)
(for reference consistent-as-needed has 317 problems in 55 files)

[refack]

I think I'm -1 on this.
I'm not blocking but would like a discussion. Is consistently better than "parsimony"?

[tniessen]

I think this is a design decision, do we prefer consistent style or do we want to leave out all quotation marks that are not strictly required?

Ongoing design decision - do not land!

[lpinca]

-0, I prefer to not use quotes unless it is really needed.

[BridgeAR]

I don't mind using as-needed. I think that most people prefer not to use quotes, that's the main point about this PR.
Using consistent is meant as a small step towards that since most objects don't need quotes at all. I tried not to churn to much and now things could be changed over time or is churning justified and I should just change this to either consistent-as-needed or to as-needed?

[refack]

I don't mind using as-needed. I think that most people prefer not to use quotes, that's the main point about this PR.
Using consistent is meant as a small step towards that since most objects don't need quotes at all. I tried not to churn to much and now things could be changed over time or is churning justified and I should just change this to either consistent-as-needed or to as-needed?

I think we have precedence for churning in order to activate an eslint rule ( @vsemozhetbyt is the king of those 👑 #12563, #13835, etc. )

IMHO as-needed is best, but up for a quick vote:

•    ❤️     — do nothing
• 👍  — consistent (current state of PR)
• 😄 — as-needed
• 🎉 — consistent-as-needed

/cc @nodejs/collaborators

[vsemozhetbyt]

I am really not sure what to prefer. I like clear minimization (as-needed), but I recall a discussion about object shorthand: some collaborators said they considered this confusing:

const foo = 'foo';

const obj = {
  foo,
  bar: 'abc'
}

vs

const obj = {
  foo: foo,
  bar: 'abc'
}

as a mind tunes in for on style and then stumbles over a different one and that hampers reading. For me, the first example is OK, but I would not want to torture those who see this differently, i.e. who possibly prefer consistent-as-needed)

[refack]

For me, the first example is OK, but I would not want to torture those who see this differently)

Since quote-props only check prop names, I think the better example would be:

const foo = 'foo';

const obj = {
  'foo': foo,
  bar: 'abc'
}

[vsemozhetbyt]

Yes, I meant we already had a discussion on a similar style issue)

[BridgeAR]

Can we get some further opinions here?

[evanlucas]

IMO, we should only quote properties that must be quoted. Rule changes tend to make cherry-picking for releases more difficult...

[mcollina]

I think this just creates churn for no particular benefit: -1. It also complicates cherry-picking.

[cjihrig]

Also -1 for the reasons listed.

[gibfahn]

I'd prefer as-needed, but I'm not sure there's enough benefit here for it to be worthwhile.

[benjamingr]

I'm also -1 for both the reasons stated above (churn) and stylistic preference. I'm making my -1 explicit - I'm fine with "as needed" as a config or not changing the config.

[refack]

@mcollina @cjihrig Just to be clear would you be Ok with as-needed (and fixing 392 problems in 71 files)?

[jasnell]

I'm -1 on this too. I much prefer omitting the prop name quotes when possible to do so.

[refack]

@mcollina @cjihrig Just to be clear would you be Ok with as-needed (and fixing 392 problems in 71 files)?

Also we could set quote-props: [warning, as-needed] for a transition period

[mcollina]

I'm -1 as as-needed and as well.

[refack]

Closing for lack of consensus