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
- Workaround for eslint bug with prettier See https://github.com/prettier/prettier/issues/1271 and following https://github.com/prettier/prettier/issues/737 — committed to ReCodEx/web-app by SemaiCZE 7 years ago
- Reformat all (#25) * Reformat all * Workaround for eslint bug with prettier See https://github.com/prettier/prettier/issues/1271 and following https://github.com/prettier/prettier/issues/737 — committed to ReCodEx/web-app by SemaiCZE 7 years ago
If I may add, it looks odd when you indent with 4 spaces, as the ternary still introduce a 2 space 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:
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:
You can’t have:
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:
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 ):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.