search

imperodesign / generator-impero

Impero's website skeleton generator, courtesy of Yeoman
Other
1 stars 1 forks source link

Use standard JS for linting #38

Closed MrSlide closed 7 years ago

MrSlide commented 8 years ago

Standard JS removes any discussion or friction because of personal preferences. It also avoids having to maintain a list of ESLint rules. There are a few tools available for IDEs as well that can facilitate in keeping the code consistent.

The code should be linted in a pre-commit hook and fail the commit if there is code that doesn't conform.

samhh commented 8 years ago

This introduces two problems.

  1. We can no longer utilise Webpack's globals/DefinePlugin feature, which is something I'm really keen on using personally.
  2. We can no longer introduce alterations that we as a group agree with. For example, in the future, I believe that Standard is going to suggest double quotes for JSX per React community convention. However that hasn't to my knowledge made it into a release yet, so we'd be forced to use single quotes - something we know is going to be incorrect and would hypothetically require a massive rewrite in the near future. That's a lot more work than maintaining a small ESLint config.

Up to you guys.

MrSlide commented 8 years ago

With Standard JS you can still define environments, which will define a preset of globals, or you can manually define global names to be ignored by the linter.

You can also define blocks of code that will have certain rules be ignored.

I've been using Standard JS as a linter while developing on the Impero web site and so far it hasn't pointed out any errors related to JSX. The only errors it has been pointing out are things like modules being loaded but never used.

samhh commented 8 years ago

That's strange... it definitely gave me warnings about JSX quotes (see: https://github.com/feross/eslint-config-standard/issues/27).

In Standard, to define the globals you (I think) have to either pollute the codebase or npm's package manifest. In which case... surely it's better to just put that configuration in a config file specifically for the linter a la ESLint?

Another concern is that if you choose to extend the template provided by this generator in the future, for example with TypeScript or something, then I'd expect Standard to have fewer supported plugins than ESLint, whereas with ESLint you can just extend right over the top of the Standard config.

pvalentim commented 8 years ago

Standard is just a set of rules for eslint. The point is to end debates about style. Having an extensible eslint config based on standard is not a bad idea but for the sake of simplicity of this generator I would favour to just use standard or maybe even have the option to choose.

As a global note about all these discussions I think we should aim for this to be base for all Impero projects. That means less is more, we don't want to clutter projects with stuff they don't need. After you generate you can and should still adapt things to the project in hand.

samhh commented 8 years ago

The simplicity to which you refer is simply not having a preconfigured .eslintrc file in your project root. I don't think that's worth the tradeoffs.

It's up to you guys though. If you go with Standard while I'm away then remember to add something for the DEVMODE DefinePlugin global. :-)

samhh commented 7 years ago

Closed for inactivity. If this is to happen then at the very least the above issue needs a solution (that I imagine would be more complex than just using ESLint...).