Rule proposal: require parens around jsx that lives on its own line

[rsolomon]

The title is not very eloquent, so I'll explain what I'm after by example.

Currently you can only require parens around multiline jsx. I'd also like to require parens around single-line jsx, but only when it lives on its own line. This is particularly useful when within an arrow function.

For example, the following would raise an error:

foo.map((bar, baz) => 
  <div>I'm on my own line!</div>
);

As would this:

const content = 
  <div>Hi there!</div>;

The following would not raise errors:

foo.map((bar, baz) => (
  <div>I'm on my own line!</div>
));

const content = (
  <div>Hi there!</div>;
);

const content = <div>Hi there!</div>;

[ljharb]

I'd love to add this to the airbnb config, since this is precisely what our style guide requires.

I feel like this might be better as an option to jsx-wrap-multilines, but I could be convinced it should be a new rule.

[rsolomon]

I agree that it belongs in jsx-wrap-multilines. Preference on option nomenclature?

[ljharb]

There's four existing syntaxes: declaration, assignment, return, arrow - this could be a fifth. Maybe standalone?

[rsolomon]

Upon a bit deeper inspection, it looks like the rule doesn't enforce parenthesis on newlines even on multiline expressions. Ex:

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

Is considered valid, even though I would prefer the linter force it to be:

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

I am wondering if this may require 2 changes:

1. A new rule or new option added to jsx-wrap-multilines that enforces newlines after the opening paren and before the closing paren of multiline jsx expressions.

2. The aforementioned standalone rule, that would treat standalone single-line jsx expressions as multiline.

[ljharb]

Those two changes sound good.

[VinSpee]

I think this is likely related:

running --fix on this code:

const MediaName = ({ name }: MediaNameProps) =>
  <Media
    queries={{
      xs: {
        minWidth: '24em',
      },
      sm: {
        minWidth: '45em',
      },
    }}
  >
    {({ sm }) =>
      <div>
        {name}
      </div>
    }
  </Media>;

results in

const MediaName = ({ name }: MediaNameProps) =>
  (<Media
    queries={{
      xs: {
        minWidth: '24em',
      },
      sm: {
        minWidth: '45em',
      },
    }}
  >
    {({ sm }) =>
      (<div>
        {name}
      </div>)
    }
    </Media>);

when I would expect

const MediaName = ({ name }: MediaNameProps) => (
  <Media
    queries={{
      xs: {
        minWidth: '24em',
      },
      sm: {
        minWidth: '45em',
      },
    }}
  >
    {({ sm }) => (
      <div>
        {name}
      </div>
    )}
  </Media>
);

is this the responsibility of this rule?

[ljharb]

I would also expect the latter.

Which rule is warning on it if you omit --fix?

[jseminck]

While looking into: #1282 - I also ran into some of the issues mentioned above.

When fixing the following:

{myCondition && 
  <div>
    <p>Hello World</p>
  </div>
}

The current auto fixer would return:

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

Which IMHO looks rather ugly. And I could not find any other ESLint rule that would format this into what we want (see below). I did manage to write the fixer in a way that it returns this for this specific case, but the code is quite fragile and probably would need a good set of unit tests to confirm it works correctly in all cases.

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

My initial fixer implementation:

function(fixer) {
  const rightNode = node.expression.right;
  return [
    fixer.insertTextAfterRange(sourceCode.getTokenBefore(rightNode).range, ' ('),
    fixer.insertTextBeforeRange(sourceCode.getLastToken(node).range, ')')
  ];
}

So in addition to the above ticket and the points mentioned in here, I think it's needed to take a deeper look into this rule and see if it can be made more useful!

[iddan]

Is someone working on this? Because we use prettier-eslint all of our codebase is now with ugly braces

[jseminck]

There's a PR for it.

[iddan]

#1285 ?

[ljharb]

It seems like #1475 covers this.