Skip to content

[react/jsx-one-expression-per-line] moves text to the beginning of line #2318

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

Closed
lkazberova opened this issue Jun 23, 2019 · 13 comments · Fixed by #3314
Closed

[react/jsx-one-expression-per-line] moves text to the beginning of line #2318

lkazberova opened this issue Jun 23, 2019 · 13 comments · Fixed by #3314

Comments

@lkazberova
Copy link

lkazberova commented Jun 23, 2019

Hi! I tried to use "react/jsx-one-expression-per-line" rule, but its behaviour is very strange and I don't know how can I fix it.

Sandbox : https://codesandbox.io/s/gatsby-starter-default-f7u3w , please do npm run lint in terminal

My config .eslintrc.json

{
   "parserOptions": {
        "ecmaVersion": 10,
        "sourceType": "module",
        "ecmaFeatures": {
            "jsx": true
        }
    },
    "plugins": [
        "react"
    ],
    "extends": [
        "eslint:recommended",
        "plugin:react/recommended"
    ],
    "rules": {
        "react/jsx-one-expression-per-line": [2, { "allow": "none" }]
    }
}

Before running eslint --fix

    <h1>Hi people</h1>
    <p>Welcome to your new Gatsby site.</p>
    <p>Now go build something great.</p>

After applying eslint

    <h1>
Hi people
</h1>
    <p>
Welcome to your new Gatsby site.
</p>
    <p>
Now go build something great.
</p>
@lkazberova lkazberova changed the title ["react/jsx-one-expression-per-line"] moves text to the beginning of line [react/jsx-one-expression-per-line] moves text to the beginning of line Jun 23, 2019
@ljharb
Copy link
Member

ljharb commented Jun 23, 2019

By itself it looks weird, but if you also enable the jsx-indent rule it will fix the indentation.

@lkazberova
Copy link
Author

lkazberova commented Jun 23, 2019

@ljharb thank you for a quick answer!
I changed my rules

    "rules": {
        "react/jsx-one-expression-per-line": [2, { "allow": "none" }],
        "react/jsx-indent": [2, 2]
    }

Unfortunately it doesn't work with plain text, but with tags - ok

Before running eslint --fix

    <h1>Hi people<button/></h1>

After applying eslint

    <h1>
Hi people
      <button/>
    </h1>

@yannickcr
Copy link
Member

The fix instruction of react/jsx-one-expression-per-line will just add some line breaks between any expression that are on the same line. If you want the tags to be correctly indented aferward then you need to enable react/jsx-indent.

However react/jsx-indent do not re-indent text nodes (798f2ca).

That's why we're getting:

    <h1>
Hi people
      <button/>
    </h1>

instead of

    <h1>
      Hi people
      <button/>
    </h1>

Surely there is some room for improvements here.

@ljharb
Copy link
Member

ljharb commented Jun 23, 2019

I would expect it to reindent text nodes.

@lkazberova
Copy link
Author

lkazberova commented Jun 23, 2019

@yannickcr @ljharb so it's a good reason for making PR 😃

@rfgamaral
Copy link

Here's one possible workaround until the issue is fixed:

Before running eslint --fix

<h1><React.Fragment>Hi people</React.Fragment><button/></h1>

After applying eslint

<h1>
  <React.Fragment>Hi people</React.Fragment>
  <button/>
</h1>

You can read more about React Fragments here if you don't know about them. But basically, the final rendered DOM tree will be like this:

<h1>
  Hi people
  <button/>
</h1>

You don't have to worry about <React.Fragment> adding an additional DOM element.

@rodomeista
Copy link

Bumping this thread - are there any intentions to resolve this issue? Having to write react fragments to properly support this feature seems a bit excessive, especially for devs who aren't sure why fragments are being used.

@ljharb
Copy link
Member

ljharb commented Dec 27, 2019

Yes, it should be fixed somehow. A PR with failing test cases would be a great start.

@ROSSROSALES
Copy link
Contributor

Hi @ljharb, could I try to tackle this?

@ljharb
Copy link
Member

ljharb commented Jun 16, 2022

@ROSSROSALES go for it!

@ROSSROSALES
Copy link
Contributor

Hi @ljharb, I used @lkazberova code as a testing example.
I recreated the same scenario initially mentioned June 23, 2019, and I did not come across the same issue.
Below is the proper output of eslint with appropriate rules.

rules:

    "rules": {
        "react/jsx-one-expression-per-line": [2, { "allow": "none" }],
        "react/jsx-indent": [2, 2]
    },

Sample Code Below:
Before running npx eslint index.js --fix

  <Layout>
    <SEO title="Home" />
    <h1>{"Hi people"}<button/></h1>
    <p>Welcome to your new Gatsby site.</p>
    <p>Now go build something great.</p>
    <h1>Hi people<button/></h1>
    <div style={{ maxWidth: `300px`, marginBottom: `1.45rem` }}>
      <Image />
    </div><Link to="/page-2/">Go to page 2</Link>
  </Layout>

Errors found from Eslint according to rules npx eslint index.js

  11:9   error  `{"Hi people"}` must be placed on a new line                     react/jsx-one-expression-per-line
  11:22  error  `button` must be placed on a new line                            react/jsx-one-expression-per-line
  12:8   error  `Welcome to your new Gatsby site.` must be placed on a new line  react/jsx-one-expression-per-line
  13:8   error  `Now go build something great.` must be placed on a new line     react/jsx-one-expression-per-line
  14:9   error  `Hi people` must be placed on a new line                         react/jsx-one-expression-per-line
  14:18  error  `button` must be placed on a new line                            react/jsx-one-expression-per-line
  17:11  error  `Link` must be placed on a new line                              react/jsx-one-expression-per-line
  17:31  error  `Go to page 2` must be placed on a new line                      react/jsx-one-expression-per-line

After applying npx eslint index.js --fix

  <Layout>
    <SEO title="Home" />
    <h1>
      {"Hi people"}
      <button/>
    </h1>
    <p>
      Welcome to your new Gatsby site.
    </p>
    <p>
      Now go build something great.
    </p>
    <h1>
      Hi people
      <button/>
    </h1>
    <div style={{ maxWidth: `300px`, marginBottom: `1.45rem` }}>
      <Image />
    </div>
    <Link to="/page-2/">
      Go to page 2
    </Link>
  </Layout>

Is there still a need to continue to look into this issue?
If so, please let me know.

@ljharb
Copy link
Member

ljharb commented Jun 25, 2022

@ROSSROSALES awesome - in that case, what we need is a PR adding a passing test case, and that PR can close this issue :-)

@ROSSROSALES
Copy link
Contributor

@ljharb I will work on it! 👍

ROSSROSALES added a commit to ROSSROSALES/eslint-plugin-react that referenced this issue Jun 29, 2022
ROSSROSALES added a commit to ROSSROSALES/eslint-plugin-react that referenced this issue Jul 5, 2022
ljharb pushed a commit to ROSSROSALES/eslint-plugin-react that referenced this issue Jul 5, 2022
ljharb pushed a commit to ROSSROSALES/eslint-plugin-react that referenced this issue Jul 5, 2022
@ljharb ljharb closed this as completed in 151a6ed Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
6 participants