Skip to content
This repository has been archived by the owner on Sep 21, 2022. It is now read-only.

Discuss Conditional JSX Expression #35

Open
jimfb opened this issue Jun 11, 2015 · 32 comments
Open

Discuss Conditional JSX Expression #35

jimfb opened this issue Jun 11, 2015 · 32 comments

Comments

@jimfb
Copy link
Member

jimfb commented Jun 11, 2015

From @syranide

http://wiki.ecmascript.org/doku.php?id=harmony:array_comprehensions could this be something to consider for JSX? I.e. statement like syntax inside braces, more readable than ternary and various other hacks while still being very much like JS. <div>{if (x) <div />}</div>, etc.

@jimfb
Copy link
Member Author

jimfb commented Jun 11, 2015

Continuing the discussion fragment in #28 (comment): Yes, I was implying statement like syntax, but not just in a JSX expression. I was proposing that all the JSX expressions BE statement syntax. So the presence of a JSX expression is equivalent to renderOutput += '<div>'. This is more inline with the way languages like PHP work. To be clear: I'm not claiming this is a better style, only that it's an interesting alternative, and naturally allows the if(foo) <div /> syntax that you were looking to achieve without breaking compliance with standard javascript.

With regards to use of non-standard javascript: Yes, the line is a little blurry. Especially as we add ES7 features before they're is agreement at the standards committees. But the difference is, imo, the answer to the question "What is the surface area of JSX?", and what is "other stuff". Unless we have some sort of statement syntax, the expression if(foo) <div /> doesn't make sense as part of ES6/7, and would therefore be part of JSX. However, if JSX were interpreted as a statement syntax, then the expression would be standard javascript and therefore completely legit, imo.

That's just my two cents. I encourage discussion on these points, and am always interested in exploring alternatives.

@jimfb jimfb changed the title Discuss Conditional JSX Discuss Conditional JSX Expression Jun 11, 2015
@syranide
Copy link

I was proposing that all the JSX expressions BE statement syntax. So the presence of a JSX expression is equivalent to renderOutput += '<div>'. This is more inline with the way languages like PHP work.

I see two big issues, it messes with implicit indicies in a way that expressions doesn't (because all children, null or not, increment the index counter too):

if (x) <A />;
<B />; // key becomes ".0" or ".1"

It's also super unintuitive behavior that only made sense in PHP because it's concatenation of partial strings, not hierarchies:

a(); // what happens to elements appended in this function?
function a() {
  b = <div />; // is this appended?
  b; // does this append it?
  // what if a() is called outside of render()?
}

Especially as we add ES7 features before they're is agreement at the standards committees.

Yeah ofc, I said that on the assumption that it will pass (no idea if it will), but array comprehensions probably aren't really relevant to this discussion. Anyway, my mine gripe is specifically with the ternary operator for conditionals, it seems like such a massive disservice to the readability of JSX and React, despite being such a common operation. Anything more complex than that is usually best hidden behind a function call anyway.

On a hunch, I would be perfectly happy with just:

<div>
  {if (x) ValueA}
  {if (x)
    ValueA
  else if (y)
    ValueB
  else
    ValueC
  }
</div>

No nesting or anything else. Again, arguable whether one should do it or not, but it's very limited in scope and I find it far preferable to the ternary operator (which is just symbol soup):

<div>
  {x ? ValueA : null}
  {x ?
    ValueA
  : y ?
    ValueB
  :
    ValueC
  }
</div>

@matthewwithanm
Copy link

I agree that JS's ternary isn't as ideal as but for the simple case you could just use JS's short circuiting logical operators, right?

<div>
  {x && ValueA}
</div>

It would be nice for JS to have if-else expressions (instead of statements) a la CoffeeScript et al for the ternary case but JSX isn't the right place to address that IMHO. I've just been breaking those things out into methods/other components.

@syranide
Copy link

I agree that JS's ternary isn't as ideal as but for the simple case you could just use JS's short circuiting logical operators, right?

That seems even worse to me, especially if you consider {x && y && ValueA} or {(x || y) && ValueA}, it stars becoming really weird real fast, even more so when things aren't as simple as x and y. Also, {0 && x} outputs 0 and you're generally messing with the actual value of children.

...but JSX isn't the right place to address that IMHO.

I guess that's the point of this discussion, there is definitely a point where JSX could go too far, but I also think there's a point where JSX doesn't do enough. Ternary operators weren't designed for this and so just because they can doesn't mean they should be used. Conditionals is a very common use-case so neglecting it for philosophical reasons seems odd.

Personally I also think there's a point in discussing how it should be, regardless of whether we do it or not (we can conclude what JS offers is a good enough approximation). To put it differently, you can say '{' expression '}' and shrug your arms, but the same argument could be applied to JSX too (just use calls/objects/arrays/etc) but most seem to agree that JSX is beneficial.

@jimfb
Copy link
Member Author

jimfb commented Jun 12, 2015

@syranide Would you be happy with:

<If test={x}>
  <A />
</If>

It's a little more verbose, but the upside is that you would also preserve the hierarchical readability. Would that be sufficient?

@lionng429
Copy link

@jimfb how would you specify or / and condition with this? Though it's readable, it's on the other hand misleading as it takes the same syntax of components

@jimfb
Copy link
Member Author

jimfb commented Jun 12, 2015

@lionng429 There are a couple of ways you could imagine specifying the else block. You could do <If test={x}>true<Else />false</If> or it could be implemented in other permutations like <If test={x}>true</If><Else>false</Else>. Doesn't really matter; those are details. The question for @syranide was if this syntax would be acceptable to him / would be an improvement over the status quo.

The fact that it looks like a component is intentional. Conceptually, you've just imported the "If" component, which renders the children iff the test condition is true. It achieves the goal (more readable conditionals) without muddying the javascript syntax in a non-standard way.

@syranide
Copy link

@syranide Would you be happy with:

<If test={x}>
  <A />
</If>

It's a little more verbose, but the upside is that you would also preserve the hierarchical readability. Would that be sufficient?

I'm not sure if I'm the ultimate authority on this, I'm simply a bit displeased with the current situation and hoping there is a better alternative :)

Anyway, not a fan, repurposing elements seems like a bad idea, you would never consider ifFunc(1 == X, () => { ... }); to be a good idea and those two seem like the same idea. Instinctively it also seems like it encourages more "lazy" practices (say <MyComp if={...} />). It also enables you to do x = <If /> which just seems like a bad idea (it's not a reason to dismiss, but it's a con). On-top of being rather cumbersome and noisy (and annoying to refactor into functions).


My view on this subject is that I really respect the hands-off approach JSX has, it introduces elements, end of story. That's great. However most languages (including JS) were not designed with deeply nested expressions, like JSX, in-mind. The only conditional expression operator (?:) available is a poor fit, it was intended for simple expressions (my interpretation). So it seems there is a vacuum introduced by JSX which the host language might not care about (I also mean this in a broader sense, what if you wrote components in C or w/e).

Intuitively it could make sense to have '{' statements '}' instead (i.e. wrap it in a lambda):

<div>
  {return Value}
  {return Cond ? A : B}
  {if (x) return <A /> else return 1}
  {
    var list = [];
    for (x of list) list.push(<Item name={name} />);
    return list;
  } // meeh, but it works (yes, you can use the normal helpers)
</div>

This solves every problem (I think?) while not intruding on the host language. The only downside is the mandatory and predominant return everywhere. So one could image introducing '{{' statements '}}' instead. So:

<div>
  {Value}
  {Cond ? A : B}
  {{if (x) return <A />; else return 1;}} // semicolons for completeness
  {{
    var list = [];
    for (x of list) list.push(<Item name={name} />);
    return list;
  }}
</div>

To me that actually seems kind of neat and unintrusive, instead of having to move all complex code out into separate functions you can now do it inline and later refactor it out if necessary. It doesn't intrude on simple expressions and the ternary operator can still find itself useful for the simpler conditionals.

One could also imagine replacing the '{' expression '}' grammar with a new grammar '{' expression_statement '}'. It could be defined as the if- and for-statement syntax but where instead of having statement blocks they have expressions (or rather expression_statement) which are implicitly returned. It's intrusive but it wouldn't really introduce unfamiliar syntax:

<div>
  {Value}
  {if (x) <A /> else 1}
  {for (x of list) <Item name={name} />}
  {for (x of list) if (x.visible) <Item name={name} />}
</div>

This syntax should be transpilable to statement syntax quite easily. But no doubt controversial, and the benefits are arguable when the '{{' statement '}}' would provide the same benefit with just some additional code.

I feel like '{{' statement '}}' doesn't entirely address conditionals in JSX, but perhaps it's good enough considering how little effort and unintrusive it is. It seems very much in line with the premise of JSX if you ask me, it simply acknowledges that not everything are expressions, statements sometimes makes sense too.


tl;dr The example below seems like it could be a reasonable compromise to me:

<div>
  {Value}
  {Cond ? A : B}
  {{if (x) return <A />; else return 1;}}
  {{
    var list = [];
    for (x of list) list.push(<Item name={name} />);
    return list;
  }}
</div>

@matthewwithanm
Copy link

...but JSX isn't the right place to address that IMHO.

I guess that's the point of this discussion, there is definitely a point where JSX could go too far, but I also think there's a point where JSX doesn't do enough. Ternary operators weren't designed for this and so just because they can doesn't mean they should be used. Conditionals is a very common use-case so neglecting it for philosophical reasons seems odd.

It's not a philosophical reason. It's a huge tradeoff in complexity, both for tools and users. Right now it's just "stuff in brackets are JS expressions." Easy for for everybody to understand. And adding a special grammar for within brackets could wind up being an even bigger nightmare when new stuff gets added to the language.

Personally I also think there's a point in discussing how it should be, regardless of whether we do it or not (we can conclude what JS offers is a good enough approximation).

