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

Disable rule suggestions #8

Closed
dferber90 opened this issue Feb 17, 2017 · 10 comments
Closed

Disable rule suggestions #8

dferber90 opened this issue Feb 17, 2017 · 10 comments

Comments

@dferber90
Copy link

dferber90 commented Feb 17, 2017

In a fairly large codebase I've found these rules to be unnecessary/conflicting when using prettier:

no-spaced-func

Prettier fixes callback(/* foo */) to callback /* foo */() at which point the rule warns.

no-confusing-arrow with allowParens option

/*eslint no-confusing-arrow: ["error", {"allowParens": true}]*/
/*eslint-env es6*/
var x = a => (1 ? 2 : 3);
var x = (a) => (1 ? 2 : 3);

Prettier removes the brackets which would avoid the error.

react/wrap-multilines

No example here.

no-unexpected-multiline

Prettier transforms

wrapper.find('FilterBy').prop('activeFilters')[1].filters.map(_ => _.key),

to

        wrapper
          .find('FilterBy')
          .prop('activeFilters')
          [1].filters.map(_ => _.key),

At which point the rule warns. But I'd maybe keep it as this can be solved by extracting the stuff into a variable. Wanted to mention it anyways.

Any thoughts on disabling these?

@lydell
Copy link
Member

lydell commented Feb 17, 2017

Thank you for the issue!

no-spaced-func and react/wrap-multilines

These rules are deprecated. So far, I’ve not added any deprecated rules. I’m not sure which way is the best: Go through all deprecated rules, or document that deprecated rules won’t be added. What do you think?

no-confusing-arrow

So far, no rule options are checked. I’m not sure if we want to do that.

We could add it as a new “special rule”, and document that the allowParens option is not a good idea. But since that option isn’t enabled by default, it might do more harm than good? Perhaps it is better to document that eslint-config-prettier isn’t 100% bullet proof and mention this rule?

I wonder if there are any other rules where one might want to check for rule options, rather than the entire rule.

no-unexpected-multiline

I have always considered this rule to be very useful, since it catches those nasty semicolon mistakes. But on the other hand, I guess prettier shows such errors implicitly. For example:

console.log("test")
["a", "b", "c"].forEach(doSomething)

The above is turned into the following by prettier:

console.log("test")[("a", "b", "c")].forEach(doSomething);

That reveals the semicolon mistake just as much as the eslint rule, doesn’t it?

So I guess we could safely add this rule. Perhaps as a “special rule”?

EDIT: Very much related: prettier/prettier#775

@lydell
Copy link
Member

lydell commented Feb 22, 2017

@dferber90 I’d love to hear your thoughts!

@dferber90
Copy link
Author

@lydell I've not forgotten about this, I have it on my todo-list. I have been super busy (flew from Munich to Rio on the weekend) and getting set up here took a bit. I'll get back to this!

@jacobp100
Copy link

Thought I'd add here rather than a new issue.

I'm using airbnb-base. I need no-confusing-arrow turned off, as prettier removes the extra parens required to get this rule to pass. I also need spaced-comment turned off for flow annotation comments!

@lydell
Copy link
Member

lydell commented Feb 23, 2017

Thanks for the feedback, @jacobp100!

Regarding no-confusing-arrow, you could also keep it on and turn the allowParens option back to the default value of false (docs). That's the crux of the issue. But I'm starting to feel like eslint-config-prettier should just turn it off and document it as one of the "special rules".

Regarding the spaced-comment rule, I don't see how it has anything to do with prettier. Could you clarify? :)

@jacobp100
Copy link

Oh I see about the parens now—I didn't realise it was so strict by default. And thinking about it, you're right about the spaced comments too! 😂

I didn't realise that there would need to be extra config on top of this—but thinking about it, for some rules you'll always need to decide yourself whether you want to keep it!

@lydell
Copy link
Member

lydell commented Feb 23, 2017

I think I have made up my mind:

lydell added a commit that referenced this issue Feb 26, 2017
There's nothing to lose here, only to win. Refs. #8.
lydell added a commit that referenced this issue Feb 26, 2017
@lydell
Copy link
Member

lydell commented Feb 26, 2017

Should be fixed in 1.4.0.

@lydell lydell closed this as completed Feb 26, 2017
@dferber90
Copy link
Author

Thank you for taking over, the decisions made sound good!

@yaminyaylo
Copy link

yaminyaylo commented Jan 21, 2019

As a solution you can use "prettier.printWidth": 100(or more) setting to extend line

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

No branches or pull requests

4 participants