Questionable new ternary cases

Musings on revisiting ternaries formatting 2022-01-15.jsx

const Musings = () => {
  return (
    <div>
      In general, I'm definitely thinking of removing parens around broken terminal alternates,
      for example instead of this:

      {component ? 
        <div>{React.createElement(component, props)}</div>
      : render ? 
        <div>{render(props)}</div>
      : (
        <div>Nothing is here</div>
      )}
      
      do this:
      
      {component ? 
        <div>{React.createElement(component, props)}</div>
      : render ? 
        <div>{render(props)}</div>
      :
        <div>Nothing is here</div>
      }
      
      That's also carry over to non-jsx situations;
      right now, we have this:
      {makeStyles({
        background:
            theme.palette.type === 'dark' ? '#535353'
            : (
                `linear-gradient(to right, #8975fb 0%, #746be7 35%), linear-gradient(to bottom, #8975fb 0%, #6f4ceb 50%), #6f4ceb`
            ),
      })}
      but it should simply be:
      {makeStyles({
        background:
            theme.palette.type === 'dark' ? '#535353'
              : `linear-gradient(to right, #8975fb 0%, #746be7 35%), linear-gradient(to bottom, #8975fb 0%, #6f4ceb 50%), #6f4ceb`,
      })}
      or perhaps:
      {makeStyles({
        background:
            theme.palette.type === 'dark' ? 
                '#535353'
              : `linear-gradient(to right, #8975fb 0%, #746be7 35%), linear-gradient(to bottom, #8975fb 0%, #6f4ceb 50%), #6f4ceb`,
      })}
      
      That latter question is related to this choice, which I currently struggle with. Musings below.
      
    </div>
  )
};


// Sometimes this looks nicer:
foo ? foo
: bar
// and sometimes this looks nicer:
foo ?
  thing.callStuff(xy, z, foo)
: thing.callStuff(xy, z, bar)
// often based on how similar the consequent and alternate look… 
// (vs how short and/or similar the test and consequent are…)

// I am keen for feedback on this area, but I think choosing something good will be hard 
// and in the interest of expediency I'm likely to bias towards the latter in absence of a clearly better solution.

// The former is also way nicer in certain chained ternaries (the latter here looks better):
encoding === "utf8" ?
  new UTF8Encoder()
: encoding === "utf16le" ?
  new UTF16Encoder(false)
: encoding === "utf16be" ?
  new UTF16Encoder(true)
// vs:
encoding === "utf8" ? new UTF8Encoder()
: encoding === "utf16le" ? new UTF16Encoder(false)
: encoding === "utf16be" ? new UTF16Encoder(true)
: throw new Error("Unsupported encoding");

// No idea how to distinguish this in a useful way…
// Maybe chains should allow consequents on the same line as the test, but single ternaries should not??
// eg; single ternary:
// start on one line
const foo = foo != null ? foo : createNewFoo()
// next, break alternate??
const foo = foo != null ? foo 
  : createNewFoo(withSomeLargArguments, thatBreakTheLine)
// or alternatively, jump straight to full break:
const foo = foo != null ? 
    foo 
  : createNewFoo(withSomeLargArguments, thatBreakTheLine)

// next, break consequent:
// if two-space indents are used:
const foo = foo != null ? 
    useTheCurrentFoo(foo, alsoWithLongArguments, thatBreakTheLine) 
  : createNewFoo(withSomeLargArguments, thatBreakTheLine)
// else, eg with 4:
const foo = foo != null ? 
        useTheCurrentFoo(foo, alsoWithLongArguments, thatBreakTheLine) 
    :   createNewFoo(
            withSomeLargArguments, 
            thatBreakTheLine
        ) // can we do this? Not sure how hard it is or how far from current behavior…

// and if the alternate breaks too, and is huggable:
const foo = foo != null ? 
    useTheCurrentFoo(foo, alsoWithLongArguments, thatBreakTheLine) 
  : createNewFoo(
      withSomeLargArguments, 
      thatBreakTheLine,
      evenWithinTheAlternateAlone,
      becauseTheyAreSoLong,
    )
// and if the alternate is not huggable:
const foo = foo != null ? 
    useTheCurrentFoo(foo, alsoWithLongArguments, thatBreakTheLine) 
  : (
    1 + 
    createNewFoo(
      withSomeLargArguments, 
      thatBreakTheLine,
      evenWithinTheAlternateAlone,
      becauseTheyAreSoLong,
    )
  );
// and if the test is long:
const foo = (
    1 === blah ||
    checkTheFoo(
      withSomeLargArguments, 
      thatBreakTheLine,
      evenWithinTheAlternateAlone,
      becauseTheyAreSoLong,
    )
  ) ? 
    theShortConsequent
  : theShortAlternate;
// and if test is long and indent != 2:
const foo = 
    (
        1 === blah ||
        checkTheFoo(
          withSomeLargArguments, 
          thatBreakTheLine,
          evenWithinTheAlternateAlone,
          becauseTheyAreSoLong,
        )
    ) ? theShortConsequent
    : theShortAlternate;



// This isn't working right now:
// next, put everything after the =
const lessShort =
  isLoudReallyLoud() ? makeNoiseReallyLoudly.omgSoLoud() : silent();


toBreakJSXAlternate.orNotToBreak = (
  <div>
    Typical jsx case:
    {showTheThing || pleaseShowTheThing ?
      <Foo attribute="such and such stuff here" />
    :
      <Bar short />
    }
    Or, instead, should it be this?
    {showTheThing || pleaseShowTheThing ?
      <Foo attribute="such and such stuff here" />
    : <Bar short />}
    
    I think the former looks weird/ugly, while the latter looks cramped.
    The latter has a readability issue of making it hard to find the closing curly brace,
    while the former has no readability issue – just an excess-space issue.
    
    What about when Bar is long?
    
    {showTheThing || pleaseShowTheThing ?
      <Foo attribute="such and such stuff here" />
    : <Bar 
        oneAttribute
        afterAnother
        andAnother
      />
    }
    
    Okay, so that actually looks more consistent with what we do elsewhere… 
    it's also compact while still being readable.
    
    So that implies this compromise solution, while ugly, should perhaps win:
    {showTheThing || pleaseShowTheThing ?
      <Foo attribute="such and such stuff here" />
    : <Bar short />
    }
    It's hard to stomach…
    
    I suspect the prettier community would ultimately like the most compact solution here, 
    specifically when the alternate does not break.
    Personally I prefer the more spaced-out option but I don't think it'll really impede my ability to read code.
    
    
    Okay, another question…
    In JSX, do we now need… *any* special casing?
    Is it okay to do this:
    {showFoo ? <Foo /> : <Bar />}
    Like yeah I think that's expected and totally fine.
    I think the only thing we need to do is ensure the trailing } gets a newline before it 
    when the alternate breaks.
    
    And then what about the null consequent case:
    
    {!thing ? null : (
      <TheThing thing={thing} />
    )}
    I definitely think that should be the behavior… but can we generalize outside of JSX?
    Like should these things also have the same behavior?
    {xs.map(() => {
      foo = bar ? bar : (
        someLongFunctionCall(withManyArguments, thatGoOnForever, breakingTheLine)
      );
      foo = bar ? null : (
        someLongFunctionCall(withManyArguments, thatGoOnForever, breakingTheLine)
      );
      foo = bar ? undefined : (
        someLongFunctionCall(withManyArguments, thatGoOnForever, breakingTheLine)
      );
      // I'm not sure tbh… This doesn't really look like it's in keeping with the rest of the feature. It looks weird.
      // Those aren't beautiful in the current spec, though, eg;
      foo = bar ? 
          undefined
        : someLongFunctionCall(withManyArguments, thatGoOnForever, breakingTheLine);
      // I feel like instead I'd definitely expect something like this:
      foo = bar ? undefined
        : someLongFunctionCall(withManyArguments, thatGoOnForever, breakingTheLine);
      // and since the ? is still close to the end of the line there, it's not crazy…
      // buut… it's still inconsistent, and somewhat needlessley so…
      // I'll set this aside for now.
      
      // Okay, coming back to this, I think if we have this format for nested, 
      // we can do it for "simple" ones too, like null, undefined, true/false, etc… 
      // can workshop the algo later. (strings? short strings? idents?)
      
    })}
    
  </div>
);

questionable-new-ternaries.js

// This comment looks funny… not sure what our desired behavior should be? Parens?
test ? foo
: /* comment
         comment
     comment
           comment
      A newline will be added after this comment, unfortunately – but it can be removed manually, see next statement.
    */
test ? foo
: bar


// Not sure about this:
func(
  () => {
    thing();
  },
  (
    something(
      longArgumentName,
      anotherLongArgumentName,
      anotherLongArgumentName,
      anotherLongArgumentName
    )
  ) ? someOtherThing()
  : somethingElse(true, 0)
);
// for a function call, should it be this instead?
func(
  () => {
    thing();
  },
  something(
    longArgumentName,
    anotherLongArgumentName,
    anotherLongArgumentName,
    anotherLongArgumentName
  ) ? someOtherThing()
  : somethingElse(true, 0)
);
// … I think I like the former better, actually, because it's more clear that the funciton call is in a conditional.


