standard / eslint-config-standard

ESLint Config for JavaScript Standard Style
https://standardjs.com
MIT License
2.6k stars 564 forks source link

extract the React parts into a separate config? #2

Closed jokeyrhyme closed 9 years ago

jokeyrhyme commented 9 years ago

Would it be possible to extract the JSX / React parts into a separate config (and remove them from this config)?

I have non-React JavaScript projects where it feels a bit weird having to install the ESLint React plugin just to be able to get a JavaScript Standard and Semi-Standard linting up and running.

This is such a useful shared config, I really appreciate the effort you've spent making it as comprehensive as it is. Great work!

dcousens commented 9 years ago

@feross I guess this would mean we'd have eslint-config-standard and eslint-config-standard-react? Assuming we do split it up anyway.

I'm not swayed either way, since I use standard, I'm not sure how many people might be using this configuration directly without the JSX support.

jokeyrhyme commented 9 years ago

@dcousens well, "extend" can take an Array of configs, so it wouldn't be necessary to publish all the different permutations. Projects can extend both a "standard" and "react" easily, or maybe combine "react" with "semistandard", or any sort of thing that a config sharer doesn't have to worry about.

jokeyrhyme commented 9 years ago

I'd actually suggest removing everything that isn't specified in the JavaScript Standard specification (e.g. the environments and ECMA features? maybe even globals?) to allow for all sorts of combinations with other useful shared configs.

feross commented 9 years ago

I think moving the React parts into another package makes sense, with the goal of making it so projects that don't use React won't need to install eslint-plugin-react. Here's an example where the Karma project had to do just this: https://github.com/karma-runner/karma/blob/67a6054835eda14bc28900d40796c4d6ece2c3ab/package.json#L223-L224

I'm less excited about @jokeyrhyme's other suggestions (removing the environments, ECMA features, globals) because I don't see the benefit. That stuff is relevant to the standard rule checks and removing it will just mean that people have to add more crap in their .eslintrc besides the barebones:

{
  "extends": "standard"
}

If you want to overwrite any of the rules, globals, etc. defined in eslint-config-standard, you can already do that. Just add those rules alongside the "extends" rule.

jokeyrhyme commented 9 years ago

@feross fair enough. Reviewing the standard again, I see that specific global variables are mentioned, so that makes sense to keep, and I agree that it's pretty easy to override those. It's quite difficult to override the React dependency, so that's the part that was causing me the most friction.

Would you accept a PR for this, or is this something you'd want to tackle yourself?

feross commented 9 years ago

It's already done :)

eslint-config-standard 3.0.0 is released with the React rules removed. eslint-config-standard-react 1.0.0 is released with just the react rules.

Going forward, the way to get both shareable configs is to do this:

{
  "extends": ["standard", "standard-react"]
}

Using an array in "extends" is only supported in eslint master at the moment, so I would wait for the next eslint release before upgrading if you need React and JSX support.

jokeyrhyme commented 9 years ago

@feross thanks!

feross commented 9 years ago

Just released standard 4.1.0 with these changes :) You can see how standard uses both configs here: https://github.com/feross/standard/blob/22e640da289a6d30e631bacbd85037ba33d776de/rc/.eslintrc