Investigate implicitly adding sugar for JSX Completion Values

[bmeck]

Rephrasing of https://github.com/facebook/jsx/issues/85

Examples work in part assuming https://github.com/facebook/jsx/issues/39 is also necessary for this. Glad to be proven wrong on that front.

Examples are also using the syntax from https://github.com/facebook/jsx/issues/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 :smile:

[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 https://github.com/facebook/jsx/issues/84#issuecomment-322645186

[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: https://github.com/facebook/jsx/issues/84#issuecomment-322715303

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 https://github.com/facebook/jsx/issues/86#issuecomment-323412452, https://github.com/facebook/jsx/issues/86#issuecomment-323415244, https://github.com/facebook/jsx/issues/86#issuecomment-323417431.

[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.

[syranide]

@bmeck Here's where I'm confused, we're talking about functions, no normal statements inside a function has a completion value. So the above examples don't make sense? It seems like you're assuming some kind of do-expression semantics that apply to all functions?

[bmeck]

@syranide In JS, statements have completion values. This can be seen in various RuntimeSemantics of the spec regarding evaluation such as https://tc39.github.io/ecma262/#sec-block-runtime-semantics-evaluation returning blockValue.

[syranide]

@bmeck Sure, but not in any meaningful way right, you cannot access these completion values other than via eval...? So AFAIK they only seem to exist for the purpose of more useful interactive prompts. It's not actually something you can use outside of that.

[bmeck]

@syranide

let a = 0;
<A />
<A />

Would return undefined as the completion value is always the first statement. Or do you see it some other way?

Yes, it would not combine. The 2 JSXElements would combine and become some form of fragment (<> or an array? [bikeshed]).

I don't see how this would cause things to not work since we are only combining adjacent JSXElements.

[syranide]

Sorry, my mistake. Updated reply below: 😄

And if it is those completion value semantics you're piggy-backing on then I don't see how it would work in practice.

<A />
let bar = 0;
<B foo={bar} />

Would return only the last <B /> as the completion value is always only the last value (other than adjacent as for your proposal, which doesn't apply here). But this above example doesn't seem very intuitive and I wonder what reason there would be to use completion value semantics instead of something more straight-forward.

[syranide]

Also, trying to transpile adjacent completion values in this way sounds like a proper nightmare if you ask me, not to mention the runtime performance overhead (hmm, perhaps it isn't bad actually).

[bmeck]

But this above example doesn't seem very intuitive and I wonder what reason there would be to use completion value semantics instead of something more straight-forward.

I agree, that example is confusing but easy to explain why it fails. I am not sure how realistic this kind of coding is to become commonplace since it doesn't have meaningful effects in current compilation chain.

Also, note that transpiling adjacent completion values in this way sounds like a proper nightmare if you ask me, not to mention the runtime performance overhead.

Since JSXElement is a compile time only construct you don't need to do it at runtime. We are not combining the results of functions. I should clarify that.

[syranide]

Ok, now I get what you're getting it. Took me a while. There's also the problem of textnodes:

<div>
  Foo
  <A />
  Bar
  <B />
</div>

Foo and Bar would try to compile as JS here, rather than textnodes. Although there are proposals to remove textnodes from JSX (and I'm in-favor of it).

Anyway, if you assume we get rid of text-nodes and forget about exact completion values semantics, then you could allow:

if (cond) {
  <A foo />
} else {
  <A bar />
}
if (cond2) {
  <B />
}
<C />
let bar = 0;
<D foo={bar} />

Which could compile to the equivalent of:

let out_1 = null
if (cond) {
  out_1 = <A foo />
} else {
  out_1 = <A bar />
}
let out_2 = null
if (cond2) {
  out_2 = <B />
}
let bar = 0;
<>
  {out_1}
  {out_2}
  <C />
  <D foo={bar} />
</>

Which might kind of make sense, but understanding the exact semantics of this is probably too complicated for any layman, and I'm not sure how you would deal with nested ifs right now, and you would probably have to automatically put a fragment around for-loops (which is problematic too).

[bmeck]

I would want let bar = 0; to stop the aggregation since it introduces a completion value. Only <B foo={bar} /> would return.

and you would probably have to automatically put a fragment around for-loops (which is problematic too).

Absolutely not, for loops have a single completion value. I do not want to change completion value semantics.

[bmeck]

We could say that empty completions do not stop aggregation?

[syranide]

Absolutely not, for loops have a single completion value. I do not want to change completion value semantics.

I don't see the merit of completion value semantics for JSX, it seems artificially limiting and unintuitive. But perhaps I'm just not seeing the light right now, anyway, I don't mean to discredit this issue if you believe there's value in it. I just don't think I can be of much help for the time being.

[bmeck]

It matches do expressions so I have absolutely zero desire to move on my statement that loops must produce a singular value.

[syranide]

@bmeck Hmm, after thinking about it a bit more I think I'm starting to see what you're getting at. But I'm not sure what real-life problem you would really be solving that couldn't be rewritten to something equally good without these semantics.

Can you think of any real-life examples where this would actually make the code significantly better?

[bmeck]

@syranide examples would all be in the same vein as #85 basically

[syranide]

@bmeck

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

Is actually super-weird to me now that I think about it. It creates a fragment, where message becomes combined with button.

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

However that would ~render only a message or a button~ always render only a button (right?). I find that super confusing and I personally can't see "Combine adjacent JSX completion values into fragments" being a generally helpful feature.

So IMHO, if you remove that I'm not sure what value there is in having completion value semantics vs just straight-up implicit return. Completion value semantics just seem more error-prone whereas implicit return would actually allow you to cut down on the clutter. What am I missing here?

[bmeck]

Is actually super-weird to me now that I think about it. It creates a fragment, where message becomes combined with button.

Making this work only within fragment syntax of <></> instead on function body make more sense?

That would complicate things since:

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

Is problematic like you said due to text. This might make the things a bit odd still since it is upon completion value instead of when they are evaluated.

I think that adjacency is pretty simple, but am open to talk about things.

So IMHO, if you remove that I'm not sure what value there is in having completion value semantics vs just straight-up implicit return. Completion value semantics just seem more error-prone whereas implicit return would actually allow you to cut down on the clutter. What am I missing here?

People will need to learn about completion values due to do expressions. I think implicit returns that are not in a completion position are a very different mechanism and would require more learning / might cause friction.