Add JS linting

[robinwhittleton]

If we’re going to lint front-end SASS assets it’d be good to extend this to JS assets as well. @dsingleton partially added this in a branch a while ago but it needed further discussion and never was merged.

What are people’s thoughts as to the use of this? We’d need to define some rules (we’ve got some already) but that could be done under the purview of the Service Patterns work.

[dsingleton]

+1 to the general idea.

I think the benefits we'd want from automatic linting are;

• Catching syntax errors, types and bug smells (unused variables)
• Rules being exposed to code editors, to apply in real time
• Enforcing a standard coding style across projects, for consistency / readability
• The enforcing being done by a computer, rather than humans (which isn't fun for anyone)

eslint seems to be a popular choice for structural linting, but doesn't cover style conventions (brackets, whitespace, etc). The branch you linked to also had issues with exposing the eslint file in a way that could be picked up by editors - which feels pretty important.

If we're considering this as part of the Service Patterns work, then defining some rules gets a little more complicated/contentious, as it applies to more than just GOV.UK. Generally i'm wary on big rule discussions/bike shedding

I think we should also consider http://standardjs.com - it's becoming increasingly popular, and side-steps the issue of debating linting rules. There are some established community rulesets (eg, airbnb's) we might want to look at.

[tvararu]

I'm a huge proponent of standard/semistandard/batteries-included and anti-bikeshedding options. As for the merits of linting, as a 3 year JSHint/JSCS/Eslint (in that order) user it's a no-brainer and I wouldn't dream to write JS today sans a linter.

eslint seems to be a popular choice for structural linting, but doesn't cover style conventions (brackets, whitespace, etc).

JSCS AFAIK still fills in more styling convention gaps than Eslint, but if I had to personally pick between more stricter styling checks and the simplicity of just running standard the second is just so much less overhead.

[tvararu]

Also: I've looked very briefly into adding support for eslint. I found this gem https://github.com/appfolio/eslint-rails but I wasn't keen on:

• The fact that a copy of eslint is included as a hardcoded blob within the source repository https://github.com/appfolio/eslint-rails/blob/master/vendor/assets/javascripts/eslint.js, which is run through ruby-rhino (the latter is fine, the former seems unnecessary).
• The fact that it appears to add additional controller entries/routes to the app https://github.com/appfolio/eslint-rails#web-interface. Also seems unnecessary and there doesn't appear to be a way to opt out of them.

[tombye]

Yes please, linting is a great idea. Before the days of JS unit tests (for me anyway) linting saved my life many a time 👍

[robinwhittleton]

In the frontend meeting today we again discussed using standard as the default solution for linting JS. @tvararu has implemented this for the prototype kit (close to being merged) and we’re going to see how that works. If it does then potentially we might want to pick a GOV.UK application and use standard locally on that. After that I think we’d be happy to implement standard as part of govuk-lint.

[fofr]

Difficulties using Standard linting in this repo described here: #63

[theseanything]

This gem has been deprecated and with a move away from an "all-in-one" linting tool