Add .eslintrc to RN project template #23901
Conversation
facebook-github-bot
added
the
CLA Signed
template/_eslintrc
Outdated
| "extends": "@react-native-community", | ||
|
|
||
| "env": { | ||
| "jest": true |
There was a problem hiding this comment.
I suggest something like this:
overrides: {
files: ['**/__tests__/**/*.js', '**/?(*.)(spec|test).js'],
env: {
jest: true,
'jest/globals': true,
},
},There was a problem hiding this comment.
Thanks @satya164, I think it's a good point.
There was a problem hiding this comment.
Maybe we should add it to community config itself?
There was a problem hiding this comment.
I think keeping eslint test runner configuration within the project is ok. Jest is integrated into RN template, but other projects might depend on a different test runner. I'm fine with both solutions tho.
There was a problem hiding this comment.
@michalchudziak can we put it into the eslint config? I think that's better, especially since people can create a project without Jest still and it makes this a bit confusing.
template/_eslintrc
Outdated
| "env": { | ||
| "jest": true | ||
| } | ||
| } |
|
The .eslintrc configuration file is deprecated according to eslint docs: My preference would be to use .eslintrc.js, so we can add comments, but .eslintrc.json would be fine if someone feels strongly otherwise. |
|
Let's do Quick question: what happens to existing projects with an |
cc @pvinis do you know what will happen? |
|
It will be a merge conflict, which is fine. Many people have their own, so it's good to see what they have and what the template suggests. The only weird case I see is if people have eslint json and we have eslint js, that might lead to some funkiness, but I don't think there are many things we can do here besides announce it to people. |
| @@ -12,6 +12,8 @@ module.exports = { | |||
|
|
|||
| env: { | |||
| es6: true, | |||
| jest: true, | |||
There was a problem hiding this comment.
Shouldn't it be under overrides like:
overrides: {
files: ['**/__tests__/**/*.js', '**/?(*.)(spec|test).js'],
env: {
jest: true,
'jest/globals': true,
},
},I think adding all jest globals to the normal env isn't great.
| @@ -24,6 +24,14 @@ module.exports = { | |||
| 'jest', | |||
| ], | |||
|
|
|||
| overrides: { | |||
| files: ['**/__tests__/**/*.js', '**/?(*.)(spec|test).js'], | |||
There was a problem hiding this comment.
Should include TS and TSX files as well
There was a problem hiding this comment.
@thymikee Are you sure that's something we want to add? TSEslint configuration on the project side will be required anyway. It seems logical to keep it on the project side for the projects that use TS.
There was a problem hiding this comment.
Eslint now supports TS (or is planning to) so it makes sense.
There was a problem hiding this comment.
Wouldn't we also need to add the parser as well? Otherwise, ESLint will fail to parse no?
Reference https://github.com/facebook/jest/blob/master/.eslintrc.js#L15
PS: I agree it'll be great to have this by default
There was a problem hiding this comment.
@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
This pull request was successfully merged by @michalchudziak in 395197d. When will my fix make it into a release? | Upcoming Releases |
Summary
The goal of this PR is to enable eslint checks in the projects generated by
react-native initcommand. I addedtemplate/_eslintrcfile, that would be replaced in an initialized project with.eslintrcfile. This PR comes in parallel with react-native-community/cli#229Changelog
[General] [Added] - Added
.eslintrcfile to generated template.Test Plan
Check out to this branch.
Follow setup from https://github.com/react-native-community/react-native-cli/blob/master/CONTRIBUTING.md
Expected result:
Eslint check ran with no issues found.