jsx-one-expression-per-line should ignore inline elements

[anthonator]

It doesn't seem to make sense to one-line inline elements.

For example, one-lining the em element in the example below doesn't make sense.

<Styled.PasswordInstructions errored={ !!formErrors.userPassword }>
  Passwords must be at least 8 characters long and can&apos;t be
  things like <em>password</em>, <em>123456</em> or <em>abcdef</em>.
</Styled.PasswordInstructions>

Here is the "fixed" version.

<Styled.PasswordInstructions errored={ !!formErrors.userPassword }>
  Passwords must be at least 8 characters long and can&apos;t be
  things like
  <em>
    password
  </em>
  ,
  <em>
    123456
  </em>
  or
  <em>
    abcdef
  </em>
  .
</Styled.PasswordInstructions>

I could also see an option allowing the developer to choose which elements to ignore along with an ignoreInline option.

[ljharb]

In that particular case i agree it looks messy; I’m not sure how to generically allow it when it’s inline with text without having things like long links be inlined.

[ryansully]

Had a similar experience when bumping up to 7.10:

<h3><i className="fa fa-user" /> <strong>User</strong></h3>
{!!onClose && <button type="button" onClick={onClose}>Close</button>}
{!!onSave && <button type="submit" onClick={onSave}>Save</button>}

becomes

<h3>
  <i className="fa fa-user" />
  {' '}
  <strong>
    User
  </strong>
</h3>
{!!onClose && (
  <button type="button" onClick={onClose}>
    Close
  </button>
)}
{!!onSave && (
  <button type="submit" onClick={onSave}>
    Save
  </button>
)}

which are among the simpler of 100+ areas of code base that had to be changed. It's the simpler JSX though that should be able to be kept inline IMO.

I think maybe an inline option in conjunction with ESLint's max-len rule would be sufficient to handle things like long inline links, if that's doable?

I like the new rule and want to be able to enforce it in some way, but in its current state it's just overkill and actually has a negative impact on code appearance for what is essentially a purely stylistic rule.

[ljharb]

See #1855; I think they're related.

[ryansully]

Somewhat, but sort of a different concern. I left feedback there as well.

[Mindaugas-Jacionis]

@ljharb the reference to #1855 is confusing, because there you point back to this thread 😄

[ljharb]

@Mindaugas-Jacionis two things that relate to each other, cross-referencing each other, shouldn't be confusing.

[MrHen]

I recently had to disable react/jsx-one-expression-per-line because it made things like this much harder to read:

        <div>
            <span>ABC</span>
            { one && <span>DEF</span> }
            <span>GHI {two} JKL</span>
        </div>

Result:

        <div>
            <span>
                ABC
            </span>
            { one &&
                <span>
                    DEF
                </span>
            }
            <span>
                GHI
                {' '}
                {two}
                {' '}
                JKL
            </span>
        </div>

What I want to prevent is someone trying to inline nested tags. I don't care about people using <span>ABC</span> -- that's easy enough to read. I can understand why the lint rule current does what it does but it would be handy to have an option to ignore inlined non-tag expressions.

[danny-andrews-snap]

Agreed. In addition, running --fix with this rule enabled completely mangles your code.

[rdsedmundo]

I'm also very unhappy of Airbnb's style guide asking for that.

I have the exact same case:

            One or more rows have failed to process. Please use the filter option below to check
            which ones have failed and report it <strong>immediately</strong> to the engineering
            team.

I'm required to split up the <strong> part in a new line, which results in the necessity of having to add two more extra lines alongside with{' '} in order to preserve the whitespace.

I'm disabling it for now.

[igorpupkinable]

This had messed up our pages with content. Text is missing meaningful spaces.

[evenfrost]

Any update on this?

[ljharb]

See https://github.com/yannickcr/eslint-plugin-react/issues/1848#issuecomment-400011655

[Davidhidalgo]

This is very annoying... :disappointed:

[guiathayde]

I recently had to disable react/jsx-one-expression-per-line because it made things like this much harder to read:

        <div>
            <span>ABC</span>
            { one && <span>DEF</span> }
            <span>GHI {two} JKL</span>
        </div>

Result:

        <div>
            <span>
                ABC
            </span>
            { one &&
                <span>
                    DEF
                </span>
            }
            <span>
                GHI
                {' '}
                {two}
                {' '}
                JKL
            </span>
        </div>

What I want to prevent is someone trying to inline nested tags. I don't care about people using <span>ABC</span> -- that's easy enough to read. I can understand why the lint rule current does what it does but it would be handy to have an option to ignore inlined non-tag expressions.

how did you that?

[nomasprime]

Agree this can hinder readability and an allow inline option is needed. Disabling rule for now.

[nidalR]

At least tags like <b> and <em> should be ignored to start with...

[TSMMark]

What's the solution? Add an option like ignoreElements: ['em', 'p'] and/or ignoreProps: ['key', 'whatever'] ?

[ljharb]

I'm not sure we have a solution yet - but it'd be great if someone wanted to propose one.

[misraelson]

@ljharb @TSMMark /* eslint-disable react/jsx-one-expression-per-line */ YOUR CODE.. /* eslint-enable react/jsx-one-expression-per-line */

[TSMMark]

Great point, it's true that one of the best things about open source packages that other people wrote for you for free is that they are purely optional and you are not actually forced to use them! A lot of people don't know that. Thanks for notifying everyone subscribed to this thread with that valuable information!

Although I wouldn't recommend your approach for globally disabling an eslint rule across your entire codebase — you can simply add one line to your .eslintrc file inside the rules object, like:

"react/jsx-one-expression-per-line": 0

[ljharb]

I’m going to hide all three of these comments; disabling an eslint rule is well-understood and well-documented, and is in no way a “solution”.