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

Add rule for React Hooks #204

Closed
wants to merge 2 commits into from
Closed

Add rule for React Hooks #204

wants to merge 2 commits into from

Conversation

Gelio
Copy link

@Gelio Gelio commented Feb 13, 2019

I created a rule for detecting improper uses of React Hooks. For more information about the rule take a look at the tslint-react-hooks repository.

This PR integrates the react-hooks-nesting into this repository.

I built tslint-react with the newly added rule and it works correctly with both react-hooks-nesting and built in this repository (example that reports both react-hooks-nesting for using a hook inside if and jsx-wrap-multiline):

image

tslint.json contained only the following:

{
  "extends": "tslint-react"
}

This PR is related to #186.

@palantirtech
Copy link
Member

Thanks for your interest in palantir/tslint-react, @Gelio! Before we can accept your pull request, you need to sign our contributor license agreement - just visit https://cla.palantir.com/ and follow the instructions. Once you sign, I'll automatically update this pull request.

Copy link

@gibbok gibbok left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@gibbok
Copy link

gibbok commented Feb 27, 2019

Would be fantastic to include your PR in tslint-react. Any update on this? Thanks for your great work!

@Gelio
Copy link
Author

Gelio commented Feb 27, 2019

@gibbok Thank you 🙂 I see many people giving a thumbs-up on this PR, but a maintainer needs to review it. Considering the fact that Palantir is moving development resources from TSLint to ESLint, it may take a while 😟

Copy link
Contributor

@adidahiya adidahiya left a comment

Choose a reason for hiding this comment

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

We're not going to add a new dependency to this package like this. If you want, you can port your rule implementation to this repo.

@Gelio
Copy link
Author

Gelio commented Mar 13, 2019

That's a pity. I would rather keep the hooks rule in my repo to be able to develop it freely. I will close this PR

@Gelio Gelio closed this Mar 13, 2019
@adidahiya
Copy link
Contributor

Sure, you're free to do that, and users can install your package and use extends: "tslint-react-hooks"... that's exactly what that feature was designed for.

@sonhanguyen
Copy link

@Gelio do you mind me doing this copy paste job?

@Gelio
Copy link
Author

Gelio commented Jun 24, 2019

@sonhanguyen Hey 👋 Yes, I do mind. I would rather not copy paste the rule's code over to this project as it would lead to duplication. Updates would have to be applied twice. Thus, I would rather keep the hooks rule in the tslint-react-hooks repository

Thanks for reaching out and suggesting 🙂

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.

5 participants