What rules do the community tend to override?

[ljharb]

I'm opening this issue specifically to gather a list of the rules that those in the community feel are controversial or problematic, such that they override them, and most importantly, why.

This is not an invitation for "+1s" (those belong as reactions on individual posts), or an invitation to hear about everyone's subjective aesthetic preferences, nor is it an indication that any of these rules necessarily will - or will not - change. I'm simply hoping to primarily gather civil, well-reasoned explanations for why certain rules are overridden, disabled, etc.

Any comments mentioning spaces vs tabs, or semicolons, will be summarily deleted :-)

We're aware of the following:

• react/jsx-filename-extension - filed as Proposal: [react][breaking] Only .js files should contain JSX. #985. Why is it controversial? Some believe that .js files should be allowed to contain JSX. Others believe this is an app-level concern, and does not belong in a shared config (like "env" settings).
• import/no-extraneous-dependencies: in test directories, for example, it needs to be overridden to the following (note: if/when eslint allows glob-based config, then we'd set up some common test directory patterns so you wouldn't have to do this manually) eslint-config-airbnb v13.0.0 is now released, which resolves this.

  "import/no-extraneous-dependencies": ["error", {
    "devDependencies": true,
    "optionalDependencies": false,
  }],

• camelcase: when you're bound to the casing selected by APIs (What rules do the community tend to override? #1089 (comment), but Airbnb suffers from this too) This might also apply to new-cap and no-underscore-dangle.
• no-param-reassign: this happens frequently with a reduce where the entire operation is pure, but the reducer is not (ie, it mutates the accumulator, but the accumulator began as {} passed into the reducer). You can use Object.assign({}, accumulator, changes), or { ...accumulator, ...changes }, however. (per What rules do the community tend to override? #1089 (comment))
• no-use-before-define: some enjoy using hoisting to define helper methods at the bottom of the file. This guide discourages relying on hoisting; instead suggesting importing the helpers from another file when possible. (per What rules do the community tend to override? #1089 (comment))
• no-mixed-operators - specifically where it relates to arithmetic operators, where the PEMDAS rule applies. We're definitely considering loosening this rule as it applies to *, /, +, and -.

Any others? I'll update the original post with new examples as I'm made aware of them.

(Note: this does not cover env settings, like "browser", "node", "mocha", etc - these are app-level concerns, and as such, your .eslintrc, tests/.eslintrc, etc should be defining them)

[jonathanong]

• arrow-body-style or whatever, because it makes converting regular functions to arrow functions difficult as it's not fixed automatically.
• camelcase - because many times i'm bound by other APIs
• no-param-reassign - makes working with koa/express/node.js difficult
• no-plusplus - because i += 1 is annoying
• prefer-template - because it isn't convenient 5% of the time
• consistent-return - annoying with early returns
• new-cap - bound by other APIs
• no-underscore-dangle - disallows "private" properties. bound by mongodb.
• no-use-before-define - disallows function declarations at the bottom

[ljharb]

@jonathanong Thanks! I'd forgotten camelcase, we have that problem too. Re arrow-body-style, would the existence of an autofixer make it not be a problem?

Can you elaborate on when prefer-template isn't convenient?

[jonathanong]

arrow-body-style - not sure, because sometimes i'd prefer having the entire { return x } block when the function is multi-line so that it's easy to add logic before the return in the future.

when i have something like path + '?' + query.

`${path}?${query}`

just seems like more work

[ljharb]

Thanks - I've added a few of your examples to the OP. (and fixed your markdown, hope you don't mind)

[chadwatson]

no-use-before-define for functions. I personally don't like to scroll down to the bottom of a file to see what a module really does. It's inevitable that for some modules it just makes sense to leave the helper functions in the same file. I prefer to see the declarative stuff first instead of having to scroll through the imperative stuff first.

[steida]

https://github.com/este/este/blob/master/.eslintrc

// Soft some rules.
"global-require": 0, // Used by React Native.
"new-cap": [2, {"capIsNew": false, "newIsCap": true}], // For immutable Record() etc.
"no-class-assign": 0, // Class assign is used for higher order components.
"no-nested-ternary": 0, // It's nice for JSX.
"no-param-reassign": 0, // We love param reassignment. Naming is hard.
"no-shadow": 0, // Shadowing is a nice language feature. Naming is hard.
"import/imports-first": 0, // Este sorts by atom/sort-lines natural order.
"react/jsx-filename-extension": 0, // No, JSX belongs to .js files
"jsx-a11y/html-has-lang": 0, // Can't recognize the Helmet.
"no-confusing-arrow": 0, // This rule is super confusing.
"react/forbid-prop-types": 0, // Este is going to use Flow types.
"react/no-unused-prop-types": 0, // Este is going to use Flow types.
"class-methods-use-this": 0, // Good idea, but ignores React render.
"arrow-parens": 0, // Not really.

[ljharb]

@steida Thanks!

Can you give me an example of your no-class-assign usage?

Also, fwiw, new-cap has capIsNewExceptions and newIsCapExceptions so you could define things like Record etc without having to alter the individual booleans.

Why would you need to disable forbid-prop-types or no-unused-prop-types if you're using Flow? It seems like they'd just be a noop if you don't have any propTypes defined.

As for class-methods-use-this, I added an exceptMethods option to the eslint rule, so the next version of the config will be excluding all the React lifecycle methods (including render), so you'll be able to remove that override soon. (update v12 of the config now handles this)

[steida]

As for no-class-assign
https://github.com/este/este/blob/5571b8ab6722d5abd75984859292f5a5258c7151/src/browser/auth/Email.js#L155

As for forbid-prop-types and no-unused-prop-types
I remember I had to set it, why? I don't remember.

As for the rest.
Thank you.

[ljharb]

@steida thanks - for the prop types ones, please file bugs on eslint-plugin-react if you find you still need the overrides. for the no-class-assign one, it seems like you could do const InnerEmail = class Email { … } and then const FocusedEmail = focus(InnerEmail, 'focus'); etc?

[jonase]

The only custom rule we're using currently is

"new-cap": [2, {"capIsNewExceptions": ["Immutable.Map", "Immutable.Set", "Immutable.List"]}]

[ljharb]

@jonase I think those are common and legit enough that if you'd like to make a PR to the base config, I'd be happy to add them.