Inconsistent formatting of nested ternary expressions

[sheerun]

This relates to sheerun/prettier-standard#76

Here's example formatting of prettierx with indents on nested ternaries enabled:

search.premium
  ? commission => {
    return suggestions.hits.map(hit => something(hit))
  }
  : something
    ? commission => {
      return suggestions.hits.map(hit => something(hit))
    }
    : null

The core of this issue is that it's possible to configure prettierx to add extra indents to nested ternary expressions, but it also enables another extra formatting rule described below:

function is normally formatted with indentation:

commission => {
  return suggestions.hits.map(hit => something(hit))
}

but when ternary expression is used (with --align-ternary-lines=false option that enables indents on nested ternaries) it doesn't have it:

search.premium
  ? commission => {
    return suggestions.hits.map(hit => something(hit))
  }
  : () => onOpen()

Command used:

prettierx --generator-star-spacing --space-before-function-paren --single-quote --jsx-single-quote --no-semi --yield-star-spacing --align-ternary-lines=false

The real issue with this formatting is that's what standard --fix actually does! So technically is nothing wrong with prettierx, but for me it seems indeed this is wrong decision and the indentation should be at the level of expression after ?, not the ? itself (this doesn't prevent expressions being indented for each level of ?:):

search.premium
  ? commission => {
      return suggestions.hits.map(hit => something(hit))
    }
  : () => onOpen()

Unfortunately this also means formatting would be divergent from what standard.js does.

In the end the correct code, with just indents on nested ternaries enabled, is:

search.premium
  ? commission => {
      return suggestions.hits.map(hit => something(hit))
    }
  : something
    ? commission => {
        return suggestions.hits.map(hit => something(hit))
      }
    : null

[sheerun]

Another example what it is currently:

search.premium
  ? somethingsomethingsomethingsomethingsomethingsomethingsomethingsomething &&
  somethingsomethingsomethingsomethingsomethingsomethingsomethingsomething
  : something
    ? somethingsomethingsomethingsomethingsomethingsomethingsomethingsomething &&
    somethingsomethingsomethingsomethingsomethingsomethingsomethingsomething
    : null

And what it probably should be:

search.premium
  ? somethingsomethingsomethingsomethingsomethingsomethingsomethingsomething &&
    somethingsomethingsomethingsomethingsomethingsomethingsomethingsomething
  : something
    ? somethingsomethingsomethingsomethingsomethingsomethingsomethingsomething &&
      somethingsomethingsomethingsomethingsomethingsomethingsomethingsomething
    : null

[sheerun]

Referencing standard/standard#1451

[brodybits]

Interesting. I think this would have to be optional (keep same formatting as Prettier by default). I generally try to use the same option name as eslint whenever possible.

I will probably need some time to look at this. A contribution would be very welcome for consideration.

Comments from the user community would also be very welcome.

[brodybits]

@sheerun I think this issue can be closed; prettierx seems to preserve the desired result if you only use the --no-semi flag, be sure to remove the --no-align-ternary-lines flag:

search.premium
  ? commission => {
      return suggestions.hits.map(hit => something(hit))
    }
  : () => onOpen()

Note that using prettier with only the --no-semi flag would give us the same, correct result in this case.

My comments about the multi-level ternary examples coming next.

[brodybits]

Your examples with multi-level or nested ternaries seem to touch upon 2 things:

• the behavior discussed in this issue, which I think can be resolved suppressed by not using the --no-align-ternary-lines flag (please remove this flag)
• indentation of multi-level or nested ternaries was removed from Prettier, and there is some desire to change it back or find some other way to improve: Nested ternary formatting - add indents back prettier/prettier#5814

I will now raise a new issue to discuss some way to improve the formatting of nested ternaries.

[sheerun]

@brodybits This issue is indeed more about nested ternaries. --align-ternary-lines=false unfortunately does two things instead of one:

1. Aligns expressions inside ternary to the ternary
2. Indents each nested ternary by one level

I'm interested in enabling just 2. and leave alignment expressions inside ternary untouched

