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

standard and prettier-standard disagree about react/jsx-indent #941

Closed
drdaeman opened this issue Jun 29, 2017 · 6 comments

Comments

@drdaeman
Copy link

commented Jun 29, 2017

Hi! I'm trying to use standard (for linting) and prettier-standard (for automatic beautifying) and while it all works great in general, two tools seem to disagree about JSX.

Here's a full class I have, after running prettier-standard:

export class ConditionalPreloader extends React.Component {
  render () {
    const { children, showPreloaderIf } = this.props
    return showPreloaderIf
      ? <Preloader />
      : <div>
          {children}
        </div>
  }
}

However standard complies about react/jsx-indent ("Expected indentation of 8 space characters but found 10" for "{children}" line) and insists on this indentation:

      ? <Preloader />
      : <div>
        {children}
      </div>

I'm not sure how things work under the hood and which package this is actually related to - so maybe I'm reporting this to a wrong project. Can someone please suggest me how to deal with this inconsistency?

Thanks!

@dadamssg

This comment has been minimized.

Copy link

commented Jun 29, 2017

Suggestion that may fix the discrepancy:

export class ConditionalPreloader extends React.Component {
  render () {
    const { children, loading } = this.props
    return loading
      ? <Preloader />
      : (
        <div>
          {children}
        </div>
      )
  }
}

Or for this particular code, just spit out the children if they don't really need that surrounding div

export class ConditionalPreloader extends React.Component {
  render () {
    const { loading, children } = this.props
    return loading ? <Preloader /> : children
  }
}

Or to make it even more concise

export function ConditionalPreloader ({ loading, children }) {
  return loading ? <Preloader /> : children
}
@drdaeman

This comment has been minimized.

Copy link
Author

commented Jun 29, 2017

Thanks for the suggestion!

Actually, prettier-standard removes unnecessary parentheses, so the first suggestion doesn't work - it will be automatically replaced whenever the file is reformatted. And then standard would start to complain :)

The third version works nicely for this particular component (<div> wasn't really needed there - thanks!), but I'm not sure it scales. For example, I have this piece of code:

...
<a className='footer-home--link' href='/about/'>
  About
</a>
{isUserAuthenticated
  ? <a className='footer-home--link' href='/accounts/logout/'>
      Logout
    </a>
  : <span>
      <a className='footer-home--link' href='/accounts/login/'>
        Log in
      </a>
      <a className='footer-home--link' href='/accounts/signup/'>
        Sign up
      </a>
    </span>}
...

Of course, it should be possible to work around this by splitting things into separate components (something like AuthenticatedUserFooterLinks), or by using consts for pieces, so the ternary operator would fit into a single line. But I feel that doing so would probably unnecessarily complicate things and make code less readable, as the reader would have to jump back and forth to see the pieces.

@dadamssg

This comment has been minimized.

Copy link

commented Jun 29, 2017

Ah. Yeah, i'm not familiar with prettier but it removing parenthesis around jsx seems questionable to me. Personally, I would raise an issue in the prettier-standard repo.

@laduke

This comment has been minimized.

Copy link

commented Aug 30, 2017

Hmm, I don't think parens can save me. If an element has more properties than fit on one line (or more classes), prettier puts the props on multiple lines. Something about a ternary being involved (it seems?) makes it say the props are indented too far.

const Asdf = ({ x }) =>
  x
    ? null
    : <span
        id='bar'
        className='class1 class2 class3'
        aria-hidden='true'
        someProp='42'
      />
@stale

This comment has been minimized.

Copy link

commented May 10, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale label May 10, 2018

@stale stale bot closed this May 17, 2018

@feross

This comment has been minimized.

Copy link
Member

commented May 20, 2018

The indentation rules changed (and were much improved!) in standard v11. This issue may very well have been fixed in that release :)

@lock lock bot locked as resolved and limited conversation to collaborators Aug 18, 2018

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