moxystudio / eslint-config

MOXY eslint configuration to be used across several JavaScript projects
MIT License
13 stars 5 forks source link

Re-evaluate our `indent` rule exceptions #80

Closed satazor closed 4 years ago

satazor commented 4 years ago

Currently, our indentation is 4 spaces, with a few exceptions:

        'indent': [2, 4, {
            'SwitchCase': 0,
            'MemberExpression': 0,
        }],

Re-evaluate these exceptions to see if we can remove them. As an example, because we are forcing MemberExpression to 0, the following code becomes unreadable:

const schema = {
    headers: Joi.object({
        'Content-Type': Joi
        .string()
        .pattern(/^application\/json\b/)
        .required(),
    }),
};

which would be much more readable like so:

const schema = {
    headers: Joi.object({
        'Content-Type': Joi
            .string()
            .pattern(/^application\/json\b/)
            .required(),
    }),
};

To circumvent the above issue, I would have to write it like so:

const schema = {
    headers: Joi.object({
        'Content-Type':
            Joi
            .string()
            .pattern(/^application\/json\b/)
            .required(),
    }),
};
satazor commented 4 years ago

Probably also related: we should lax newline-per-chained-call, which is has a max depth of just 2.

zebateira commented 4 years ago

Agreed 👌

Would also do it for SwitchCase, but maybe that's less standard.

ruipneves commented 4 years ago

Please do, the newline-per-chained-call is a pain in the *** as it is at the moment and it becomes strange when you have just 3 calls for example because one will be going to the next line. Either we define it as 1 and it all goes in a new line or we make it bigger

paulobmarcos commented 4 years ago

Yep, not only it becomes more readable but also seems more inline with what we are used to do and what our current rules are enforcing.

acostalima commented 4 years ago

Please do, the newline-per-chained-call is a pain in the *** as it is at the moment and it becomes strange when you have just 3 calls for example because one will be going to the next line. Either we define it as 1 and it all goes in a new line or we make it bigger

Yeah, I must admit the above is quite annoying in my opinion...

zebateira commented 4 years ago

Please do, the newline-per-chained-call is a pain in the *** as it is at the moment and it becomes strange when you have just 3 calls for example because one will be going to the next line. Either we define it as 1 and it all goes in a new line or we make it bigger

@ruipneves but you can put all of them inline still right? (https://eslint.org/docs/rules/newline-per-chained-call) Or you mean it sucks because the autofix puts it 2 (first lines) + 1 (next line)? Do agree that max depth of 2 is weird - only one or everything in each new line sounds better @ruipneves 👌

ruipneves commented 4 years ago

It sucks because of the new line 😛

array.filter().map()

.filter()
zebateira commented 4 years ago

@ruipneves yeah, what I mean is you can still put them in each line right?

array
    .filter()
    .map()
    .filter()

this would be valid right? (a part from indentation)

ruipneves commented 4 years ago

Yes, at the moment that is valid as well. The real problem is when you want everything in a one liner having 3 calls

hugomrdias commented 4 years ago

Wouldn't prettier solve all the formatting issues and simplify the config. Feel free to ignore me as I'm not very familiar with the repo.

satazor commented 4 years ago

@hugomrdias Prettier does have have an overlap with eslint in regards to formatting, especially now that eslint has autofix. Still, eslint does much more than just formatting, which is *linting.* As an example, it detects errors based on static analysis that prettier doesn't do.

@ritazoliveira do you want to take on this? So overall, we need to:

This is a breaking change, so the commits should reflect that. It might be worth making these as separate PRs.

hugomrdias commented 4 years ago

@satazor yes I know all that, just saying leave the formatting to prettier and linting to eslint. This setup is pretty standard even with styles (prettier/style lint).

acostalima commented 4 years ago

I'm not familiar with prettier, but if eslint does as much as prettier and even more, what would be the point to maintain a prettier config as well?

ritazoliveira commented 4 years ago

Both PR's are now open for review.

satazor commented 4 years ago

Addressed by #83 and #82