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

"jsx-indent" -> "react/jsx-indent" #958

Merged
merged 1 commit into from Jan 29, 2017
Merged

Conversation

Jorundur
Copy link
Contributor

No description provided.

@Jorundur
Copy link
Contributor Author

There are some things I don't understand in this rule. The rule's description is
"jsx-indent": [<enabled>, 'tab'|<number>]
but there is no further description on <enabled>.

I would assume that it's a boolean, either 0 or 1, but in every single example <enabled> is 2, e.g.:

// 2 spaces indentation
// [2, 2]

// tab indentation
// [2, 'tab']

// no indentation
// [2, 0]

What does that mean?

Furthermore, we can see this contradiction:

The following patterns are considered warnings:
...
// tab indentation
// [2, 'tab']

  

...
The following patterns are not warnings:
// tab indentation
// [2, 'tab']

  

These patterns are identical and yet one is considered a warning and the other not.

@ljharb
Copy link
Member

ljharb commented Nov 15, 2016

"enabled" refers to 0/1/2 or off/warn/error, just like every eslint rule in the ecosystem and core is configured.

If this change were to be made (adding "react/"), we'd have to do it on all the rules - as this is how they're all currently documented.

@Jorundur
Copy link
Contributor Author

Jorundur commented Nov 15, 2016

@ljharb Thanks for the clarification regarding <enabled>.

Regarding the addition of react/ on all rules, you're right, that would be quite the hassle.
However, I might suggest that for consistency you then do the opposite thing and remove react/ from jsx-filename-extension and require-extension that currently include the lines:

"rules": {
  "react/jsx-filename-extension": [1, { "extensions": [".js", ".jsx"] }],
}

and

"rules": {
  "react/require-extension": [1, { "extensions": [".js", ".jsx"] }],
}

Lastly, what about the contradiction I mentioned, is there a reason for it or am I misunderstanding something?

@ljharb
Copy link
Member

ljharb commented Nov 15, 2016

Because tabs vs spaces are very hard to convey in markdown format, all the examples use spaces, but have a comment directly above them describing the intended indentation.

Copy link
Collaborator

@lencioni lencioni left a comment

Choose a reason for hiding this comment

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

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

3 participants