Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation conflicts with object-property-newline setting? #2007

Open
jabacchetta opened this issue Feb 14, 2019 · 5 comments
Open

Documentation conflicts with object-property-newline setting? #2007

jabacchetta opened this issue Feb 14, 2019 · 5 comments

Comments

@jabacchetta
Copy link
Contributor

jabacchetta commented Feb 14, 2019

It seems like the documentation consistently promotes having a new line for each item in multiline lists, whether it be in a function invocation or in an array (both of which mention bad examples of not having each item on a new line).

And even the object examples (although not referring to this specific rule) seem to favor this approach.

With that said, what is the reasoning behind allowing object properties to all appear on the same line?

It would seem more consistent to remove that option for object-property-newline. But at the very least, the reasoning and/or use case should probably be documented.

@ljharb
Copy link
Collaborator

ljharb commented Feb 15, 2019

The guideline is, if the entire construct can fit on one line, it should be on one line; if not, each item should be on its own line.

In other words, for any of { … }, ( … ), [ … ].

@jabacchetta
Copy link
Contributor Author

jabacchetta commented Feb 15, 2019

Actually, after rereading the ESLint documentation, it looks like I misunderstood the allowAllPropertiesOnSameLine option. I didn't realize setting it to false would also mark the line as incorrect if the properties were on the same line as the curly braces.

So it looks like ESLint doesn't have the configuration available to make the following happen:

// correct
const obj = { foo: "foo", bar: "bar", baz: "baz" };

// correct
const obj2 = {
    foo: "foo",
    bar: "bar",
    baz: "baz"
};

// incorrect, but ESLint allows it with `allowAllPropertiesOnSameLine` set to true
const obj3 = {
    foo: "foo", bar: "bar", baz: "baz"
};

According to what I thought the Airbnb style guide promoted (and what I believe you're confirming) is that obj3 is incorrect, since it spans three lines (multiline), but does not place each item on its own line?

But unfortunately we can't currently force the error to be picked up by ESLint (not without having obj1 become a false positive).

@ljharb
Copy link
Collaborator

ljharb commented Feb 15, 2019

Is there not a different rule that complains about that one? If not, then we should propose one to eslint :-)

@jabacchetta
Copy link
Contributor Author

It looks like array-element-newline needs a new ESLint rule as well (to handle the same exact case for arrays). And then on the same topic of multiline, this issue applies as well, requiring another ESLint rule.

I'll go ahead and submit feature requests for all of the above when I have the time to work on the pull request for the implementation of those rules.

@axten
Copy link

axten commented Sep 5, 2019

you mean feature request like this?
eslint/eslint#11620

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

No branches or pull requests

4 participants
@ljharb @axten @jabacchetta and others