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

Forbid class props in react #24

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Forbid class props in react #24

wants to merge 1 commit into from

Conversation

trotzig
Copy link
Contributor

@trotzig trotzig commented May 19, 2017

In the past, we've had a custom overcommit hook protecting against using
class instead of className as a prop. For React newcomers, this is a
common mistake to make, so it's good to have some protection in place.
Instead of the custom overcommit hook, we can instead just prevent this
through the react/forbid-component-props rule [1].

[1] https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/forbid-component-props.md

In the past, we've had a custom overcommit hook protecting against using
`class` instead of `className` as a prop. For React newcomers, this is a
common mistake to make, so it's good to have some protection in place.
Instead of the custom overcommit hook, we can instead just prevent this
through the react/forbid-component-props rule [1].

[1] https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/forbid-component-props.md
@trotzig trotzig requested a review from MatthewHerbst May 19, 2017 07:48
Copy link
Contributor

@MatthewHerbst MatthewHerbst left a comment

Choose a reason for hiding this comment

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

LGTM, but the react/no-unknown-property rule should in theory be covering this use case already

@trotzig
Copy link
Contributor Author

trotzig commented May 19, 2017

Awesome, thanks for pointing that out. For some reason, that isn't catching things for me. We'll have to investigate before deciding whether to merge this or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants