Ternary and React Components about eslint-plugin-proper-ternary

[finnmerlett]

I can see this repo probably isn't being actively maintained at the moment, but I use it and get a lot of use out of it, so I thought I'd add my two cents here. I have a React component like this

const NetworkIndicator: React.VFC = () => {
// online: boolean | "backOnline"
  const online = useSelector(s => s.context.appOnline);
  const showIndicator = !online || online === 'backOnline';

  return showIndicator ? (
    <OfflineTopBar $online={!!online}>
      {!online && <Icon inline icon={IconLib.ERROR} />}
      {online ? 'Back online!' : 'No internet connection'}
    </OfflineTopBar>
  ) : null;
};

I get an error from this plugin saying that I have a nested ternary that is disallowed. I don't want to allow that setting as I want to avoid nested ternaries.. except in this kind of situation where there is a React component in between. I think there should be an option to make it so returning a JSX element from a ternary resets the main nest-count. Perhaps also could make it so that JSX element nesting had a separate max count you could set.

For now I have to go with either of these options to keep my coding style consistent, both of which I think look less neat (though I prefer the latter)

const NetworkIndicator: React.VFC = () => {
  const online = useSelector(s => s.context.appOnline);
  const showIndicator = !online || online === 'backOnline';

  return (
    (showIndicator && (
      <OfflineTopBar $online={!!online}>
        {!online && <Icon inline icon={IconLib.ERROR} />}
        {online ? 'Back online!' : 'No internet connection'}
      </OfflineTopBar>
    )) ||
    null
  );
};

const NetworkIndicator: React.VFC = () => {
  const online = useSelector(s => s.context.appOnline);
  const showIndicator = !online || online === 'backOnline';
  const message = online ? 'Back online!' : 'No internet connection';

  return showIndicator ? (
    <OfflineTopBar $online={!!online}>
      {!online && <Icon inline icon={IconLib.ERROR} />}
      {message}
    </OfflineTopBar>
  ) : null;
};

[getify]

I believe the AST holds JSXElement for those items, not the transpiled-equivalent function call expression. I think that's why "call": false isn't applying, to suppress the error.

I have tried turning off all the options, but an error still emerges. It must be because it's a "complex" case.

Yeah, since it's a JSXElement, it doesn't fit with any of the other patterns checked for, so as the documentation says, it defaults to "complex". The documentation also points out my reasoning for why there's no "complex": false option, because such an option would moot the whole purpose of the rule, and the rule should just not be used.

I think the first approach is a bit more legible than the second one. What are your thoughts?

TBH, I think this is exactly the kind of case that should be considered "complex". Which is to say, to my eyes, the first version has the potential of being much less readable than the second version. In fact, I would actually much rather that code be formatted like this:

(customHandler && typeof customHandler === 'function')
        ? (
           <Clickable onClick={clickHandler} className="ui-menu__option">{title}</Clickable>
        )
        : (
           <a href={href} className="ui-menu__option">{title}</a>
        )

To be clear, this plugin doesn't support purely whitespace-style questions like this -- maybe it should, but that's also a pandora's box of complexity that I wanted to avoid -- so that's just my personal preference, not something that I currently use a tool to enforce.

---

To your bigger question...

I agree that JSX showing up in ternaries is common, so this is at least something relevant to discuss here -- it's not just a niche corner case.

But I'm not convinced that a JSXElement deserves to be classified separately from other complex expressions. JSX elements have arbitrary complexity (attributes, nested elements, etc), so opening up a JSXElement to be allowed without parens means allowing all that complexity, OR it means needing a set of rules to control to what extent we want to allow JSX without parens vs where it requires parens. That would be a lot of subjective decision making, I think, and I'm not sure it would be worth the effort.

I'm just asking if you have any other reasons for why a JSXElement should be a specifically configurable option, other than that you like the look of it a little better? Would you want to place any limits on the JSX complexity-tree? Where/how would we draw those lines?

[escudero89]

Thank you for the swift and thoughtful response @getify.

I'm just asking if you have any other reasons for why a JSXElement should be a specifically configurable option, other than that you like the look of it a little better?

I tried adding this plugin to a couple of projects we have, but since we are used to using the first approach, there're lots of linter errors being raised because of this. In an ideal world, I'd like to add this parens rule for all the other cases, and to ignore the JSXElements.

A simple option just a { "jsx": false } would be amazing, but I understand your point about being an actual complex element.

Would you want to place any limits on the JSX complexity-tree? Where/how would we draw those lines?

I would ignore completely JSXElements, or not. No gray areas.

Cheers.

[getify]

I can see this repo probably isn't being actively maintained at the moment

This repo is still under maintenance. If it appears "not active" it's only because there's not much to do. It's not super high on my priority list. But it's definitely not abandoned or anything.

I think there should be an option to make it so returning a JSX element from a ternary resets the main nest-count.

That's a very different kind of question than was asked in the OP. I think that would need a separate issue so we can discuss it at length.

---

But I can say that I find it very hard to imagine justifying that. I would need a lot more clarity into the reasoning behind why a JSX element gets to "reset" the nesting where other expression types don't. From what I can tell of that code, it's no different than a big object literal with a bunch of nesting of ternaries at different levels, and that's nearly exactly the case envisioned for limiting nesting.

var C = {
   x: (condOne ? 
      [ 1, 2, 3 ] :
      {
         y: (condTwo ?
            [ 4, 5, 6 ] :
            { z: 42 }
         )
      }
   )
};

// vs

var C = <MyComp>
   <x>
      {(condOne ?
         <>1, 2, 3</> :
         <y>
            {(condTwo ?
               <>4, 5, 6</> :
               <z>42</z>
            )}
         </y>
      )}
   </x>
</MyComp>;

Point being, I don't understand the thinking that treats JSX as "special and simple, not complex" when IMO it's absolutely on the far end of the scale in terms of visual complexity. It's far more visually dense and easy to lose the ternaries when visually scanning.

If you open this up in another thread, we can discuss further. But I just have a hard time accepting this perspective. It doesn't make any sense to me to distinguish these two cases here.