prettier: Wrong indentation on ternary operator

I can’t reproduce it on http://mib.im/try-prettier/, but if I run:

$(npm bin)/prettier --single-quote --trailing-comma es5 --write "{src,scripts,tests}/**/*.js"

On my Popper.js, the file src/tooltip/index.js at line 59 will have:

    // get events list
    const events = typeof options.trigger === 'string'
      ? options.trigger.split(' ').filter(trigger => {
          return ['click', 'hover', 'focus'].indexOf(trigger) !== -1;
        })
      : [];

Note that the return line has wrong indentation.

About this issue

  • Original URL
  • State: closed
  • Created 7 years ago
  • Reactions: 2
  • Comments: 49 (23 by maintainers)

Commits related to this issue

Most upvoted comments

If I may add, it looks odd when you indent with 4 spaces, as the ternary still introduce a 2 space indent. indent

We don’t treat eslint as an authority to how to format things, so it’s true that it doesn’t really matter if eslint thinks it’s wrong or not. You shouldn’t be using any of eslint’s formatting rules with prettier unless you are fixing them afterwards with eslint --fix to get a different style.

The core of the issue is that the return is indented 2 spaces relative to it’s outer syntax (options). That’s why it works that way.

There’s no right or wrong here. While you think this code looks good:

 const events = typeof options.trigger === 'string'
      ? options.trigger.split(' ').filter(trigger => {
        return ['click', 'hover', 'focus'].indexOf(trigger) !== -1;
      })
      : [];

To me it looks a little strange because the end curly brace is mis-aligned with the expression that it’s closing. There’s nothing dictating that every single line can only be indented by specifically 2 spaces. There are rare times where you want to align something specifically, requiring an additional space or so.

With all of that said, we are probably going to change ternary formatting, as discussed here: https://github.com/prettier/prettier/pull/1171 So this probably is going to change. It’s difficult to find a good syntax for wrapping ternaries, and we’ll continue to iterate on it.

The indentation must always be:

something
  else
    with
      always
        2
          more
            spaces

You can’t have:

something
   with
       four spaces

Honestly I’m a bit scared to have to discuss about this with one of the contributors of the most raging formatting library of the moment…

This is still a broken feature, since it doesn’t work for those using 4 space indents as shown above.

Arg fail :p

That’s unlikely @joamag. See our option philosophy for a better explanation.

Yes this a matter of taste, so debate could last more than infinity.

I just gave up my own preference in indentation and followed prettier: nice advantage is we all write the same (think of new comers who will benefit that).

Oh I get it, the aim is not to indent as such, its to align the rest of the lines within the ternary operator with the first text after the ? itself. This is therefore 2 spaces in, one for the ? and then the space between it and the condition.

In my case, the only way to achieve that aim and still use the normal indent character(s) would be to insert a tab between ? and environment:

image

tslint is happy but I agree that is an unusual use of white space in the middle of the lines.

I will leave it as a hard to resolve problem, apart from simply turning off many tslint rules.

@azz oh yeah you’re right, thanks a lot! 😅

The ending bracket is not misaligned with the expression it’s closing, since the ? can be considered part of that expression. For example, you would not add the arbitrary indentation to the following, because the ending bracket does not need to be in the same character column ( which @Platane pointed out still doesn’t work depending on tab width ):

var myFn = options.trigger.split(' ').filter(trigger => {
  return ['click', 'hover', 'focus'].indexOf(trigger) !== -1;
});

I’m not a collaborator here, just a user like you 😄

What prettier does here is aligning the closing brace to where the opening brace comes from (options.trigger.split(' ').filter(trigger => {) instead of the opening ? in the ternary. I think that prettier’s current behavior correct, while eslint thinks it should align with the ?.

This looks like eslint/eslint#6606, so I think you either will have to disable the eslint rule (which makes sense in a prettier context) or add // prettier-ignore above it so prettier leaves it alone. Or see if the maintainers agrees with you that the current behavior is wrong.