Investigate implicitly adding sugar for JSX Completion Values

[bmeck]

Rephrasing of #85

Examples work in part assuming #39 is also necessary for this. Glad to be proven wrong on that front.

Examples are also using the syntax from #84 for fragments, but assume this is just sugar for now.

I would like to investigate implicitly combining adjacent completion values of statements that resolve to JSX expressions.

<div>
  <A />
  <B />
</div>

Is an example where implicit returns struggle. However, if instead we treat the completion value of statements as the mechanism for adding sugar we can do a few things.

Combine adjacent JSX completion values into fragments

• This should only combine JSXElement literals, no other literal kinds should be combined. Completion positions not using JSXElement literals are not combined.
• This should only combine places that have a normal completion

This works fairly well, but can have some odd syntax going on with semicolons and blocks:

<A />;
<B />;

Would be equivalent to both:

<A />
<B />

And

<>
<A />
<B />
</>

Implicit return for JSX completion values of functions

arr.map(item => {
  if (Date.now() % 2) <Odd></Odd>
  else <Even></Even>
});

The function has a completion value that is JSX. I would make a warning or error if mixing types that are not JSX and JSX here:

arr.map(item => {
  if (Date.now() % 2) <Odd></Odd> // <Odd> is in a completion position for the function
  else "even"; // ERROR: implicit return of JSXElement cannot collide with a non-JSXElement
});

[bmeck]

A bit odd but interesting example:

render() {
  if (this.state.invalid) {
    <Message>{this.state.invalid.text}</Message>
  }
  <Button></Button>
}

Becomes a fragment.

[syranide]

@bmeck I was writing a reply to this in the other issue but apparently I was no longer allowed to.

Anyway, there's a fundamental problem with having all children be some kind of "implicit return" as you showcase above. It's a super interesting concept (it's insanely flexible too), but it breaks implicit indices (and thus reconciliation). This means that in your example <div><A/><B/></div> and then rendering <div><B/></div> that <B/> will be recreated, because it changed from index 1 to 0 (or worse, wrongly reuse the wrong state). You can key it, but that becomes fragile and very tedious. You can imagine the implicit key being tied to the source code location, which I think is an interesting idea, but it has its own host of even more unintuitive problems I'm afraid.

IMHO one of the not really well appreciated aspects of JSX today is that you are essentially defining a "fixed structure" for children, all children are put in predefined slots, and then each slot can have different values (but whenever it is more than one value, you need to key it, because the implicit index is unreliable). This is what gives rise to reliable implicit indices and that's an extremely useful thing. I'm not sure if this explanation makes any sense 😄

[bmeck]

You can imagine the implicit key being tied to the source code location, which I think is an interesting idea, but it has its own host of even more unintuitive problems I'm afraid.

This is an investigation! Lets go over those problems. This also reminds me of #84 (comment)

[syranide]

Just a note about a variation of your example:

render() {
  if (this.state.invalid) {
    <Message>{this.state.invalid.text}</Message>
  }
}

Will have one of two behaviors when "this.state.invalid" is false.

1. Return undefined, logical because no JSX-element was defined thus it should not return "anything JSX". But it means that all "generated" return-statements must now conditionally check whether the array is empty and return undefined then, this causes overhead.

2. Return JSX fragment, this is dangerous because all functions that can return a JSX-element now magically becomes a "JSX function". Although you could solve this by having some kind of scope "dojsx { ... }" which makes the magic explicit (EDIT: this sounds fine to me on the surface).

Bonus, what happens if you do <B />; return 0?

[syranide]

This is an investigation! Lets go over those problems. This also reminds me of #84 (comment)

@bmeck

if (cond) {
  <B foo />
} else {
  <B bar />
}

Will not reuse the <B /> when switching between them. This is not necessarily bad, you just have to be very careful when keying. EDIT: But this is kind of a big deal because of how many actually don't understand importance of keying today even.

---

for (...) {
  <B foo={bar} />
}

They will all have the same key, perhaps it can fall back to index, but that is a problematic behavior for keys in general I think. You could also just warn when the keys conflict in this case as we already do I guess.

---

The bigger problem I think comes from calling functions and other kinds of less straight-forward uses:

function foo(bar) {
  <B bar={bar} />;
}
<>
  (cond ? foo(1) : null)
  foo(2)
</>

This has the same problem as in my earlier comment, when you re-render that it will either require explicit keys (tedious and weird, because you have to make foo take a key) or the implicit indices will break when you re-render.

---

These are just the obvious off the top of my head.

I'm quite sure warnings cannot be emitted for most of these as the false-positives would be overwhelming.

EDIT: JSX today behaves logically and when you're providing dynamic arrays it's easy to detect and issue warning for lack of keys, I think this intuitive behavior is imperative. But it seems it doesn't translate over to "implicit returns" without making all child-expression semantics JSX-specific too, possibly.

[bmeck]

@syranide

render() {
 if (this.state.invalid) {
   <Message>{this.state.invalid.text}</Message>
 }
}

Was this a mis-copy? It is missing <Button>.

As it stands we should probably throw an error or warning since the completion value contains a non-JSX value of undefined. This is briefly mentioned at end of original issue text.

<B />; return 0

The statement in the original issue is only that you combine Combine adjacent JSX completion values

We should clarify that non JSX literals such as a number are not combined.

We can also further clarify that only normal completions are combined which would prevent the return from combining with an ExpressionStatement.

if (cond) {
  <B foo />
} else {
  <B bar />
}

I would leave this as is, automatically deduping should be a separate issue.

for (...) {
  <B foo={bar} />
}

The for loop only has a completion value of the last iteration. So you would only end up with one <B>. See eval("for (var i = 0; i < 3; i++) {i;}"); for an example of this behavior.

You could also just warn when the keys conflict in this case as we already do I guess.

Yes, this idea does not get rid of conflict resolution.

function foo(bar) {
 <B bar={bar} />;
}
<>
 (cond ? foo(1) : null)
 foo(2)
</>

I might not be understanding this well enough. Is there something that forces "the implicit indices will break when you re-render" to be true?

[bmeck]

fixed up original issue text .

[syranide]

Ok, wait. I assumed this was mostly a continuation of the previous issue and just glossed over your text. My bad.

If it's just about adjacent JSX elements then these are my thoughts on that: #84 (comment)

But IMHO that feature is largely irrelevant if there is larger discussion. It doesn't change anything, it's just sugar. EDIT: It is also error-prone and confusing because <i />foo()<i /> would actually try to call foo(), and would not become a textnode as you would normally assume.

---

arr.map(item => {
  if (Date.now() % 2) <Odd></Odd>
  else <Even></Even>
});

Sure, like I mentioned in the previous issue, implicit return is a possibility (ASI is an issue though), but again, it's just a cosmetic thing, you could do this via do-expressions too. So you can argue the merits of it on its own, but personally I'm not a fan of it, it is unexpected and breaks with how JS otherwise works. It is also somewhat dangerous as it can hide code errors. Anyway, that's my personal opinion. 🤷‍♂️

[bmeck]

The ASI is an issue that is solved the same way as other ASI issues are solved, by adding a semicolon. Right now it fails to parse which is much better than ASI issues like:

return
{};

but again, it's just a cosmetic thing, you could do this via do-expressions too.

One without the other isn't very valuable, the value is in combining the 2 pieces proposed here.

The do expression equivalent can be exceedingly verbose for some things like:

render() {
  if (this.state.invalid) {
    <Message>{this.state.invalid.text}</Message>
  }
  <Button></Button>
}

render() {
  return [ // ASI hazard if on new line as well...
    ...do { // BIKESHED (we have not specified in this thread if this *always* vs sometimes generates an index)
      if (this.state.invalid) {
        [<Message>{this.state.invalid.text}</Message>]
      }
      else []; // this branch doesn't translate cleanly
    },
      <Button></Button>
    ];
}

[syranide]

The ASI is an issue that is solved the same way as other ASI issues are solved, by adding a semicolon. Right now it fails to parse which is much better than ASI issues like:

Well ASI is an official feature of JS and writing without colons is valid coding style, you solve return with parenthesis. So JSX should at least have a sensible solution to it. "Just add a colon" doesn't sound OK to me given that from looking it at it you would expect it to compile and someone not intimately familiar with it would be totally stumped.

The do expression equivalent can be exceedingly verbose for some things like:

I don't follow.

render() {
  if (this.state.invalid) {
    <Message>{this.state.invalid.text}</Message>
  }
  <Button></Button>
  <Button></Button> // added second button to actually showcase adjacent JSX elements
}

If we assume that the fragment syntax <> is accepted, then that would currently look like (without do-expressions):

render() {
  if (this.state.invalid) {
    return <Message>{this.state.invalid.text}</Message>
  }
  return <>
    <Button></Button>
    <Button></Button>
  </>
}

Sure enough it is different, but your example isn't exactly transformative. It's just more concise.

I personally also find it confusing, from looking at your example I would assume that the buttons would be rendered no matter what (i.e. execution does not stop on a JSX element), because it looks like a templating language and that's how I would say templating languages work.

It also seems like you've mischaracterized how it would look with do-expressions:

render() {
  return do {
    if (this.state.invalid) {
      <Message>{this.state.invalid.text}</Message>
    } else {
      <Button></Button>
    }
  };
}

---

... Or are you saying that the button IS rendered in your example, regardless of the invalid state? Because that is a whole different beast and we're back at the problems of #86 (comment), #86 (comment), #86 (comment).

[bmeck]

The completion value of the if is a JSXElement, it gets combined with the adjacent completion value which is also a JSXElement. The if statement is not in a function body completion position and is not subject to implicit return.

[syranide]

@bmeck Maybe it's just me or I don't fully understand the purpose of "phrasing it that way".

If we take the original example as a starting-point:

render() {
  if (this.state.invalid) {
    <Message>{this.state.invalid.text}</Message>
  }
  <Button></Button>
  <Button></Button>
}

Then it should translate to something akin to the following:

render() {
  if (this.state.invalid) {
    push(<Message>{this.state.invalid.text}</Message>)
  }
  push(<Button></Button>)
  push(<Button></Button>)
}

Right?

[syranide]

I should just clarify exactly instead:

render() {
  let out = [];
  if (this.state.invalid) {
    out.push(<Message>{this.state.invalid.text}</Message>)
  }
  out.push(<Button></Button>)
  out.push(<Button></Button>)
  return out
}

EDIT: Are we on the same page or am I misunderstanding you in some way?

[bmeck]

@syranide kind of? Though you need to be careful about some things like loops that do not have completion values per iteration and try{} catch (_) {} finally {} which re-uses the completion value. This is not about adding to an array whenever any JSXElement is evaluated.