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 · 8 comments

Comments

@feross
Copy link
Member

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>
    )
  }
})

@feross feross added the enhancement label Dec 5, 2016

@dcousens

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Contributor

commented Dec 18, 2016

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

This comment has been minimized.

Copy link

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

@plonko

This comment has been minimized.

Copy link

commented Nov 2, 2017

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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link

commented Jun 18, 2019

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

This comment has been minimized.

Copy link
Member Author

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 in ccaf439 Aug 14, 2019

@feross

This comment has been minimized.

Copy link
Member Author

commented Aug 14, 2019

Let's ship it in standard 14!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
7 participants
You can’t perform that action at this time.