Fix ESLint setup #240

friederbluemle · 2021-12-16T10:22:26Z

There are several issues related to the current ESLint/TypeScript setup with the latest version of this template.

Steps to reproduce:

Create new project: npx react-native init rntsapp --template react-native-template-typescript
Run yarn lint
Observe errors

error  Definition for rule '@typescript-eslint/no-shadow' was not found  @typescript-eslint/no-shadow

and

WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.

Also, .eslintrc.js contained a few quote warnings/errors that were also fixed.

/path/rntsapp/.eslintrc.js
  1:1   error    Definition for rule '@typescript-eslint/no-shadow' was not found                                    @typescript-eslint/no-shadow
  5:5   error    Replace `"no-shadow":·"off"` with `'no-shadow':·'off'`                                              prettier/prettier
  5:5   warning  Strings must use singlequote                                                                        quotes
  5:18  warning  Strings must use singlequote                                                                        quotes
  6:5   error    Replace `"@typescript-eslint/no-shadow":·["error"` with `'@typescript-eslint/no-shadow':·['error'`  prettier/prettier
  6:5   warning  Strings must use singlequote                                                                        quotes
  6:38  warning  Strings must use singlequote                                                                        quotes

Related: #112 #150 #238

Fixes #237

emin93

Hi @friederbluemle,

Thanks for the PR. The changes related to fixing ESLint look good and are welcome.

The unrelated changes I advise you to create a PR in the RN repo, so we can move it over as soon it's merged there.

emin93 · 2021-12-16T10:27:18Z

template/_prettierrc.js

  bracketSpacing: false,
-  jsxBracketSameLine: true,


I know that this has been deprecated, but since this template aims to be consistent with the default RN template, I advise you to create a PR for that in the RN repo: https://github.com/facebook/react-native/blob/v0.66.4/template/_prettierrc.js

Makes sense! I only checked that main branch in react-native, which has this addressed already.

emin93 · 2021-12-16T10:27:55Z

template/_prettierrc.js

@@ -1,7 +1,7 @@
 module.exports = {
+  arrowParens: 'avoid',


Here also, please undo the re-sort, so it's consistent with the default RN template.

Okay, I'll undo this, and we'll wait until this arrives in the next release of react-native.

template/_eslintrc.js

friederbluemle · 2021-12-16T10:43:39Z

Hi @emin93 - Thanks for the super fast review. I agree with what you said and removed the commit that changes .prettierrc.js

emin93

Looks good, thanks for the updates 👍

mikehardy · 2021-12-16T11:13:45Z

Sorry about the no shadow issue! That was cherry picked from a config change I needed but didn't work in isolation I guess. Is there no CI here that runs these things?I could look into that if not

mikehardy

Didn't realize those deps for no shadow weren't here yet, bad testing on my part from last PR

emin93 · 2021-12-16T12:04:40Z

@mikehardy Good idea with testing this in the CI. You are more than welcome to look into it. 👍

Summary: ESLint configuration is derived from `react-native/eslint-config`, which has supported Flow, vanilla JS, and TypeScript. react-native-community/react-native-template-typescript#238 and a related PR ended up adding new rules to the TypeScript config, to fix bugs with TS linting. It required a followup change, to specify the parser/plugin, and explicity reference them as devDependencies. react-native-community/react-native-template-typescript#240 `react-native/eslint-config` already includes setting the plugin/parser. But overriding rules requires declaring that again, and directly referencing a plugin means a need for the app to declare dependencies, since ESLint resolves modules from the current config. The rules overridedn were later fixed in `react-native/eslint-config`, which is really the right place for the fix (e.g. facebook#32644). I noticed this when deriving from the TS template in facebook#32644 and removed the rule overrides, but didn't have the historical context to realize this means we can then: 1. Remove the explicit parser/plugins since `react-native/eslint-config` already sets them, and we are no longer overriding any rules. 2. Remove the devDependencies, to let the versions be managed entirely by `react-native/eslint-config`. Changelog: [General][Changed] - Simplify Template ESLint Configuration Differential Revision: D41652699 fbshipit-source-id: 67620f12f8fa0ec94fc4db2cbbd2a60a62b70874

Summary: Pull Request resolved: #35529 ESLint configuration is derived from `react-native/eslint-config`, which has supported Flow, vanilla JS, and TypeScript. react-native-community/react-native-template-typescript#238 and a related PR ended up adding new rules to the TypeScript config, to fix bugs with TS linting. It required a followup change, to specify the parser/plugin, and explicity reference them as devDependencies. react-native-community/react-native-template-typescript#240 `react-native/eslint-config` already includes setting the plugin/parser. But overriding rules requires declaring that again, and directly referencing a plugin means a need for the app to declare dependencies, since ESLint resolves modules from the current config. The rules overridedn were later fixed in `react-native/eslint-config`, which is really the right place for the fix (e.g. #32644). I noticed this when deriving from the TS template in #32644 and removed the rule overrides, but didn't have the historical context to realize this means we can then: 1. Remove the explicit parser/plugins since `react-native/eslint-config` already sets them, and we are no longer overriding any rules. 2. Remove the devDependencies, to let the versions be managed entirely by `react-native/eslint-config`. Changelog: [General][Changed] - Simplify Template ESLint Configuration Reviewed By: cortinico Differential Revision: D41652699 fbshipit-source-id: 8e3313dbf27407c5866f3c2432cffc2ecec1b01d

Summary: Pull Request resolved: #35529 ESLint configuration is derived from `react-native/eslint-config`, which has supported Flow, vanilla JS, and TypeScript. react-native-community/react-native-template-typescript#238 and a related PR ended up adding new rules to the TypeScript config, to fix bugs with TS linting. It required a followup change, to specify the parser/plugin, and explicity reference them as devDependencies. react-native-community/react-native-template-typescript#240 `react-native/eslint-config` already includes setting the plugin/parser. But overriding rules requires declaring that again, and directly referencing a plugin means a need for the app to declare dependencies, since ESLint resolves modules from the current config. The rules overridedn were later fixed in `react-native/eslint-config`, which is really the right place for the fix (e.g. #32644). I noticed this when deriving from the TS template in #32644 and removed the rule overrides, but didn't have the historical context to realize this means we can then: 1. Remove the explicit parser/plugins since `react-native/eslint-config` already sets them, and we are no longer overriding any rules. 2. Remove the devDependencies, to let the versions be managed entirely by `react-native/eslint-config`. Changelog: [General][Changed] - Simplify Template ESLint Configuration Reviewed By: cortinico Differential Revision: D41652699 fbshipit-source-id: 8e3313dbf27407c5866f3c2432cffc2ecec1b01d # Conflicts: # template/_eslintrc.js

Summary: Pull Request resolved: facebook#35529 ESLint configuration is derived from `react-native/eslint-config`, which has supported Flow, vanilla JS, and TypeScript. react-native-community/react-native-template-typescript#238 and a related PR ended up adding new rules to the TypeScript config, to fix bugs with TS linting. It required a followup change, to specify the parser/plugin, and explicity reference them as devDependencies. react-native-community/react-native-template-typescript#240 `react-native/eslint-config` already includes setting the plugin/parser. But overriding rules requires declaring that again, and directly referencing a plugin means a need for the app to declare dependencies, since ESLint resolves modules from the current config. The rules overridedn were later fixed in `react-native/eslint-config`, which is really the right place for the fix (e.g. facebook#32644). I noticed this when deriving from the TS template in facebook#32644 and removed the rule overrides, but didn't have the historical context to realize this means we can then: 1. Remove the explicit parser/plugins since `react-native/eslint-config` already sets them, and we are no longer overriding any rules. 2. Remove the devDependencies, to let the versions be managed entirely by `react-native/eslint-config`. Changelog: [General][Changed] - Simplify Template ESLint Configuration Reviewed By: cortinico Differential Revision: D41652699 fbshipit-source-id: 8e3313dbf27407c5866f3c2432cffc2ecec1b01d

friederbluemle requested review from emin93 and radko93 as code owners December 16, 2021 10:22

emin93 requested changes Dec 16, 2021

View reviewed changes

emin93 reviewed Dec 16, 2021

View reviewed changes

template/_eslintrc.js Show resolved Hide resolved

Fix ESLint setup

55f9da4

friederbluemle force-pushed the fix-eslint branch from 312805c to 55f9da4 Compare December 16, 2021 10:42

emin93 approved these changes Dec 16, 2021

View reviewed changes

emin93 merged commit acff43c into react-native-community:main Dec 16, 2021

friederbluemle deleted the fix-eslint branch December 16, 2021 11:00

mikehardy approved these changes Dec 16, 2021

View reviewed changes

kelset mentioned this pull request Dec 1, 2022

fix(deps, template): remove unnecessary ts eslint deps from template facebook/react-native#35527

Closed

NickGerleman mentioned this pull request Dec 1, 2022

Simplify Template ESLint Configuration facebook/react-native#35529

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix ESLint setup #240

Fix ESLint setup #240

friederbluemle commented Dec 16, 2021 •

edited

Loading

emin93 left a comment

emin93 Dec 16, 2021

friederbluemle Dec 16, 2021

emin93 Dec 16, 2021

friederbluemle Dec 16, 2021

friederbluemle commented Dec 16, 2021

emin93 left a comment

mikehardy commented Dec 16, 2021

mikehardy left a comment

emin93 commented Dec 16, 2021

Fix ESLint setup #240

Fix ESLint setup #240

Conversation

friederbluemle commented Dec 16, 2021 • edited Loading

emin93 left a comment

Choose a reason for hiding this comment

emin93 Dec 16, 2021

Choose a reason for hiding this comment

friederbluemle Dec 16, 2021

Choose a reason for hiding this comment

emin93 Dec 16, 2021

Choose a reason for hiding this comment

friederbluemle Dec 16, 2021

Choose a reason for hiding this comment

friederbluemle commented Dec 16, 2021

emin93 left a comment

Choose a reason for hiding this comment

mikehardy commented Dec 16, 2021

mikehardy left a comment

Choose a reason for hiding this comment

emin93 commented Dec 16, 2021

friederbluemle commented Dec 16, 2021 •

edited

Loading