archived text from ternaries PR description

outdated_todods.md

Parts I'm less sure of / things that could use help

NB: This section may be outdated, I can't remember what the current status is since I wrote this originally a long time ago.

Personally, I feel pretty good about shipping this largely as-is (modulo any bugs or code review issues) and I don't consider the below issues to be blockers – if anything, they might be best addressed in follow-on PR's. But, I raise them here since I anticipate they may garner discussion and I'm not so sure that my current answer is one that I'll like later on.

1. Whether and when to shorten to "case-style" for non-nested ternaries

As written, the PR uses "case-style" when the consequent is visually short (under ten characters long), eg:

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

The thinking here is that the following would be needlessly long and harder to read:

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

The heuristic of "ten characters" (without a line break in the original) was eventually settled upon because it fits the word "undefined" and will allow a variety of patterns like foo ? foo : …, foo ? [foo] : …, foo ? foo(1) : …, foo ? null : …, foo ? false : …, etc – and the fact that the word is visually short should allow the reader to ~quickly notice the ? near the end of the line indicating that the expression is a ternary.

However, this is not a heuristic we use elsewhere, and there are a few cases I've seen in demo situations where the longer-form might be clearer, like this:

const filters = Object.keys(filterValues).reduce(
  (acc, key) =>
    keysToRemove.includes(key) ? acc
    : { ...acc, [key]: filterValues[key] },
  {}
);

My hunch is that this is an edge-case we can iron out in future PR's if needed, as we get feedback from users, but I'm open to pulling this heuristic out, refening it, or expanding it.

2. Whether to include a special case for null-consequents in JSX

As written, the PR has a special case for when the consequent in a jsx ternary is null, eg:

<div>
  {!foo ? null : (
    <Foo foo={foo} />
  )}
</div>

This is for symmetry with the equivalent boolean:

<div>
  {foo && (
    <Foo foo={foo} />
  )}
</div>

I'm leaning towards tearing out this special case, which would result in this:

<div>
  {!foo ? null
  : <Foo foo={foo} />}
</div>

but I'm open to input. It's also easy to tear it out later.

3.a. Newlines after assignment operators (bug?)

Right now, there's a ¿bug? which prevents this form from being used:

const foo = 
  conditional ? result : alt;

instead, it'll always go straight to this form:

const foo = conditional ?
    result
  : alt;

even if the first would fit.

Frankly I just haven't figured out how to fix this and would appreciate a tip on how to do so. But it also doesn't come up often enough in practice for me to consider this a blocking bug, personally (I'm happy to ship like this).

3.b. Newlines after assignment operators (parens)

Pretty rare edge case, but when a ternary's test is complicated, it shows up like this:

const foo = (
    isItThis
    || orThis
  ) ?
    thenDoThis
  : elseThis;

I think I'd rather the opening paren appear on the next line:

const foo =
  (
    isItThis
    || orThis
  ) ?
    thenDoThis
  : elseThis;

but this seems like a viable follow-on PR.