Skip to content
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

JSX conditionals requires a lot of workarounds a lot of the time #690

Closed
syranide opened this issue Dec 21, 2013 · 21 comments
Closed

JSX conditionals requires a lot of workarounds a lot of the time #690

syranide opened this issue Dec 21, 2013 · 21 comments

Comments

@syranide
Copy link
Contributor

The one good example

Conditionals are currently solved with the ternary ?: or && + || and actually works surprisingly well. However, what can be returned by them is another story.

The academic textbook example works great:

<div>
  {condition ?
    <span></span>
  : null}
</div>

or

<div>
  {condition &&
    <span></span>
  }
</div>

Looks pretty good, works great.

Any other is a disaster

But in order to return 2+ tags you need to return an array, which also requires that you separate them by commas (good bye beautiful JSX):

<div>
  {condition ?
    [
      <span></span>,
      <span></span>
    ]
  : null}
</div>

If you want a text-node instead then you have to actually rewrite your JSX-style code to pure JavaScript instead, which is perhaps even worse:

<div>
  {condition ?
    'My name is ' + name + '!'
  : null}
</div>

Let's also do the complete version, a text-node surrounded by tags, whitespace rules fly out the window and it no longer looks anything like JSX:

<div>
  {condition ?
    [
      <span></span>,
      'My name is ' + name + '!',
      <span></span>
    ]
  : null}
</div>

And while we're at it, if there are nested ifs then it looks like this, so let's go all out ugly, despite how simple it really should be:

<div>
  {condition ?
    [
      (condition ?
        [
          <span></span>,
          'My name is ' + name + '!',
          <span></span>
        ]
      : null),
      'Hello ' + name + '!'
    ]
  : null}
</div>

That is barely even understandable code. When without conditionals it would look like this:

<div>
  <span></span>
  My name is {name}!
  <span></span>
  Hello {name}!
</div>

So in my opinion, something is clearly wrong here, adding a conditionals should never require you to be working with arrays and rewriting inline text to use JavaScript string literals instead, or in general require you to massacre your code.

The problem(s)

Multiple tags

The easy part problem is simply that we currently cannot return more than one tag before having to resort to arrays. Intuitively this should be relatively easy to fix in JSX by simply automatically wrapping such as arrays. Simple, intuitive and good output?

Conditionals

While it may not be a big deal, it's weird that the syntax for an if-statement <tag>{cond ? true : false }</tag> is different from a nested if <tag>{cond ? (cond ? true : false) : false}</tag> (braces replaced with parenthesis, because we're already in an expression) and it's back to braces if it's inside a tag <tag>{cond ? <tag>{cond ? true : false}</tag> : false}</tag>.

Text and expressions

The last big issue is that we can transition from JS to JSX-HTML by adding a root tag <tag></tag>, but we cannot transition directly to JSX-text and JSX-expressions My name is {name}! without it being wrapped in a tag <tag>My name is {name}!</tag> or without being rewritten to plain JavaScript 'My name is ' + name + '!'. Which is necessary because conditionals are currently implemented as expressions.

Solutions

New conditional syntax

So it seems to me like the root of the problem is that conditionals are implemented as JSX expressions, which also causes the problem with having to rewrite JSX text/expressions when adding conditionals unless they are wrapped by tags. If they did not piggyback on JSX expressions then everything but non-parented JSX text/expressions would be solved, and really, that's not an issue at all.

Nesting of JSX expressions

Another solution is to allow JSX-expressions to be nested <tag>{cond ? {cond ? true : false} : false}</tag>, JSX text/expressions inside conditionals will still be a problem, but that could reasonably be solved by backtick strings <tag>{cond ? My name is {name}! : null}</tag>.

Or...

Or do you think there are better ways one should use to structure the code? Am I missing something?

I realize that this perhaps is approaching it from the perspective of old-style templates, but from where I'm standing now, these things seem to make it quite cumbersome in more complex situations... there are a lot of situations in which one has to change surrounding code simply because one added a tag, or because one removed a tag... or moved a conditional.

PS. We had some discussion in the chat, and I agree that most of this is probably just "old-school template habits", however, returning multiple tags is biting me in the ass a lot. While everything content inside conditionals could be wrapped in divs/spans to make it play nice, it seems weird to have to add markup just to be able to write reasonable JSX.

@syranide
Copy link
Contributor Author

zpao made me a believer, <frag> is the way to go. It solves everything beautifully, it just needs to be implemented somehow.

@esamattis
Copy link

I can you explain (or link to one) what <frag> is and how it solves this?

I'm finding exactly the same issues you have described here.

@syranide
Copy link
Contributor Author

syranide commented Apr 6, 2014

@epeli Sorry for the later answer. The idea is simply that <frag> represents a document fragment, i.e, a list of nodes without a parent. It's currently out of reach it seems (implementation-wise) but it seems like they do want that feature. <frag> = <span> is good enough for now unless you have your CSS set up very strictly, this is my own workaround.

What <frag> solves is that everything kind of makes sense (today) as long as what you're doing has a single parent node, when you have multiple parents (need to use [<div />, <div />]) or text-nodes (needx to use "text") things get bad. <frag> provides a single parent for all use-cases, thus allowing you to do {cond ? <frag>Text</frag> : <frag><div /><div /></frag>}.

I'm currently not a huge fan of the JSX conditionals, but I suspect I need to think about them differently, I've experimented with separating it out into separate methods and it seems to work good (and seems to make sense).

@esamattis
Copy link

Thanks for your response! I guess it makes it simpler to use Javascript conditionals in JSX.

I think it would also make it possible to implement something like TAL:

For example

var If = React.createClass({                                                                                                         
    render: function() {                                                                                                             
        if (this.props.condition) {                                                                                                  
            return <frag>{this.props.children}</frag>;                                                                               
        }                                                                                                                            
        return <frag></frag>;                                                                                                        
    }                                                                                                                                
}); 

Usage:

<If condition={this.isUserLoggedIn()}>
  <UserPanel /> 
</If>

Has anything like this been considered for React/JSX?

@syranide
Copy link
Contributor Author

syranide commented Apr 7, 2014

@epeli I'm only relaying my interpretation of it now, but the answer is yes and no. They have considered it (adding dedicated syntax for conditionals) but feel that it is the wrong solution and do not intend to go that route. I'm still finding my way around but I'm half-way inclined to agree with them, what you are suggesting is an intuitive go-to solution, it's kind of flawed it seems.

That being said, just to make it clear, conditionals are currently possible in React in the form of:

<div>
  {cond ?
    <div />
  :
    <div />
  }
</div>

<div>
  {cond &&
    <div />
  }
</div>

That is currently the recommended approach to conditionally inserting elements. Personally if the branches contain lots of code, I usually refactor them into individual methods (or individual components where it makes sense), thus:

<div>
  {cond ? this.renderLoggedIn() : this.renderNotLoggedIn()}
</div>

You actually can introduce If and similar constructs yourself if you really want to, but I strongly recommended against it, but it could look as (untested):

function If(props) {
  if (props.condition) {
    return arguments.slice(1);
  }
}

<If condition={this.isUserLoggedIn()}><div /></If> is sugar for If({condition: this.isUserLoggedIn()}, React.DOM.div()}), by not making it an actual component but a function with your own logic you can do some pretty nifty stuff. However, as I mentioned above, I strongly recommended against doing this.

Generally speaking, a big issue all templating languages have is that the syntax is really poor for conditionals, sometimes they "solve" it by not allowing the else-clause, sometimes it's nested, sometimes it's a weird special-case <If><Else></If>, etc. I'm not sure what the best solution really is, it seems like using markup for it is flawed though. The recommended approach at the top I like from a theoretical perspective, but from a practical perspective it's very ugly looking to me, but it works great when the content has been refactored into methods.

@patryk-kabaj-axomic
Copy link

Hey guys, do we have something that helps it today or is this still the case for JSX?

@GetContented
Copy link

frag would be absolutely awesome to have...

@AlexGilleran
Copy link

If you're used the condition/looping constructs of traditional templating languages and would prefer to use them like me, you might consider https://www.npmjs.com/package/jsx-control-statements. Basically it turns neater <If condition={condition></If>-style syntax into the ternary syntax described above.

@jquense
Copy link
Contributor

jquense commented Jun 5, 2015

If anyone is annoyed by this as I was and uses babel, I wrote a plugin to implement (in part) the <frag> syntax mentioned above.

https://github.com/jquense/babel-plugin-jsx-fragment

@stoeffel
Copy link

If anyone is interested, I just released a module for conditional rendering. Feedback is more than welcome.
https://github.com/stoeffel/react-cond

@rymohr
Copy link

rymohr commented Sep 25, 2015

@stoeffel looks like a great solution. Thanks for sharing!

@gfdev
Copy link

gfdev commented Jan 8, 2016

If anyone is interested yet, there is another solution for conditional rendering.
https://github.com/gfdev/javascript-react-if-component

@jimfb
Copy link
Contributor

jimfb commented Jan 9, 2016

@gfdev et al: If statements can not be implemented as a react component for reasons: https://github.com/AlexGilleran/jsx-control-statements/wiki/Why-Transform

Alex has done a great job maintaining a transform that implements various control statements:
https://www.npmjs.com/package/jsx-control-statements

@atticoos
Copy link

atticoos commented Feb 5, 2016

