Javascript: 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 #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 (https://github.com/airbnb/javascript/issues/1089#issuecomment-249396262, 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 https://github.com/airbnb/javascript/issues/1089#issuecomment-249396262)
• 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 https://github.com/airbnb/javascript/issues/1089#issuecomment-249396262)
• 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.

[oliviertassinari]

From Material-UI:

'no-console': 'error', // airbnb is using warn
'no-return-assign': 'off', // airbnb use error, handy for react ref assign.
'operator-linebreak': ['error', 'after'], // aibnb is disabling this rule

'react/jsx-handler-names': ['error', { // airbnb is disabling this rule
  eventHandlerPrefix: 'handle',
  eventHandlerPropPrefix: 'on',
}],
'react/jsx-filename-extension': ['error', {extensions: ['.js']}], // airbnb is using .jsx
'react/jsx-max-props-per-line': ['error', {maximum: 3}], // airbnb is disabling this rule
'react/no-danger': 'error', // airbnb is using warn
'react/no-direct-mutation-state': 'error', // airbnb is disabling this rule

[bebraw]

I have set the following at Reactabular:

"rules": {
  "comma-dangle": ["error", "never"], // personal preference
  "prefer-arrow-callback": 0, // mocha tests (recommendation)
  "func-names": 0, // mocha tests (recommendation)
  "import/no-extraneous-dependencies": 0, // monorepo setup
  "import/no-unresolved": [1, { ignore: ['^reactabular'] }], // monorepo setup
  "no-underscore-dangle": 0, // implementation detail (_highlights etc.)
  "no-unused-expressions": 0, // chai
  "no-use-before-define": 0, // personal preference
  "react/sort-comp": 0, // personal preference
  "react/no-multi-comp": 0 // personal preference
}

[sompylasar]

"no-nested-ternary": 0,  // because JSX
"no-underscore-dangle": 0,  // because private properties
"arrow-body-style": 0,  // because cannot enforce what's personally preferred: 
  // (x) => ( some ? ( comple + x ) : ( e + x + pression ) )
  // (x) => some.trivialExpression()
  // (x) => { return ( <JSX /> ); }  // to add something later without re-indenting.

[broguinn]

At my company, we've overridden some rules that don't play nice with Mocha:

• arrow-body-style means before hooks can't access the all-important this
• no-unused-expressions makes expect(true).to.be.true; to cause a linting error

Also:

• prefer-rest-params is not supported in Node 4.x

[ljharb]

@oliviertassinari Thanks! Most of yours are strengthening rules we don't have on/as errors. For ref assign, ref callbacks don't have a return value, so instead of ref => this.ref = ref, the guide recommends you use (ref) => { this.ref = ref; } which doesn't collide with the rule.

@bebraw re no-underscore-dangle, JS doesn't have private properties :-/ hopefully you've read the recent guide section on them. re no-unused-expressions, see below.

@broguinn arrow-body-style in mocha tests does do that, but i've found that using this in mocha tests is an antipattern, and an unfortunate API choice of mocha itself. For the few times we need it, we just use a normal function and give it a name. Re expect(true).to.be.true, that's a hugely problematic pattern, because expect(true).to.be.yogurt passes too - it's a noop. We recommend expect(true).to.equal(true) or expect(!!truthy).to.equal(true); which can't silently fail. Re prefer-rest-params, the guide does recommend/assume you are using babel.

To all: regarding no-nested-ternary, I understand why a single ternary statement is useful in JSX. Can someone provide me a code example of why a _nested_ ternary is useful in JSX?

[relekang]

The interesting changes from our config is that we changed these to be able to use flowtype in a good way:

    "no-duplicate-imports": [0], // Does not support import of types from same file/module as is already used.
    "import/no-duplicates": [2], 
    "react/sort-comp": [2, {
      "order": [
        "type-annotations",  // sort type annotations on the top
        "static-methods",
        "lifecycle",
        "/^on.+$/",
        "/^(get|set)(?!(InitialState$|DefaultProps$|ChildContext$)).+$/",
        "everything-else",
        "/^render.+$/",
        "render",
      ],
    }],

You can find the config here. The other changes are mostly adding extra rules and disabling a few because we haven't always used airbnb as a base.

[bebraw]

@ljharb Yeah, I'm not using _ as private. It's a special case (convention) where I use it to signify a field with special meaning (i.e. something generated). I wouldn't go as far as to remove the rule from this preset.

[ljharb]

@relekang Thanks! re no-duplicate-imports, does Flow not support importing multiple things all in one import statement? import/no-duplicates has been enabled in our config since May, so you can safely remove it.

[relekang]

@ljharb I am not sure, but I have not found a way to combine

import {someFunction} from 'module';

and

import type {someType, someOtherType} from 'module';

into one line.

import/no-duplicates has been enabled in our config since May, so you can safely remove it.

Nice 👌 Need to read the changelog closer.

[ljharb]

@relekang if that's true, i'd file that as a bug on flow :-)

[devillexio]

Thanks for soliciting community feedback @ljharb!

We override the following rules (that fall outside of the aesthetic preference category):

• require-yield: 0 - Working with Koa.js, there are some cases where we don't yield from a generator.
• strict: [0, "global"] - We work mostly with ES6 modules, which are strict already.

[lelandrichardson]

i'd argue that accepting duplicate import paths when one is import type and another is just import should probably be filed as a corner case with the import eslint plugin. Seems like a valid use case to me.

[lencioni]

It looks like there was an issue to support import type https://github.com/benmosher/eslint-plugin-import/issues/225 which has been fixed https://github.com/benmosher/eslint-plugin-import/pull/334. The docs for the rule claim to support this, too, so I think you should be good to go!

There is an open issue about allowing exporting types from import/prefer-default-export: https://github.com/benmosher/eslint-plugin-import/issues/484

[relekang]

@ljharb @lelandrichardson @lencioni I did not mean to say that the rule from the import plugin did not work. I tried to say that the rule no-duplicate-imports from eslint itself gives what I would think of as false positives with flowtype imports. Is there a reason that both are in use here? It looks like the one from the import plugin covers the cases that the one from eslint does.

no-duplicate-imports is turned on in es6.js

[ljharb]

@relekang @lencioni @lelandrichardson since no-duplicate-imports is an eslint core rule, they'd need to support flow in order to detect that corner case. I think it would make more sense for Flow to allow you to combine import types and imports all into one statement.

[jcreamer898]

@ljharb

I'm a little confused about the react/forbid-prop-types change, why is it bad to pass an object?

What's the alternative?

Thanks!

[ljharb]

@jcreamer898 it's not specific enough - use shape or objectOf (and arrayOf over array, etc)

[jcreamer898]

Ahhh, objectOf was what I was missing. Thanks!

[vdh]

I like to lean towards accepting Airbnb's rules (despite what this lengthy list seems to indicate), so any chance to reduce my own overrides list would be great 😃

---
rules:
  import/no-extraneous-dependencies:
    - error
    -
      devDependencies: true
  curly:
    - error
    - all
  new-cap:
    - error
    -
      newIsCap: true
      capIsNewExceptions:
        # Allow Immutable.js classes
        - List
        - Map
        # ...etc
  no-unused-vars:
    - warn
    -
      vars: all
      args: all
      varsIgnorePattern: ^_
      argsIgnorePattern: ^_

  # Downgraded to warnings
  no-unused-vars: warn
  no-useless-concat: warn
  prefer-template: warn
  semi: warn
  spaced-comment: warn
  react/prefer-stateless-function: warn

  # Disabled
  no-mixed-operators: off
  no-continue: off
  no-plusplus: off
  react/no-children-prop: off
  react/no-unescaped-entities: off
  react/forbid-prop-types: off
  react/no-unused-prop-types: off
  jsx-a11y/no-static-element-interactions: off

• import/no-extraneous-dependencies: We keep our build tools and development-only deps in devDependencies.
• curly: I find if (foo) { return 'bar'; } to be less ambiguous than if (foo) return 'bar'; for single-line blocks.
• no-mixed-operators: I use a && b || c all the time for short-circuit evaluation, and requiring brackets for a + b * c feels like a betrayal of everything I learned in high-school algebra. Logical or arithmetic operators feel too core to be putting heavy-handed bracket requirements over them.
• no-continue: continue is useful when controlling loop flow, and a lot of function-based loops use return as a substitute for it, so why disallow the real thing?
• no-plusplus: Unary operators are common for loop structures, not every unary needs to be expanded to += 1. If there was a rule to restrict unary to _only_ the inside of loop blocks, or to disallow whitespace around unary operators (It feels like a huge mistake to let that be a valid syntax), I might like that.
• react/no-children-prop: <a children="foo" /> feels just as valid as <a>foo</a> is when children are simple values like strings.
• react/no-unescaped-entities: This one feels like it's crippling the wonderful built-in escaping already provided by React, just to provide some typo catching. I would actually prefer an opposite no-escaped-entities rule that would forbid HTML entities entirely.
• react/forbid-prop-types: I really like the sentiment behind this one, to enforce descriptive prop types, but it really hampers prop inheritance. E.g. if a <Page> component is going to pass a user object down to a <Header> component that has the descriptive shape prop type validation. <Page> never uses user directly, so it only has to validate that user is an object, and the more in-depth validation is handled by <Header>.
• react/no-unused-prop-types: Problematic when handling shape, and also has trouble detecting when a prop is used outside of render, or passed down to child components via prop inheritance.
• jsx-a11y/no-static-element-interactions: Also one that I like the sentiment for, but some designs I'm given it's sometimes extremely awkward to attempt to avoid click events for static elements. If this rule could be split on mouse vs keyboard events, I would love to enforce no static elements for keyboard events.
• react/prefer-stateless-function: Usually arrives before the class is actually finished being written, so the downgrade to warning is to make it less persistent.
• no-unused-vars, no-useless-concat, prefer-template, semi, spaced-comment: Downgraded to warnings because they're style-related rules that can be dealt with after more important lint errors.
• new-cap: Already covered earlier, and I think my shorthand use of Immutable classes is too niche to worry about, especially since I'm shadowing the ES6 Map.
• no-unused-vars: Also might be too niche, using a _ prefix for purposefully-unused vars and arguments.

[ljharb]

@vdh Thanks! Some responses:

• re import/no-extraneous-dependencies, see the OP
• re no-mixed-operators, I can see the argument that PEMDAS shouldn't require disambiguating parens, but value selection operator precedence is far too often confusing. I've added arithmetic operators to the OP.
• no-continue and no-plusplus: the guide disallows loops of any kind, so i++ shouldn't be needed.
• no-children-prop: "children" is a special prop, and this rule enforces that it be treated specially.
• no-unescaped-entities allows you to use the real character, you'd just need to put it in curlies - ie, <div>{'a "quote"'}</div> instead of <div>a "quote"</div>.
• forbid-prop-types: far better here would be to make Header's shape for user be shared, and just reuse that on Page. Page doesn't just need an object - it needs whatever Header needs, and the propTypes should be conveying that.
• no-unused-prop-types: it's still a bit buggy, so this one is fair.
• re the "style" things - we feel that style rules are precisely as important as all other rules, and absolutely should break the build. Sloppiness in one aspect implies sloppiness in others - kind of like a brown M&Ms test.
• The next version should obviate the need for your new-cap exceptions.
• no-static-element-interaction: when this happens, we simply tell our designers that they need to design things better :-) designs that violate native element interaction behaviors aren't good designs.
• no-unused-vars: if it's something like $foo.each((_, i) => console.log(i)) - it should already ignore unused "not-the-last" args. What's an example where it's warning here?

[AlicanC]

• no-underscore-dangle: Class privates.
• no-duplicate-imports: #1054
• no-mixed-operators: We configure this with "{ allowSamePrecedence: true }".
• allow-parens: We configure this with "always".
• react/sort-comp: More annoying than useful.
• import/imports-first: Does not fit the way we order our imports:

// Imports that introduce global effects. The same ordering below applies to this.
import 'babel-polyfill';
import './appStyle.css';

// Third-party modules.
import React from 'react';
import _ from 'lodash';

// Shared code. Sourced in the project, but not specific to current component
import CoolButton from 'components/CoolButton'; // webpack alias
import CoolUtils from '../utils/CoolUtils'; // Upper directory

// Private code. Also sourced in the project, but are specific to current component and never required from anywhere else
import Styles from './styles';
import TabOne from './tabs/TabOne';
import TabTwo from './tabs/TabTwo';

• import/newline-after-import: Conditional imports:

import Foo from 'foo';
const Bar = (Platform.OS === 'ios') ? require('bar-ios') : null;
import Baz from 'baz';

• no-continue: We are not sure if this really is a good practice. We code like this:

