hapijs / eslint-config-hapi

Shareable ESLint config for the hapi ecosystem
Other
22 stars 23 forks source link

Migrate to eslint v2 #31

Closed spruce-bruce closed 8 years ago

spruce-bruce commented 8 years ago

http://eslint.org/docs/user-guide/migrating-to-2.0.0

I wouldn't mind submitting a PR, but I won't have time for a couple days probably.

AdriVanHoudt commented 8 years ago

:+1: (I want to help but also not a lot of time :/ )

spruce-bruce commented 8 years ago

Added a PR #32 - some notes:

The main changes here are

  1. Update tests to check for new message format (using single instead of double quotes)
  2. no-arrow-condition rule was removed. In order to check for if ((foo) => 1) { you have to use no-constant-condition instead, which also checks for if (true) {, etc
  3. space-after-keywords has been removed. keyword-spacing is used in its place. keyword-spacing is a more far reaching rule and includes spaces before keywords as well. In this PR keyword-spacing is on, but we can configure this rule to require spaces after and disallow spaces before keywords. It doesn't look like this can be configured to require spaces after and not care about spaces before. I'm not sure if this is important.
cjihrig commented 8 years ago

Closed in bd415cbebb3c6d4d25447206b92eecee0ff302dc

prashaantt commented 8 years ago

I just updated to v9. I know this doesn't properly fit in hapi's domain but I wanted to leave this here, fwiw.

My hapi project uses ES6 modules in the client-side code. With the new space-after-keywords rule, things like

import { Component } from 'react';
import * as actions from './actions';

don't lint for spaces around the from and as. I have had to now override the rule in my project as:

"keyword-spacing": [2, { "after": true }]

which then doesn't care about spaces before keywords. Perhaps it would be good to leave out the "before": false rule from this plugin itself as it could potentially cause similar such issues.

cjihrig commented 8 years ago

This is a really annoying situation. In older versions of ESLint we used space-after-keywords, but did NOT use space-before-keywords (in accordance with the hapi style guide). Now that we have to use keyword-spacing, my understanding (someone correct me if I'm wrong on all of this) is that the before setting is either true or false, and that there is no option to just ignore. Your proposal of leaving out before: false actually means making it before: true. I would be OK with doing this if we can prove that it doesn't create any new linting errors for code that adheres to the style guide.

cjihrig commented 8 years ago

OK, I've tested a few hapi modules and having before: true seems fine. I'll take a PR for it.

prashaantt commented 8 years ago

You're right, rewriting the above rule as follows clarifies the intent:

"keyword-spacing": [2, { "before": true }] // by default, `after` is true, `before` is false

As for whether this won't break existing code, space-before-keywords used to default to always, which to me looks equivalent to the new "before": true. I think we should be fine here.

edit: oops, you beat me to it :) PR on the way.

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.