New Ternary Formatting: A Curious Case of the Ternaries

[rattrayalex]

Summary

Adds an --experimental-ternaries option, which moves the ? in multiline ternaries to the end of the first line instead of the start of the second, along with several related changes.

While it might look weird at first, beta-testing shows that after a little use, developers find it makes nested ternaries much more readable and useful.

This PR resolves one of our most highly-upvoted issues in a way that fixes the problems the naive solution reintroduced, following extensive discussion, exploration, and experimentation.

Example

// "Questioning" ternaries for simple ternaries:
const content = 
  children && !isEmptyChildren(children) ? 
    render(children) 
  : renderDefaultChildren();

// "Case-style" ternaries for chained ternaries:
const message =
  i % 3 === 0 && i % 5 === 0 ? "fizzbuzz"
  : i % 3 === 0 ? "fizz"
  : i % 5 === 0 ? "buzz"
  : String(i);
  
// Smoothly transitions between "case-style" and "questioning" when things get complicated:
const reactRouterResult = 
  children && !isEmptyChildren(children) ? children 
  : props.match ?
    component ? React.createElement(component, props)
    : render ? render(props)
    : null
  : null

For more examples, see the tests:

• https://github.com/rattrayalex/prettier/blob/postfix-ternaries/tests/format/js/conditional/new-ternary-spec.js
• https://github.com/rattrayalex/prettier/blob/postfix-ternaries/tests/format/js/conditional/new-ternary-examples.js

Walkthrough

1. Every line that ends with a question mark is an "if".
   • If you see foo ? it's like asking a question about foo – "if foo? then, …".
2. Every line that starts with a : is an "else".
   • If you see : foo that means, "else, foo".
   • If you see : foo ? that means "else, if foo?".
3. Every line that just has an expression is a "then".
   • If you just see foo, that means, "then foo".

Reception

Typically, developers say they dislike this at first glance. Fortunately, beta testing shows that after using it for a little while, developers find it much more readable than other formatting options, and don't want to ever go back.

For example, here's someone commenting on a PR adding this style to their codebase at work:

And here they are a few weeks later:

Someone else from the same team, who was also quite skeptical at first, had this to say:

My first hour with the rule on, it felt a little odd. But by hour two, I’d used it a few times to solve problems that otherwise would have been ugly refactors to if statements. I’m not going back. 

Like JSX and Prettier itself, which both had many detractors at first, we expect most skeptics to become advocates after using this for a while.

Context and Motivation

Printing nested ternaries nicely in a wide variety of scenarios is a tricky challenge.

Prettier’s original, naive approach of indentation worked fine in simple cases, but didn’t scale and had other problems.

In 2018, we replaced that with flat ternaries, which the community did not receive well – the issue asking it to be reverted has over 500 upvotes.

Over the last few years, we’ve worked hard to find a solution which would satisfy our constraints in a wide variety of situations.

Ideally, we'd find one scheme that would fluidly flow from a single ternary, to a chain of 2, to a long chain of simple cases, to something more complex with a few nested conditions. The syntax in JSX, TypeScript conditional expressions (which cannot be expressed with if), and normal JS should all look and feel the same. And in all cases, it should be easy to follow what's the "if", what's the "then", and what's the "else" – and what they map to.

• Fixes Nested ternary formatting - add indents back #5814 , Prefer breaking after = in type aliases if RHS is a conditional type chain #10325 , and others.
• Supersedes Add indentation back to nested ternaries #9559, Use JSX style for all nested ternaries #9552 , and others.

Details and Notes

Note that it mostly removes parens from ternaries in JSX (there is almost no special-casing for JSX needed anymore), eg:

<ListItemAvatar>
  {customers[record.customer_id] ?
    <Avatar
      src={`${customers[record.customer_id].avatar}?size=32x32`}
    />
  : <Avatar />}
  Or, reversed:
  {!customers[record.customer_id] ?
    <Avatar />      
  : <Avatar 
      src={`${customers[record.customer_id].avatar}?size=32x32`}
    />
  }
</ListItemAvatar>

Checklist

• I’ve added tests to confirm my change works.
• (If changing the API or CLI) I’ve documented the changes I’ve made (in the docs/ directory).
• (If the change is user-facing) I’ve added my changes to changelog_unreleased/*/XXXX.md file following changelog_unreleased/TEMPLATE.md.
• I’ve read the contributing guidelines.

✨Try the playground for this PR✨

[thgreasi]

The only part that I would prefer as in the current release, would be for simple ternaries to be:

const ifElse = shouldShowStuff()
  ? showTheStuff()
  : hideTheStuff();

[sosukesuzuki]

I will review this PR this weekend.

[Evertt]

@sosukesuzuki how about this weekend? 😇

[kheck-slingshot]

@sosukesuzuki how about this weekend? 😇

Does this one work?!

[Evertt]

Gotta say, after reading it more thoroughly, I'm not a huge fan of how this looks...

I hear the OP in that he experienced that he has come to like this style more and more after using it for a while, and so that he hopes people would be willing to try it for a while before jumping to conclusions.

The thing is, I'm not sure how I could try this for a while before casting my final vote let's say. Like, either this gets merged and then everybody can (/ will be forced to) "try" it, but I'm pretty sure after it's been merged, it's very unlikely to be reverted if people turn out to really dislike it.

Or this PR doesn't get merged yet for a while, but then how am I supposed to be able to try it? @rattrayalex, am I supposed to point to your fork and branch in my package.json for the prettier dependency? Is that what you had in mind?

Edit

To be more specific about the things that, at least at first glance, I don't like:

// "Questioning" ternaries for simple ternaries:
const ifElse = shouldShowStuff() ?
    showTheStuff()
  : hideTheStuff();

I'm so used to seeing the ? and : either completely aligned or indented, that when I read this style, I have a moment of confusion and I really need to go looking for the ? to prove to myself that I haven't lost my sanity.

The case-style is also new to me, but that one I can more easily imagine getting used to and liking after having some more experience with it.

const redirectUrl = pathName ? pathName
  : nextPathName + nextSearch || defaultAuthParams.afterLoginUrl;

This one I can live with. But that's just because I'm sensitive to line lengths and I prefer to see multiple lines that are similar in length than to see one outlier that is significantly longer or shorter than the lines around it.

const redirectUrl = pathName ?
    pathName
  : nextPathName + nextSearch || defaultAuthParams.afterLoginUrl;

Same example as the first one. Like, okay besides the fact that now my line-length-sensitivity is triggered... 😅 But okay, besides that, I mean really, if the consequent has to go to to the next line, then why not bring the ? with it? It looks neater, and it's a style that most people are just way more familiar with and thus will find easier to read. And isn't the end-goal (or at least one of them) to apply a consistent style to people's code that will make it easier to read for most people?

const redirectUrl = pathName
  ? pathName
  : nextPathName + nextSearch || defaultAuthParams.afterLoginUrl;

I do understand what you say that you have come to read this const redirectUrl = pathName ? as a question and therefor it feels more natural for you. Maybe I'll have the same experience, and maybe other will too. But so far we have no certainty on that whatsoever.

So then I think it should be up to you and the (other) maintainers of this formatter to provide us with a trial period. Like this gets merged, but it's opt-in or opt-out via a newly added configuration option.

And then a few months later we take a poll on whether the new style indeed has grown on people or not.

What do you guys think about that? @rattrayalex @sosukesuzuki

[rattrayalex]

Thanks Evert. I really appreciate your willingness to try this out, even though it looks weird.

But okay, besides that, I mean really, if the consequent has to go to to the next line, then why not bring the ? with it?

A full blog post is due to answer this question, but here's a quick attempt off the cuff, from what I can remember:

1. When you look at the first line, you can immediately see that in const redirectUrl = pathName, pathName isn't an assignment, but a question.
2. As you mention, this looks very wrong/strange at first, but IME/IMO lends much more clarity once you're used to it.
3. Over time, you realize it makes your ternaries less like a weird boolean, and more like a concise if-expression (which JS does not have, and by the looks of it, won't get anytime soon).
4. It makes for a much more consistent evolution of a ternary as it grows into the case-style chain.
   A) The change from a traditional style to the case style looks quite jarring in practice, the question mark flies around in weird ways.
   B) The case style demands a question mark at the end of the line when the consequent is indented. Without a question mark always following the consequent, my concern is this will not only be inconsistent, but make it harder for people to quickly and intuitively read the case-style.
5. It lets you read any line or segment of a ternary, and immediately know which part of the ternary you're in – the question (ends with a question mark), the consequent (naked), or the alternate (starts with a colon).
   A) It's what we're used to, but not actually intuitive, for the consequent to start with a question mark (there's nothing about a ? foo that says "then foo", but foo ? very much says "if foo" – does that make sense?).
   B) In a long chain of ternaries, even with indentation as prettier originally did and as the community is asking for, it's mind-numblingly difficult to tease out which things are questions and which things are answers. With this change, it becomes very intuitive and easy to read once you're used to it.

Does that help clarify anything at all? I'm keen for feedback – which of these arguments make the most sense / feel most compelling, and which the least?

[rattrayalex]

Oh, and as for this:

how am I supposed to be able to try it?

You can set "prettier": "rattrayalex/prettier#postfix-ternaries" in your package.json

[vjeux]

I like the parenthesis and structure here

[vjeux]

Nice that the condition now fits in one line

[vjeux]

Would be great to ship all those examples in master (regardless of this PR) and rebase so we can see the before/after your changes

[rattrayalex]

Done!
PR here: #13952 (just extracts the first commit from this branch to a separate PR)
Diff here: 615d1c1

[cherscarlett]

Would it be possible to split this into two PRs? One from the previous discussion reverting to old behavior, and another to consider introducing your additional concerns?

[cNoveron]

Great to see people are actually doing something about this. I stopped using prettier precisely because of this issue, so I really hope this PR solves it for good.

[ehoogeveen-medweb]

1. When you look at the first line, you can immediately see that in const redirectUrl = pathName, pathName isn't an assignment, but a question.

I was thinking about this because my gut reaction was "well I don't need the question mark to see there's more going on" and I realized it's probably because I rely on the presence of a semicolon to know that a line is finished. If I configured Prettier to format without semicolons and rely on ASI then the ? would be pretty helpful, but currently (and I guess this is the default configuration) my eyes instantly move to the next line to keep reading.

[rattrayalex]

Now that #9559, this PR is blocked by:

1. Reviews by @sosukesuzuki and @fisker (and optionally other maintainers)
2. A blog post by me (I had an earlier draft, and I'm brushing it off now) along with a feedback form, which I plan to make today.

@sosukesuzuki and @fisker, please let me know if there's anything I can do to help you in review! I've tried to structure commits such that you can see the diff between this and main in both the false and true setting, etc.

[sosukesuzuki]

The release blog should be created at ./scripts/draft-blog-post.js. Also, other features and fixes should be mentioned.

Therefore, I would like it to be removed from this PR.

Based on the content of this post, I'll be opening a PR to create a new release blog, and I'll be asking for your reviews at that time as well.

[rattrayalex]

Sure, will remove shortly! Any feedback on the PR itself (is all good there?) or the other blog post?

[fisker]

I still prefer keep the old ternary.js unchanged, rename the new one as another name. Not very important, won't block.

[sosukesuzuki]

If my review comments are solved, LGTM.

[sosukesuzuki]

Can I separate this blog addition into a separate PR? I don't want to merge features and post blogs at the same time. I think it's best to post blog at the same time as the 3.1 release.

[sosukesuzuki]

@rattrayalex I've removed two blog posts from this PR. I'll re-create PRs for each blog posts.

[rattrayalex]

Terrific, thank you so much @sosukesuzuki ! I'll resume work on the blog post in a separate PR.