function myFunc() {
  const foo = ...;
  if (!foo) return;

  const bar = ...;
  if (!bar) return;

  const baz = ...;
  if (!baz) return;

  // There is no reason to stop, do it
  doWork();
}

We want to apply the same style to loops, but without continue, we end up with indentation-hell:

for (const thing of things) {
  const foo = ...;
  if (foo) {
    const bar = ...;
    if (bar) {
      const baz = ...;
      if (baz) {
        doWork();
      }
    }
  }
}

[ljharb]

@AlicanC thanks! The only ones I think need commenting are underscore (there's no such thing as "privates"); imports-first: imports are hoisted, so your conditional import will run after every static one anyways - the rule enforces that the code's order matches evaluation order; and no-continue: the guide prohibits any loops at all, so this shouldn't come up (I'd recommend filter chains on things for example)

[knpwrs]

As far as import/no-extraneous-dependencies is concerned, the rule itself supports glob configuration: https://github.com/benmosher/eslint-plugin-import/blob/2dec0a7f33b737371efb7acd60c8a30a48562ccb/docs/rules/no-extraneous-dependencies.md

[vdh]

@ljharb

• I completely agree that those rules should break builds (and CI tests), I use --max-warnings 0 to tell ESLint to exit 1 on any warnings. I downgrade them only for uncommitted development code purposes, when they are either non-breaking or will be auto-corrected by Babel (e.g. missing semicolons). Perhaps I need to put more focus on the --fix option during development instead…
• Re forbid-prop-types: I've actually started trying to use something like that! 😄 But I've had trouble stripping them out in production, since babel-plugin-transform-react-remove-prop-types only strips the propTypes. The "best" solution I've found so far was webpack's DefinePlugin and:

export const sizeShape = process.env.NODE_ENV === 'production' ? null : /* ... */

• Re loops: That would be 11.1 right? Unfortunately I'm the only self-appointed champion of linting amongst my cohorts, so it's hard to find the time to enforce anything that doesn't have a complimentary ESLint rule.
  We do use Immutable.js and use map/filter/reduce very extensively when transforming data (with both Immutable instances and ES6 Arrays), so I suppose we're most of the way there anyway.
• Re no-children-prop: I'm curious to learn what makes children a special prop? I remember looking into the React source code to check if React treated children vs config.children any different inside createElement(type, config, children), and I couldn't see any issues. As long as someone doesn't do something stupid and mix the two on a non-self-closing tag (<a children="foo">bar</a>, bar overrides foo), <a children="foo" /> and <a>foo</a> should create the exact same React element?

[ljharb]

@vdh if you're not passing children inside jsx, such that you get a closing tag, what's the point of jsx? "children" is special because React.createElement accepts them as a third argument, and because they can be used as actual jsx descendants (altho you're correct that they end up identical inside react).

[oliviertassinari]

Another point as nobody raised it yet.
jsx-boolean-value is enforced to ['error', 'never']. But don't we lose expressivity?
It seems than @zpao is against it.

[ljharb]

@oliviertassinari jsx is html, and this is another similarity to html. In general explicit is much better than implicit, but this is one of the many HTML vagaries you can't get away with not knowing anyways.

[sompylasar]

JSX boolean rule makes sense because one case is expressed in strictly one way: true without value, false with value. Otherwise true may be expressed in two ways. There might be a rule that forbids the other way, i.e. true without the value, but I haven't seen such.

[vdh]

@ljharb I still don't understand this vilification of the children prop. It was proposed and added with little explanation or reasoning in its documentation aside from the fact that it's "special" because of how JSX compiles nesting into the third argument of React.createElement. It's very arbitrary.

If you're not passing children inside jsx, such that you get a closing tag, what's the point of jsx?

Isn't the point of JSX to be a syntactic sugar to nest multiple levels of components inside children props? If I'm at a "leaf" node, why do I need to enforce the same nesting syntax on my non-component values? <a> tags _can_ have further nesting, but if I write <a children="Foo" /> this is a visual indicator that this _particular_ <a> tag is currently being used as a leaf (and although they _are_ technically another nested node, text nodes are very rarely treated as such).

I've even found a previous discussion where someone suggested removing it from the resulting props altogether, but the end result of that discussion seems to be that it is _not_ that special. It's just another prop. It's only been special in the context of validation and rendering. To quote the first reply in that issue:

This seems odd to me, I agree that the way children is currently implemented is weird, but IMHO that's not because it doesn't belong in props. It's because React.createElement provides the convenience of specifying children through varargs, which to me is a convenience provided for non-JSX users but otherwise not a good API decision.

I'm sorry to be so stubborn about this, but I've searched for and found no evidence in the React docs or any comments from the React developers discouraging the use of passing children via prop. If avoiding it was an official stance, or if there was a real technical hurdle to using it, I would drop this. But I don't like the idea of just arbitrarily banning something without technical merit.

[ljharb]

@vdh it's not an official stance of the React team, that's for sure. The issue is that without nested children, we should be using React.createElement calls instead of jsx - there is a large technical cost to using a non-standard-JS syntax. The primary arguments for using JSX is that when nesting children, the closing element (like XML/HTML) contains the opening element's name, which makes nesting more visually identifiable than a closing paren. In other words, I'm claiming that if you're not nesting children anywhere, then you probably shouldn't be using JSX anywhere either. Let's open a new issue though if you want to discuss this further, since the point of this issue is to discuss which rules are overridden, and NOT to debate specific rules.

[ljharb]

@knpwrs thanks - eslint-plugin-import started supporting globs for that rule in v1.15.0, so I'll update eslint-config-airbnb-base soon with that enabled.

[kevinSuttle]

 "rules": {
    "prefer-template": 2,
    "comma-dangle": [2, "always-multiline"],
    "object-curly-spacing": [
      "error",
      "always"
    ],
    "import/extensions": [2, "never", { "js": "never", "jsx": "never", "json": "never"}],
    "import/newline-after-import": 0,
    "import/no-extraneous-dependencies": [
      "off",
      {
        "devDependencies": false,
        "optionalDependencies": false
      }
    ],
    "react/jsx-filename-extension": [1, { "extensions": [".js", ".jsx"] }]
  }

An easy way to find out: https://medium.com/google-cloud/static-javascript-code-analysis-within-bigquery-ed0e3011732c

[ljharb]

@kevinSuttle prefer-template, comma-dangle, and object-curly-spacing are already defined in this config, so you don't need to override those. The others are pretty straightforward, thanks!

[milesj]

I turn these off:

• no-param-reassign - I think this is fine when the value is scalar/primitive, which is the majority use case for me.
• padded-blocks - I like spacing between my conditionals.
• class-methods-use-this - Makes any kind of inheritance annoying.
• react/jsx-filename-extension - I'm too lazy to update all my tooling to understand the "jsx" extension.

And then I have "no-unused-vars": [2, { "vars": "all", "args": "none" }], which is primarily to support flowtype.

I also disable a handful more for tests, as it makes writing tests more cumbersome.

{
  "rules": {
    "max-len": 0,
    "no-undef": 0,
    "func-names": 0,
    "prefer-arrow-callback": 0,
    "import/no-extraneous-dependencies": 0,
    "import/prefer-default-export": 0
  }
}

[ljharb]

@milesj can you show me an example of when it's a scalar/primitive that you think it's OK?

For tests, you should use "import/no-extraneous-dependencies": [2, { "devDependencies": true }] for safety.

[milesj]

Something like this, very rudimentary.

function update(data, action = true) {
  if (condition) {
    action = false;
  }

  // ...
}

[jkeljo]

no-mixed-operators - allowSamePrecedence: true. Having to separate arithmetic operators of the same precedence was unnecessary work. The linter still helps you get it right even for operators whose precedence is less commonly understood, because if it hasn't made you put parens anywhere you know they must have been at the same precedence.

no-plusplus - allowForLoopAfterthoughts: true. The guide forbids loops. But in places where I use one anyway, the afterthought of a for loop is immune to the semicolon insertion problem that motivates the no-plusplus rule to begin with.

no-underscore-dangle - yes, putting an underscore does not a private field make. Yet the Crockford pattern or WeakRef is a lot of ugliness that I'm not going to bother with in most cases. I find I'd rather have a hint than nothing. There's at least one Babel transform out there to make underscores really private, but I haven't gone there yet.

react/jsx-no-bind - I tighten this to forbid arrow functions and not ignore refs, because the former can cause shouldComponentUpdate implementations to erroneously return true, and the latter results in a lot of extra calls to the ref callback.

react/sort-comp - the default order and the style guide order both just feel weird. Can't put all props at the top, or put render and its helper methods in reading order, for example.

I also turn off some rules that either don't play nice with Flow or where Flow does a better job than the linter at detecting the problem.

[AlicanC]

no-unused-vars is also problematic in Express applications. callback.length must be 4 for a callback to be considered an error handler:

// We won't use "next" but we can't omit it
app.use((err, req, res, next) => { /* ... */ });
// If we omit it we get `req, res, next` instead of `err, req, res`:
app.use((err, req, res) => { /* err is req, req is res, res is next and you are f'd */ });

I don't override this rule though, I just disable it for the line. The problem is not the rule, it's Express' API.

I might override it in the future since I like to keep unused arguments as documentation.

[gaurav21r]

Thank you @ljharb for this issue, its good to know you are interested in hearing out the community even in cases where your stand is clear.

I just thought dangling underscores no-underscore-dangle deserves a mention of its own . #490 has been around for a year. 6 comments above, have them in their overrides.

Your stance on the fact that JavaScript has no real private properties is well taken! However I think most people are using this convention because the framework of their choice uses this convention. Eg: Polymer. Whether or not it is the _right_ thing to do, it is highly prevalent in the Javascript industry as of today.

[ljharb]

@gaurav21r if a framework has made the choice to codify the wrong thing to do, then users of that framework can simply disable the rule in their projects. I think my reply to you on that very thread summarizes our position :-)

