Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

use tslint rules for React hooks linting#3517

Closed
sqs wants to merge 1 commit into
masterfrom
lint-react-hooks
Closed

use tslint rules for React hooks linting#3517
sqs wants to merge 1 commit into
masterfrom
lint-react-hooks

Conversation

@sqs

@sqs sqs commented Apr 20, 2019

Copy link
Copy Markdown
Member

Uses https://github.com/Gelio/tslint-react-hooks for linting React hooks code to find common mistakes, as recommended by https://reactjs.org/docs/hooks-rules.html.

Addresses https://github.com/sourcegraph/sourcegraph/pull/3392#discussion_r275895266.

Note: We plan to switch from TSLint to ESLint soon (https://github.com/sourcegraph/sourcegraph/issues/2461). In that case, we will use the more standard ESLint rules for React hooks instead.

Confirmed working; if I add a useEffect in an if-statement, it reports:

/home/sqs/src/github.com/sourcegraph/sourcegraph/shared/src/components/linkPreviews/WithLinkPreviews.tsx:27:9
ERROR: 27:9     react-hooks-nesting               A hook cannot appear inside an if statement

Uses https://github.com/Gelio/tslint-react-hooks for linting React hooks code to find common mistakes, as recommended by https://reactjs.org/docs/hooks-rules.html.

Addresses sourcegraph/sourcegraph#3392 (comment).

Note: We plan to switch from TSLint to ESLint soon (sourcegraph/sourcegraph#2461). In that case, we will use the more standard ESLint rules for React hooks instead.

Confirmed working; if I add a `useEffect` in an if-statement, it reports:

```
/home/sqs/src/github.com/sourcegraph/sourcegraph/shared/src/components/linkPreviews/WithLinkPreviews.tsx:27:9
ERROR: 27:9     react-hooks-nesting               A hook cannot appear inside an if statement
```
@sqs sqs added the webapp label Apr 20, 2019
@sqs sqs requested a review from felixfbecker April 20, 2019 06:29
@sqs sqs mentioned this pull request Apr 20, 2019
1 task
@codecov

codecov Bot commented Apr 20, 2019

Copy link
Copy Markdown

Codecov Report

Merging #3517 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

Impacted Files Coverage Δ
web/src/user/settings/backend.tsx 28.12% <0%> (ø)

@felixfbecker felixfbecker left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to not add this to our shared TSLint config?

@sqs

sqs commented Apr 21, 2019

Copy link
Copy Markdown
Member Author

@felixfbecker No, good call. Closing this in favor of sourcegraph/tslint-config#9.

@sqs sqs closed this Apr 21, 2019
@sqs sqs deleted the lint-react-hooks branch April 21, 2019 01:29
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants