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 ignore config to no-unknown-property rule #631

Merged
merged 1 commit into from
Jun 15, 2016
Merged

Add ignore config to no-unknown-property rule #631

merged 1 commit into from
Jun 15, 2016

Conversation

insin
Copy link
Contributor

@insin insin commented Jun 10, 2016


```js
"rules": {
"react/no-unknown-property": [1, { "allow": ["class", "for"] }],
Copy link
Member

Choose a reason for hiding this comment

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

As far as I know, React itself doesn't support "class" or "for", and requires "className" and "htmlFor". Can you point to some documentation that allows for this usage?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

React doesn't support it, but you can use Babel plugins to handle usage of 'class' and 'for' at transpile time for teams who prefer to keep using them, e.g: insin/babel-plugin-react-html-attrs#4 (comment)

@yannickcr
Copy link
Member

That's a very specific use case, but I am not against adding such option.

Just a couple of things:

  • I'd prefer to use an ignore option instead of allow (like prop-types or jsx-pascal-case)
  • You are referring to your specific use case in the documentation. I think it can be confusing. Maybe just saying that you can ignore some properties is enough ?

@insin insin changed the title Add whitelist config to no-unknown-property rule Add ignore config to no-unknown-property rule Jun 15, 2016
@insin
Copy link
Contributor Author

insin commented Jun 15, 2016

Changed the option to ignore and made the documentation more generic, as per other rules.

@yannickcr yannickcr merged commit 4418534 into jsx-eslint:master Jun 15, 2016
@yannickcr
Copy link
Member

Merged, thanks!

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

Successfully merging this pull request may close these issues.

3 participants