I've seen a lot of React components that aim to introduce control flow behavior, but I personally prefer to keep those pieces functional and as part of the language. This is just a personal preference, I think the component based approaches mentioned above are fantastic pieces of work.

I just published a really simple function that can be used to compose conditional render blocks. https://github.com/ajwhite/render-if

It's pretty much what you might expect, predicate => element => predicate && element

As an in-line expression

class MyComponent extends Component {
  render() {
    return (
      {renderIf(1 + 2 === 3)(
        <span>The universe is working</span>
      )}
    );
  }
}

As a named function

class MyComponent extends Component {
  render() {
    const ifTheUniverseIsWorking = renderIf(1 + 2 === 3);
    return (
      {ifTheUniverseIsWorking(
        <span>The universe is still wroking</span>
      )}
    )
  }
}

As a composed function

const ifEven = number => renderIf(number % 2 === 0);
const ifOdd = number => renderIf(number % 2 !== 0);

class MyComponent extends Component {
  render() {
    return (
      {ifEven(this.props.count)(
        <span>{this.props.count} is even</span>
      )}
      {ifOdd(this.props.count)(
        <span>{this.props.count} is odd</span>
      )}
    );
  }
}

Or just vanilla

const ifEven = number => element => elseElement => {
  if (number % 2 === 0) return element;
  return elseElement;
}

class MyComponent extends Component {
  render() {
    return (
      {ifEven(this.props.count)(
        <span>{this.props.count} is even</span>
      )(
        <span>{this.props.count} is odd</span>
      )}
    );
  }
}

The main goal for me was to stay flexible and composable. I feel that this solved the issue I was seeking. It doesn't

@jimfb
Copy link
Contributor

jimfb commented Feb 5, 2016

@ajwhite This suffers from the same problems I just mentioned one post prior. This can't be solved without at transform, as per https://github.com/AlexGilleran/jsx-control-statements/wiki/Why-Transform

Which is why you need something like https://www.npmjs.com/package/jsx-control-statements

@atticoos
Copy link

atticoos commented Feb 5, 2016

@jimfb thank you for these links. Terrific points, my approach promotes the idea that the block will not be evaluated if the predicate fails, yet it certainly will. ⚠️ ⚠️ ⚠️

Mentioned a thought on using a callback to avoid evaluation on our other thread reactjs/react-future#35 (comment). Will keep discussion moving there. Ty for feedback!

@milesj
Copy link
Contributor

milesj commented Feb 5, 2016

Wouldn't this be easily circumvented through stateless components?

<div>
  <listSpans condition={condition} />
</div>

function listSpans(props) {
    if (!props.condition) return null;

    return  [  
        <span></span>,
        <span></span>
    ];
}

It's a little more syntax, but it solves the issue at hand.

@jimfb
Copy link
Contributor

jimfb commented Feb 5, 2016

@milesj That is a LOT of syntax for an if statement. Also, it means your nodes are no longer in the same place (you have to scroll to a different file, or at least a different part of the file) to see what is going to get rendered there if the condition is true. Also, stateless components can't return a list. Bottom line: no, it doesn't solve it.

@milesj
Copy link
Contributor

milesj commented Feb 5, 2016

IMO, if you're returning multiple tags, or complex conditionals like this:

<div>
  {condition ?
    [
      (condition ?
        [
          <span></span>,
          'My name is ' + name + '!',
          <span></span>
        ]
      : null),
      'Hello ' + name + '!'
    ]
  : null}
</div>

It's probably better to just extract this into a component. More syntax isn't necessarily bad. Components should be updated to support arrays if they do not.

islemaster referenced this issue in code-dot-org/code-dot-org Mar 3, 2016
Any time React renders an array of components, each component in the array must have a "key" property that helps React manage the components efficiently.  I'm doing a sort of clumsy update here to meet this requirement - there may be a cleaner way to restructure these.

Also updated the Authored Hints components to follow our new declaration standard, so their names show up nicely in the React dev tools.

FYI @Hamms
@shaedrich
Copy link

shaedrich commented Nov 22, 2017

<If condition={this.isUserLoggedIn()}>
  <UserPanel /> 
</If>

is a nice approach, but it would be even more if JSX could support something like this natively as it is used in Ruby:

[1, 2, 3].each do |n|
  puts "Number #{n}"
end

in templates:

<% if this.isUserLoggedIn()? %>
    <%= partial "UserPanel" %>
<% end %>

so in JSX it could look like this:

{if this.isUserLoggedIn()}
    <UserPanel />
{end}

@gaearon
Copy link
Collaborator

gaearon commented Nov 22, 2017

See facebook/jsx#98 and facebook/jsx#99.
I’ll lock this issue because it is more about JSX than React. Hopefully the above links are helpful.

@facebook facebook locked and limited conversation to collaborators Nov 22, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests