buildo / eslint-config

buildo's eslint shared config

eslint-config-buildo

buildo's eslint shared config

Usage

In your project's .eslintrc

{
  "extends": "buildo"
}

If you need to override or turn off rules:

{
  "extends": "buildo",
  "rules": {
    "comma-dangle": 0, // disable trailing commas
    "jsx-quotes": "prefer-double" // change
  }
}

You will need the following npm packages:

npm i --save-dev babel-eslint\
                 eslint\
                 eslint-config-buildo

If you'd rather install them globally, you need to explictly require some plugins as well:

npm i -g babel-eslint\
         eslint\
         eslint-config-buildo\
         eslint-plugin-no-copy-paste-default-export\
         eslint-plugin-no-loops\
         eslint-plugin-react

Proposing a new rule

If you are proposing a new rule, you can use linto to pre-check whether it will cause new errors or warnings in the existing projects.

First, get the latest version of linto:

yarn global add linto
# or if you have time
npm install -g linto

Then you can use the configuration you'll find in this repo as a base.

Example

Suppose you want to propose the addition of guard-for-in. Edit the linto-config.yml by adding the rule under test to the eslintConfig section:

repos:
  // ...
eslintConfig:
  // ...
  rules:
    guard-for-in: 2
}

Then run linto

linto run -c linto-config.yml

This will generate a report that you can copy-paste on a GitHub issue.

NOTE

linto purposely ignores the project-specific configurations, so you may run into errors not directly related to the rule under test. If that's the case, simply turn off the offending rules in the configuration, to get more precise results. For instance, if you get a lot of max-len errors, just do:

repos:
  // ...
eslintConfig:
  // ...
  rules:
    max-len: 0
    guard-for-in: 2

You can also do the same for a single repo. For example, this turns off the semi rule only for the 'buildo/foo' repo:

repos:
  - owner: buildo
    name: foo
    eslintConfig:
      rules:
        semi: 0
  // ...
eslintConfig:
  // ...
  rules:
    guard-for-in: 2

Proposing a new plugin

Similarly to rules, you can test new plugins (and their rules). If you add any entries to the plugins section of the ESLint configuration, they will get installed and made available while linto runs.

Example

Suppose you want to propose the addition of eslint-plugin-import. Proceed like before, using a configuration like:

repos:
  // ...
eslintConfig:
  // ...,
  plugins:
    - import
  rules:
    import/named: 2

eslint-config's Issues

update eslint peer dep

description

eslint peer dependency is fixed to 2.x while eslint has been using for months 3.x.

how to reproduce

• {optional: describe steps to reproduce defect}

specs

update eslint peer dependency

misc

{optional: other useful info}

new rules proposal

Add no-extend-native and no-native-reassign

/cc @buildo/frontend

kill the comments

e.g. // (fixable)

they are useless and tedious to keep in sync with reality

Disallow Use of `undefined` Variable (no-undefined)

Disallow Use of undefined Variable (no-undefined)

eslint rule: http://eslint.org/docs/rules/no-undefined

Implicit return and arrow functions

I find this confusing:

[1, 2, 3].map((foo) =>
  bar.something(blabla).foo()
);

That's an implicit return, but I personally can't parse it easily because it's on a separate line.

I propose to always stick to either

[1, 2, 3].map((foo) => bar.something(blabla).foo());

or

[1, 2, 3].map((foo) => {
  return bar.something(blabla).foo();
});

This improves readability and predictability, imho. @buildo/frontend, thoughts?

upgrade eslint-plugin-react

requirements

let's upgrade eslint-plugin-react to v6.x

specs

{optional: describe technical specs to implement this feature, if not obvious}

misc

{optional: other useful info}

want: no-magic-numbers

http://eslint.org/docs/rules/no-magic-numbers

Add consistent-return

requirements

Have a convention for returns.

specs

http://eslint.org/docs/rules/consistent-return

misc

@buildo/frontend thoughts?

let's define our base sets of eslint rules!

@gabro @giogonzo @francescogior @ascariandrea

Hi everyone! In the last week only I fixed/updated like 4 .eslintrc configuration files so, as the base configuration is always the same, I created this new package where we can define our base sets.

For now we only have two files: default.js and production.js