So do I! I just don't agree that that's how it should be (:

I don't mean to be dismissive or shrug my arms. When I say that I don't think JSX is the right place to address this, I don't mean that I don't think it should be addressed. But this is a problem in all of JS. Have you considered putting together a proposal for the language?

@jimfb
Copy link
Member Author

jimfb commented Jun 12, 2015

And adding a special grammar for within brackets could wind up being an even bigger nightmare when new stuff gets added to the language.

Agree. I'm also not convinced the {{ grammar would even be unambiguous; it certainly does require infinite lookahead to differentiate between an object definition and a statement definition. But the more important point is the one @matthewwithanm raised: The syntax would intrude on the language space of future versions/extensions of javascript.

A favorite quote from Sebastian:

It is our intention to claim minimal syntactic real estate... That way we leave the door open for other extensions.

FWIW: The component/element syntax is available today: https://github.com/valtech-au/jsx-control-statements (personally I'm a fan).

@syranide
Copy link

It's not a philosophical reason. It's a huge tradeoff in complexity, both for tools and users. Right now it's just "stuff in brackets are JS expressions." Easy for for everybody to understand. And adding a special grammar for within brackets could wind up being an even bigger nightmare when new stuff gets added to the language.

@matthewwithanm Yeah I'm not disagreeing that there are trade-offs for some approaches, but JSX has a purpose and if it's not 100% fit for that purpose we should evaluate if we can make it better.... that's why we have JSX to begin with, JSX is a monstrous trade-off for something which has barely quantifiable benefits, but we all agree it's worth it. So obviously it's not that binary.

But this is a problem in all of JS. Have you considered putting together a proposal for the language?

I don't see how it's a problem with JS. Sure you could extend JS with it but I'm not sure what I would use it for other than possibly for JSX expressions. That last one wasn't really a serious proposal though.

Agree. I'm also not convinced the {{ grammar would even be unambiguous; it certainly does require infinite lookahead to differentiate between an object definition and a statement definition. But the more important point is the one @matthewwithanm raised: The syntax would intrude on the language space of future versions/extensions of javascript.

@jimfb If you think of it in-terms of JSX {...} --> JS (() => ...)() then you'll see that JS fat arrow functions suffer from the exact same ambiguity, you disambiguate those with ({}). The transpiler is allowed to drop the generation of fat arrow functions for expressions because they serve no purpose.

But the exact syntax isn't my point, but whether or not JSX has shortcomings. My experience is that JSX works surprisingly well as-is today when you have the time to carefully plan and organize your components. That is a luxury. Being able to move fast, experiment and later refactor is important. This seems like a small price to pay for that flexibility.

@sebmck
Copy link

sebmck commented Jul 9, 2015

FWIW, Babel actually implements do expressions. With this it means that the original example of:

<div>{if (x) <div />}</div>

can be represented as:

<div>{ do { if (x) <div />; } }</div>

with a do expression.

Babel implements the correct completion records semantics and outputs simple code and deopts to an IIFE for code it can't explode into a single expression. You can play around with it in the REPL here.

IMO a more generic JavaScript solution (ie. do expressions) should be sought rather than adding additional complexity to the existing JSX syntax.

@sebmck
Copy link

sebmck commented Jul 9, 2015

Also, I really like @syranide's proposal of allowing an abitrary list of statements inside JSX expression containers, I particularly like the completion record example of:

<div>
  {Value}
  {if (x) <A /> else 1}
  {for (x of list) <Item name={name} />}
  {for (x of list) if (x.visible) <Item name={name} />}
</div>

It doesn't impose any new syntax and reuses existing JavaScript semantics. This could also quite trivially be implemented in Babel at least and the benefits would be enormous.

@mathieumg
Copy link

@sebmck With regards to do expressions you mentioned (didn't know about them), I imagine it wouldn't be possible to use switch? (See REPL)

If I add returns inside the switch it complains that it's not a function, even though it is once transpiled. Is this by design/in line with the spec? A workaround is this but it's neither intuitive, nor pretty.

As for @syranide 's proposal, 👍 as well.

@matthewwithanm
Copy link

do expressions are exactly the kind of thing I was hoping for! Thanks for the link @sebmck! If the "statement problem" is solved in JS land like that, the only change in JSX semantics is automatic wrapping in do expressions and there's no chance of issues. Would love to see that happen.

@sebmck
Copy link

sebmck commented Jul 9, 2015

@mathieumg Yeah, switches aren't possible. Easiest way to test out completion records (rather than digging through the spec) is to use the return value of eval:

eval("switch (1) { case '1': 2; break; }");
eval("switch (1) { case '1': 2; }");

@matthewrobb
Copy link

@sebmarkbage Just curious if there is ANY documentation for do-expressions on the babel site? I couldn't find the proposal mentioned on the tc39 github thing so I did open this issue: tc39/ecma262#49

@sebmck
Copy link

sebmck commented Jul 13, 2015

@matthewrobb There isn't. It's a deliberate choice because there's no current proposal document or spec text.

@sebmarkbage
Copy link
Contributor

JSX isn't the right context to discuss this really. The whole point of JSX, as opposed to templates, is to follow the general purpose language around it. If something like this needs to be added to JSX, it also needs to be added to the language, e.g. in an object literal or any other expression form, and we should follow that. We're not inventing a new language here so I kind of feel like this discussion is in the wrong place. It should be at es-discuss or something like it.

Making do-expressions implicit (facebook/jsx#39) is a lot more palatable since the do keyword is really just working around a legacy syntax issue and we could avoid that for a little nicer syntax. @syranide makes a good point about comparing it to arrow functions though.

We can keep this issue open for discussions of the implications but I would suggest that we refrain from proposals that doesn't make sense in JS outside of JSX. IMO, if control flow syntax doesn't work in an object literal, it shouldn't work in JSX.

I feel like facebook/jsx#39 + regular JS proposals like array comprehensions cover everything here. We can add a section to this repo with examples that demonstrate them. PR?

@atticoos
Copy link

atticoos commented Feb 5, 2016

JSX isn't the right context to discuss this really. The whole point of JSX, as opposed to templates, is to follow the general purpose language around it.

I felt similarly when thinking about the "conditional challenge" in React. I've seen a lot of component based approaches, all of which are really well done, but I like the separation of control flow and my components.

I put together a functional approach to this problem that I think contributes to this discussion. https://github.com/ajwhite/render-if

I've shared this in another discussion over in facebook/react#690 (comment), so I'm just going to grab some samples I posted in verbatim:

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>
      )}
    );
  }
}

