Xo: Add `object-curly-newline` rule

[sindresorhus]

http://eslint.org/docs/rules/object-curly-newline

'object-curly-newline': [2, {multiline: true, minProperties: 1}],

The above will enforce putting the contents of objects (everything inside the braces) on a newline unless there's only one property.

The main issues with this rule is that it then enforces that all objects with one property is inline {foo: true}. I think it would make more sense not to enforce anything then, as having it inline or not when only property depends on the situation.

The second issue is that it also applies to object destructuring, where having multiple properties inline is quite ok. const {foo, bar} = unicorn();.

[jfmengels]

I kind of like being able to specify quick objects on one line when the number of keys is relatively small.
Another case where this is nice and this rule gets in the way IMO is when you create objects with the ES6 same-name shorthand (don't know the name of this)

const foo = 1, bar = 2;
const object = {foo, baz};

I'm not currently a big fan of this rule at the moment.

[sindresorhus]

Official name: Object Literal Property Value Shorthand

[SamVerschueren]

I agree with @jfmengels. I have even disabled it on some files at work because of those "issues". It would've been nice if we could define a maximum number of props in an object that are allowed on one line.

[sindresorhus]

But consistency and readability! :) Isn't readability better than slight convenience? I applied XO on many projects today and a lot of code became clearer when the object properties got on separate lines. That also improves diffing and reduces chances of merge conflicts. That being said, I'm open to reconsider if everyone feels it's wrong, but please try it out a little bit first. It might just be something you need to warm up to.

when the number of keys is relatively small.

How do we enforce "relatively small"? What number is that?

Can you guys provide some real-world examples of it getting in the way?

Btw, I know it's confusing, but this rule is actually different from the one that enforces multiple properties on new lines. See: https://github.com/sindresorhus/eslint-config-xo/commit/7ff915813633977ae1e2c8c96a1d3dfbffdaa191 and https://github.com/sindresorhus/eslint-config-xo/pull/23

[jfmengels]

please try it out a little bit first. It might just be something you need to warm up to

Sure, no problem with trying it out.

when the number of keys is relatively small.

How do we enforce "relatively small"? What number is that?

If I had to write the rule now, I'd go for a character length limit (meaning the number of characters between { and } would need to < X), but frankly you'd need a stronger heuristic between number of characters, whether they use literal property value shorthand and whether the properties have long names.

foo({a: 1, b: 2}) // nice and readable IMO (at least readable enough)
foo({
  a: 1,
  b: 2
}) // Wouldn't do it, but I get that it could be considered more readable
foo({
  someProperty: Object.assign(foo, {ok: 2}),
  someProperty2: Object.assign(foo, {ok: 3})
}) // Definitely would do it
foo({a: 1, b: 2, c: 3, d: 4}) // starting to get crowded.
foo({a, b, c, d}) // Reasonably readable
foo({
  a,
  b,
  c,
  d
}) // Definitely overkill

foo({prettyLongPropertyName1, prettyLongPropertyName2}) // readable
foo({
  prettyLongPropertyName1,
  prettyLongPropertyName2
}) // might start considering it, but probably wouldn't do it on my own
foo({prettyLongPropertyName1, prettyLongPropertyName2, prettyLongPropertyName3}) // getting crowded...?
foo({prettyLongPropertyName1: someValue, prettyLongPropertyName2: foo(a, b, c), prettyLongPropertyName3: prettyLongPropertyName3}) // (╯°□°）╯︵ ┻━┻

tl;dr: I'm far from having hard science and hard opinions here. Don't mind trying it out a bit. I'd say that minProperties: 2 would be a nice compromise between these cases.

[sindresorhus]

Let say we allowed two properties. I too have done a lot of foo({a: 1, b: 2}). Don't you think users would abuse it and do this all the time?

foo({someProperty: Object.assign(foo, {ok: 2}),someProperty2: Object.assign(foo, {ok: 3})})

