Eslint-plugin-react: react/jsx-one-expression-per-line

[tjbenton]

There's a bug with the --fix part of this rule when you have plain text inside of the jsx element.

Original code:

const something = () => (
  <Text>Plain text</Text>
)

Result of eslint --fix:

const something = () => (
  <Text>
Plain text
  </Text>
)

Expected result:

const something = () => (
  <Text>
    Plain text
  </Text>
)

[ljharb]

Indentation should be handled by the indent and react/jsx-indent rules; it's typical and common for autofixers to need to work in tandem.

[temowemo]

It looks like neither indent nor react/jsx-indent can perform the proper indentation in this case unless you wrap "Plain Text" in brackets and quotes like so:

ESLint Rules:

"react/jsx-one-expression-per-line": "error",
"react/jsx-indent": ["error", 2]

Original code:

const something = () => (
  <Text>{"Plain text"}</Text>
)

Result of eslint --fix:

const something = () => (
  <Text>
    {"Plain text"}
  </Text>
)

So while the react/jsx-one-expression-per-line rule should definitely not be expected to perform indentation, maybe react/jsx-indent should handle this indentation problem?

[ljharb]

Yes, that seems reasonable. We could perhaps add an option to jsx-indent that covered plain text?

[temowemo]

That sounds like a great idea to me! I would actually benefit from this option. I would do the honors, but unfortunately I do not feel comfortable submitting a PR as I have never done so, and I am not familiar with the procedure.

[peter-mouland]

Would it be easy to add an option to turn off this rule for plain text content?

[ljharb]

I don’t know if it would be easy, but i think the rule definitely needs two options - one for plain text content, and another for elements with only one child.

[dmeents]

Has anybody found a suitable fix for this in the meantime?

[ljharb]

The only fix, besides someone submitting a PR here, is disabling the rule for the time being.

[alexzherdev]

@ljharb I noticed this is the last (?) item blocking eslint 5 support in airbnb/javascript. I can take a break from my prop-types work and try to handle this. My understanding is that two options are necessary:

• allowPlainTextOnly that would leave <Text>Plain text and nothing else</Text> as is;
• allowSingleChild that would leave <Text><OnlyOneChildHere></Text> as is.
  Is that right? Also, would allowSingleChild only apply for a single JSX element inside, or plain text as well, i.e. is the second option a superset of the first one?

[ljharb]

That sounds right, and that'd be great - I think that yes, allowSingleChild would be a superset of allowPlainTextOnly, so maybe better would be allow that can take text or single-child, where "text" covers "any non-element children"?

[alexzherdev]

allow sounds good.
The following would also qualify as non-element children, which I don't think an option called text is expected to touch though?

<Text>{foo && <Bar />}</Text>

A JSXExpressionContainer could contain a lot of different content. I was assuming a single JSXExpressionContainer to qualify for single-child rather than text.

[ljharb]

ok fair, maybe just "text or numbers" for the text option?

[alexzherdev]

I think a single Literal or JSXText as a child should fit.

[ljharb]

in that case, let's call the options single-child vs literal vs none (the default)?

[alexzherdev]

Hmm let's say we allow: 'literal'. What about the following case:

<Text>foo
</Text>

Should that still warn? Same for single-child with a non-literal child.
In other words, should the option only take effect if the single child is on the same line as both opening and closing tags?

[ljharb]

Yes, I think that the option should only take effect if the single child is on the same line as opening and closing tags.