ESLint file about development HOT 14 CLOSED

Would it make sense to add a short explanation comment of each set option directly in the "official" .eslintrc file?

from development.

I'd vote for adding a comment at the top of the file with a link to the page detailing the explanations. But I'd be open to what you're suggesting as well @anselmbradford.

from development.

I just figure for something like this the settings mean what they mean so they're not going to be changing much and it keeps everything together.

Another thought on this: Is there any advantage to having the config as a .eslintrc file vs a visible JSON or YAML file? If the config were in a JSON/YAML file the .eslintrc file could be added to .gitignore and allow local overrides of the settings for a project, but maybe we want to discourage that.

from development.

@anselmbradford I just followed the convention of other projects I'd seen, but I have no objection to a visible file. We should be able to add it to the .gitignore either way though.

Does anyone have specific thoughts on the settings above? Is it too verbose? Are there things that should be changed or removed? Once we have a file we like, I'd be happy to add it to the cf-generator and work on writing a plain language summary.

from development.

I think your first pass is a good one. Here are my own personal thoughts on what I would adjust:

Rules I would relax

no-process-exit: 1 Downgrade to warning. Even in large apps you'll want to control how the app exits at some point so it's not always bad to explicitly exit.
no-shadow: 1 Downgrade to warning. To me there's legitimate cases for shadowing vars, a common one is when you have nested functions with callbacks of the same name, and you really shouldn't call any outside of your scope anyway so the shadowing is a good thing. It can also be annoying to have to choose multiple names for the same thing.
curly: [2, "multi-line"] Omitting braces on single-line IFs should be OK I think.
no-use-before-define: [2, "nofunc"] Make exception to allow functions to be defined out of the way at the bottom of blocks containing them.
quotes: [2, single, "avoid-escape"] Allow doubles when string contains singles, to avoid escaping.

Rules I would enforce

indent: 2 We should probably all agree on an indentation style, regardless of which. I personally really like 4 spaces.
default-case: 2

from development.

Those are all fantastic suggestions. Thanks @cjfont!

from development.

I'm in favor of warnings in cases like the above post by @cjfont, now that I think about it. I favor a warning-dominant approach, I think... ⚠️ ⚠️ ⚠️

And not to start the Two-or-Four Space Wars (again? 👾 🚀 🌠 ), but the CFPB already converted me to a 2-spacer (from a not-carer), and I think that's the majority opinion. Am I right?

from development.

And not to start the Two-or-Four Space Wars (again? 👾 🚀 🌠 ), but the CFPB already converted me to a 2-spacer (from a not-carer), and I think that's the majority opinion. Am I right?

Yeah, I personally prefer 2 spaces, but I don't care much either way as long as it's consistent. eRegs, is actually 4 spaces because we were switching between JS and Python and it just seemed easier at the time.

from development.

And by "consistent" I mean consistent at the project level.

from development.

Agreed that the most important thing here is consistency. The problem is that in ESLint in order to ensure that indentation is being done correctly, it seems you are forced to pick a style. Wouldn't it be nice to have an option that auto-senses what style you're trying to use, and enforces it for the entire document?

from development.

That would be awfully nice. I'd say that projects could be free to tweak their ESLint file to match their specific needs. If we're linting every JS file moving forward, we're already in a better place than we are now.

from development.

Here's an updated fule based on @cjfont's feedback:

ecmaFeatures:
  ecmascript: 6

env:
  browser: true
  node: true

globals:
  jQuery: true
  $: true

rules:
  # Possible errors
  no-console: 1
  no-extra-parens: 2
  no-irregular-whitespace: 2
  no-reserved-keys: 2

  # Best practices
  block-scoped-var: 2
  curly: [2, "multi-line"]
  default-case: 2
  eqeqeq: [2, smart]
  guard-for-in: 1
  no-alert: 2
  no-caller: 2
  no-eval: 2
  no-extend-native: 2
  no-floating-decimal: 2
  no-undef: 2
  no-with: 2
  wrap-iife: [2, inside]

  # Variables
  no-shadow: 1
  no-shadow-restricted-names: 2
  no-use-before-define: 2

  # Strict Mode
  strict: [2, "global"]

  # Node.js
  no-mixed-requires: 2
  no-process-exit: 1

  # Stylistic issues
  indent: [2, 2]
  brace-style: 2
  camelcase: 2
  new-cap: 1
  no-lonely-if: 2
  no-mixed-spaces-and-tabs: 2
  no-space-before-semi: 2
  no-trailing-spaces: 2
  no-underscore-dangle: 0
  semi: [2, "always"]
  space-after-keywords: 2
  space-in-brackets: [2, never]
  space-infix-ops: 2
  quotes: [2, single, "avoid-escape"]

from development.

We could consider matching our format directly to the eslint source lint file structure. The original idea of a comprehensive lint file is it helps with diffs, but ours doesn't help matching against the one in the official eslint repo. However, their's is JSON and our's is YAML, so that's an additional hurdle to syncing. Anyway, just Friday thoughts...

from development.

@anselmbradford has been all over this. Closing!

from development.