[brodybits]

Thanks @sheerun. The fact that this issue and sheerun/prettier-standard#76 are about nested ternary expressions was not clear to me until now. Can you please do me and others a favor and clarify as follows:

• use the word nested in the title of Weird formatting of long nested ternary expression sheerun/prettier-standard#76 and of this issue
• update the description of this issue with at least one nested ternary example and "unfortunately does two things' info from Inconsistent formatting of nested ternary expressions #41 (comment)

I do think the solution should be pretty straightforward (actually got a quick solution working in my workarea), challenge would be to determine what and how should be configured.

[sheerun]

@brodybits Sure, done

[sheerun]

I think extra option --indent-nested-ternary would be probably the best thing to do

[sheerun]

btw. don't blame yourself that you didn't get it, because I didn't as well at the time :P It took me few months to fully realise what's the actual issue

[brodybits]

done

I see you fixed it in both places, thanks!

extra option --indent-nested-ternary

Sounds nice. I will ask you to review when I raise the PR, where we can discuss further if needed.

don't blame yourself
[...]

Bullseye, a major benefit of GitHub issues and using GitHub itself:)

I did try a quick fix which then broke something else. I will test each of the existing cases of using options.alignTernaryLines before moving forward with a bug fix.

And thanks for giving credit in sheerun/prettier-standard@b8687b1, even better if it would really be consistent:)

[brodybits]

If we fix it the way we discussed, I am now thinking to name it --offset-ternary-expressions for the sake of consistency with eslint/eslint#12556.

From the number of issues and discussions related to this one and prettier/prettier#5814, I am now thinking of an alternate solution that I proposed in #71.

[brodybits]

I think what we are looking for is (more) balanced indentation of nested ternary expressions, which would be more like Prettier was before prettier/prettier#5039 was merged. I also discovered prettier/prettier#5257, which I am now tracking in #73.

I will probably need some more time to figure this one out. I tried changing a few things, nothing seems to work 100% right for me right now. We have seen things go wrong with ternary formatting in both this fork and upstream Prettier, would ideally like to get it right with more thorough testing this time.

The more verbose ternary formatting solution I proposed in #71 would probably be easier and quicker, though likely to be affected even worse by #73.

Any contribution would be welcome for consideration. As I said before, I would be especially picky about the option name.

[brodybits]

I think I figured out how to get the balanced nested ternary indentation working, may need some more time to test and investigate before fixing.

[brodybits]

@sheerun I have updated the title and pinned this issue as a conflict with standard. Unfortunately I have found a case from brodybits/react-native-module-init#85 where the formatting of a nested ternary by standard 16 does not look right:

const mockCallSnapshot = []

jest.mock('execa', () => (cmd, args, opts) => {
  mockCallSnapshot.push({ execa: [cmd, args, opts] })
  return cmd === 'git'
    ? args[1] === 'user.email'
        ? Promise.resolve({ stdout: 'alice@example.com' })
        : Promise.resolve({ stdout: 'Alice' })
    : Promise.resolve()
})

Here is how I think it should be formatted:

const mockCallSnapshot = []

jest.mock('execa', () => (cmd, args, opts) => {
  mockCallSnapshot.push({ execa: [cmd, args, opts] })
  return cmd === 'git'
    ? args[1] === 'user.email'
      ? Promise.resolve({ stdout: 'alice@example.com' })
      : Promise.resolve({ stdout: 'Alice' })
    : Promise.resolve()
})

I hope we can get this fixed on standard somehow. Yes I may be possible for me to make prettierx format nested ternaries more consistently with standard 16 but think this would be extra work in the wrong direction.

[brodybits]

Issues that I discovered still lurking in eslint & "Standard JS":

• Incorrect indenting of ternary with multi-line function calls with offsetTernaryExpressions: true eslint/eslint#14058
• Ternary indenting issues from eslint standard/standard#1624

I hope to get these resolved in the near future.

[brodybits]

The update with --offset-ternary-expressions in release 0.17.0 did not really resolve this issue, see issue #468. Further discussion will be in #468.