Skip to content
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

Prevent missing parentheses around multiline JSX (jsx-wrap-multilines) #710

Closed
feross opened this issue Dec 5, 2016 · 9 comments
Closed

Comments

@feross
Copy link
Member

feross commented Dec 5, 2016

Wrapping multiline JSX in parentheses can improve readability.

Rule page: https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-wrap-multilines.md (Automatically fixable.)

This is an error:

var Hello = React.createClass({
  render: function() {
    return <div>
      <p>Hello {this.props.name}</p>
    </div>
  }
})

This is not an error:

var singleLineJSX = <p>Hello</p>

var Hello = React.createClass({
  render: function() {
    return (
      <div>
        <p>Hello {this.props.name}</p>
      </div>
    )
  }
})
@dcousens
Copy link
Member

dcousens commented Dec 5, 2016

Isn't this already a thing?

Any way, I usually do:

var Hello = React.createClass({
  render: function() {
    return (<div>
      <p>Hello {this.props.name}</p>
    </div>)
  }
})

@cesarandreu
Copy link
Contributor

This matches how I use React as well.

It also works for this style, which I've seen being used around:

export const Hello = ({ name} ) => (
  <div>
    <p>Hello {name}</p>
  </div>
)

@pluma
Copy link

pluma commented Jan 24, 2017

AFAICT the proposed rule seems to be a de facto standard in the React community.

I'm so used to it that I often even wrap single-line JSX in parens (and indent it as multi-line) but that seems excessive.

@feross feross added this to the standard v11 milestone Apr 4, 2017
@seb-thomas
Copy link

Anyone know of anything like this for ES Lint? This is my main pet peeve in JSX! :)
Why would anyone want to not use brackets? :o

@justnewbee
Copy link

justnewbee commented Dec 26, 2018

i really don't like it... my favorite style is without those parentheses, i think the jsx structure is quite clear by itself, no need to wrap them

i want to "never" it and report error on those wrapped, but there seem not to have an option to do so...

please add one, for example ["error", "never"]?

@jsphstls
Copy link

The necessity of parenthesis depends on whether misaligned JSX tags are allowed: https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-closing-tag-location.md

The added cognitive complexity of optional wrapping parenthesis is not always needed because JSX must be wrapped in a single parent anyways. The wrapping parenthesis are only useful after return and to maintain alignment in ternaries.

const something = 
  <div>
    <button />
  </div>

const something = () =>
  <div>
    <button />
  </div>

const something = () => {
  console.log('something')

  return (
    <div>
      <button />
    </div>
  )
}

const something = isWhatever
? (
  <div>
    <button />
  </div>
) : (
  <span>
    <button />
  </span>
)

@feross feross modified the milestones: standard v13, standard v14 Jul 5, 2019
@feross
Copy link
Member Author

feross commented Aug 14, 2019

The configuration that I want to go with is:

    "react/jsx-wrap-multilines": ["error", {
      "declaration": "parens-new-line",
      "assignment": "parens-new-line",
      "return": "parens-new-line",
      "arrow": "parens-new-line",
      "condition": "parens-new-line",
      "logical": "ignore",
      "prop": "ignore"
    }]

I think this configuration is the least controversial, best for readability (in my subjective opinion), and seems to match the React community's dominant style. This currently passes the entire test suite. (We don't actually have that many repos with JSX in the suite, though we do have bitmidi.com, webtorrent-desktop, and a few others.)


Here's a list of what is allowed and not allowed

declaration

The following patterns are considered warnings when configured {declaration: "parens-new-line"}.

var hello = <div>
  <p>Hello</p>
</div>;
var hello = (<div>
  <p>Hello</p>
</div>);

The following patterns are not considered warnings when configured {declaration: "parens-new-line"}.

var hello = (
  <div>
    <p>Hello</p>
  </div>
);

assignment

The following patterns are considered warnings when configured {assignment: "parens-new-line"}.

var hello;
hello = <div>
  <p>Hello</p>
</div>;
var hello;
hello = (<div>
  <p>Hello</p>
</div>);

The following patterns are not considered warnings when configured {assignment: "parens-new-line"}.

var hello;
hello = (
  <div>
    <p>Hello</p>
  </div>
);

return

The following patterns are considered warnings when configured {return: "parens-new-line"}.

function hello() {
  return <div>
    <p>Hello</p>
  </div>;
}
function hello() {
  return (<div>
    <p>Hello</p>
  </div>);
}

The following patterns are not considered warnings when configured {return: "parens-new-line"}.

function hello() {
  return (
    <div>
      <p>Hello</p>
    </div>
  );
}

arrow

The following patterns are considered warnings when configured {arrow: "parens-new-line"}.

var hello = () => <div>
  <p>World</p>
</div>;
var hello = () => (<div>
  <p>World</p>
</div>);

The following patterns are not considered warnings when configured {arrow: "parens-new-line"}.

var hello = () => (
  <div>
    <p>World</p>
  </div>
);

condition

The following patterns are considered warnings when configured {condition: "parens-new-line"}.

<div>
  {foo ? <div>
      <p>Hello</p>
    </div> : null}
</div>
<div>
  {foo ? (<div>
      <p>Hello</p>
  </div>) : null}
</div>

The following patterns are not considered warnings when configured {condition: "parens-new-line"}.

<div>
  {foo ? (
    <div>
      <p>Hello</p>
    </div>
  ): null}
</div>

logical

We configure nothing for this one. Anything goes.

<div>
  {foo &&
    <div>
      <p>Hello World</p>
    </div>
  }
</div>
<div>
  {foo &&
    (<div>
      <p>Hello World</p>
    </div>)
  }
</div>
<div>
  {foo && (
    <div>
      <p>Hello World</p>
    </div>
  )}
</div>

prop

We configure nothing for this one. Anything goes.

<div foo={<div>
    <p>Hello</p>
  </div>}>
  <p>Hello</p>
</div>;
<div foo={(<div>
    <p>Hello</p>
  </div>)}>
  <p>Hello</p>
</div>;
<div foo={(
  <div>
    <p>Hello</p>
  </div>
)}>
  <p>Hello</p>
</div>;

@feross feross closed this as completed in ccaf439 Aug 14, 2019
@feross
Copy link
Member Author

feross commented Aug 14, 2019

Let's ship it in standard 14!

@anton-bot
Copy link

Isn't it true though that parens are not needed in ternaries? You can maintain proper alignment without any parens at all:

condition
  ? <div>
      <img />
    </div>
  : <div />

feross added a commit to standard/eslint-config-standard-jsx that referenced this issue Oct 28, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 26, 2021
suchitg8 pushed a commit to suchitg8/standard that referenced this issue Apr 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Archived in project
Development

No branches or pull requests

8 participants