Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add common .eslintrc file #16

Merged
merged 2 commits into from
Aug 8, 2015
Merged

Conversation

bryanrsmith
Copy link
Contributor

Based on https://github.com/airbnb/javascript/tree/master/packages/eslint-config-airbnb, updated for ESLint 1.0. Aurelia projects can include their own .eslintrc that extends this config and provides any project-specific overrides needed. I opted to fork the airbnb config rather than extend it because it includes a dependency on a react plugin that ESLint does not support overriding, and I didn't want to install that dev dep into every aurelia project.

"space-before-function-paren": [2, "never"], // http://eslint.org/docs/rules/space-before-function-paren
"space-infix-ops": 2, // http://eslint.org/docs/rules/space-infix-ops
"space-return-throw-case": 2, // http://eslint.org/docs/rules/space-return-throw-case
"spaced-comment": [0, "always", { // http://eslint.org/docs/rules/spaced-comment
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to disable this rule because there's a bug that causes it to detect doc comment /** patterns as a violation. We can re-enable when the bug is fixed.

eslint/eslint#3276

@EisenbergEffect
Copy link
Contributor

@bryanrsmith Is this ready to be merged then?

@bryanrsmith
Copy link
Contributor Author

Yep!
On Sat, Aug 8, 2015 at 8:40 AM Rob Eisenberg notifications@github.com
wrote:

@bryanrsmith https://github.com/bryanrsmith Is this ready to be merged
then?


Reply to this email directly or view it on GitHub
#16 (comment).

@EisenbergEffect EisenbergEffect merged commit a11da28 into aurelia:master Aug 8, 2015
@EisenbergEffect
Copy link
Contributor

Merged and released to npm.

@baerrach
Copy link

baerrach commented Jul 16, 2019

@bryanrsmith @EisenbergEffect
Is there anything that discusses why rules are the way they are?

For example, https://github.com/airbnb/javascript says prefer const but this config has prefer-const off.

I've valued having Aurelia's opinions being made explicit with reasonings, as it helps in the cases where my reasoning differs and I can attempt to judge whether I should change my opinion, especially if it's just because that's the way it's always been.

@EisenbergEffect
Copy link
Contributor

@baerrach We’ve actually been migrating a bunch of Aurelia vCurrent away from using this and nothing in vNext uses it. The tools library is a bit of a legacy for us, dating back to the early days of Aurelia, when transpilers were quite different. I wouldn’t necessarily follow the lint rules here. Regarding this particular rule, I would certainly prefer const. I’m not sure why that is off. If I recall, once upon a time, before const was widely shipping in browsers, sometimes the transpilers (Traceuer in particular) would do very inefficient things with it. That may be where this stems from, but I can’t honestly remember anymore.

@baerrach
Copy link

@EisenbergEffect

I try to follow Larry Wall's 3 virtues. So if I can I leverage someone else's hard work on deciding these things. Thus I am left to bike shed the bits left over, and I free up more headspace for core responsibilities.

Does Aurelia have a JavaScript style guide enshrined in an eslint config somewhere else?
vCurrent for templating at least still references "extends": "./node_modules/aurelia-tools/.eslintrc.json"

@EisenbergEffect
Copy link
Contributor

The best place to look is in our vNext TypeScript setup actually. It may not be what you are looking for, if you aren't using TS. But, you can find that here: https://github.com/aurelia/aurelia/blob/master/tslint.json Linting is a ticky subject as we often don't all agree even amongst our team on the best set of rules. Also, let me /cc @BBosman who has really been helping on vNext to ensure our code is consistent in style. He may have some thoughts for you on that subject.

@baerrach
Copy link

I haven't drunk the TypeScript cool aid yet.

These resource indicate that TSLint is being abandoned with the effort moving to support ESLint.

The existing ESLint rules are out of date, and at the time of creationg there probably wasn't the separation of airbnb's rulesets (e.g. to remove react from the base). Now airbnb have things separated so I am able to move forward with this

module.exports = {
  extends: "airbnb-base",
  parser: "babel-eslint",
  env: {
    browser: true, // browser global variables
    node: true, // Node.js global variables and Node.js-specific rules
  },
  parserOptions: {
    ecmaVersion: 6,
    ecmaFeatures: {
      legacyDecorators: true,
    },
  },
  rules: {
    // Override https://github.com/airbnb/javascript when they don't make sense for MEA
  },
}

Install eslint 5 since airbnb hasn't updated to 6 yet.

npm i eslint@^5 -D

Install airbnb peerDependencies listed in:

npm info "eslint-config-airbnb-base@latest" peerDependencies

Peer dependencies:

npm i eslint-plugin-import -D
npm i eslint-config-airbnb-base -D

@baerrach
Copy link

@EisenbergEffect As for disagreements on linting we all have those :) Even prettier is starting to groan under the weight of disagreements.

Is defining those are priority in the Aurelia code base?
Is defining those for cli generated projects a priority? (Auerlia has an opinion on a lot of things and I think it should have one on linting as well that is baked into projects that get created)

Airbnb JavaScript Style Guide is a starting point for that discussion.

Some things I prefer:

  • javascript over other configuration file format. You get comments and all the other niceties of javascript without the limitations of json or yaml.
  • make a stance and include your reasonings. Even if people disagree, its better than nothing and tooling support often helps people fix their code to be compliant.
  • anything too controversial can be left out. The linter is there to catch things. It won't magically make well engineered code, people still have to have a brain and use it. A linter is useful to help make the code base more consistent so that the cognitive load of understanding the code because of differences in style can be reduce/elimintated leaving me free to use my brain cells on understanding how the code works.

@EisenbergEffect
Copy link
Contributor

I think we will have a recommended lint for vNext as part of the skeleton, most likely. As you mention, things are sort of in a transitional state now since TSLint is basically dead. I’m eagerly waiting the new ESLint support so that we can unify all of this and move forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants