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

Potential autofix improvement for jsx-wrap-mulitlines #1576

Merged

Conversation

sharmilajesupaul
Copy link
Contributor

@sharmilajesupaul sharmilajesupaul commented Nov 30, 2017

This change removes the newline that comes before opening parentheses, if "parens-new-line" is specified as an option and the multiline JSX starts on a new line.

i.e. when "parens-new-line" is specified, instead of fixing these cases:

  <div prop={
    <div>
      <p>Hello</p>
    </div>
  }>
    <p>Hello</p>
  </div>

// &

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

like this:

  <div prop={
(
    <div>
      <p>Hello</p>
    </div>
)
  }>

// &

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

this change will inline the opening parentheses:

  <div prop={(
    <div>
      <p>Hello</p>
    </div>
)}>

// &

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

I did not change the position of closing parens in this PR, I'm not sure if there is another rule that might cover that? if not, I can also try and collapse the bottom parens to be more consistent.

cc. @ljharb

Update: this change will now inject the closing paren before the token after the jsx body, so the autofix will not leave the closing paren on its own line.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM but I think it does need to cover the closing paren also

const ATTR_PAREN_NEW_LINE_AUTOFIX = `
<div prop={(\n<div>
<p>Hello</p>
</div>\n)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this one needs to be )}> on the next line

@sharmilajesupaul
Copy link
Contributor Author

@ljharb added change to cover closing paren, ptal thanks!

This change should remove the newline that comes before a parentheses,
if "parens-new-line" is specified as option and the multiline jsx
starts on a new line. The autofix will also inject the closing parentheses
before the next enclosing character as opposed to being on its own line.
@ljharb ljharb merged commit b801624 into jsx-eslint:master Nov 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

2 participants