Eslint: New rule: destructuring-property-newline

[morgs32]

Please describe what the rule should do:
It would function almost exactly like object-property-newline except on destructured variables. Much of the below is an adaptation of that rule.

It destructure lots of variables at once like so:

const { foo, bar, baz, abc, xyz, foobar, foobaz, bazfoo, barfoo, abcxyz, xyzabc, foobarbaz, bazbarfoo } = foobarbazabcxyz;
the line length can get pretty long. Readability decreases. I think it could be useful if there was a rule that detected the amount of characters in a line or the amount of variables being destructured. If that number happens to be too large (not sure what the right number here is), throw a warning/error suggesting a multiline destructuring like so:

const {
  foo,
  bar,
  baz,
  abc,
  xyz,
  foobar,
  foobaz,
  bazfoo,
  barfoo,
  abcxyz,
  xyzabc,
  foobarbaz,
  bazbarfoo
} = foobarbazabcxyz;

Is this rule preventing an error or is it stylistic?

What category of rule is this? (place an "X" next to just one item)

[x] Enforces code style
[ ] Warns about a potential error
[ ] Suggests an alternate way of doing something
[ ] Other (please specify:)

Provide 2-3 code examples that this rule will warn about:

Examples of incorrect code for this rule:

/*eslint object-property-newline: "error"*/
const { foo, bar, baz, abc, xyz, foobar, foobaz, bazfoo, barfoo, abcxyz, xyzabc, foobarbaz, bazbarfoo } = foobarbazabcxyz;

Examples of correct code for this rule:

const {
  foo,
  bar,
  baz,
  abc,
  xyz,
  foobar,
  foobaz,
  bazfoo,
  barfoo,
  abcxyz,
  xyzabc,
  foobarbaz,
  bazbarfoo
} = foobarbazabcxyz;

Options

This rule has an object option:

• "allowMultiplePropertiesPerLine": true allows all keys and values to be on the same line

allowMultiplePropertiesPerLine

Examples of additional correct code for this rule with the { "allowMultiplePropertiesPerLine": true } option:

/*eslint object-property-newline: ["error", { "allowMultiplePropertiesPerLine": true }]*/

var { foo: "foo", bar: "bar", baz: "baz" } = obj

var {
    foo: "foo", bar: "bar", baz: "baz"
} = obj2