// Do we like this?
(valid ? helper.responseBody(this.currentUser)
: helper.responseBody(this.defaultUser)
).prop;
// Or would this be better?
(
  valid ? helper.responseBody(this.currentUser)
  : helper.responseBody(this.defaultUser)
).prop;
// or even this?
(valid ? 
  helper.responseBody(this.currentUser)
: helper.responseBody(this.defaultUser)
).prop;
// or perhaps even this?
(valid ? helper.responseBody(this.currentUser) : (
  helper.responseBody(this.defaultUser)
)).prop;

// Related:
const decorated = (arg, ignoreRequestError) => {
	return (
		(
			typeof arg === "string" ||
			(arg && arg.valueOf && typeof arg.valueOf() === "string")
		) ? $delegate(arg, ignoreRequestError)
		: handleAsyncOperations(arg, ignoreRequestError)).foo();
};
// shouldn't that be:
const decorated = (arg, ignoreRequestError) => {
	return (
		(
			typeof arg === "string" ||
			(arg && arg.valueOf && typeof arg.valueOf() === "string")
		) ? $delegate(arg, ignoreRequestError)
		: handleAsyncOperations(arg, ignoreRequestError)
	).foo();
};
// and another one:
bifornCringerMoshedPerplexSawder =
    askTrovenaBeenaDependsRowans +
    ((
        glimseGlyphsHazardNoopsTieTie === 0 &&
        kochabCooieGameOnOboleUnweave === Math.PI
    ) ? averredBathersBoxroomBuggyNurl
    : anodyneCondosMalateOverateRetinol
    ).Fooooooooooo.Fooooooooooo;


// Other interesting cases (not pulling directly from tests):
foo ? bar
: (
  function foo() {
    // ...
  }
);
foo ? 
  {
    something: 'long',
    goes: 'here',
  }
: (
  {
    something: 'else',
    goes: 'here',
  }
);
foo ? bar
: (
  [
    1,
    2,
  ]
);
// I think instead these should all be like this:
foo ? 
  [
    4,
    5,
    6,
  ]
: [
    1,
    2,
  ];
// But what about this?
foo ? bar
: (
  some,
  really,
  reallyLongArgs,
  thatBreakTheArgs,
) => {
  the();
  fn()
  body();
};
// Call Expressions?
foo ? bar
: someFn(
  some,
  really,
  reallyLongArgs,
  thatBreakTheArgs,
);
// … I think no…

// Curried Arrows?
foo ? bar
: (a) => 
  (b) => 
    (c) => {
      a + b + c
    }
// ugh, nope.
foo ? bar
: (
  (a) => 
    (b) => 
      (c) => {
        a + b + c
      }
)
// hmm, so if we indent the else, we need to use two-space spacing, which is bad…
// when the consequent and alterate are both multiline, I think the parens aren't so bad:
foo ? 
  [
    4,
    5,
    6,
  ]
: (
  [
    1,
    2,
  ]
);
// but when the consequent fits on the same line as the test, it's super weird:
foo ? bar
: (
  [
    1,
    2,
  ]
);
// so we could instead keep the colon on the same line:
foo ? bar : (
  [
    1,
    2,
  ]
);
foo ? bar : (
  {
    something: 'else',
    goes: 'here',
  }
);
// and maybe hug objects and arrays:
foo ? bar : [
  1,
  2,
];
foo ? bar : {
  something: 'else',
  goes: 'here',
};
// Hmm… no, better with parens there I think…



// Looking at react-admin, I don't love this:
const useStyles = makeStyles(theme => ({
    root: {
        background:
            theme.palette.type === 'dark' ? '#535353'
            : (
                `linear-gradient(to right, #8975fb 0%, #746be7 35%), linear-gradient(to bottom, #8975fb 0%, #6f4ceb 50%), #6f4ceb`
            ),
// It should either be this:
const useStyles = makeStyles(theme => ({
    root: {
        background: theme.palette.type === 'dark' ? '#535353' : (
            `linear-gradient(to right, #8975fb 0%, #746be7 35%), linear-gradient(to bottom, #8975fb 0%, #6f4ceb 50%), #6f4ceb`
        ),
// Or this:
const useStyles = makeStyles(theme => ({
    root: {
        background: theme.palette.type === 'dark' ? 
                '#535353' 
            : (
                `linear-gradient(to right, #8975fb 0%, #746be7 35%), linear-gradient(to bottom, #8975fb 0%, #6f4ceb 50%), #6f4ceb`
            ),
// Which, come to think of it, should perhaps be this:
const useStyles = makeStyles(theme => ({
    root: {
        background: theme.palette.type === 'dark' ? 
                '#535353' 
            : `linear-gradient(to right, #8975fb 0%, #746be7 35%), linear-gradient(to bottom, #8975fb 0%, #6f4ceb 50%), #6f4ceb`,
// Or perhaps:
const useStyles = makeStyles(theme => ({
    root: {
        background: theme.palette.type === 'dark' ? '#535353' 
            : `linear-gradient(to right, #8975fb 0%, #746be7 35%), linear-gradient(to bottom, #8975fb 0%, #6f4ceb 50%), #6f4ceb`,

			     
// Another random thought, sometimes this looks nicer:
foo ? foo
: bar
// and sometimes this looks nicer:
foo ?
  thing.callStuff(xy, z, foo)
: thing.callStuff(xy, z, bar)
// often based on how similar the consequent and alternate look… 
// (vs how short and/or similar the test and consequent are…)

// The former is also way nicer in certain chained ternaries (the latter here looks better):
encoding === "utf8" ?
  new UTF8Encoder()
: encoding === "utf16le" ?
  new UTF16Encoder(false)
: encoding === "utf16be" ?
  new UTF16Encoder(true)
// vs:
encoding === "utf8" ? new UTF8Encoder()
: encoding === "utf16le" ? new UTF16Encoder(false)
: encoding === "utf16be" ? new UTF16Encoder(true)
: throw new Error("Unsupported encoding");

// No idea how to distinguish this in a useful way…
// Maybe chains should allow consequents on the same line as the test, but single ternaries should not??
// eg; single ternary:
// start on one line
const foo = foo != null ? foo : createNewFoo()
// next, break alternate
const foo = foo != null ? foo : (
  createNewFoo(withSomeLargArguments, thatBreakTheLine)
)
// next, break consequent:
// if two-space indents are used:
const foo = foo != null ? 
    useTheCurrentFoo(foo, alsoWithLongArguments, thatBreakTheLine) 
  : createNewFoo(withSomeLargArguments, thatBreakTheLine)
// else, eg with 4:
const foo = foo != null ? 
        useTheCurrentFoo(foo, alsoWithLongArguments, thatBreakTheLine) 
    : (
        createNewFoo(withSomeLargArguments, thatBreakTheLine)
    )
// and if the alternate breaks too, and is huggable:
const foo = foo != null ? 
    useTheCurrentFoo(foo, alsoWithLongArguments, thatBreakTheLine) 
  : createNewFoo(
      withSomeLargArguments, 
      thatBreakTheLine,
      evenWithinTheAlternateAlone,
      becauseTheyAreSoLong,
    )
// and if the alternate is not huggable (or if the indent is not two spaces):
const foo = foo != null ? 
    useTheCurrentFoo(foo, alsoWithLongArguments, thatBreakTheLine) 
  : (
    1 + 
    createNewFoo(
      withSomeLargArguments, 
      thatBreakTheLine,
      evenWithinTheAlternateAlone,
      becauseTheyAreSoLong,
    )
  );
// and if the test is long:
const foo = (
    1 === blah ||
    checkTheFoo(
      withSomeLargArguments, 
      thatBreakTheLine,
      evenWithinTheAlternateAlone,
      becauseTheyAreSoLong,
    )
  ) ? 
    theShortConsequent
  : theShortAlternate;
// and if test is long and indent != 2:
const foo = 
    (
        1 === blah ||
        checkTheFoo(
          withSomeLargArguments, 
          thatBreakTheLine,
          evenWithinTheAlternateAlone,
          becauseTheyAreSoLong,
        )
    ) ? theShortConsequent
    : theShortAlternate;

things I found after updates on 2022-01-15 I find somewhat notable.tsx

const foo = {
    onSuccess: onSuccess !== undefined ? onSuccess
        :   () => {
                notify(
                    'ra.notification.deleted',
                    'info',
                    { smart_count: 1 },
                    true
                );
                redirect(redirectTo, basePath || `/${resource}`);
                refresh();
            },
    onFailure: error =>
        notify(
            typeof error === 'string' ? error
            :   error.message || 'ra.notification.http_error',
            'warning'
        ),
}

// I think this kind of thing is where case-style really shines.
const redirectUrl = pathName ? pathName
    :   nextPathName + nextSearch ||
        defaultAuthParams.afterLoginUrl;

// Here, case-style strikes me as pretty confusing
const filters = Object.keys(filterValues).reduce(
    (acc, key) =>
        keysToRemove.includes(key) ? acc
        :   { ...acc, [key]: filterValues[key] },
    {}
);

// I dont love this, if-else is better here IMO:
         join(
           [
             ",",
-            insideAtRuleNode(path, ["extend", "custom-selector", "nest"]) ?
-              line
+            insideAtRuleNode(path, ["extend", "custom-selector", "nest"]) ? line
             : hardline,
           ],

<div>
{editing && !locked && (
  sideBySideInput ? 
    <div className="flex">
      <Input {...props} />
      {props.children}
    </div>
  : 
    <Input {...props} />
)}
</div>