But the point of this discussion is decide which sets we need and create a set of rule for each one (for example we could remove production.js if we don't need it or we could split default.js into es6.js, react.js )

How to use it:

If you want to use these rules in a project (a buildo project you're contributing to or a personal project) you can simply install this package and then extend your .eslintrc like this:

// ALL
"extends": "eslint-config-buildo"

// SOME
"extends": [
  "eslint-config-buildo/rules/SOME_RULE",
  "eslint-config-buildo/rules/SOME_OTHER_RULE"
]

add wrap-iife: [2, "inside"]

http://eslint.org/docs/rules/wrap-iife.html

for readability, I think

(a => {
 // ...
})();

is better than both

a => {
// ...
}();

and

(a => {
// ...
}());

e.g. see https://github.omnilab.our.buildo.io/buildo/labonline/pull/1510/files#diff-5987c54300ae1129b26846f0f758a1c6R90

consider adding no-extra-parens

http://eslint.org/docs/2.0.0/rules/no-extra-parens

jsx-quotes: 'prefer-single'

Currently we use jsx-quotes with prefer-double which enforces double quotes on JSX strings, but at the same time we use single quotes for strings.

Many others (see airbnb) do the same, and this is the usually the motivation:

Why? JSX attributes can't contain escaped quotes, so double quotes make conjunctions like "don't" easier to type. Regular HTML attributes also typically use double quotes instead of single, so JSX attributes mirror this convention.

However, in our case:

• you typically need apostrophes while writing English text in attributes. We don't usually do this (strings are in a separate json file) and in the rare circumstances you can always fallback on double quotes (eslint won't complain if you use a single quote inside the string)
• about consistency with HTML attributes, I personally couldn't care less, do you?

I vote to keep consistency between strings and use single quotes for JSX too.

Thoughts?

/cc @buildo/frontend-opensource

fix dependencies

required dependencies are currently declared as dev-dependencies!

curly

http://eslint.org/docs/rules/curly

set on "all", basically:

ok

if (something) {
  somethingElse();
}

not ok

if (something) somethingElse();

Given that there are only 2 errors in the current LOL codebase (and both by @dhinus :P) it should be a relatively shared rule among buildojsers

@buildo/frontend thoughts?

some established rules have disappeared!

no-unused-vars, comma-dangle, semi, no-trailing-spaces, quotes
where have they gone? :) @gabro @giogonzo

I think we should definitely re add them, especially no-unused-vars (detects useless imports too)

require space after keywords

I think we should enforce this rule http://eslint.org/docs/rules/space-after-keywords.html

ex:

// bad
if(condition) {
    doSomething();
}else{
    doSomethingElse();
}

// good
if (condition) {
    doSomething();
} else {
    doSomethingElse();
}

re-enable no-use-before-define

requirements

re-enable the rule when this is fixed babel/babel-eslint#249

dot-notation

http://eslint.org/docs/rules/dot-notation

we're already doing this, but it can be useful if we forget things like this:

upgrade eslint-plugin-no-loops

requirements

upgrade eslint-plugin-no-loops to v0.3.x

specs

{optional: describe technical specs to implement this feature, if not obvious}

misc

{optional: other useful info}

Add `eqeqeq`

http://eslint.org/docs/rules/eqeqeq

rule proposal: promise plugin

Essentially this: https://github.com/xjamundx/eslint-plugin-promise

Thoughts? @buildo/frontend

rule proposal: method names

proposal: method names should never start with _. I mostly agree with AirBnb, after reading this discussion airbnb/javascript#1024

We could use this brand new eslint plugin to enforce it https://github.com/buildo/eslint-plugin-method-names/tree/master 💯

@buildo/frontend

max-len should ignore URLs, strings and imports

requirements

IMO max-len (to be finally more useful than annoying) should ignore URLs, strings and imports.
http://eslint.org/docs/rules/max-len#ignoreurls

@buildo/frontend

specs

• ignore strings, URLs and templateLiteral
• add regexp to ignore imports

misc

{optional: other useful info}

Dependency on eslint?

Is it correct that we depend on eslint?

https://github.com/buildo/eslint-config/blob/master/package.json#L28

My spidey sense tells me it should only be a peerDependency

want: dot-location

http://eslint.org/docs/rules/dot-location

Add spaced-comment and no-multi-spaces

Proposing these together since both very minor.. note that both are fixable

spaced-comment and no-multi-spaces

