Add ESLint configuration #52
Conversation
…-native-cli into feat/eslint
| @@ -34,4 +34,4 @@ const withPods = { | |||
| ios: ios.pod, | |||
| }; | |||
|
|
|||
| module.exports = {flat, nested, withExamples, withPods}; | |||
There was a problem hiding this comment.
so we go with non-fb style of extra spacing in destructuring statements?
There was a problem hiding this comment.
Back to the roots! https://github.com/rnpm/rnpm/blob/master/src/config/android/index.js#L110
On a serious note: I don't have preference here. Just sticking to whatever ESLint preset is recommending. I guess we can always change that in case we agree on a general preset for all RNCommunity projects - right now, it's pretty much up to project maintainers.
| let commands = acc.commands; | ||
| let platforms = acc.platforms; | ||
| let haste = acc.haste; | ||
| let { commands, platforms } = acc; |
There was a problem hiding this comment.
Yes, but this would cause another error: using "let" where "haste" is not re-assigned. Wanted to avoid double-destructing statement for the sake of it.
|
Thanks @thymikee - that's right! Luckily, most of the changes were performed automatically by ESLint, so I am quite confident they will not cause major issues. |
| @@ -10,8 +10,6 @@ | |||
| import type { ContextT } from '../core/types.flow'; | |||
| import type { CommandLineArgs } from './bundleCommandLineArgs'; | |||
|
|
|||
| ('use strict'); | |||
There was a problem hiding this comment.
You probably shouldn't remove 'use strict' if you want your scripts to run in strict mode as you're not using ES6 modules.
There was a problem hiding this comment.
That's already handled by Babel plugin
There was a problem hiding this comment.
@thymikee which plugin does it? according to babel's docs, the sourceType defaults to "script" and will be "module" only if you're using import/export. so since the code here uses require and not import/export, it shouldn't insert 'use strict'.
There was a problem hiding this comment.
This is what's used: @babel/plugin-transform-strict-mode. Does it matter which sourceType we use when having that?
There was a problem hiding this comment.
@thymikee if you use that it shouldn't matter I think, but where is it specified? i couldn't find it here: https://github.com/react-native-community/react-native-cli/blob/master/packages/local-cli/babel.config.js#L4
There was a problem hiding this comment.
nevermind. it's newly added in the diff
There was a problem hiding this comment.
Yea, I requested it :D glad you're up to date now :)
This PR adds ESLint configuration with Prettier and automatically fixes most of the issues. Some of the changes had to be done manually.
ESLint is now part of our testing pipeline.
3 warnings remaining for disabled tests, to be implemented in upcoming PRs.
I excluded "debugger-ui" folder from ESLint based on historical issues we had with running ESLint in Haul on that folder. This runs in browser context "as-is" without transpilation and should not be changed.
"templates" folder will be gone in favour of the one present in React Native master (to be done as a next PR).