[gaurav21r]

@ljharb I know your position :P but as I mentioned I deemed this one rule worthy of special mention in this thread based on why you've opened it :)

[tleunen]

I usually override max-len because with test descriptions (describe/it), it's very hard to keep it under 100 or 120 chars.

Every indent because I prefer using 4 spaces than 2, just a preference.
And no-extraneous-dependencies with the devDeps enabled for the tests.

Otherwise, I use something very close to what airbnb suggests.

[ljharb]

@tleunen good news! you no longer have to override max-len for that :-) it now ignores lines with long strings.

[SimenB]

Overrides:

module.exports = {
  rules: {
    'max-len': ['error', 140], // I'm on a widescreen anyways
    'linebreak-style': 'off', // This is enforced but git in `.gitattributes` (with auto), why make things more difficult for windows users?
    'no-multiple-empty-lines': ['error', { max: 1, maxEOF: 1, maxBOF: 0 }], // I don't like padding, makes me scroll more
    'no-plusplus': 'off', // Don't see any reason to disallow it
    'quote-props': ['error', 'consistent-as-needed'], // I find it easier to read if either none or all are quoted, but it should only be quoted at all if it's actually needed
  },
};

Additions:

module.exports = {
  rules: {
    'arrow-parens': ['error', 'as-needed'], // Cuz why not
    'lines-around-directive': 'error', // Again, I don't like padding
    'no-warning-comments': ['warn', { terms: ['todo', 'fixme', '@todo', '@fixme'], location: 'start' }], // I think warning on TODOs (and it's cousins) is useful
    'prefer-numeric-literals': 'error', // Cuz why not
  },
};

[ljharb]

@SimenB Thanks! Most of those are subjective preferences or stricter enforcements (except for the "why not"s, for which the guide usually goes into great detail as to why not). re linebreak-style - as far as I recall, Windows users are easily capable of editing files that contain only LFs - all Windows tools I'm familiar with treat LFs the same as CRLFs, so there's no difficulty for Windows users in always using LFs (and every editor can be configured to produce them). Can you clarify the difficulty?

[SimenB]

That might be me just stuck from the old days of poor editors, and not an actual problem.