Our average code base does not agree but I think mostly because it was never enforced. There's no convention in the failing projects and they are inconsistent

	repo	errors	warnings
✅	buildo/aliniq	0	0
❌	buildo/web-shared	19	0
❌	buildo/lexdoit	36	0
❌	buildo/ipercron	10	0
❌	buildo/bobafett	6	0
❌	contactlab/contacthub-sdk-browser	3	0
✅	contactlab/contacthub-sdk-nodejs	0	0
❌	buildo/avenger	2	0
❌	buildo/react-components	7	0
❌	buildo/rc-datepicker	2	0
❌	buildo/react-cookie-banner	2	0
✅	buildo/react-autosize-textarea	0	0
✅	buildo/react-input-link	0	0
✅	buildo/react-flexview	0	0
✅	buildo/react-placeholder	0	0
❌	buildo/local-date	1	0
❌	buildo/formo	12	0
✅	buildo/hophop	0	0
✅	buildo/nemobot	0	0
❌	buildo/prisma	3	0
✅	buildo/pledge	0	0

remove `production` rules

requirements

let's have only one source of truth for our rules, having production.js seems more complex than useful
(refer to comments in #1)

specs

move rules from 'production.js' to default one

Create test project to test our rules against

requirements

The idea is to have a test suite so that we can update rules and dependencies with some confidence.

specs

My current proposal is to create a comprehensive project compliant with our ESLint standards.

This allows us to run an updated configuration against the project and verify that nothing breaks.

comma dangle

I'm really tired of seeing diffs like this

const props = {
-  allowBlah: true
+  allowBlah: true,
+  isBaz: false
};

😠

or

const props = {
- allowBlah: true,
- isBaz: false
+ allowBlah: true
};

😧

I'm proposing to change comma-dangle from never to always-multiline, so that we can have nice diffs:

const props = {
  allowBlah: true,
+ isBaz: false,
};

🎉

or

const props = {
  allowBlah: true,
- isBaz: false,
};

💯

I'm not a big fan of it from a purely aesthetic point of view, but the advantages are pretty huge when it comes to diffs.

It also makes much easier and less error prone to manipulate objects and arrays, as you don't have to remember to add/remove commas to the last lines.

/cc @buildo/frontend

custom eslint rules to enforce conventions

original issue: https://github.omnilab.our.buildo.io/buildo/labonline/issues/1305

requirements

eslint can be very effective to enforce some conventions. We're already using it to verify if a component's name is the same of its filename.
New rules to help us enforce some conventions::

• a Component can import only one sass file and only if it has the same name (but in camelCase)
  • ex: MyBeautifulComponent can have import './myBeautifulComponent.scss'
• the imported sass must be in the same folder (./*.scss)
• if a component has a main class, this should be the component's name in kebab-case
  • ex: MyBeautifulComponent can have <div className='my-beautiful-component' ...

remove no-extra-parens (revert #53)

requirements

as discussed in #39 we should revert #53 and remove no-extra-parens as it's too strict for us

plugin idea: eslint-plugin-tcomb

A plugin to enforce some conventions in tcomb.

Possible rules:

no-unnamed-types

description

types should always have a name

specs

check last arguments of type definitions

---

no-loose-structs

description

structs should be strict by default

specs

second argument of t.struct should be an object with strict: true

List npm modules

requirements

The Readme should list the required npm modules.

specs

Add npm i ...

misc

Airbnb does it ;)

Update eslint to v2

requirements

• update to [email protected]
• update/replace deprecated rules

specs

• update rule by rule and check for warnings and errors

Empty lines

I would like to propose some rules to prevent enforce rules about empty lines:

• no-multiple-empty-lines
  I would go for the default setting
• padded-blocks
  I would set: blocks => "never", classes => "always" and switches => "always"
• newline-after-var
  I think this rule will make the code more readable (at the expense of more lines)
• newline-before-return
  Maybe this is too strict, but I like it :)

@buildo/frontend what do you think??

add more rules to default config

some experiments in: https://github.omnilab.our.buildo.io/buildo/labonline/blob/web-1416-test_new_eslint_plugins/web/.eslintrc

that .eslintrc should contain exactly the proposed diff wrt current buildo/default

(note that no-this-before-super is already here but disabled (for no reason?))

Some of the rules that I added are a bit opinionated, e.g. no-plusplus.. but it made me discover this beauty: https://github.omnilab.our.buildo.io/buildo/labonline/blob/web-1416-test_new_eslint_plugins/web/src/app/components/Basic/Tabs/Tabs.js#L43-L56

Let's discuss these and/or other possible additions in this issue

Disable global find

requirements

• eslint doesn't mark find as undefined variables (cause it's a js native function)

specs

• turn off the global find var

misc

• my experience is I only use the lodash find method and sometimes I forget to import in the current file. No lint errors, but code doesn't work and I lose time debugging :/

Find a way to document code

requirements

The code documentation should be enforced, at least for some cases, to let external devs understand better how app works, to improve code quality and organization.

specs

For "documentation" I don't mean jsdoc or any other standard, but I'm referring to the concept itself. So we need to define first what parts of code need to be documented and how.

misc

A possible solution could be an eslint rule (or a combo of rules):

• require-jsdoc
• valid-jsdoc

but could be a lot of overhead.

why are we repeating some rules from eslint:recommended?

Just something we need to check / understand:

as pointed out here #15 , some rules weren't being checked without explicitly repeating them even though they are part of eslint:recommended (I think just no-unused-vars, comma-dangle, see http://eslint.org/docs/rules/)

consider more plugins to include

I have these on my list:

• https://github.com/jussi-kalliokoski/eslint-plugin-no-implicit-side-effects (forget the void stuff, we can just disable inline when needed, and it should be in some very special cases) (also there's a good list of other rules we could add at the end of the readme)
• https://github.com/benmosher/eslint-plugin-import

what do you think?

react/jsx-equals-spacing doesn't work

Atom complains about rule definition not found and eslint doesn't complain at all about react/jsx-equals-spacing.

Seems related to eslint-react-plugin version: jsx-eslint/eslint-plugin-react#21

default rule config is not supported anymore by eslint 2.0

description

Updating to eslint 2 may have caused some plugin rules to be turned off, as the ruleConfig key of plugins is now ignored.

See https://github.com/buildo/eslint-plugin-no-loops for an example.

specs

Run through all plugins we use and explicitly activate the rules we expect to be on.

/cc @buildo/frontend

pin eslint-plugin-react to <6.10.1

requirements

Due to jsx-eslint/eslint-plugin-react#1117, eslint-plugin-react is breaking our builds.

specs

Pin eslint-plugin-react to version <6.10.1

misc

{optional: other useful info}

add eslint-plugin-import/first

requirements

From their doc

Notably, imports are hoisted, which means the imported modules will be evaluated before any of the statements interspersed between them. Keeping all imports together at the top of the file may prevent surprises resulting from this part of the spec.

This is currently happening in every project we have that uses:

• babel6
• metarpheus
• prelude

In these projects we call the prelude function before metarpheus import, but since babel6 that import is (correctly) hoisted to the top so the prelude is actually called after it leading to some defects

specs

{optional: describe technical specs to implement this feature, if not obvious}

misc

{optional: other useful info}

Evaluate eslint-plugin-compat

See https://github.com/amilajack/eslint-plugin-compat

Add array-bracket-spacing

Add http://eslint.org/docs/rules/array-bracket-spacing

We essentially already follow the default, but using linto I've found a few instances where this isn't true.

linto report:

	repo	errors	warnings
❌	buildo/aliniq	4	0
✅	buildo/web-shared	0	0
❌	buildo/react-components	10	0
❌	buildo/ipercron	4	0
✅	buildo/avenger	0	0
❌	buildo/bobafett	10	0
✅	buildo/formo	0	0
✅	buildo/hophop	0	0
✅	buildo/pledge	0	0

add object-shorthand

not sure if it was there or not, but it's not anymore and it's not part of eslint:recommended

it should be: "object-shorthand": [2, "always"]

[Rules] Add max-len rule

requirements

We should add the rule to lint very long lines.
The default value for code line is 80, which could be fine (to test before) and comments are configurable.

specs

• eslint max-len

misc

{optional: other useful info}

c/c @buildo/frontend

Add gdsm to linto config

Enfore space before '/>' in self-closing tags

requirements

In case of self-closing tags (e.g., <MyComponent />) a space should always be present before the closing />.

Example:
The following should be considered an error: <MyComponent/>

specs

{optional: describe technical specs to implement this feature, if not obvious}

misc

{optional: other useful info}