-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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 rule with ternary operators on multiple lines and JSX elements #454
Comments
I think what it wants might be more like: const foo = bar
? (<p>
{'Bar is true!'}
</p>)
: (<p>
{'Bar is false!'}
</p>); |
Eugh, really? Those round brackets look awful... Moreover, that is still invalid. This is the syntax my eslint is accepting (but maybe it's due to me using 4 spaces instead of 2 for indentation): const foo = bar
? (<p>
{'Bar is true!'}
</p>)
: (<p>
{'Bar is false!'}
</p>); This becomes even uglier when I use self-closing tags and the const foo = bar
? (<div
k={1}
/>)
: (<div
k={2}
/>); But maybe there's no "right" way to indent those mixes of js and jsx... |
Yeah, that's not right. It should be keying off the opening tag, no matter the context: Should be possible: return canRender
? <Foo>
className="foo"
</Foo>
: null But both |
I'm using syntax mentioned by @neverfox very often and it would be extremely useful if |
I am also using something similar to @neverfox, would love to see support for the following for jsx-indent-props Instead of return colHidden
? <span
className="hidden">
</span>
: <span
className="shown"
ref="myCol">
<span>foo</span>
</span>; Should be return colHidden
? <span
className="hidden">
</span>
: <span
className="shown"
ref="myCol">
<span>foo</span>
</span>; |
there's also another related issue that also has to do with With:
I think this should be valid (with one prop):
but eslint expects:
|
I use it this way: isSomething
? <SomeComponent />
: <AnotherComponent
className={css.component}
onSomething={props.doSomething}
/> And it fails w/o: /* eslint react/jsx-indent-props: 0, react/jsx-closing-bracket-location: 0 */ |
I have to say I agree with the indenting as described by @goffreder. The ternary operator is indented by one level and the values are either a single line or contained within parenthesis where the content is indented (again) and the closing parenthesis is on the same level as the operator;
To me it just feels the most natural as the operators are indented in a way that's similar to chained methods and the parenthesis indentation is similar to the indentation of curly brackets for control statements. The parenthesis are purely used to indicate scope for multi-lined elements and make it easier to determine where the current part of the operator ends. This is also why I think the brackets should close on the same indentation as the matching question mark or colon, as you can spot the next part of the ternary operator in a glance when reading the code. Obviously, if the element (or statement) is only a single line the parenthesis can be left out. But that's just my humble opinion :) |
Any update on this please? |
Hello, +1 for supporting
if possible :-) |
@yannickcr @ljharb Guys please share your thoughts on this issue? |
I am happy to help review a PR that improves this rule. It sounds like there are a few styles that folks desire, so it would be good if the rule could enforce each style with this rule, chosen via an option. |
This example: (condition)
? <Comp
foo={x}
bar={y}
>
<ChildrenComponent />
</Comp>
: <AnotherComp /> breaks 3 rules: |
@straku I don't think I fully understand what you are criticizing and what you are proposing. Can you give us some examples of each? |
It would also be helpful to provide a few examples that only violate one rule each - so we can see each individual modification you'd be interested in. |
@lencioni You were saying that you would like to enforce different styles by an option. It certainly would be nice to have that option but I am wondering how would you like to do it when this style of writing ternary expressions breaks more than one rule? The same option for each rule? Some global config? Sorry if my comment sounded like criticizing, just wanted to know what would be the best way of doing it. |
@straku my expectation would be a collection of differing options for each rule, that put together give you the style you like. |
Currently
|
@sassanh If I understand correctly, eslint's built in There's a lot of things like ternary operators that the current version will simply leave as is, which is awful when everything around it gets re-indented... |
@aij great, looking forward for the new version. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Any chance it would be implemented? |
@odragora a PR implementing it would be extremely helpful. |
I would like to contribute to working on this issue. @ljharb |
Go for it! |
@ljharb Upon further research on this topic, I believe that there is a bug within eslint that prevents indentation with ternary operators in jsx. Therefore, I do not believe this issue can be solved under the current version of eslint. |
If it's a bug in the |
This code
violates the
jsx-indent
rule on the lines with the<p>
opening tags, saying that everything should be brought one level back.I use a lot this syntax, especially when I have to render one complex component or another according to a boolean variable, and I can't seem to find a workaround to avoid the linting error (apart from not using the ternary operator, which I don't like).
The text was updated successfully, but these errors were encountered: