New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposals for nested ternary formatting #9561
Comments
Option A |
Option B |
To "my taste", they're both inferior to both current "flat", and previous "binary-tree" options. |
I prefer option B over option A, but still prefer an unimproved reversion to nested types per #9559, as having a concise (or even attractive) representation is far far less of a priority for me than regaining the ability to quickly glean the logical relationships between paths even when only looking at a subset of a large ternary... and IMO nothing even approaches the nested format on that front. As an avid user of TS conditional types, I would add a vote for supporting the nested structure there as well. It's not uncommon for me to end up in the "complex" case, where sub conditions exist in both branches at multiple depths. But regardless of which way this goes, I just want to thank you @rattrayalex for putting in all this thought and work on the different options, and including solid examples for each proposal 🙌 (Edited for clarity) |
😄 thanks Mike! I really appreciate the kind words. Just to be clear, you're saying that you prefer the behavior in #9559 to either of the behaviors in this proposal for TS conditional types? EDIT: And by the way, if you'd be willing to find a particularly complex example in your codebase, I'd much appreciate it! No worries if you're not able to. |
I vote for "Option A". And actually we can create "Option C": const message =
i % 3 === 0 && i % 5 === 0
? "fizzbuzz" :
i % 3 === 0
? "fizz" :
i % 5 === 0
? "buzz" :
String(i); It is something like compromise between Option A and Option B. |
Hi @rattrayalex! Regarding an example, much of our public code has been refactored over the past year or so to remove these kind of ternaries due to readability issues with the formatting. However, here is a public file of ours with various ternaries of different depths and complexities impeded to different degrees by the current rules. In case it's helpful, here are some more examples from non-public code in a codebase of mine that uses a patch to restore the original nested format: const args = [
context.requestId,
context.sessionId,
context.userId,
...columnFields.map(([field, { encode, default: d }]) =>
encode
? encode(
typeof data[field] === "undefined"
? typeof d === "undefined"
? null
: d
: data[field]
)
: typeof data[field] === "undefined"
? typeof d === "undefined"
? null
: d
: data[field]
)
];
// --------------------------------------------------
// get the field
return definition.encode
? definition.encode(
typeof row[field] === "undefined"
? typeof definition.default === "undefined"
? null
: definition.default
: row[field]
)
: typeof row[field] === "undefined"
? typeof definition.default === "undefined"
? null
: definition.default
: row[field]; I think that my greatest frustration with the current behavior, and my concern with some of these other proposals, is that a reader has to expect different formatting based on the logic itself. This means you need to understand the logic before the formatting is helpful, which defeats the whole point of formatting. Given this and my stance that the nested format is the only useful one for the complex case, I do think my real preference is for restoration of the indented form, per #9559. I do acknowledge the issue of "scrunching lines" with deep indentation, but this is an acceptable tradeoff in my use cases. |
Also, regarding your request for examples of TypeScript conditionals, I'm not sure what your timeline is on this ternary issue, but this major initiative in graphql-js is on the docket immediately following or part of the codebase conversion to TS. This feature will require extensive use of conditionals - I suspect it will require heavier use than any other codebase I've worked with or read. The feature essentially uses the GraphQL object configurations to check or infer resolver call signatures, and given that ternaries are the only mechanism for describing conditional types, there is a good chance that this will find the limits of any formatting strategy 🙃 |
I slightly prefer Option B but either Option A or Option B would be a vast improvement over the previous methods (flat or nested), "to my taste". 😉 Thanks for taking the time to write this all up. |
Thanks for the real-world examples, Mike! I decided to peel off the last one and see how they compare. Turns out Option A would be the exact same as indented, since this isn't a flat-nested ternary chain, and there's only 2 cases. However, I illustrated what it would look like if the minimum was a ternary chain length of 2 instead of 3. Examples with the code as written:// indented:
const result = definition.encode
? definition.encode(
typeof row[field] === "undefined"
? typeof definition.default === "undefined"
? null
: definition.default
: row[field]
)
: typeof row[field] === "undefined"
? typeof definition.default === "undefined"
? null
: definition.default
: row[field];
// option A – actually the same as above(!!), since the longest ternary chain is 3.
// If we were to adjust it to a minimum length of 2, this is how it'd look:
const result =
definition.encode ? (
definition.encode(
typeof row[field] === "undefined"
? typeof definition.default === "undefined"
? null
: definition.default
: row[field]
)
) : typeof row[field] === "undefined" ? (
typeof definition.default === "undefined"
? null
: definition.default
) :
row[field];
// option B:
const result =
definition.encode
? definition.encode(
typeof row[field] === "undefined"
? (typeof definition.default === "undefined"
? null
: definition.default)
: row[field]
)
: typeof row[field] === "undefined"
? (typeof definition.default === "undefined"
? null
: definition.default)
: row[field];
// option B2 (see below):
const result =
definition.encode ?
definition.encode(
typeof row[field] === "undefined" ?
typeof definition.default === "undefined" ?
null :
definition.default :
row[field]
) :
typeof row[field] === "undefined" ?
typeof definition.default === "undefined" ?
null :
definition.default :
row[field]; Ultimately, I thought all 3 looked pretty similar in this case. However, I was also curious what it would look like if the undefined-checks were flipped, making the ternaries more of a flat chain: Examples with the ternaries flattened into a chain:// indented:
const result = definition.encode
? definition.encode(
typeof row[field] !== "undefined"
? row[field]
: typeof definition.default !== "undefined"
? definition.default
: null
)
: typeof row[field] !== "undefined"
? row[field]
: typeof definition.default !== "undefined"
? definition.default
: null
// option A:
// (note that one of the ternary chains is length 3, and the inner one length 2, so they look different. Don't love it.)
const result =
definition.encode ? (
definition.encode(
typeof row[field] !== "undefined"
? row[field]
: typeof definition.default !== "undefined"
? definition.default
: null
)
) :
typeof row[field] !== "undefined" ? row[field] :
typeof definition.default !== "undefined" ? definition.default :
null
// option B:
const result =
definition.encode
? definition.encode(
typeof row[field] !== "undefined"
? row[field]
: typeof definition.default !== "undefined"
? definition.default
: null
)
: typeof row[field] !== "undefined"
? row[field]
: typeof definition.default !== "undefined"
? definition.default
: null
// option B2 (see below):
const result =
definition.encode ?
definition.encode(
typeof row[field] !== "undefined" ?
row[field] :
typeof definition.default !== "undefined" ?
definition.default :
null
) :
typeof row[field] !== "undefined" ?
row[field] :
typeof definition.default !== "undefined" ?
definition.default :
null; Honestly, this gave me a lot of pause about Option A with the rule around only entering it with a chain of 3 or more. It just makes ternaries look inconsistent and even more confused – it's readable enough, but your eyes don't learn where to look to figure out the structure of a complex expression involving ternaries. This makes me lean personally towards using Option A for TS Conditional Types only, which it turns out is already being worked on in #7948 (just saw for the first time). Of course, if the format turns out to be popular, we could consider adopting it in ternaries more widely in the future. I'm not sold on this – I think Option B performed nicely in the above examples, and Option A was mostly only confusing when situated next to shorter ternaries – but more hesitant about Option A than I was before. EDIT: I have added Option B2, which I think actually looks the best in these cases. |
I prefer Option B to Option A. However, I would like to propose a modification of Option B that I like more than either of those. The modification is to place the Option B2: if-else-style, trailing punctuationconst message =
i % 3 === 0 && i % 5 === 0 ?
"fizzbuzz" :
i % 3 === 0 ?
"fizz" :
i % 5 === 0 ?
"buzz" :
String(i);
const paymentMessage =
state == "success" ?
"Payment completed successfully" :
state == "processing" ?
"Payment processing" :
state == "invalid_cvc" ?
"There was an issue with your CVC number" :
state == "invalid_expiry" ?
"Expiry must be sometime in the past." :
"There was an issue with the payment. Please contact support.";
const simple =
children && !isEmptyChildren(children) ?
children :
null
const simpleChained =
children && !isEmptyChildren(children) ?
children :
component && props.match ?
React.createElement(component, props) :
render && props.match ?
render(props) :
null
const complexNested =
children && !isEmptyChildren(children) ?
children :
props.match ?
component ?
React.createElement(component, props) :
render ?
render(props) :
null :
null;
type TypeName<T> =
T extends string ?
"string" :
T extends number ?
"number" :
T extends boolean ?
"boolean" :
T extends undefined ?
"undefined" :
T extends Function ?
"function" :
"object";
type Unpacked<T> =
T extends (infer U)[] ?
U :
T extends (...args: any[]) => infer U ?
SomeVeryLongNameOfSomeKind<U> :
T extends Promise<infer U> ?
U :
T; This was first proposed by @0x24a537r9 in this comment. I never saw any subsequent comments discuss it. pros compared to Option B
cons compared to Option B
I am aware that this Option B2 doesn’t fix all the cons of Option B as compared to Option A, but I think it is an improvement overall and should be the main contender against Option A. |
I actually made a PR based on that comment: #4767, but it was never merged. |
Can we see how Option A will work when nested in truthy branch? All I see is perfect switch-case like scenarios for that option, which doesn't really help. |
Thanks @roryokane for reviving @0x24a537r9 idea (and @zeorin for putting up the earlier PR). I'll admit I had overlooked this approach, and seeing the examples and advantages spelled out by @roryokane is quite compelling to me (thanks for doing the work there!!). I particularly like that it brings ternaries into consistency with boolean operators, and the simple heuristic that "a ? at the end means it's a condition", which is both natural and clear. Honestly, looking at some examples, Option B2 looks the most like "prettier-formatted" code, probably because of the operator-at-end-of-line consistency. I agree with the removal of parens in this case, for the reasons stated. I am also optimistic that Option B2 may work better for TS Conditional Types than any of the proposed alternatives, but I think that needs further exploration... I think that, going back to #4767, B2 leaves us with a big question – do we adopt this formatting for non-nested ternaries? Doing so would certainly cause quite a lot of churn, and probably an uproar to go with it. Based on my experience, I can't imagine prettier's maintainers would be okay with that. So I think the key question may be, would it be prohibitively surprising/confusing a jump to go from a single ternary with BOL operators to a nested ternary with EOL operators? |
See the Option description: const complexNested =
children && !isEmptyChildren(children) ? children :
props.match ? (
component ? React.createElement(component, props) :
render ? render(props) :
null
) :
null; |
Exploring TS Conditional Types specifically with this format, I wonder if we could special-case things to get a nice blend of Option A and Option B2, in the spirit of @thorn0 's comment here. Specifically, B2 could turn into A when the consequent is an Identifier or scalar Literal (ie; For example: type Unpacked<T> =
T extends (infer U)[] ? U :
T extends (...args: any[]) => infer U ?
SomeWrapper<U> :
T extends Promise<infer U> ? U :
T; What do you think @thorn0 @mmiszy? (This approach could perhaps also apply to JS ternaries, but I'm nervous that it would be fickle – might be better to start with just TS conditionals). |
Whether or not Option B2 is adapted to turn into Option A for some TS Conditional Types, I basically agree that B2 seems very likely to be strictly superior to either fully-flat (current production behavior) or fully-indented (#9559) for TS. It scales better than fully-indented to long conditionals (which may be necessary) and looks cleaner and less confusing than fully-flat. In fact, I think TS Conditionals were often given as an example of why fully-indented wasn't good in the first place. |
TS conditionals are really a separate thing, for multiple reasons. What's good for them isn't necessary good for normal conditionals. For one, don't forget that it's not always |
I definitely see that argument, and I agree that it's fine for them to have their own logic, and for that logic to be considered separately/later. But starting from a point of consistency between ternaries and TS conditionals would definitely be preferable if possible. If the two looked totally different, that'd be unfortunate. Also worth keeping in mind that we're racing against the clock of merging #9559, and past experience has shown that TS users don't love that behavior for TS conditionals. So I'd really like to come up with something that's better than #9559 for TS conditionals. All that said, I basically think Option B2 fits the bill nicely, and as noted above, probably helps make it easier to merge #7948 more than anything. And I agree that whether to merge/modify/replace #7948 can be a separate question, outside of the scope of this issue. Sound right to you? |
At the moment, #9559, on condition that it includes #9559 (comment), seems like the least evil to me, but I probably need to have another look at all the proposals. Also it wouldn't solve the problem with simple TS conditionals (#7940). That would still need a solution. BTW, I've just had another thought about TS conditionals. Because of the prominent |
Fully-flat looks strange in TS only for simple types, like the example in #7940. For more complex types, as I wrote in #7948 (comment), it's pretty good. |
@rattrayalex thanks for testing out using my example. I have a couple follow-up comments based on the results and a bit more thought here. Instead of just announcing my preferences here, I think it might be more useful to explain my priorities and criteria for whatever a "good" solution here would exhibit. Maybe other folks agree or disagree:
Adding a new configuration option is obviously out. However, there is precedent in prettier for applying different rules based on the manually-formatted context. For example, prettier is perfectly happy to leave both of these alone, assuming they don't break the max column width: const a = { a: "a", b: "b" };
const b = {
a: "a",
b: "b"
}; Said a different way, prettier uses the existing formatting to decide whether the object's field definitions will be inline or on distinct lines. Could we do the same for ternaries? If we use the ternary root as a cue, we could apply the "chosen" formatting to all child paths. This would allow the best format to be "chosen" by the developer and enforced by prettier without the need for arbitrary heuristics. This would satisfy my 2 biggest criteria, and leave the third in the hands of me and my team. |
A configurable option is the most sensible way forward. As can be clearly seen in this collection of issues, there will never be agreement on what is the preferred formatting, and different developers use ternaries in different ways. I know the whole “no options!” mantra will continue to be chanted but at some point you have to realise that there are multiple use cases for ternaries and forcing it into a single format for all users is counter productive (the whole purpose of prettier is productivity is it not?). To blindly reject a logical way forward, not because it’s a bad solution, but because it goes against your motto, is foolish. The main prettier page says “Has few options”. An option for this could be added and that statement would still be true. I would argue that adding an option is less bad than the alternative, which is suddenly changing the formatting of all your users’ code… again. |
I'm pretty big on immutable coding practices and using |
I couldn't have put it better myself. |
Option E: parens-style// JavaScript
const message =
i % 3 === 0 && i % 5 === 0 ? (
"fizzbuzz"
) : i % 3 === 0 ? (
"fizz"
) : i % 5 === 0 ? (
"buzz"
) : (
String(i)
);
const paymentMessage =
state == "success" ? (
"Payment completed successfully"
) : state == "processing" ? (
"Payment processing"
) : state == "invalid_cvc" ? (
"There was an issue with your CVC number"
) : state == "invalid_expiry" ? (
"Expiry must be sometime in the past."
) : (
"There was an issue with the payment. Please contact support."
);
const simple =
children && !isEmptyChildren(children) ? (
children
) : (
null
)
const simpleChained =
children && !isEmptyChildren(children) ? (
children
) : component && props.match ? (
React.createElement(component, props)
) : render && props.match ? (
render(props)
) : (
null
)
const complexNested =
children && !isEmptyChildren(children) ? (
children
) : props.match ? (
component ? (
React.createElement(component, props)
) : render ? (
render(props)
) : (
null
)
) : (
null
); // JSX
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>
) // TypeScript
type TypeName<T> =
T extends string ? (
"string"
) : T extends number ? (
"number"
) : T extends boolean ? (
"boolean"
) : T extends undefined ? (
"undefined"
) : T extends Function ? (
"function"
) : (
"object"
);
type Unpacked<T> =
T extends (infer U)[] ? (
U
) : T extends (...args: any[]) => infer U ? (
SomeVeryLongNameOfSomeKind<U>
) : T extends Promise<infer U> ? (
U
) : (
T
); Pros:
Cons:
I've been writing my ternary expressions this way since about 2018, when I first got into functional-style JavaScript. (Ironically, it's the main reason I'm not using Prettier yet!) 😅 (Edited to add) Here's how I think my proposal meets the goals outlined by @rattrayalex :
|
@SpadeAceman , it may be more verbose, but you're touching on an excellent point that nowadays I believe ternary operators have to be more verbose anyway if we want to use them for complex expressions spanning over multiple lines. In the simplest cases, the |
Oops, apparently my Option E proposal may just be how Prettier already formats JSX ternaries. 😬 After seeing this in the playground over lunch ... {greeting.endsWith(",") ? (
" "
) : (
<span style={{ color: "grey" }}>", "</span>
)} ... I searched again for previous discussions on this topic, and found this comment, which seems to confirm it. If this is indeed how things currently work, then I guess my proposal basically boils down to using Prettier's JSX-style ternaries for all multi-line ternary expressions. I would argue that formatting multi-line ternary expressions the same consistent way everywhere would be highly beneficial - just like how Prettier's consistent visual formatting provides benefits for code readability and comprehension. |
Can we actually do something about it? Instead of sitting around? Let's have another vote and decide on something so I can finally remove the 10000 |
It's been 2.5 years and there's been no update from anyone at all about this. |
We haven't made much progress on this work because we don't have enough time. This work is progressing little by little, led by @rattrayalex. It's not dead yet. please wait. |
In a week it will have been 3 years since this issue was opened. Look at this code, it's indented 3 tabs too much. Why is it just randomly indented?
Please just add a temporary option to add back in how it used to be, or just some temporary solution. Those that don't care about some random formatting changes in commits can use that option for now, those that do care, don't have to. |
What's almost worse about this is that prettier-ignore ignores everything, so if it formats something like this badly, nothing inside it will get formatted either, so occasionally have to remove the prettier-ignore, format it, then put it back in and remove the bad formatting again. |
We're all developers here. I'm a developer too. And as developers we all know that if an outstanding improvement request or piece of technical debt hasn't been resolved after three years, it's not that "we don't have enough time", it's more a case of "this wasn't ever prioritised as something particularly important". I'm not having a go or being stroppy, I'm merely pointing out that in a thread full of people who want a thing to be done and then waited 3 years for it, citing "not enough time" as the reason may come across as somewhat... disingenuous. |
I think we are conflating 2 different issues: long/complex expressions and nested ternaries. Perhaps instead of trying to find a unifying theory, we can identify usage categories, and apply one style for each. Perhaps just 2, simple vs complex. I favor declaring long expressions in a separate variable or function: const longValue = "There was an issue with your CVC number, and here is some extra info too."; For me, all I want is pattern matching. So this is my favorite style, not that I'm proposing it: const message =
i % 3 === 0 && i % 5 === 0 ? "fizzbuzz" :
i % 3 === 0 ? "fizz" :
i % 5 === 0 ? "buzz" :
String(i);
const paymentMessage =
state == "success" ? "Payment completed successfully" :
state == "processing" ? "Payment processing" :
state == "invalid_cvc" ? "There was an issue with your CVC number, and here is some extra info too." :
state == "invalid_expiry" ? "Expiry must be sometime in the past." :
"There was an issue with the payment. Please contact support.";
I'm a vertical reader, meaning I read my version as
The other styles I read as
The left part of the code is the most important part of the code. There are many names for this style, I call it column reading. As for git concerns over white space, I think VsCode helps or eliminates the problem. Still, I commit code maybe 4-5 a week. I might read it dozens of times a day. |
I read the new post: https://prettier.io/blog/2023/11/13/curious-ternaries.html And found the example for the new format is not much readable to me.
const animalName =
pet.canSqueak() ? 'mouse'
: pet.canBark() ?
pet.isScary() ?
'wolf'
: 'dog'
: pet.canMeow() ? 'cat'
: pet.canSqueak() ? 'mouse'
: 'probably a bunny';
const animalName =
pet.canSqueak() ? 'mouse' :
pet.canBark() ?
pet.isScary() ? 'wolf' : 'dog' :
pet.canMeow() ? 'cat' :
pet.canSqueak() ? 'mouse' :
'probably a bunny'; |
Something I've personally learned is that moving operators to the start of the line immensely helps with readability and navigation by to making it easier to scan code and orient oneself without reading / parsing entire lines. My biggest issue with the new format is that it breaks that completely - it is not possible to quickly scan the left side of the new ternary to understand the composition of code. For example taking the example from the blog post: const animalName =
pet.canSqueak() ? 'mouse'
: pet.canBark() ?
pet.isScary() ?
'wolf'
: 'dog'
: pet.canMeow() ? 'cat'
: pet.canSqueak() ? 'mouse'
: 'probably a bunny'; Taking a subrange from that code that one might see when scanning the left-side: : pet.canB...
pet.isSc...
'wolf' It quickly illustrates a weakness of this formatting. You can no longer understand the structure here - all you know is that the code is part of the alternate branch of a ternary. In order to glean more information (eg is there method chaining here? is there an array? is there a nested ternary?) you need to stop and read and parse more of the line to understand things and keep scanning. In comparison the indented form of this code: const animalName = pet.canSqueak()
? "mouse"
: pet.canBark()
? pet.isScary()
? "wolf"
: "dog"
: pet.canMeow()
? "cat"
: pet.canSqueak()
? "mouse"
: "probably a bunny"; Taking the same 3 line subrange: : pet.canB...
? pet.is...
? "wol... It's immediately clear what the exact structure of the code is without needing to read any further. In fact you only need to read the first character of the line to immediately understand the structure of the code. This is the same reason that I personally prefer operators at the start of the line (#3806) over operators at the end of the line, but that's a tangent we need not get into in this discussion. |
I read the post and came here as a result. The proposed formatting is an improvement, but I dislike the mix of curious style and case style even though I find both of those styles fairly readable on their own. My particular problem is that it makes the pattern inconsistent and harder to scan. This would be my ideal proposal, which is essentially a small tweak of Option B2 proposed before to be consistent with what has been shipped in prettier behind the flag. The way to read this is even simpler than the rules in the current proposal:
This is the closest you can get to make ternaries read like a chain of Let’s call it Option B3:Option B2: if-then-else-style, trailing const message =
i % 3 === 0 && i % 5 === 0 ?
"fizzbuzz"
: i % 3 === 0 ?
"fizz"
: i % 5 === 0 ?
"buzz"
: String(i);
const paymentMessage =
state == "success" ?
"Payment completed successfully"
: state == "processing" ?
"Payment processing"
: state == "invalid_cvc" ?
"There was an issue with your CVC number"
: state == "invalid_expiry" ?
"Expiry must be sometime in the past."
: "There was an issue with the payment. Please contact support.";
const simple =
children && !isEmptyChildren(children) ?
children
: null
const simpleChained =
children && !isEmptyChildren(children) ?
children
: component && props.match ?
React.createElement(component, props)
: render && props.match ?
render(props)
: null
const complexNested =
children && !isEmptyChildren(children) ?
children
: props.match ?
component ?
React.createElement(component, props)
: render ?
render(props)
: null
: null;
type TypeName<T> =
T extends string ?
"string"
: T extends number ?
"number"
: T extends boolean ?
"boolean"
: T extends undefined ?
"undefined"
: T extends Function ?
"function"
: "object";
type Unpacked<T> =
T extends (infer U)[] ?
U
: T extends (...args: any[]) => infer U ?
SomeVeryLongNameOfSomeKind<U>
: T extends Promise<infer U> ?
U
: T;
// EDIT: More examples from Theo
type Something<T> =
T extends string ?
"string"
: T extends number ?
"number"
: "other";
type Something<T> =
T extends string ?
T extends "a" ?
"a"
: "b"
: T extends number ?
"number"
: "other";
// Very complex example.
type Schema<TParams> =
TParams['_input_out'] extends UnsetMarker ?
$Parser
: inferParser<$Parser>['out'] extends Record<string, unknown> | undefined ?
TParams['_input_out'] extends Record<string, unknown> | undefined ?
undefined extends inferParser<$Parser>['out'] ?
undefined extends TParams['_input_out'] ?
$Parser
: ErrorMessage<"Cannot chain an optional parser to a required parser">
: $Parser
: ErrorMessage<"A;; input parsers did not resolve to an object">
: ErrorMessage<"A;; input parsers did not resolve to an object">; ProsIt’s easier to tell which lines are conditions and which lines are values: It reads almost exactly like For complex “then” values, parenthesis can still be used in a clean readable way. It’s already what the proposed formatting would generate as long as the conditions for long enough to trigger wrapping. ConsNested ternaries in the “then” case lead to multiple levels of nesting hurting readbility. Increases the vertical height of the code. Further improvement to readability?In my opinion, wrapping any nested ternary expression in parenthesis further helps the readability at the cost of even more vertical space. Here in an example with and without parenthesis. // Without parenthesis for nested ternary:
const complexNested =
children && !isEmptyChildren(children) ?
children
: props.match ?
component ?
React.createElement(component, props)
: render ?
render(props)
: null
: null;
// With parenthesis for nested ternary:
const complexNested =
children && !isEmptyChildren(children) ?
children
: props.match ?
( component ?
React.createElement(component, props)
: render ?
render(props)
: null )
: null;
// With parenthesis where every line that start with punctuation is a condition
const complexNested
= children && !isEmptyChildren(children) ?
children
: props.match ?
( component ?
React.createElement(component, props)
: render ?
render(props)
:
null
)
:
null
;
// This is the most readable IMO.
// Here's the Very complex example with parens.
type Schema<TParams>
= TParams['_input_out'] extends UnsetMarker ?
$Parser
: inferParser<$Parser>['out'] extends Record<string, unknown> | undefined ?
( TParams['_input_out'] extends Record<string, unknown> | undefined ?
( undefined extends inferParser<$Parser>['out'] ?
( undefined extends TParams['_input_out'] ?
$Parser
:
ErrorMessage<"Cannot chain an optional parser to a required parser">
)
:
$Parser
)
:
ErrorMessage<"A;; input parsers did not resolve to an object">
)
:
ErrorMessage<"A;; input parsers did not resolve to an object">
; I don't have a strong feeling about the With the last variant, I think the rules become very simple:
|
Currently I get this: this[prop.name] = !handler
? newVal
: newVal === null // attribute removed
? 'default' in handler
? handler.default
: null
: handler.from
? handler.from(newVal)
: newVal but I manually change it to this which is more readable to me: // prettier-ignore
this[prop.name] = !handler
? newVal
: newVal === null // attribute removed
? 'default' in handler
? handler.default
: null
: handler.from
? handler.from(newVal)
: newVal (sorry GitHub renders tabs so wide) The second is cleaner because you can clearly see each ternary. Has that format been considered? |
I think, 5 years later, after multiple changes to ternary formatting that always sufficiently annoy a significant portion of users, it may be time to accept that there is no one-size-fits-all ternary format. Creating an option to allow a user to choose a style would be great. Allowing for the community to create plugins for different styles would also be a way to allow for flexibility (or is this already possible with the current plugin API?). |
Agreed. Reading the Option Philosophy page, this sentence stood out to me:
The existence of the So, if we admit that this is still an area of active and ongoing research, then the potential need for configuration in this area can't be dismissed out of hand. Especially if this ongoing research ultimately concludes (as I believe it will) that no one-size-fits-all ternary format is possible. |
Has someone consulted about formatting of nested ternarries with any AI? I still vote for flatting else-branches and indeting the if branches, and consider the adding/removing one case must affect only this case. By the way, it is also a good idea to use ternarries like tables (but in case of else-branching only and if the const message =
i % 3 === 0 && i % 5 === 0 ? "fizzbuzz" :
i % 3 === 0 ? "fizz" :
i % 5 === 0 ? "buzz" :
String(i);
const paymentMessage =
state == "success" ? "Payment completed successfully" :
state == "processing" ? "Payment processing" :
state == "invalid_cvc" ? "There was an issue with your CVC number, and here is some extra info too." :
state == "invalid_expiry" ? "Expiry must be sometime in the past." :
"There was an issue with the payment. Please contact support."; |
Why don't we make them even more like an if-else block? (Like prettier do it for jsx) Look how easy it is to read: const animalName =
pet.canSqueak() ? (
'mouse'
) : (
pet.canBark() ? (
pet.isScary() ? (
'wolf'
) : (
'dog'
)
) : (
pet.canMeow() ? (
'cat'
) : (
pet.canSqueak() ? (
'mouse'
) : (
'probably a bunny'
)
)
)
); UPD, shortened form (adding braces only when there is not a primitive value) const animalName =
pet.canSqueak() ? 'mouse' : (
pet.canBark() ? (
pet.isScary() ? 'wolf' : 'dog'
) : (
pet.canMeow() ? 'cat' : (
pet.canSqueak() ? 'mouse' : 'probably a bunny'
)
)
); |
@olosegres |
@DScheglov I've added example with shortened form, what do you think about it? p.s. we spend a lot of time understanding already written logic, which means by improving this we win, even if there is something wrong with git-diffs |
@olosegres See my proposal above which is already modelling if-else but using only indentation. In your updated case, I am against the "then" case ever being on the same line as that hurts scannability. The other nit-pick with your proposal is that it doesn't model "else if". Instead it's an "if" nested within an else. Here's your example updated to my proposal which reads like const animalName
= pet.canSqueak() ?
'mouse'
: pet.canBark() ?
( pet.isScary() ? 'wolf' : 'dog' ) // Simple cases on 1-line.
: pet.canMeow() ?
'cat'
: pet.canSqueak() ?
'mouse'
: // empty line for clear "or else" case.
'probably a bunny'
; // semi-colon for a clear "end" Every line that starts with a punctuation is an "if" every ":" is an "else". |
I am in agreement with @bradzacher #9561 (comment) But I would also consider a few additional adjustments I ended up employing to help with readability and navigation. Example codeinfo.type === "playlist"
? info.creationDate
? <div className="lt-info-stats">
<span className="text pure">Created on {info.creationDate}</span>
</div>
: null
: info.type === "artist"
? <div className="lt-info-stats">
<span className="text pure">{info.genre}</span>
</div>
: <div className="lt-info-stats">
<span className="text pure">{info.releaseDate}</span>
<span className="cdot" style={{ fontWeight: "bold", margin: "1px" }}>·</span>
<span className="text pure">{info.genre}</span>
</div> There are two adjustments made in the above example:
These adjustments help be visually separate the elements into blocks much like the brackets in an if/else: EditOver some time, I have switched between the adjustments I added and the formatting @bradzacher shows in his last examples and I have changed my opinion. While my minor adjustments are more easily digestible visually to me, I realized that
So, I am currently inclined to retracted my suggestions... Fyi, here is a little gif showing the visual difference: |
Here is my approach I like to use to format ternary operators. I find this works well because of the way the layout follows these principals:
The cons as I see it are:
const message = i % 3 === 0 && i % 5 === 0 ? "fizzbuzz"
: i % 3 === 0 ? "fizz"
: i % 5 === 0 ? "buzz"
: String(i);
const paymentMessage = state == "success" ? "Payment completed successfully"
: state == "processing" ? "Payment processing"
: state == "invalid_cvc" ? "There was an issue with your CVC number"
: state == "invalid_expiry" ? "Expiry must be sometime in the past."
: "There was an issue with the payment. Please contact support.";
const simple = children && !isEmptyChildren(children) ? children
: null
const simpleChained = children && !isEmptyChildren(children) ? children
: component && props.match ? React.createElement(component, props)
: render && props.match ? render(props)
: null
const complexNested = children && !isEmptyChildren(children) ? children
: props.match ? component
? React.createElement(component, props)
: render ? render(props)
: null
: null;
type TypeName<T> = T extends string ? "string"
: T extends number ? "number"
: T extends boolean ? "boolean"
: T extends undefined ? "undefined"
: T extends Function ? "function"
: "object";
type Unpacked<T> = T extends (infer U)[] ? U
: T extends (...args: any[]) => infer U ? SomeVeryLongNameOfSomeKind<U>
: T extends Promise<infer U> ? U
: T; |
@tsnelson1 hi, I'm not sure that it works as you described: const message = i % 3 === 0 && i % 5 === 0 ? "fizzbuzz"
: i % 3 === 0 ? "fizz"
: i % 5 === 0 ? "buzz"
: String(i); The rule "Each line immediately identifies itself as a ternary operator" doesn't work for the first line. To match the rule your example must look like this: const message = false ? assertUnreachable()
: i % 3 === 0 && i % 5 === 0 ? "fizzbuzz"
: i % 3 === 0 ? "fizz"
: i % 5 === 0 ? "buzz"
: String(i); Why do I think so. Becuse when we are talking about Additionally it is not a good practice to have two ways to do the same. We already discussed in this or similar threads more consistent approaches, as instance the following; const message =
i % 3 === 0 && i % 5 === 0 ? "fizzbuzz" :
i % 3 === 0 ? "fizz" :
i % 5 === 0 ? "buzz" :
String(i); or like this const message =
i % 3 === 0 && i % 5 === 0 ? "fizzbuzz" :
i % 3 === 0 ? "fizz" :
i % 5 === 0 ? "buzz" :
String(i); Sometimes I use the following formating: const paymentMessage =
// prettier-ignore
state === "success"
? "Payment completed successfully" :
state === "processing"
? "Payment processing" :
state === "invalid_cvc"
? "There was an issue with your CVC number" :
state === "invalid_expiry"
? "Expiry must be sometime in the past." :
// otherwise
"There was an issue with the payment. Please contact support."
; |
Background
In #737, an alternative format was proposed for nested ternaries, on the grounds that indenting every level of a nested ternary looked bad, especially for long nested ternaries.
However, there were concerns from core maintainer @vjeux that the proposed format would not be compatible with the way we currently do ternaries, and would present problems with long lines.
The community suggested several alternatives, and the one chosen was the simplest – simply flatten them: #5039. Unfortunately, this did not work well in many cases, and will likely be reverted, which brings us back to the drawing board.
It would be nice to come up with a solution for the cases where indented nested ternaries are objectionable prior to merging the revert, to minimize churn in the community. However, I think we should timebox this search to a week or two.
In #9552, I made one attempt at improving nested/chained ternary formatting, but it wasn't great, so I decided to scour proposed solutions and found two candidates that I think could be made to work well.
Note that we may wish to apply these proposals in only certain circumstance, at least at first; more on that below.
Goals
I'd like to find a solution that:
I don't think something that fully satisfies all of the above constraints, so tradeoffs may be required.
Option A: case-style
This is the format that was originally proposed in #737.
case-style pros
case-style cons
?
, obscuring the program structure.Incremental adoption
Since this proposal is a little bolder and risks being disruptive, we should ease into it.
I think we should merge #9559, but carve out one or two special cases where case-style clearly makes sense, namely:
If these were well-received, we could consider adopting the style more broadly at a later time.
If not, the blast radius will be much more limited (both of these cases are likely fairly rare) and we can revert without causing huge amounts of churn.
Option B: if-else-style
This is a compromise between fully flattened (#5039) and fully indented (#9559) that feels more familiar than the solution above:
I first saw this approach outlined here by @JAForbes.
if-else pros
if-else cons
paymentMessage
above.complex nested
, above) are a little difficult to read.Personally, I think this option is strictly superior to both the fully-flat and fully-nested behaviors. However, I'm concerned that it still leaves TS in the lurch.
My take
EDIT: upon further exploration, I think Option A gets too special-case-y and will result in strange git diffs, and big/strange differences in code style that will frequently surprise and confuse users. Unless we were to shift to using it for ternaries everywhere, I think we should not adopt Option A.
Personally, I actually quite like Option B2, proposed by @roryokane below, with a prior implementation by @zeorin in #4767, and I think both Option B and Option B2 warrant further exploration.
The text was updated successfully, but these errors were encountered: