Create a shareable ESLint configuration package #689
Conversation
| @@ -0,0 +1,21 @@ | |||
| { | |||
There was a problem hiding this comment.
We'll also want a readme here, explaining in a few words how to use this package as standalone.
| "eslint-plugin-flowtype": "^2.18.1", | ||
| "eslint-plugin-import": "^1.12.0", | ||
| "eslint-plugin-jsx-a11y": "^2.2.2", | ||
| "eslint-plugin-react": "^5.2.2" |
There was a problem hiding this comment.
I would like to keep dependencies pinned, and update them manually. Our setup is a little more conservative.
There was a problem hiding this comment.
While we're at it, can you look into updating us to eslint-plugin-react@6? Have any rules changed?
There was a problem hiding this comment.
I pinned the dependencies and added a README. I can look into updating the plugins tomorrow.
| 1. Install this package, ESLint and the necessary plugins: | ||
|
|
||
| ``` | ||
| npm install eslint-config-react-app [email protected] [email protected] [email protected] [email protected] [email protected] [email protected] --save-dev |
There was a problem hiding this comment.
I think having to install all the dependencies separately like this is ridiculous, but until ESLint supports plugin dependencies for shareable configs, there isn't anything we can do about it, I'm afraid :(
There was a problem hiding this comment.
We can do the same trick as here: https://github.com/airbnb/javascript/tree/master/packages/eslint-config-airbnb#eslint-config-airbnb-1. I'm just not sure what the Windows version would be like.
There was a problem hiding this comment.
I guess we could write it in JS and have a node -e '<SCRIPT>' here, so it would be the same for Windows? I assume everyone who is going to use this will have node installed.
There was a problem hiding this comment.
I think having the command that you can copy paste to your console here is better than that trick, as long as we keep the version numbers up-to-date. It's a bit more work for us, but easier for the users.
| useEslintrc: false | ||
| }, | ||
| // @remove-on-eject-end |
There was a problem hiding this comment.
Don't we need this for prod config too?
There was a problem hiding this comment.
We copy the .eslintrc to the ejected project, so we don't need to specify a custom configFile path or to disable the .eslintrc. I think people prefer the rc files for configuration in an ejected project (see #410).
There was a problem hiding this comment.
I didn't mean ejected, I meant webpack.config.prod.js. Don't we need to update path in it as well as .dev.js?
There was a problem hiding this comment.
Ah, great point. I forgot we had ESLint for the build too. Updated now.
| "eslint": "3.5.0", | ||
| "eslint-config-react-app": "file:packages/eslint-config-react-app", |
There was a problem hiding this comment.
Wow, had no idea you could do that
|
Thanks! |
* Move ESLint configuration to a separate package * Remove the ESLint configuration, moved to eslint-config-react-app * Update ESLint instructions * Pin the package versions in eslint-config-react-app * Add a README for eslint-config-react-app * Update README.md * Update README.md * Update README.md * Update README.md * Update package.json * Update package.json * Update production eslint-loader config * Add the ESLint config to devDependencies of the repo
Move the ESLint configuration to a separate package
eslint-config-react-app. This allows using the same configuration in projects not using CRA. It also makes the amount of configuration in an ejected project smaller: the starter ESLint config of an ejected the project will be just{"extends": "react-app"}.Test plan:
[x] Check that linting errors are shown correctly when running
npm start.[x] Check that it also works end to end. (Tested with a local npm server.)
(NB: the e2e test will break at the moment because the package hasn't been published yet, and it tries to install it from npm. However, I tested by publishing it to a local npm registry with sinopia, and it worked. I think we might want to come up with a way to end-to-end test all the packages together without having to manually pack all of them in the test script. But for now pulling the linter config from npm is probably good enough?)