Why should this rule be included in ESLint (instead of a plugin)?
For the reasons we have the rule (object-property-newline)
[https://github.com/eslint/eslint/pull/5933], which draws from (JSCS requireObjectKeysOnNewLine)[http://jscs.info/rule/requireObjectKeysOnNewLine.html]

To quote the reasons for object-property-newline:

While formatting preferences are very personal, a number of style guides require that object properties be placed on separate lines for better readability.

Another argument in favor of this style is that it improves the readability of diffs when a property is changed:

[morgs32]

@platinumazure don't mind if I tag you? I am actually excited to try my hand at the PR. This weekend... mostly ripping off the object-property-newline rule. Whaddya think.

[platinumazure]

@morgs32 You are welcome to start work on a PR this weekend, or at any time! However, I wanted to draw your attention to our rule change acceptance process, which requires that 4 team members are on board with the proposal: One "champion", and three other people who :+1: the proposal (and nobody who :-1:s the proposal).

I'll champion this, so that's one less thing to worry about. But we won't accept/merge any PRs until the issue is accepted. We like to advise contributors to consider carefully whether they want to work on a PR before an issue is accepted, on the (small) chance that the issue might ultimately be closed due to insufficient support from the team. That said, even if we were to close the issue, your work in creating a PR could be useful because you could use this as a basis for a custom rule in your project, so you would be able to get what you need.

Of course, part of my job as champion is to try to get this accepted by the team, so. :smile:

[platinumazure]

@eslint/eslint-team: This should be a pretty simple stylistic rule for us to add, and it covers a core functionality gap (no way to enforce object destructuring style).

The proposal is to create a rule destructuring-property-newline which would be very similar to object-property-newline, but cover object destructuring.

I don't think it's a good idea to augment object-property-newline to handle destructuring because object properties and destructuring properties are syntactically and semantically different.

Can we get some :+1:s for the issue?

[not-an-aardvark]

¯\_(ツ)_/¯ I'd be in favor of enhancing object-property-newline, personally, given that we already handle destructuring in object-curly-newline under basically the same premises. But I don't feel strongly about it either way.

Does this rule only apply to object destructuring, and not array destructuring?

[kaicataldo]

I'd also be in favor of modifying object-property-newline instead of creating a whole new rule. Despite the naming convention is a bit weird (I believe these rules were created before we had destructuring in the language), I think it makes sense to stay consistent.

If we extend this to arrays, I'd also be in favor of modifying array-element-newline and array-bracket-newline, respectively.

[morgs32]

@platinumazure I started the PR but am stalling out on some of the tests, specifically the ones of import statements.
I'm not sure if we ever reached consensus. But I figured what the hell.
https://github.com/eslint/eslint/pull/9563

[ZooeyMiller]

Are there any updates on the status of this issue? The PR was last updated a few months ago, I think this is a really useful rule which would be really worthwhile, I'd be happy to have a look at the comments on the PR and help any way that I'm able, if there's still interest in this rule existing.

[PaulInglis]

+1 for this.

Adding an option to ensure the order is alphabetical would also be awesome!

[nzakas]

@platinumazure it looks like you championed this rule but it has not received enough support to be accepted.

@PaulInglis Please don't leave +1 comments, as it triggers notifications to everyone. You can use :+1: on the issue description to indicate your support.

[flux627]

It seems like this has been implemented (?)

When I add this to my .eslintrc.json, I no longer have errors on when destructuring:

    "object-curly-newline": [
      "error", {
        "ObjectExpression": "always",
        "ObjectPattern": {"multiline": false}
      }
    ]

Unless I'm misunderstanding this issue?

[subwai]

Could there be a rule for minProperties?
We would like small deconstructs such as:

const { foo, bar } = baz;

to stay one-line, while:

const { foo, bar, foobar } = baz;

should turn into:

const {
    foo,
    bar,
    foobar
} = baz;

[sergeysova]

Have any progress here? @platinumazure @not-an-aardvark

[kaicataldo]

@platinumazure It looks like there's team support for modifying object-property-newline instead of making a new rule. Can we update this proposal and then see if we get support from the team for that?

[neiker]

Hey, any progress? If there's any PR where I can help just say it :)

[morgs32]

@neiker I started on one here if it's worth using. https://github.com/eslint/eslint/pull/9563

[kaicataldo]

We don't have a concrete proposal at this point (see https://github.com/eslint/eslint/issues/9259#issuecomment-378972323). I'll try to whip something up today but if someone beats me to it, please feel free!

[pawelkleczek]

Is there any update about this feature?

[TSMMark]

Would love to see this added. It would help reduce git conflicts if you have a large line of imports, for example:

import { clone, constant, every, filter, flatten, map, mapValues, pickBy, reduce } from 'lodash'

If one contributor adds compact and another adds findIndex, for example, you would get a conflict, whereas on multiple lines you would get no conflict.

Would love to be able to enforce this in our codebase. I've contributed eslint plugins before like react/jsx-one-expression-per-line — let me know if I can help at all, or add documentation for an existing PR or anything.

[kaicataldo]

Sorry, I lost track of this and don't have time at to follow up right now. @platinumazure I believe we still need a concrete proposal here.

[coderaiser]

multiple-properties-destructuring of eslint-plugin-putout can be used for this purpose. It turns:

const {foo, bar, foobar} = baz;

into:

const {
    foo,
    bar,
    foobar
} = baz;

It is an addition to putout linter/code transformer that fixes everything it can find.

[timkraut]

I could probably invest some time to build this feature. Is the preferred approach still to modify object-property-newline? Do you want to have a new option for this?

[morgs32]

@timkraut there's some work done here: https://github.com/eslint/eslint/pull/9563
If you want to use it.

[kaicataldo]

Before we can move forward, we still need a concrete proposal that details the change to object-property-newline (and we should decide if we also want to do something for array destructuring). Once we have that, the team can discuss and vote on it.

[mdobroggg]

It would be great if the rules were configurable similar to https://eslint.org/docs/rules/object-curly-newline so that people could decide on how they want this rule to be enforced on their project.

We are running into this on our team and would love to have the option to add a rule to enforce and also --fix any current issues with it.

[platinumazure]

I'm not sure I understand why we want to focus on modifying object-curly-newline. I still think a new rule would make more sense, especially if we also want to handle array destructuring there as well.

@eslint/eslint-team Are we still looking for a concrete proposal here?

[ljharb]

Realistically, what’s needed is a generic rule for newlines in any curly braced structure, that allows configuring by import/destructuring/etc.

[loren138]

@platinumazure @kaicataldo
I'm not sure if a new issue or a comment would be best for a concrete proposal. If a new issue is best, I'm happy to open one.

Modifications to include object destructure/import/export in the current object-property-newline rule would require strange defaults or waiting for ESLint 7. As such, I propose we deprecate object-property-newline and create a new rule curly-property-newline.

I would like to propose that the options for curly-property-newline match those of object-curly-newline.

curly-property-newline has 1 main option (allowAllPropertiesOnSameLine)

But it also allows specifying for each of 4 subgroups:

You can specify different options for object literals, destructuring assignments, and named imports and exports:

{
    "curly-property-newline": ["error", {
        "ObjectExpression": { "allowAllPropertiesOnSameLine": true },
        "ObjectPattern": { "allowAllPropertiesOnSameLine": true },
        "ImportDeclaration": { "allowAllPropertiesOnSameLine": false },
        "ExportDeclaration": { "allowAllPropertiesOnSameLine": true }
    }]
}

"ObjectExpression" configuration for object literals
"ObjectPattern" configuration for object patterns of destructuring assignments
"ImportDeclaration" configuration for named imports
"ExportDeclaration" configuration for named exports

A shorthand config of

{
    "curly-property-newline": ["error", { "allowAllPropertiesOnSameLine": true }]
}

parses to

{
    "curly-property-newline": ["error", {
        "ObjectExpression": { "allowAllPropertiesOnSameLine": true },
        "ObjectPattern": { "allowAllPropertiesOnSameLine": true },
        "ImportDeclaration": { "allowAllPropertiesOnSameLine": true },
        "ExportDeclaration": { "allowAllPropertiesOnSameLine": true }
    }]
}

Default config would be:

{
    "curly-property-newline": ["error", {
        "ObjectExpression": { "allowAllPropertiesOnSameLine": false },
        "ObjectPattern": { "allowAllPropertiesOnSameLine": false },
        "ImportDeclaration": { "allowAllPropertiesOnSameLine": false },
        "ExportDeclaration": { "allowAllPropertiesOnSameLine": false },
    }]
}

If we were to modify the existing rule, default config would need to be:
(which is why I'm proposing a new rule to support the more sensible defaults above without a backwards breaking change)

{
    "curly-property-newline": ["error", {
        "ObjectExpression": { "allowAllPropertiesOnSameLine": false },
        "ObjectPattern": "off",
        "ImportDeclaration": "off",
        "ExportDeclaration": "off"
    }]
}

(where off is a string option to disable to check for that type of object)

[ljharb]

Imports and exports don’t have properties, so perhaps another name? curly-brace-newline or so?

it’d be ideal if it also covered if, for, while, do, just blocks (like standalone curlies), function bodies, class bodies, etc, to be truly generic.

[loren138]

@ljharb This rule would not be for the newlines of the braces themselves so much as for the items within the brace (as such it would not be replacing object-curly-newline just object-property-newline)

For ifs and functions, you're looking for the brace-style rule: https://eslint.org/docs/rules/brace-style

I am, however, open to other names of the rule.

[ljharb]

aha, ok - maybe curly-brace-contents-newline? Naming is hard.

[jsphstls]

newline-curly-contents ?

[v1z]

any news about this rule?

[kaicataldo]

@platinumazure Looks like you're championing this change. How should we move forward? I'm :+1: to the proposal outlined here, pending the name (this one is tough - I'm not sure what we should call it either).

[handeyeco]

Would love to see this added. It would help reduce git conflicts if you have a large line of imports, for example:

import { clone, constant, every, filter, flatten, map, mapValues, pickBy, reduce } from 'lodash'

Like @TSMMark mentioned, it would be very nice for imports.

I imagine it would also work with function params too? Something like:

function SomeComponent({ a, b, c = 10 }) {
  // ...
}

function SomeComponent({
  a,
  b,
  c = 10
}) {
  // ...
}

Figure it would handle all destructuring the same, but I thought I would mention it none-the-less. 🤷‍♀️

[Pyrolistical]

we need this rule as there's nothing to handle mixed breaks in destructuring

Bad ❌

const {
  start, waitForStart,
  stop
} = require('./server');

Good ✔️

const {
  start,
  waitForStart,
  stop
} = require('./server');

[TSMMark]

@ljharb Since ES module imports and destructing are actually two different language features, would we be better off breaking this into two distinct rules? Or are we all in agreement that the two syntaxes are similar enough that they should be bundled under one rule?

[ljharb]

One rule but controlled separately by options seems sensible.

[ljharb]

(with a more generic rule name, ofc)

[TSMMark]

That sounds good to me if we can land on a generic name that makes sense. Is there a common term to describe both named imports and destructured assignments? I'm not aware of any and a quick search did not yield much for me. My favorite proposal so far is newline-curly-contents but that may be too vague, as object-property-newline is already its own rule and curlies are used frequently in JS syntax, obviously.

Also I'm not sure if this is a real concern, but the term "curly" backs the rule into a corner if we wanted to support enforcing newline for array destructuring assignment as well, e.g.

const [
  index,
  setIndex,
] = useState(0)

[ljharb]

Probably a single rule, called, say, list-newline, that can potentially govern all forms of (comma-separated) lists in the future?

function signatures and calls, object declarations and destructurings, named imports and exports, array declarations and destructurings, comma operator statements - did i miss anything?

[g-plane]

@ljharb I think that is off-topic. You can create a new issue to propose that.

[ljharb]

@g-plane i don’t think discussing a broader generalization of the rule under discussion is off topic, nor do i think it makes sense to file a new issue with the intent of making this one obsolete.

[kaicataldo]

@g-plane I don't think this discussion is off topic. I think it's important to discuss our long-term strategy for enforcing this kind of pattern.

@ljharb Not sure about the name, but I like the idea of having a singular rule with granular configuration options! In general, I'm of the mind that we should combine these more generic styling rules into one rule, because I think the trade off of more complex configuration is worth it to not have to keep track of multiple rules (which can also be added/removed). Some relevant discussion around that (regarding other rules) can be found here.

[Pyrolistical]

@ljharb I like your idea, but its make me wonder if this should just be part of comma-style, or should we should also eat comma-style as list-style

[kaicataldo]

The style in question is about new lines between comma separated list items. I don’t think that should be tied to the location of commas.

[ljharb]

I think “lists” should be the general concept, regardless of the name (not commas, altho lists are comma-separated)

[shermendev]

Any updates on the issue?

[TaylorAbraham]

Would love to see updates on this, it's been bothering me a lot. Can the community help with this any way?

[kaicataldo]

@platinumazure is no longer on the team and so we don't have a champion at this point. Unless someone from the team wants to champion this, given how old this proposal is, I think it's time to close it.

[ljharb]

It would be very helpful if someone was willing to champion, especially a broader/more general rule for covering all forms of comma-separated lists.

[anikethsaha]

I think it's better to go with a separate rule which will cover both for object and array .

if someone is championing this, I would like to give it a try and implement this 👍

[ljharb]

@anikethsaha the problem is that there's many more categories than "object" and "array". There's also "function arguments", "named imports", "named exports", "comma operator"; for both "object" and "array" there's "literal" and "destructuring". There's probably more I didn't think of just yet.

A single rule is the only thing that makes sense and is extensible without littering eslint core with more rules.

[anikethsaha]

what I meant, was to have a separate rule having both ( originally and others which you mentioned ) use cases in it rather than an option.
But either of them looks fine as object-property-newline does have a similar purpose.

[ljharb]

Ah, then I wholeheartedly agree :-) a new rule, that could replace object-property-newline, and add the granular (options) ability to control each kind of thing separately, would be ideal.

[electrovir]

Note that I couldn't get the package suggested from last year to flag or fix destructuring 😕. So there doesn't seem to be an easy alternative for this right now.

[coderaiser]

@electrovir could you please provide an example of destructuring you couldn't get to fix :)?

[pcarn]

@coderaiser It doesn't seem to work for function params:

function SomeComponent({ a, b, c = 10 }) {
  // ...
}
should turn into:
function SomeComponent({
  a,
  b,
  c = 10
}) {
  // ...
}

[coderaiser]

@pcarn yep, for this case it doesn't work, and will not. I don't think that it's very readable. But you can always use code like:

function SomeComponent(params) {
    const {a, b, c = 10} = args;
    // ...
}

And this case will work as expected :). And transformed into:

function SomeComponent(params) {
    const {
        a,
        b,
        c = 10,
    } = args;
    // ...
}

[aladdin-add]

Thanks for your interest in improving ESLint. As of 2020, unfortunately, we have decided to only accept rules related to new ECMAScript features. We prefer that new rules be implemented in plugins, so I'm closing it.

Since ESLint is pluggable and can load custom rules at runtime, it doesn't need to be a blocker for you using this in your project, if you'd find it useful. It just means that you would need to implement the rule yourself, rather than using a bundled rule that is packaged with ESLint.