I still think this is a problem git deals with better anyways (it just converts it for you under the hood, which this rule makes not possible (as windows user can't have CRLF in their checked out code)). Having something like consistent would make sense, but that's not available.

Not the rule I feel the strongest for, though!

[mglidden]

In addition to a few subjective ones, we also disable:

class-methods-use-this: Let's say you have classes A and B, which both inherit from the same class. Class B might not use this, but can't be static because A needs this. This is a common enough pattern for us that we turned it off.

arrow-body-style: The current setting allows one to write both of these functions:

const fn1 = (list, arg) => (
  _.find(list, { foo: arg })
);
const fn2 = (list, arg) => {
  _.find(list, { foo: arg });
};

fn1 returns the value, while fn2 doesn't. The similarity is confusing, so we don't want to allow functions in the style of fn1 (specifically, multi-line implicit returns – single line are much more obvious). ESLint can't enforce this, so we just manually watch for it.

[ljharb]

@mglidden can you provide an example of classes A, B, and the base class?

[mglidden]

Here's a simpler case with just 2 classes:

class Parent {
  getProperty() {
    return "static value";
  }
}
class Child extends Parent {
  getProperty() {
    return this.property;
  }
}

This will warn on the parent's implementation of getProperty, even though it can't be made into a static method.

[ljharb]

@mglidden what would be a noncontrived version of that? in other words, what's the actual concrete reason you'd need that behavior?

[mglidden]

We've seen this issue in several places, mostly around polymorphic classes.

To try and condense one example: we have several classes that provide data to a specific part of the application, and are called DataSources. One of them, the StaticDataSource, just provides data from a handful of constants. Another one provides data based on arguments passed into its constructor. Both subclass the DataSource parent class. Most consumers of this data don't care about which specific DataSource subclass they are dealing with. Some consumers check arguments to make sure they have a DataSource.

None of the methods in StaticDataSource use this, while other subclasses do. So StaticDataSource would be filled with incorrect warnings.

[ljharb]

Gotcha, thanks! Polymorphism anywhere, and excessive inheritance, are things we generally discourage, but we don't need to get deeper into it here :-)

[QuentinRoy]

However I think most people are using this convention because the framework of their choice uses this convention.

@gaurav21r Well, I think many people use this convention just because they need internal properties and cannot find a clean way to forbid their access (some even think the underscore convention is actually a quite sophisticated solution, after all python has been successfully using it for 30+ years).

[ljharb]

The clean way is to rearchitect so you don't need it. What Python does, just like what any other language does, is entirely irrelevant. JS is JS.

[QuentinRoy]

What Python does, just like what any other language does, is entirely irrelevant. JS is JS.

Does that mean that we cannot hunt for inspiration from other practice and take out what works?

The clean way is to rearchitect so you don't need it.

That may mean everything. And everything may be a treatment worth that the "problem" if not properly designed. The example from your guide might suggest that one should just put everything "public". Is it what you mean? If not, is it about using workarounds like Signal, Weakref or others? Or is it about something more subtle and in this case, would you mind sharing an example of what you do?

[ljharb]

Everything in JS that's exposed is already public. Sticking a character in the name doesn't change that.

The new private properties proposal is the first example of true privacy, and you can simulate that using WeakMap. Short of that? Everything's public, and your architecture shouldn't bury its head in the sand to try to pretend it's not.

[QuentinRoy]

Again, nobody is pretending to forbid any access. I am clearly not a fan of the WeakMap workaround but then that's me.

Everything's public, and your architecture shouldn't bury its head in the sand to try to pretend it's not.

I think we are missing the point here. Are you saying that you never have the _need_ for any internal values?

[ljharb]

Sometimes I do! But since JS currently lacks that facility (instance privacy ofc; closure privacy has always been available), any design that requires it is automatically inappropriate for use in JS.

[QuentinRoy]

any design that requires [internal values] is automatically inappropriate for use in JS

That looks like an awful lot of "inappropriate" designs to me but alright, I see your stance. I think I'll keep my no-underscore-dangle override though :).

EDIT: I don't want to restart this discussion, but I just discovered that even the W3C is using the underscore convention. Considering that these guys are those standardising JS, IMHO, it makes the "inappropriate for JS" argument even more questionable.

[michaelBenin]

These are overrides we use:

{
  "extends": "airbnb",
  "rules": {
    "comma-dangle": [2, "never"],
    "react/jsx-filename-extension": [0, { "extensions": [".js"] }],
    "prefer-arrow-callback": ["error", { "allowNamedFunctions": true }]
  },
  "env": {
    "es6": true,
    "node": true,
    "browser": true,
    "mocha": true
  }
}

[moimael]

We override those :

  rules: {
    indent: ['error', 2, {
      SwitchCase: 1,
      VariableDeclarator: 1,
      outerIIFEBody: 1,
      MemberExpression: 1,
      FunctionDeclaration: {
        parameters: 1,
        body: 1,
      },
      FunctionExpression: {
        parameters: 1,
        body: 1,
      },
    }],
    'no-useless-escape': 'off',
    'no-multi-str': 'off',
    'react/jsx-filename-extension': [1, {
      extensions: ['.js', '.jsx'],
    }],
    'import/no-extraneous-dependencies': 'off',
    'import/no-dynamic-require': 'off',
    'react/no-multi-comp': 'off',
    'arrow-parens': 'off',
    'react/forbid-prop-types': 'off',
    'react/no-unused-prop-types': [1, {
      skipShapeProps: true,
    }],
  },

The indent override is for MemberExpression: 1, which mainly allows us to properly indent chained functions. no-useless-escape seems to have issue with regex.
no-multi-str do not seems to be relevant anymore, except for really old browsers.

[ljharb]

@moimael can you provide an example of code that's indented properly with our settings, versus with yours?

As for no-useless-escape, I'm quite sure it has no issues but I'd love to hear what they are :-)

no-multi-str is an issue because you should be using template literals instead of multiline strings.

[eddyerburgh]

no-plusplus: ["error", { "allowForLoopAfterthoughts": true }] - cases where semicolons could be auto added will be caught, and the common usage of plusplus in for loops is allowed.

comma-dangle: ["error", "never"] - dangling commas just look wrong to me!

[moimael]

@ljharb so for example with promises, this code would throw no errors using the default airbnb settings :

  it('should return exception on failure', (done) =>
    engine.renderRequest('/broken')
.then(res => expect(res).toBeNull())
.then(done)
.catch(e =>
  expect(e).toEqual(
    new Error('Could not find a matching modifier named not-a-modifier'),
  ),
)
.finally(done),
  );

Where with our settings (MemberExpression: 1) it will be valid only in this case :

  it('should return exception on failure', (done) =>
    engine.renderRequest('/broken')
      .then(res => expect(res).toBeNull())
      .then(done)
      .catch(e =>
        expect(e).toEqual(
          new Error('Could not find a matching modifier named not-a-modifier'),
        ),
      )
      .finally(done),
  );

After enabling again no-useless-escape, I don't have the silly errors i had before, so it seems it has been fixed.

Template literals for multiline string in a properly indented code looks like a mess, let's take this example :

class Something {
  function test() {
    const thisisavariable = `i want
multiline string`;
  }
}

Indentation doesn't look good at all (and can look a lot worse with more nested code and bigger multiline string), and if I indent the string properly then I get a lot of extraneous space (which is how template literals are supposed to work) so in this case i would prefer a multiline string.

[ljharb]

@eddyerburgh thanks! Our guide disallows for loops too, so the no-plusplus issue doesn't come up for us. I don't actually like the look of trailing commas either - but the thing that swayed me is that git diffs don't ever spill out onto the next or preceding line in a multiline comma-separated list of things.

@moimael ah, interesting - we may want to enable MemberExpression in our own config then. I'll look into it with our codebase; if it seems compatible with your "only in this case" example (which I agree is the only correct way to indent that) then we'll enable it! In the template literal case you mention, I'd recommend looking into https://www.npmjs.com/package/dedent and/or https://www.npmjs.com/package/deline :-)

[moimael]

@ljharb Awesome ! For the multiline template literals, I tried those package before, the thing that I find very annoying is having to import them each time you need it, which makes it in my opinion less convenient that multiline string.

[rhbecker]

@AlicanC

no-unused-vars is also problematic in Express applications

I encountered the same problem and found the argsIgnorePattern option to be preferable to turning off the rule entirely, or via inline comments: "no-unused-vars": ["error", { "argsIgnorePattern": "next" }].

I actually came to this issue in looking for how folks deal with no-param-reassign in working with express, as setting values within middleware is commonly done by modifying res.locals.

I wonder whether the maintainers of eslint would consider a similar ignore pattern option for the no-param-reassign rule ... I may ask over there. I found an open issue on this topic.

[DanielFGray]

I prefer having a space after !

"space-unary-ops": [
  "error", {
    "words": true,
    "nonwords": false,
    "overrides": {
      "!": true
    }}],

Without the space it can be easy to skip over them and not immediately notice there's negation happening.

Compare
``` javascript
if(!someThing(foo)) {...}
// vs
if(! someThing(foo)) {...}
````

[ljharb]

@DanielFGray how does that work when coercing to a boolean with !!?

[jeffvandyke]

My big one is:

    // matches airbnb, only 'everything-else' is moved to just after 'lifecycle',
    // because they are usually internal methods that render doesn't care about,
    // but lifecycle and event handlers both do, so the logical place is in between those groups.
    'react/sort-comp': ['error', {
      order: [
        'static-methods',
        'lifecycle',
        'everything-else',
        '/^on.+$/',
        '/^(get|set)(?!(InitialState$|DefaultProps$|ChildContext$)).+$/',
        '/^render.+$/',
        'render'
      ],
    }],

[TiddoLangerak]

My only override:

// All relevant browsers support both nesting and ids, so it's really not needed to require both
// See: https://github.com/evcohen/eslint-plugin-jsx-a11y/issues/51
'jsx-a11y/label-has-for': ['error', {
    components: ['label'] ,
    required : {
        "some" : ["nesting", "id"]
    }
}]

[DanielFGray]

@ljharb i tend to use Boolean(foo) rather than !! so it works okay

[ljharb]

@TiddoLangerak all relevant accessibility devices may not, though.

[TiddoLangerak]

@ljharb The relevant issue at jsx-a11y was inconclusive in that regards, but it hinted that there is little reason to believe that there are still relevant accessibility devices for which this rule is required. I'm not even remotely an expert on accessibility devices, but when we turned the rule off we couldn't find any evidence of any relevant accessibility device that would be impacted by it. I'd love to be proven wrong though.

[TiddoLangerak]

It seems that requiring both nested and id really isn't needed anymore, even for accessibility: I stumbled upon a website with accessibility tests for different screenreaders here. The TL;DR is that all but one of the tested screenreaders have perfect support for both id-based and nested labels. The only not perfect screenreader (SaToGo 3.4.96.0) only had issues with tabbing through nested labels with text after the form control, but also only had a 1.5% marketshare in 2015 (according to webaim).

[SimenB]

I have a blind co-worker who's our expert on a11y, and he also says there is no difference between nesting and for when using screen readers, at least

[ljharb]

@TiddoLangerak @SimenB that's good to hear anecdotally; but doing both has been the best practice for years and I'd want to see lots of evidence before changing that recommendation. Even 1.5% marketshare is still nonzero, and a11y is about including everyone, no exceptions.

[vforge]

Our overrides:

    'max-len': [2, 120, 2],
    'object-curly-spacing': [2, 'never'],
    'quotes': [2, 'single', {'allowTemplateLiterals': true}],
    'comma-dangle': ['error', {
      'arrays': 'always-multiline',
      'objects': 'always-multiline',
      'imports': 'never',
      'exports': 'always-multiline',
      'functions': 'ignore',
    }],

Switch from spaces to tabs plus requirement of JSDocs as you can see here: https://github.com/Wikia/eslint-config-fandom/blob/master/index.js

[Jamesernator]

I override template-curly-spacing as it's more consistent with the other forms of curly rules:

const x = `Hello ${ foo }`
const y = { a: 10 }
const { z } = llama

[ralphholzmann]

We had to disable no-use-before-define in one of our files due to having two mutually recursive functions that referenced each other.

[ljharb]

@Jamesernator seems like reasonable logic.

@ralphholzmann thanks, that sounds like a perfectly sensible reason to override that rule :-)

[caub]

"quotes": ["error", "single", { "avoidEscape": true, "allowTemplateLiterals": true }],
"no-unused-vars": ["error", { "argsIgnorePattern": "^_", "varsIgnorePattern": "^_" }],
"comma-dangle": ["warn", { "arrays": "never", "objects": "always-multiline" }],
"max-len": ["warn", 180, 4],

"arrow-body-style": "off",
"arrow-parens": "off",
"consistent-return": "off",
"prefer-destructuring": "off",
"function-paren-newline": "off",
"object-curly-newline": "off",
"global-require": "off",
"prefer-template": "off",

"no-plusplus": "off",
"no-underscore-dangle" : "off",
"no-path-concat": "off",
"no-param-reassign": "off", // for ORMs, sessions..
"no-nested-ternary": "off",
"no-mixed-operators": "off",
"no-return-assign": "off",

"react/jsx-no-bind": "warn",
"react/forbid-prop-types": "warn",
"react/no-unused-prop-types": "warn",
"react/jsx-filename-extension": "off",
"react/no-danger": "off",

example: function-paren-newline complains about:

  const wrapper = mount(
    <Router>
      <NotFound />
    </Router>
  );

which looks fine to me

ideally this too:

"object-curly-spacing": ["error", "never"],
"indent": ["error", "tab"],

[ljharb]

@caub for function-paren-newline, i'd suggest this:

  const wrapper = mount((
    <Router>
      <NotFound />
    </Router>
  ));
/* or */
  const wrapper = mount(
    <Router>
      <NotFound />
    </Router>,
  );

[bschlenk]

To add on arrow-body-style:
comma-dangle is required, the biggest reasons being that it makes commits nicer, i.e. you see only one line changed instead of two when adding an element to an array. arrow-body-style, on the other hand, tends to cause the exact opposite, more lines to be changed in a diff. If I want to add more logic to an implicit return arrow function, I need to first convert it before I can add anything. This is combersome and also bloats git diffs.

[divyanshu013]

I can't seem to understand the order for reordering of custom methods in react/sort-comp

[ljharb]

@divyanshu013 indeed if you follow the error messages one at a time, it's a very slow and tedious process. However, the desired ordering is in the config itself.

[dtinth]

no-void: off I use void to signify that a variable is intentionally unused.

// Nothing part of a maybe functor
const Nothing = () => ({
  map(f) {
    void f; // Nothing to map here.
    return Nothing();
  }
});

[ljharb]

@dtinth why include the f in the signature if it's unused? iow, why not map() { return Nothing(); }?

[dtinth]

@ljharb Good question. Some frameworks checks the arity of the functions. e.g. express error handling middleware (err, req, res, next). Also, when I do polymorphism I prefer all implementations to have the same signature. e.g. there's also Some(thing) whose map(f) function does invoke f.

[ljharb]

In that case, you can configure eslint to ignore the last unused argument - that seems much better than adding a line of production code.

[tunnckoCore]

Agree with @ljharb. Also if i remember, the err is excluded by default - both here and in StandardJS.

As about the title of the thread. These are my overrides and comments to them. In anyway Airbnb's style guide is awesome, because handles as much rules as possible.

module.exports = {
  rules: {
    // Enforce using named functions when regular function is used,
    // otherwise use arrow functions
    'func-names': ['error', 'always'],


    // Always use parens (for consistency).
    // https://eslint.org/docs/rules/arrow-parens
    'arrow-parens': ['error', 'always', { requireForBlockBody: true }],


    'prefer-arrow-callback': ['error', { allowNamedFunctions: true, allowUnboundThis: true }],


    // enforces no braces where they can be omitted
    // https://eslint.org/docs/rules/arrow-body-style
    // Never enable for object literal.
    'arrow-body-style': [
      'error',
      'as-needed',
      {
        requireReturnForObjectLiteral: false,
      },
    ],


    // Allow functions to be use before define because:
    // 1) they are hoisted,
    // 2) because ensure read flow is from top to bottom
    // 3) logically order of the code.
    'no-use-before-define': [
      'error',
      {
        functions: false,
        classes: true,
        variables: true,
      },
    ],


    // Same as AirBnB, but adds `opts` and `options` to exclusions
    // disallow reassignment of function parameters
    // disallow parameter object manipulation except for specific exclusions
    // rule: https://eslint.org/docs/rules/no-param-reassign.html
    'no-param-reassign': [
      'error',
      {
        props: true,
        ignorePropertyModificationsFor: [
          'acc', // for reduce accumulators
          'e', // for e.returnvalue
          'ctx', // for Koa routing
          'req', // for Express requests
          'request', // for Express requests
          'res', // for Express responses
          'response', // for Express responses
          '$scope', // for Angular 1 scopes
          'opts', // useful to ensure the params is always obect
          'options', // and when using Object.assign for shallow copy
        ],
      },
    ],


    /**
     * Ensure small functions and no nested/callback hell.
     * If you need more, consider extracting some logic.
     * Also you have cool destructuring feature, use it.
     */


    // Enforce the max number of lines in a file, half of the AirBnB's.
    // http://eslint.org/docs/rules/max-lines
    'max-lines': [
      'error',
      {
        max: 150,
        skipBlankLines: true,
        skipComments: true,
      },
    ],


    // http://eslint.org/docs/rules/max-params
    'max-params': ['error', { max: 3 }],


    // http://eslint.org/docs/rules/max-statements
    'max-statements': ['error', { max: 20 }],


    // http://eslint.org/docs/rules/max-statements-per-line
    'max-statements-per-line': ['error', { max: 1 }],


    // http://eslint.org/docs/rules/max-nested-callbacks
    'max-nested-callbacks': ['error', { max: 5 }],


    // http://eslint.org/docs/rules/max-depth
    'max-depth': ['error', { max: 5 }],
  },
}

[jimmywarting]

Not being able to use features that are in the current version of javascript. I can live with style guides but forbidding the use of a feature like for..of, iterators/generators, Symbols i can't understand...

[ericblade]

@jimmywarting for..of is blocked because all for loops are blocked. Iterator/generator due to performance/bundle size issues if i remember correctly, and transpiling is still definitely a major concern to most users, almost definitely to a big business such as airbnb. Symbol because polyfills are not 100% compatible with the real thing. (the guide does specifically state "they should not be used when targeting browsers/environments that don't support them natively")

So, one is a style concern (Airbnb's style forbids for), one is a performance concern, and one is a compatibility concern. I'd say those are pretty valid concerns, but of course, you're welcome to override them. Personally, I've never come up with a good reason to use iterator/generator, but that may be significantly due to that i've never used them, so my imagination for good places to use them is lacking example.

[sompylasar]

@ericblade

Personally, I've never come up with a good reason to use iterator/generator, but that may be significantly due to that i've never used them, so my imagination for good places to use them is lacking example.

Example: redux-saga.

[jimmywarting]

For loop are grate if you use async/await. Makes code so much more readable. Iterators are cool and I have came up with many good valid reasons why to use them that it can solve that you can't do in any other "good" way.

For me readable code is more preferred above all else, specially if you are working in team and other need to understand your code. Spending time Optimising in the beginning is a waste of time if it turns out that nobody is using it or if it's not used heavenly like converting a long movie

And when you are building a electron or a node app that many of us do you only have to worry about one platform. And if performance is important to you then you should definitely use for loops instead of utility functions
If you play around with iterators and generators for One month or two and really think of cases where you can use it instead of your regular way of solving things you will start to realise what you are missing out

[ljharb]

@jimmywarting this is one of the important reasons to use a styleguide. We ban with and eval as well - something being a language feature in no way automatically means it's good to use.

If you're targeting only environments with native async/await, you can certainly override the rule.

[Billy-]

To all: regarding no-nested-ternary, I understand why a single ternary statement is useful in JSX. Can someone provide me a code example of why a nested ternary is useful in JSX?

<div>
  { loading
    ? <Loader />
    : error
    ? <Error />
    : somethingElse
    ? <SomethingElse />
    : <Default />
  }
</div>

Also, I don't think this applies only to jsx:

const result = a > b
  ? 'greater'
  : a < b
  ? 'less'
  : 'same'

@ljharb

[jeffvandyke]

@Billy- I agree, and that's why I also felt it necessary to override this rule. This is also why I can't wait for https://github.com/tc39/proposal-pattern-matching to land in Babel in a stable/usable form, with do-expression semantics (which is the plan, AFAIK). That might be the best solution to examples like these.

[ljharb]

@Billy- thanks - both of those examples seem highly unreadable to me. I'd split up the jsx one into multiple { } expressions, and i'd use if/else on the second one.

I thoroughly agree that pattern matching would be cleaner than all of these.

[GabrielDuarteM]

@ljharb It's really hard for me to understand why exactly the rule linebreak-style is enforced... What are the benefits that this rule brings?

I know Windows users can override the linebreak styles to use the unix style, but why make it harder for them?

From the eslint rule description: "[...] the default behavior of git on Windows systems is to convert LF linebreaks to CRLF when checking out files, but to store the linebreaks as LF when committing a change.", so the commited files will always contain LF linebreaks anyway...

This is a rule I always override when using the airbnb preset, because I see no apparent benefit from it, and it makes it harder for windows users to contribute.

[ljharb]

@GabrielDuarteM i somewhat agree; see #1224. The thing that makes it harder for windows users, however, is that their global gitattributes aren't set to auto-convert prior to cloning repos (which isn't the default behavior on older gits, i believe).

Separately, I think it's reasonable to override that rule if you have windows devs; but i also think it's reasonable to enforce that your windows devs configure their editors to use LF exclusively.

[rhbecker]

My team gets windows machines on which we develop apps that run on linux. Our lives are less complicated when we just stick with LF everywhere, so we happen to like the rule as-is.

[GabrielDuarteM]

@ljharb Still, I'm failing to see what would be the benefits of having that rule enabled, which would justify giving windows devs such a burden.

Is it only to avoid people using older versions of git to commit files with different linebreak styles? If that's the case, I don't know much about linebreaks, but can you clarify what problems would that bring? Besides, on this comment, you said that legacy isn't the best reason to make choices, so if that's the case, should we really apply that rule, based on older git versions?

I know its relatively easy to make the change to LF on windows, but i just don't see a compelling reason to do so...

Also, if someone wants to have all files on the repo using LF, isn't it easier to do it on a local .gitattributes and commit that to the repo?

We all want people to start contributing, regardless of the platform they are on, and to new developers, seeing hundreds/thousands of errors when linting, just after cloning the repo, could definitely scare them away from contributing...

[rhbecker]

LF works everywhere. CRLF does not.

In a way, your last paragraph is a good answer to your value question. If I'm writing code that isn't going to work well across platforms, I'd like my linter to tell me. If I only realize that after I've created a large number of files, I've added burden to change.

As you mention, it can be noisy when you decide after-the-fact to enforce some rule you weren't previously enforcing. So, that's an inconvenience for old code. If you don't feel like updating that old code, it's easy enough to override that rule. But for all future code you write, wouldn't it be preferable to just start off using LF and avoid the complexity thereafter?

[ljharb]

@GabrielDuarteM a local gitattributes won't help, unfortunately, it has to be set globally before the repo is cloned in the first place to avoid issues.

[stevemao]

"no-underscore-dangle": "warn". Sometimes it's not possible with third party libs

In some folders (EG: serverless projects) I have to do:

"import/prefer-default-export": "off",
"import/no-default-export": "error"

[stevemao]

Just ran into this issue:
I need to have two labels for one input. So the markup might be a bit messy if I have to nest input inside of two labels:

<label>
  <label>
    <input />
  </label>
</label>

I'm not sure if this is possible but if one input already has a label as parent, the other label only having htmlFor should be enough.

jsx-a11y/label-has-for

[ljharb]

@stevemao I’m very confused; can you explain why you need two labels for the same input? As to the other part of your comment, if all of that appears in the same component, it seems reasonable to allow that - please file that on eslint-plugin-a11y.

[VincentLanglet]

The only one rule I change is react/jsx-filename-extension because I use typescript.
Too bad it does not automatically switch for a tsx extension check in typescript file.

[bencmbrook]

Require without extensions

"import/extensions": ["error", "always"],

@ry Ryan Dahl regrets allowing require without the extension with this reasoning:

• Needlessly less explicit.
• Not how browser works.
• The module loader has to guess.

cc @jimmywarting https://github.com/transcend-io/conflux/pull/23#discussion_r301924827

[ljharb]

@bencmbrook his regrets about the very reasons node became so popular don't change what's the best practice - also "how the browser works" is that extensions are ignored, and it's impossible for node to match that since it reads from the filesystem, so that's an inherently incorrect reason.

[QuentinRoy]

@ljharb I don't quite get your argument. Why is it best practice? The fact that browsers ignore extensions has little impact on the use or not use of this rule. I do agree with @bencmbrook (and Ryan Dahl) that I would rather have node being consistent with the browser. Not that I plan to stop using a bundler any time soon, I wish I still had the choice for example.
The reason I stopped using @bencmbrook rules, however, is that you cannot (easily) have an extension when loading node_modules, and I was not fond of having inconsistencies between loading local files, and loading modules.

[jimmywarting]

To add to @bencmbrook comment, in Deno you can't import modules without extension

[ljharb]

@QuentinRoy node can never be consistent with the browser in this way because the browser has out-of-band metadata via the <script> tag and the HTTP server's MIME type, that node lacks. Desiring that is fine, expecting you'll ever get it is at best naive. Omitting extensions is a best practice because it aids refactoring and avoids having to think about the module format you're importing, which should only be a concern of the author, not the consumer.