Weird formatting of long nested ternary expression

[JStonevalley]

Hi
prettier standard formats my ternary expression without indent when breaking a line that is too long. This does not seem correct.
The simplified code snippet looks as follows:

When formatted it looks like this:

My only config is:
{ "printWidth": 120 }
Regards

[sheerun]

This would be solved after brodybits/prettierx#44 is fixed

[brodybits]

brodybits/prettierx#44

I hope to resolve it soon, no promises right now.

[NT-RX0]

Any updates? still broken.

[sheerun]

I think @brodybits removed this issue from prettierx but it still needs to be fixed there. If anyone would like to see it fixed, please controbute to prettierx repository

[brodybits]

The issue was moved to brodybits/prettierx#13 (original repository was renamed, issue was transferred, but link did not seem to redirect). And I think the issue was already fixed by @aMarCruz.

@sheerun, I think this repo needs to use a newer version of prettierx.

[sheerun]

@brodybits Good to know, I'll try to upgrade this repo

[aMarCruz]

@sheerun , use the "alignTernaryLines": false in your prettierrc.json

with alignTernaryLines as false you have this output:

The logic is the same as ESLint indent.flatTernaryExpressions

[sheerun]

Ternary expressions should be fine in 10.0.0

[sheerun]

Hmm I need to revert because there are unexpected issues with other syntax

[sheerun]

Here's an example how prettier-standard formats right now:

const Component = () => (
  <ul className='has-text-centered'>
    {landmarks.hits.length > 0
      ? landmarks.hits.map(({ name, objectID }) => (
          <li key={name}>
            <a href={`${process.env.DOMAIN}` + `/city/${objectID}`}>{name}</a>
          </li>
        ))
      : null}
  </ul>
)

And here's how after fixing ternary expressions:

const Component = () => (
  <ul className='has-text-centered'>
    {landmarks.hits.length > 0
      ? landmarks.hits.map(({ name, objectID }) => (
        <li key={name}>
          <a href={`${process.env.DOMAIN}` + `/city/${objectID}`}>{name}</a>
        </li>
      ))
      : null}
  </ul>
)

As you can see there's missing indent even though there's no second ternary expression.

[NT-RX0]

And here's how after fixing ternary expressions:

const Component = () => (
  <ul className='has-text-centered'>
    {landmarks.hits.length > 0
      ? landmarks.hits.map(({ name, objectID }) => (
        <li key={name}>
          <a href={`${process.env.DOMAIN}` + `/city/${objectID}`}>{name}</a>
        </li>
      ))
      : null}
  </ul>
)

@sheerun It's the same result as current version of standard --fix, so let's take this step and feedback the upstream I guess?

[aMarCruz]

Should be. I based my PR on the ESLint rule code, which is the one that Standard uses.

[sheerun]

I'm sorry that this issue is this deep, but I think it should be fixed in standard first. Nested ternary expressions are far less common than single ternary with JSX code or map (as shown above) and I think this change would make formatting less readable. Formatting my project which this change re-formats tenths of files where nested ternary is not used.

[sheerun]

Here is another example of inconsistent formatting:

standard --fix

const Component = () => (
  <ul className='has-text-centered'>
    {landmarks.hits.length > 0
      ? landmarks.hits.map(({ name, objectID }) => {
        return someverylongvariablelololololololololololololololol
      }).map(({ name, objectID }) => {
        return <foo />
      })
      : null}
  </ul>
)

But similar code with standard --fix (with new line after before .map) yields:

const Component = () => (
  <ul className='has-text-centered'>
    {landmarks.hits.length > 0
      ? landmarks.hits
        .map(({ name, objectID }) => {
          return someverylongvariablelololololololololololololololol
        }).map(({ name, objectID }) => {
          return <foo />
        })
      : null}
  </ul>
)

On the other hand currently prettier-standard formats both examples consistently (it doesn't matter where .map is):

const Component = () => (
  <ul className='has-text-centered'>
    {landmarks.hits.length > 0
      ? landmarks.hits
          .map(({ name, objectID }) => {
            return someverylongvariablelololololololololololololololol
          })
          .map(({ name, objectID }) => {
            return <foo />
          })
      : null}
  </ul>
)

[sheerun]

Also, here's original example formatted with standard --fix if .map is on new line:

const Component = () => (
  <ul className='has-text-centered'>
    {landmarks.hits.length > 0
      ? landmarks.hits
        .map(({ name, objectID }) => (
          <li key={name}>
            <a href={`${process.env.DOMAIN}` + `/city/${objectID}`}>{name}</a>
          </li>
        ))
      : null}
  </ul>
)

[bsiddiqui]

@sheerun any update on this?

[sheerun]

No, someone needs to fix this at prettierx

[brodybits]

We already made a fix before, see brodybits/prettierx#13.

In case of troubles, someone please raise a new issue in https://github.com/brodybits/prettierx/issues with a minimal, reproducible example ([1]).

[1] https://stackoverflow.com/help/minimal-reproducible-example

[sheerun]

I've described issue in more detail in brodybits/prettierx#41

So in short probably standard.js would need to be convinced first to make this change...

[sheerun]

So now the plan is as follows...

1. First eslint needs to merge PR I've prepared that introduces option for indent rule that fixes this issue: New: Add offsetTernaryExpressions option for indent rule eslint/eslint#12556

2. Then I need to convince standard to use this rule as default: Indentation/alignment within ternary branches (consequent/alternate) standard/standard#927

3. Finally I need to convince prettierx to change how --no-align-ternary-line works:
   Inconsistent formatting of nested ternary expressions brodybits/prettierx#41