Linting is strict for a reason. Sometimes it affects convenience, but in the long run it's better for everyone. In addition to readability and diffing benefits, I must mention refactoring/changing. It's a lot easier to delete and add properties when they're on separate lines.

For background: I've been thinking about this rule for a long time, as I was unsure about it too, but I've had so many PRs with cluttered code from inline objects that I decided to pull the trigger now.

See https://github.com/eslint/eslint/issues/6211 regarding ignoring object shorthands. I agree it doesn't make sense to put those on separate lines.

I'd say that minProperties: 2 would be a nice compromise between these cases.

The object-property-newline doesn't support a minProperties option. It's either all or nothing. (Again, not to be confused with object-curly-newline which does support such option.

[jfmengels]

See eslint/eslint#6211 regarding ignoring object shorthands. I agree it doesn't make sense to put those on separate lines.

Ah cool!

I've had so many PRs with cluttered code

When it comes to linting for submitted PRs, you have a lot more experience and frustration than me ;)

Let's try it out, but I'd love to see eslint/eslint#6211 come to fruition to cut a break for object shorthand.

[sindresorhus]

Ok, I've disabled the rule until we decide on something here and https://github.com/eslint/eslint/issues/6211 is done.

[SamVerschueren]

It might just be something you need to warm up to.

When I started using XO, I had to change a lot of my previous practices. It's all frustration in the beginning, but at the end, you notice the beauty of all the strict rules. So yes, it's perfectly possible that we/I have to getting used to this rule.

[sindresorhus]

See: https://github.com/eslint/eslint/issues/6251

[sholladay]

Looks like object-curly-newline may have grown up since this issue was opened. You can configure it to have different behavior for object literals, destructuring, imports, and exports. And it no longer has the behavior Sindre described of forcing object literals with one property to always be one way or the other. minProperties does what you'd expect, it ignores things that have fewer properties, which is great.

I've been very happy with this config:

https://github.com/sholladay/eslint-config-tidy/blob/d7b980ac5c86d530f093bcc923143a277a70d6e5/index.js#L102-L123

'object-curly-newline'       : ['error', {
    ObjectExpression : {
        multiline     : true,
        minProperties : 2,
        consistent    : true
    },
    ObjectPattern : {
        multiline     : true,
        minProperties : 4,
        consistent    : true
    },
    ImportDeclaration : {
        multiline     : true,
        minProperties : 4,
        consistent    : true
    },
    ExportDeclaration : {
        multiline     : true,
        minProperties : 2,
        consistent    : true
    }
}],

I wrote some tests to prove that it behaves nicely:
https://github.com/sholladay/eslint-config-tidy/blob/e0fbf554ae22937d7c8bbc49b789ae23b8a385d7/test.js#L34-L82

[sindresorhus]

I tried your above config now and --fix changes:

Object.assign(stream, {isTTY: true, columns, rows});

to

Object.assign(stream, {
    isTTY: true, columns, rows
});

[sholladay]

If you want to allow one-line object literals for objects with three properties, then change the ObjectExpression.minProperties to 4.

[sindresorhus]

No, I meant, it should have fixed it to:

Object.assign(stream, {
    isTTY: true,
    columns,
    rows
});

[sholladay]

This rule only affects the newlines for the curly braces, not the properties. There is a separate rule for the latter.

Edit: Nevermind, I see that was already discussed. I guess the real problem is object-property-newline not supporting minProperties. Is there anything preventing adding object-curly-newline, though?

[fregante]

A couple of ideas:

Ok

{name, email, subject, sign} // Only keys
{name: 'Alpha', email: '[email protected]'} // Not a long line

Not ok

{name, email, subject, sign: true} // Not "only keys" anymore
{name: 'Alpha the son of Thor Junior and Laquisha', email: '[email protected]'} // Line is too long

[sindresorhus]

Is there anything preventing adding object-curly-newline, though?

PR welcome :)