stylelint / eslint-config-stylelint

Stylelint org's shareable config for eslint
MIT License
62 stars 9 forks source link

Remove `no-confusing-arrow` because of the deprecation of formatting rules #276

Closed ybiquitous closed 4 months ago

ybiquitous commented 4 months ago

Which issue, if any, is this issue related to?

None.

Is there anything in the PR that needs further explanation?

First, this rule was deprecated in ESLint v8.53.0. The alternative is the third-party plugin @stylistic/eslint-plugin-js.

Second, I sometimes feel annoyed by this rule. For example, see:

const result = () => (type === 'foo' ? foo() : bar());
/*             ^^ Arrow function used ambiguously with a conditional expression [no-confusing-arrow] */

In this case, I think it's readable enough, thanks to () around the function body. So, I think this rule is no longer necessary for this shared config.

Ref:

[!NOTE] This rule was added in #74.

Mouvedia commented 4 months ago

My problem is that it conflicts with prettier.

thanks to () around the function body

There's an option for that: { "allowParens": true }

ybiquitous commented 4 months ago

@Mouvedia Can you tell me how to reproduce the conflict with Prettier?

Mouvedia commented 4 months ago

Try

            const fix = () =>
                (messageType === 'expected' ? fontFamilyNode.addQuotes() : fontFamilyNode.removeQuotes());

It will be reformatted.

ybiquitous commented 4 months ago

@Mouvedia Sorry, I didn't understand what you meant. The example code you posted is a problem of Prettier, not no-confusing-arrow, right?

Mouvedia commented 4 months ago

If you were using { "allowParens": true } instead of removing the rule then it would conflict with prettier because prettier removes the parentheses. If you remove the rule, it's irrelevant.

ybiquitous commented 4 months ago

Ah, I now understand. Assuming the following example:

/*eslint-disable no-unused-vars, no-undef*/

/*eslint no-confusing-arrow: ["error", {"allowParens": true}]*/

const fix = () =>
    (messageType === 'expected' ? fontFamilyNode.addQuotes() : fontFamilyNode.removeQuotes());

Prettier removes (...) from the arrow function body:

const fix = () =>
-   (messageType === 'expected' ? fontFamilyNode.addQuotes() : fontFamilyNode.removeQuotes());
+   messageType === 'expected' ? fontFamilyNode.addQuotes() : fontFamilyNode.removeQuotes();

Then, ESLint emits an error:

const fix = () =>
/*          ^^ Arrow function used ambiguously with a conditional expression */
    messageType === 'expected' ? fontFamilyNode.addQuotes() : fontFamilyNode.removeQuotes();

By the way, if an arrow function is within a single line, Prettier doesn't format the code:

const fix2 = () => (type === 'expected' ? addQuotes() : removeQuotes());

In summary, I think removing no-confusing-arrow is better to prevent any confusion.

Mouvedia commented 4 months ago

By the way, if an arrow function is within a single line, Prettier doesn't format the code

Good to know.

ybiquitous commented 4 months ago

I'm merging this PR since there are no objections.