Eslint rules discussion

[acabreragnz]

"rules": {

    // Our base eslint with airbnb
    "comma-dangle": [
      "error",
      "never"
    ],

    // Necessary to follow a clean code approach
    "no-use-before-define": [
      "error",
      {
        "functions": false,
        "classes": false
      }
    ],

   // Code can have empty space to separe block of things
    "no-trailing-spaces": [
      "error",
      {
        "skipBlankLines": true
      }
    ],

    // In express applications is very comon to modify the response
   // In general I think that is not a bad practice to update a property
    "no-param-reassign": [
      "error",
      {
        "props": false
      }
    ],

    // 80 is the default, Intellij allows 121
    "max-len": [
      "error",
      121
    ],

    // Don't allow ternary operator, why?
    "no-unused-expressions": [
      "error",
      {
        "allowTernary": true
      }
    ],

    // In express, error handlers are a special kind of middlewares with 4 arguments
   // In the last error handler is unnecesary to call the last argument (next), but it must appear
    "no-unused-vars": [
      "error",
      {
        "args": "none"
      }
    ],

    // I prefer {color: 'blue'} than { color: blue }
    "object-curly-spacing": [
      "error",
      "never"
    ],

    // I prefer function() than function ()
    "space-before-function-paren": [
      "error",
      "never"
    ]
}

In test directory we should define another .eslintrc.json with this rule

  "rules": {
    "no-unused-expressions": 0
  }

Specially if we are using chai.js, this kind of expect gives an error:
expect(null).to.be.null;

Let discuss about the rules I mentioned above, what do you think?

[rkmax]

I liked the changes you did, I'm using this one https://github.com/feross/eslint-config-standard but things like space-before-function-paren and object-curly-spacing are annoying for me, so 👍 x 💯

[rkmax]

@acabrera91 I would like to propose use the "standard" plus this rules, something like follows (sorry no comments), the following content is yours plus a couple of thing more, like babel and add the standard rules of javascript

{
  "parser": "babel-eslint",
  "extends": [
    "standard"
  ],
  "plugins": [
    "babel"
  ],
  "rules": {
    "key-spacing": 0,
    "max-len": [
      2,
      120,
      2
    ],
    "comma-dangle": [
      "error",
      "never"
    ],
    "no-use-before-define": [
      "error",
      {
        "functions": false,
        "classes": false
      }
    ],
    "no-param-reassign": [
      "error",
      {
        "props": false
      }
    ],
    "no-unused-expressions": [
      "error",
      {
        "allowTernary": true
      }
    ],
    "object-curly-spacing": [
      "error",
      "never"
    ],
    "space-before-function-paren": [
      "error",
      "never"
    ],
    "semi": ["error", "always"]
  }
}

For this one the dependencies will look like

...
    "eslint": "^3.15.0",
    "eslint-config-standard": "^6.2.1",
    "eslint-plugin-babel": "^4.0.1",
    "eslint-plugin-promise": "^3.4.2",
    "eslint-plugin-standard": "^2.0.1",
...

[jac1013]

Hello @acabrera91 I have doubts about this:

 // In express, error handlers are a special kind of middlewares with 4 arguments
 // In the last error handler is unnecesary to call the last argument (next), but it must appear
    "no-unused-vars": [
      "error",
      {
        "args": "none"
      }
    ],

imho what you are explaining in the comments is a problem with express, to me this rule make sense and helps to write better code.

[jac1013]

Hey guys, so what about joining what @rkmax improved in his comment and use what @acabrera91 created and start from that? Toni I'm still waiting on the response to the comment above, what do you think?

[jac1013]

Any update on this @acabrera91 @rkmax ?

[rkmax]

I think we're waiting for @acabrera91 answer

[ghost]

These rules looks good to me, I would like to start using what @rkmax proposed + @acabrera91

 // In express, error handlers are a special kind of middlewares with 4 arguments
 // In the last error handler is unnecesary to call the last argument (next), but it must appear
    "no-unused-vars": [
      "error",
      {
        "args": "none"
      }
    ],

About this one maybe for that case it will be good to use inline comments to disable that rule.

// eslint-disable-line no-unused-vars

[rkmax]

I Agree with you @evasquez26 because it's special use case and not the regular way

[ghost]

I would also like to propose a change to the max-len rule:

    "max-len": [
      "error",
      {
        "code": 120,
        "tabWidth": 2,
        "ignoreStrings": true,
        "ignoreTemplateLiterals": true,
        "ignoreRegExpLiterals": true,
        "ignorePattern": "^\\s*var\\s.+=\\s*require\\s*\\(/"
      }
    ]

[acabreragnz]

I agree with all of you.

BTW, what are your thougts about using plusplus operator?

In my project I've desabled this rule,
"no-plusplus": 0

[jac1013]

@acabrera91 I'm fine with it, I rarely use that or --, when are we going to be ready to lock this version of lint rules? I think we should provide a file now and if we want to make improvement we create new issues later, I would like to start using something.

[acabreragnz]

I think I need the help of @rkmax He proposed to use the standard instead of airbnb

[jac1013]

Cool! Hey @rkmax can you come up with a final-initial version of the file so we can start using it now?

[ghost]

By the way I used the package config of @rkmax and this package is missing: babel-eslint

[ghost]

eslint-plugin-import is also missing

[ghost]

eslint-plugin-node is algo missing on the packages @rkmax suggested. I'm just keeping track of this