To me, this approach felt a more at-home with how React is structured by following the language around it. I find myself in agreement that control flow tags don't feel quite at home built into React as components.

@matthewrobb
Copy link

@ajwhite The only issue I see with those approaches is that the components will be created every time no matter what. You aren't leveraging any language construct to prevent needless construction of components or elements.

@jimfb
Copy link
Member Author

jimfb commented Feb 5, 2016

@matthewrobb Yes, you are absolutely correct.

@ajwhite This is a bigger problem than you might be imagining, and is the same reason that If statements can not be implemented as React components: 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

Ah great point guys. Yep, this absolutely becomes evaluated.

I suppose the only way to avoid evaluation here would be pass in as a callback

{ifEvent(this.props.count)(() => (
  <span>{this.props.count} is even</span>
))

However, this starts to lose the value I see in the goal of keeping this minimal. Thanks for this feedback, that can pose a big problem if attempting to access properties you only expect to exist when the predicate is satisfied.

@bzbetty
Copy link

bzbetty commented Aug 17, 2016

I think Microsoft got it right when making their Razor templating language. They tried to reduce the number of control characters {} and making the compiler just a little smarter. It's actually very similar to JSX in a lot of ways. @ puts you into code mode, until it hits an html literal or the end of the expression. Ends up much cleaner than handlebars.

<div>
    @if(this.props.count) {
      <span>@this.props.count is even</span>
    }
</div>

@chrisregnier
Copy link

I'm not a fan of the special control statements that I've seen suggested so far, so for only the simplest (IF) case I'd like to suggest annotation blocks on JSX expressions. (This also shares this new syntax with #66 Custom attribute namespace.)

Then use the truthy value of the annotation block as the conditional and it makes simple boolean conditional rendering quite nice. (the object form below is the custom attribute namespace stuff)

let showError = false;
return (
    <div>
        {@ showError}<span>Error</span>
        {@ { ref: callback } }{ (<span/>) }
    </div>
);

Which could roughly look like this:

let showError = false;
return React.createElement('div', null,
    (showError)&& React.createElement('span', { _meta: (showError) }, 'Error'),
    ( { ref:callback} ) && React.createElement('span' , { _meta: { ref: callback } } )
);

@rozzzly
Copy link

rozzzly commented Oct 25, 2016

@chrisregnier I like the look of that. It does introduce a new control character, but doesn't clutter the code or feel awkward. I'm interested to see how else/else if could be worked in, I think that support for those control flow statements (and potentially others) must be a key concern 🤔

What about

<div>
  // a one liner; and like js it doesn't require explicit "end" token/syntax (But would require conditional and jsx stmt to be on the same line
  { @if foo === 'bar' } foo is a <span style'color: red;'> bar!</span> 
  // Now a more complex example
  { @if bar === 'foo' } 
      Bar is a <b>foo</b>
      Block scoping/I whatever plays nice
  { @else if true  === true } another one liner
  { @else } 
     Some
     MultiLine
     Block
  { @end else } // not that sexy 😳 perhaps just an ubiquitous @end? Please not bash-esque fi/esle/etc..
</div>

That's semantic, allows for single lines or blocks, can do if/else if/else and I don't see why it couldn't do other things like for loops. It stands out amid other jsx code but isn't a huge departure from what we've got now. Pretty sure it would support nesting too. I haven't thought it all through, and I'm not sure how the emitted code would look (especially with the for/while) but think this is a step in the right dire

@bzbetty
Copy link

bzbetty commented Oct 25, 2016

@rozzzly I know my suggestion might not be the easiest to implement but it really does end up looking a lot cleaner and would be way easier for people to learn. (as I said microsoft have used this syntax perfectly fine in the past https://github.com/aspnet/Razor )

With the exception that it wouldn't support text literals directly in a code block (not dissimilar to jsx anyway), and probably require exactly one html element per code block.

That said given that {} is already a control statement I can see why this could be quite problematic / ambiguous in certain situations.

<div>
    @if(foo === 'bar') { <span style='color: red;'> bar!</span> }

    @if(bar === 'foo') { 
        <div>Bar is a <b>foo</b>
        Block scoping plays nice
        </div>
    } else if (true  === true) { 
      <span>another one liner</span>
    } else {
      <ul>
         <li>Some</li>
         <li>MultiLine</li>
         <li>Block</li>
      </ul>    
    } 
  </div>

One of the best parts of jsx is that it went down the route of smart parser and not custom templating language. Stick with that and use javascript syntax for control statements and html for html.

@rozzzly
Copy link

rozzzly commented Oct 25, 2016

I don't mind that, but I see it potentially making it a little more confusing what is expected (js or jsx) within the { }s currently it's js=>jsx=>js=>jsx but with this it'd be more like js=>jsx=>js unless it follows a @if/etc in which case then it's supposed to be jsx, etc. That's potentially a deal breaker for me so I'm interested to see what iterations others can make on either of our suggestions. Much less clunky-feeling than some of the proposed <If condition={ true === !false }> something </If> style syntax

@syranide
Copy link

facebook/jsx#39

@bzbetty
Copy link

bzbetty commented Oct 25, 2016

@rozzzly other reason I like my suggestion (although yours would work with a small tweak) is that it opens up other code constructs too. The <If> style ideas would need it to be coded for each concept.

eg a for loop is suddenly very easy.

<ul>
  @for(var i in items) {
     var t = fn(i); // in code mode until it sees a tag
     <li />
  }
</ul>

@scottmas
Copy link

scottmas commented Apr 6, 2017

Along with @kittens & @syranide I would love an approach like

<div>
  {Value}
  {if (x) <A /> else 1}
  {for (x of list) <Item name={name} />}
  {for (x of list) if (x.visible) <Item name={name} />}
</div>

These code constructs work in vanilla javascript. For instance, type this in your console - it works:

for(var i of [1,2]) if(i === 1) console.log(1)

Also, according to @kittens, this could be trivially implemented in Babel at least. I'm not sure how much more complicated it would make the JSX spec.

Also, FWIW, I strongly dislike the idea of adding @for and similar syntactic extensions. Let's just keep JSX as close to plain ol' Javascript as possible.

@calebmer @RReverser @sebmarkbage and other Facebook peeps have an opinion?

And FWIW, I think this is an essential question to iron out for the future of React. I don't think {test && <Component/>} gets the React ecosystem nearly far enough. Flow control is a huge pain point, and libraries like Angular are doing it in a more developer friendly way with attribute directives like <div *ngIf="test" *ngFor='var i of list'/>. Adding something like this would just be one step towards improving React.

@mnpenner
Copy link

mnpenner commented Jun 5, 2017

Also, FWIW, I strongly dislike the idea of adding @for and similar syntactic extensions. Let's just keep JSX as close to plain ol' Javascript as possible.

But @for wouldn't be a special directive. @ switches you into JS-mode, and then the for is plain ol' Javascript. We're essentially trading { for @ and dropping the closing }.

Would this work with your proposed syntax?

<ul>
  {for(var i in items) {
     var t = fn(i); // in code mode until it sees a tag
     <li />
  }}
</ul>

Also, I hate attribute directives like Angular has. Stringly-typed code in faux-attributes is the devil.

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