ESLint and styleguides

[Paul7Peterson]

Hello everyone!

I open this discussion so we can have a debate about a thing that I start to find necessary in the project, especially considering that is open to everyone to contribute: the ESLint config.

Here are some of the basic ones that I will propose based on what I've been seeing in the project:

{
   // ...
   "overrides": [
    {
      "rules": {
        "no-unused-vars": ["error", { 
          "vars": "all",
          "varsIgnorePattern": "^_"
        }],
        "semi": ["error", "always"],
        "quotes": ["error", "single"],
        "no-multi-spaces": ["error", { 
          "ignoreEOLComments": false 
        }],
        "comma-dangle": ["error", "always"],
        "no-delete-var": "error",
        "no-var": "error"
      }
    },
  ]
}

The reasons for these rules I propose:

• "no-unused-vars": unused var is dead code included in the build and that is not bringing anything. If something is temporarily decoupled, must be commented with a reason to keep the code there for now. Otherwise, we have version control.
• semi: semicolons are optional, and so it is washing your teeth. But the semicolon avoids some issues like the case of an array after brackets, casting, and many other situations where the absence of semicolon can produce issues.
• quotes": single quoting is cleaner and the single quote should not be used for the genitive (or using the scaped version) and the double quotation is used in the English grammar occasionally. The idea is to be consistent and avoid problems.
• comma-dangle: makes the refactoring way easier.
• no-delete-var: a bad practice.
• no-var: let and const are way more predictable and embrace good practices. I would even set the use of const as compulsory when there's no resignation of the variable.

Feel free to propose yours with the right arguments. 😉

[ericbolikowski]

Hey @Paul7Peterson, thanks for raising this topic. I'm a big fan of a consistent coding style/syntax across the entire project.

What I'd like to know first is how the rules you propose differ from the Nx defaults. We can probably find those defaults by digging a bit, but AFAIK at least a few rules are already in place, including comma-dangle. no-multi-spaces and quotes.

What I'm unsure about is whether existing rules are set up with a "error" configuration. I set up my VsCode to format-on-save which works nicely.

I'd like to see/look into:

• pre-commit hook disallowing non-compliant code
• include a guide in the README.md on how to setup format-on-save in VsCode

Your thoughts to the above?

[Paul7Peterson]

When I'm running yarn lint, I'm not receiving any errors or warnings because of the absence of comma-dangle, and I haven't tested for quotes but I think it will be the same. The multi-space must be overridden because of the exception { "ignoreEOLComments": false } for the JSDocs. In any case, being explicit doesn't hurt.

In the config of .vscode/setting.json file, we can add extra configurations for typescript.format.x settings and other interesting things for being common for the people contributing and an extra layer of standardization. That avoids the need for your second bullet.

For the first, you can lint --fix as part of the pipelines, for sure. But some things have to be manually done. I don't know if I would set the lint as a PR condition...