Eslint-plugin-react: jsx-indent wrong works with ternary operator

Created on 6 Nov 2016  路  12Comments  路  Source: yannickcr/eslint-plugin-react

Version: 6.6.0

2016-11-06 23 49 28

Most helpful comment

Thanks for all your examples, there is a lot of different ways to indent ternaries and seems we missed some of them.

I will add more tests cases to the jsx-indent tests suite and make sure they all pass.

All 12 comments

ditto for the ternary operator being inline with the logical if/brackets

{ isFoo ?
  <div>lorem</div>
:
  <div>ipsum</div>
}

However the following passes, but forces an inline else statement (not ideal, but passes linting)

{ isFoo ?
  <div>lorem</div>
: <div>ipsum</div>
}

yes my biggest remaining problem is with terneries and brackets e.g. when brackets are used, usually indentation goes up by 1, but within a ternary, it currently does not.

This was caused by a fix to allow this kind of formatting:

cond ?
  <div/> :
  <span/>

Before 6.6.0, jsx-indent would have required:

cond ?
  <div/> :
    <span/>

But this breaks the also common style with : on its own line :/

examples formatted how 6.6.0 expects vs expectation

        return isDisabled ?
            content :
            (
            <Touchable onTap={this.handleTap}>
                {content}
            </Touchable>
        );

but I'd expect

        return isDisabled ?
            content :
            (
                <Touchable onTap={this.handleTap}>
                    {content}
                </Touchable>
            );

next one

    return secondaryTitle ?
        (
            <div>
                <p>{primaryTitle}</p>
                <p>{secondaryTitle}</p>
            </div>
        ) : (
        <p>{primaryTitle}</p>
        );

but I would expect

    return secondaryTitle ?
        (
            <div>
                <p>{primaryTitle}</p>
                <p>{secondaryTitle}</p>
            </div>
        ) : (
            <p>{primaryTitle}</p>
        );

Makes it really hard when combined with the operator-linebreak, which eg. comes with the standard package, as that doesn't allow you to move your : either. But mostly it's just bad to have the linting tool suddenly change it's mind about the linting and encourage a bad coding style rather than a good one, so hoping for a swift fix.

This apparently was fix of #901 and #897 that was made as a response to another bug around this that was introduced in 6.4.0.

Code like this also produces a linter error now (not previously):

{condition ? <Hello /> : (
  <World />
)}

The error is Expected indentation of 0 space characters but found 2 react/jsx-indent on the <World /> line.

If someone could review my PR #950, then that would be great. It fixes the main case reported in this issue.

@voxpelli Not sure if you noticed, but your fix causes some issues with other implementations

@alasdairhurst I have not, all tests in the module passes as did the linting on my largest React project so please add any comments to #950 on any additional problems that the fix in there might cause and it will have to be ensured that additional tests are added for those cases and the code modified to pass also those tests.

Thanks for all your examples, there is a lot of different ways to indent ternaries and seems we missed some of them.

I will add more tests cases to the jsx-indent tests suite and make sure they all pass.

@yannickcr this fixes a breaking change - may we get a new release so our CI is happy?

@rosskevin I hope to publish a new release in the coming hours.

Was this page helpful?
0 / 5 - 0 ratings