Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(lint): typescript requires special no-shadow #238

Merged
merged 1 commit into from
Nov 29, 2021

Conversation

mikehardy
Copy link
Contributor

In pure javascript eslint config from @react-native-community, the no-shadow rule is on,
which is good, but for typescript it generates false-positives and needs a specific configuration change
to use a typescript-specific rule

Reference:

In pure javascript eslint config from `@react-native-community`, the no-shadow rule is on,
which is good, but for typescript it generates false-positives and needs a specific configuration change
to use a typescript-specific rule

Reference: 
- https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/docs/rules/no-shadow.md
- typescript-eslint/typescript-eslint#2552 (comment)
mikehardy added a commit to mikehardy/luna that referenced this pull request Nov 28, 2021
Hopefully integrates via PR upstream in `@react-native-community`,
this appears to be the standard configuration for no-shadow in
typescript projects, see:

react-native-community/react-native-template-typescript#238
@mikehardy mikehardy requested a review from luancurti November 28, 2021 20:48
@emin93 emin93 merged commit a67375c into main Nov 29, 2021
@emin93 emin93 deleted the mikehardy-eslint-no-shadow branch November 29, 2021 07:43
LunatiqueCoder pushed a commit to LunatiqueCoder/luna that referenced this pull request Nov 29, 2021
Hopefully integrates via PR upstream in `@react-native-community`,
this appears to be the standard configuration for no-shadow in
typescript projects, see:

react-native-community/react-native-template-typescript#238
@friederbluemle friederbluemle mentioned this pull request Dec 16, 2021
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request Dec 1, 2022
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
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Dec 1, 2022
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
kelset pushed a commit to facebook/react-native that referenced this pull request Dec 13, 2022
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
OlimpiaZurek pushed a commit to OlimpiaZurek/react-native that referenced this pull request May 22, 2023
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants