facebook / jsx

The JSX specification is a XML-like syntax extension to ECMAScript.
http://facebook.github.io/jsx/
1.96k stars 133 forks source link

RFC: React Expressions with Implicit Do-Generator Semantics #98

Closed clemmy closed 6 years ago

clemmy commented 6 years ago

RFC: React Expressions with Implicit Do-Generator Semantics

Background

A highly requested feature of React seems to be a nicer way to loop and write conditionals inside JSX. For example this StackOverflow question about loops in JSX has over 300k views, and there are popular libraries such as jsx-control-statements written in order to address this fundamental feature of a templating language. This fostered discussions such as https://github.com/facebook/jsx/issues/39, https://github.com/facebook/jsx/issues/88, and https://github.com/reactjs/react-future/issues/35, suggesting syntax in order to allow for similar capabilities in JSX. With the new JSX semantics, you'd be able to do something like this:

<div>{ for (let item of items) yield item; }</div>
// loops!
<div className={ if (foo) 'bar'; } />
// conditionals!

while keeping the old behavior as well.

<div>{ items.map(i => <li key={i.key} />) }</div> // expression

<div style={{ position: 'absolute' }} /> // object literal

// these still work!

Proposal

The proposal is to use a combination of implicit do expression semantics as well as implicit generator expression semantics to fulfill a wide range of use cases. (By implicit, I mean omitting the do in do-expressions and the * in generator expressions). We'll extend the grammar for the JSXExpression block, to accept a list of statements. These statements are what you would usually find inside a Block, but it also allows the yield keyword. Additionally, there will be the following criteria:

  1. If it starts with {, then that is an ObjectLiteral, not a BlockStatement (Ignoring leading comments and open parenthesis). Similarly, if it starts with a (async) function expression, class expression, or generator expression, then it is of that following type.

  2. continue is not allowed at the top scope. They are allowed inside a nested block as long as they don't reference at a higher scope level than their own.

  3. return is not allowed except inside nested functions.

This allows for code like the following:

<div>{ for (let item of items) yield item; }</div>

The semantics for the evaluation of the code in-between the braces {...} are as follows:

// if yield keyword is used inside JSXExpression
$ReactJSXExpression((function*() { return do { ... } })())

function $ReactJSXExpression(generator) {
  let yields = [];
  let next;
  while (next = generator.next()) {
    if (next.done) {
      break;
    } else {
      yields.push(next.value);
    }
  }
  return yields;
}
// if no yields inside the JSXExpression
$ReactJSXExpression(do { ... })

// adapted from https://github.com/facebook/jsx/issues/88#issuecomment-339022316

Test cases

https://gist.github.com/clemmy/f0de896e4853172d88f664ab23c517d9

Alternative Considerations

@sebmarkbage and I converged on the proposal I outlined above after a lot of discussion. At first glance, it probably looks a bit weird since the behavior of the same syntax differs depending on whether a yield is present or not, but hey, that's better than one of our thoughts to make that behavior dynamic based on runtime. 🙂 Here are some of the alternative proposals we considered:

Always using generator expression semantics:

Cons:

This would be a breaking change with current JSX since yields will always be returned as an array; the following wouldn't be valid anymore:

<div style={{ position: 'absolute' }} /> // object literal

In order to fix this, we use a modified generator expression semantics approach:

Always using generator expressions semantics # 2

$ReactJSXExpression((function*() { return do { ... } })())

function $ReactJSXExpression(generator) {
  let yields = [];
  let completion = undefined;
  let next;
  while (next = generator.next()) {
    if (next.done) {
      completion = next.value;
      break;
    } else {
      yields.push(next.value);
    }
  }
  if (yields.length > 0) {
    return yields;
  } else {
     return completion;
  }
}
// adapted from https://github.com/facebook/jsx/issues/88#issuecomment-339022316
Cons:

The behavior is dictated by the runtime (e.g. whether we return completion or yields is based on the runtime evaluation of the length of yields).

Do Expression by default and explicit generator expression syntax

For example, we can use [ ... ] to indicate that it's a generator expression. This allows the following code, which is actually quite nice:

<div>[ for (let item of items) yield <Item key={item.id} item={item} />; ]</div>
Cons:

This would break existing JSX where an opening [ is a part of the text inside JSXChildren.

Do Expression by default and explicit generator expression syntax # 2

Similar to the above [ ... ], but we use a star instead (*{ ... }). It was hard to decide between this one and the main proposal.

Related Links

sebmarkbage commented 6 years ago

Ok, I noticed another problem with this change.

function and class now become declarations instead of expressions.

This makes this syntax errors:

<div onClick={function() { }} /> <Foo class={class { }} />

Since you can't have unnamed function/class declarations.

These two evaluate to undefined.

<div onClick={function handleClick() { }} /> <Foo class={class Foo { }} />

That's a pretty bad breaking change.

We'll have to figure something out for these.

We could do the same thing that we do for object literals and lookahead?

They'll all have the same problem. They require over-parenthesization when used in the statement list form.

<div style={if (c) ({ })} /> <div onClick={if (c) (function() { })} /> <Foo class={if (c) (class { })} />

syranide commented 6 years ago

:+1: But I would much prefer making do-expression vs generator explicit.

PS. Would {*...} not work too? Or might it be problematic in the future? IMHO *{...} seems very unintutive, <div>1*{x}</div>.

kasperpeulen commented 6 years ago

I agree with @syranide. If we can find a way to be more explicit, about that it is a do* instead of a do expression, I think that would be easier to parse, more consistent and more predictable.

We were discussing about introducing keywords with an asterisk behind (if, for, switch, while), in in this issue:

https://github.com/sebmarkbage/ecmascript-generator-expression/issues/1#issuecomment-342539225

It would then look like this:

<div>{ for* (let item of items) yield item; }</div>
clemmy commented 6 years ago

@loganfsmyth brought up a good point which I'll quote here:

I think for me the selling point of JSX, and the main reason I've felt fine having it grandfathered into Babylon even though we don't normally allow much non-standard syntax, is that it had a very clear PrimaryExpression position and otherwise was mostly straightforward parsing-wise. I will say the ambiguity between JSX and Flowtype is a huge pain though. I don't have as much of a problem with implicit do because at least that's behavior that is defined in the spec based on completion value already, so instead of "implicit do" you could frame it as "implicit statement completion" or something to not tie it to the do expression proposal specifically, but the implicit generator parsing seems super scary to me, because now you're actually co-opting existing language functionality that you'd expect to work one way to instead work another way, due to this hidden function wrapper.

const buildElement = co.wrap(function* fn(){
return (
<div class={yield foo()} />
);
});

for example would semantically map fine to just JSX + do expressions, as a way to incrementally build up a JSX result, but now you've co-opted the existing yield keyword into something that in a very unclear that requires the users to know intricate details of how JSX is handled.

The further JSX diverges from standard JS semantics, the less I feel comfortable supporting it and promoting it in the community.

sebmarkbage commented 6 years ago

First, let me just state my perspective on the state of things. I think that JSX is not a given winner. There are alternative approaches like templates that can come up with whatever syntax to overcome obstacles like this. But also JavaScript as the source language isn't even a given winner for this since new forks can avoid existing syntactic quirks. If we don't stay on top of getting the best expressivity then JSX will probably die out anyway. That's why I'm overall pretty positive towards risky and aggressive designs since worst case it'll just go away naturally anyway.

The * in generators is a wart on JavaScript IMO. Esthetically it is very problematic and it is a JavaScript specific quirk. E.g. C# just has the yield keyword be the thing that determines it (the return type also helps clarify that). Python just has the yield keyword but also has different semantics for generators.

One of the reasons I constantly stay away from using generators in APIs is because of that quirk. That also seems to be the case in JS overall. Maybe it's just that we don't have use cases that require them but I also wonder if people partly stay away from designing APIs to use them because of the ugly star. If I thought it worth while I'd propose dropping it in function generators too.

That said, I'd be fine starting off with a * disambiguator which is also what I have in the generator expression proposal. Then possibly revisiting an extension that makes it implicit (perhaps even for function declarations, function expressions and arrow functions).

Where should it go though? Put it on other statement keywords like for*, if*... won't work because a primary use case of these is to introduce temporary variables.

<div>*{ let i = 0; for (let item of items) yield <Item index={i} item={item} />; }</div>

I prefer *{ ... } overall since it looks like a generator function with things removed rather than an inside out generator function. It is also what I had in the generator expression proposal but that might not work in that context due to the ** operator. let x = x ** {} is already a thing. EDIT: Never mind. It works because ** is a separate token.

Putting it on the inside also seems risky and looks weird outside the JSX context.

We already put ... on the inside but that's slightly different since it's not combined with a block.

IMO double curlies is a non-starter: <div>{*{ }}</div>

In a way it almost seems less risky to make it implicit since it avoids potential future syntax quirks. JSX can also have breaking changes so it's not necessarily forever if it turns out to be a mistake.

Regarding @loganfsmyth's example:

const buildElement = co.wrap(function* fn(){
    return (
        <div class={yield foo()} />
    );
});

That's a pretty compelling counter-example if people used generators more. I'm not too worried about this showing up that might in practice because they're so rarely used today. I could probably be convinced that there is a future use case.

A sigil would be fine if we could find one that is natural so it sufficiently solves the problem of indicating that something special is going on. Having a very long indicator like double bracing or keywords is probably a non-starter. Also doing nothing at all and leaving this as a PrimaryExpression is not an option IMO.

sebmarkbage commented 6 years ago

I think for generator expressions for JS outside JSX maybe do * { } is appropriate. Since we're opting for do like semantics implicitly here. It might make sense to use the star before the block as a parallel.

<div>*{ for (let item of items) yield item }</div>
<Component
  prop="foo"
  items=*{ for (let item of items) yield item }
/>

Most likely they will be multi-line:

<div>
  *{
    let i = 0;
    for (let user of activeUsers) {
      i++;
      let isEven = (i % 2 === 0);
      let id = user.id;
      let name = user.name;
      if (id && name !== 'DELETED') {
        yield <User name={name} id={id} />;
      }
    }
  }
</div>

I think the confusing (and breaking) case is when you actually use * as part of JSX text.

<div>
  This is a paragraph* with some asterisk.
  *{explanation}
</div>

It would render the explanation without the *. But that's linter space to warn about.

ljharb commented 6 years ago

We should optimize for the "pit of success" - minimizing a super easy-to-stumble-into footgun as "linter space" isn't going to achieve that.

sebmarkbage commented 6 years ago

@ljharb Do you have a concrete concern? Not sure which angle you're referring to.

ljharb commented 6 years ago

@sebmarkbage sorry for the confusion; I was responding to

I think the confusing (and breaking) case is when you actually use * as part of JSX text.

sebmarkbage commented 6 years ago

Right. In this case the * helps because it would kick off this type of lint rule. Right? I think most of the scenarios I've been able to think of has similar easily lintable things. Do you foresee other issues with this approach?

ljharb commented 6 years ago

fwiw, that particular lint rule also inspired require-await, which is a horrible rule and actively harmful; I'm not sure that's a great litmus test.

Forgetting yield makes a generator useless, but not broken. In this case, the lint rule would be required to avoid something being broken.

In general, I don't like the implicit nature of this proposal - I think when do expressions exist, {do { }} works just fine, and requires no extra syntax.

sebmarkbage commented 6 years ago

I honestly don't know how to weigh criticism in syntax discussions. This happens a lot in TC39 too.

I feel like in these discussions it either always turn out as 1) fear of new syntax was overly cautious, people learned it quickly and benefited despite concerns. 2) people learned how to use something and benefitted but the overall complexity and gotchas increased. 3) it only increased gotchas and people stopped using it after a while or at least a majority did.

Every single addition there is a request for more explicitness given previous history and context. In cases where that happens, every time there is a request to drop the bloated syntax to make it cleaner or a feature isn't used because it is incumbent by bloated syntax.

What criteria should we use to determine? It's hard.

syranide commented 6 years ago

Every single addition there is a request for more explicitness given previous history and context. In cases where that happens, every time there is a request to drop the bloated syntax to make it cleaner or a feature isn't used because it is incumbent by bloated syntax.

@sebmarkbage tl;dr ahead. Indeed that is an objectively almost impossible task. I'm nitpicking on words perhaps, but it sounds like being explicit avoids your concerns, because it should not increase gotchas as you actually have to choose (single value/function vs push array/generator) and once you choose the behavior is consistent with JS counterparts. I think it also helps readability, to make it clear if what you're looking at is a do-expression or generator function.

If you make it implicit, then yes, the user does not need to choose and it just "works". But if they do not know whether they want to emit a single value or a push array then they're just blindly typing code and hoping for the best. I would be genuinely surprised if you wouldn't find at least a handful of people that would exclusively use yield, because "it's just more powerful". People already have problems understanding why keying is important and I think implicit generators will make that even less obvious. So if they do not make a conscious decision and not really understand WHY it just works, then I just see the gotchas piling up. Including refactoring, you have to actively choose between function/do-expression vs generator, you still have to understand the difference.

IMHO, the biggest failure of JS is the == and related operators. It would have been extremely useful IF it had worked consistently well. The fundamental problem is that it tries to remove the need for the user to understand "types", the goal is praiseworthy. But without that knowledge you'll also not understand the dangers and pitfalls of it. Once you understand it, this helpfulness just becomes complexity as you try to figure out whether or not it's going to do what you think it should, instead of just telling it explicitly what it should do.

Now, it's not the same, but if the TC39 committee had decided to merge functions and generators then it would be a nobrainer to do the same for JSX, but I can only assume they did so for a good reason. If we keep them separate, then it's easy to just point to the documentation of both, many will probably already know it. If you mix the two, you've invented something new that people has to learn that is entirely JSX-specific.

As you say, it's a problematic situation. But I think it boils down to a simple question; is JSX an extension of JavaScript, or a designer DSL:ish templating language on-top. If it's an extension of JS then it should be faithful to JS, it's only meant to sand down the rough edges between "React" and JS. If DSL, then go wild and do anything that you think simplifies the lives of "designers". But I don't think it should try to be both. Right now I think it's somewhere in between (with proposals on both sides as well), but still salvageable both ways.

Anyway, this is just my perspective on situation and trying to add to the discussion. Do what you want with it. There is no objectively best solution without an unambiguously stated philosophy to benchmark against. So ultimately I think progress is most important and if the discussion is not changing your mind then I have more faith in your vision of what JSX should be over bike-shed "design by committee".

kasperpeulen commented 6 years ago

The * in generators is a wart on JavaScript IMO. Esthetically it is very problematic and it is a JavaScript specific quirk. E.g. C# just has the yield keyword be the thing that determines it (the return type also helps clarify that). Python just has the yield keyword but also has different semantics for generators.

@sebmarkbage First, let me say that I'm super happy there is actively being worked and thought about this. My main concern is that JSX works differently then Ecmascript.

I think it is worth discussing if we can "improve" the syntax for generators. If C# implicitly uses yield to determine if a function is a generator or a normal function, then that would be interesting to see if that could also work for javascript. That would also solve the problem that arrow functions can't be generators.

But I would prefer that JSX and Ecmascript works the same.

So, if JSX doesn't require a star, but uses the yield keyword, to implicitly determine if if it is a generator expression or a do expression, then I would expect that functions (and async functions) work like this, and that do expression work like this.

For example, I would expect the below two are valid syntax:

let gen = (() => {
  yield 1;
  yield 2;
})();

let gen = do {
  yield 1;
  yield 2;
};

If so, this proposal is not really needed anymore: https://github.com/sebmarkbage/ecmascript-generator-expression

You could then also in ecmascript just write:

let arrayOfUsers = [...do {
  for (let user of users)
    if (user.name.startsWith('A'))
      yield user;
}];

This would do then the exact same as the proposed *{...} generator expression blocks, you only need one char more, and you get rid of the (ugly?) star.

kasperpeulen commented 6 years ago

In general, I don't like the implicit nature of this proposal - I think when do expressions exist, {do { }} works just fine, and requires no extra syntax.

@ljharb This is not true, you can not create generators with the current do expression proposal, and babel plugin. For the non-generator case, there is one big annoyance as it comes to do expressions in JSX templates. It gets very noisy quickly and you get a lot of indentation. For example, consider this example:

<div className="col">
  {
    do {
      if (timer == null) {
        <button className="btn btn-info btn-lg" onClick={this.startTimer}>
          Start Timer
        </button>;
      } else {
        <button className="btn btn-danger btn-lg" onClick={this.endTimer}>
          End Timer
        </button>;
      }
    }
  }
</div>

With this proposal, you have less noise and 4 spaces less indentation:

<div className="col">
  {if (today.breaks.current() == null) {
    <button className="btn btn-info btn-lg" onClick={this.startTimer}>
      Start Timer
    </button>;
  } else {
    <button className="btn btn-danger btn-lg" onClick={this.endTimer}>
      End Timer
    </button>;
  }}
</div>

This is also depends on how prettier would handle this, and maybe prettier could parse do expressions, so that it looks less noisy. But I do think this proposal would be a win readability.

ljharb commented 6 years ago

@kasperpeulen i'm talking about the part where the last value is returned. If jsx wanted to add do * { } or something, that'd be fine and explicit - but i think the do { } wrapper is required in order for "the last statement magically ends up being the completion value" to make sense.

clemmy commented 6 years ago

@sebmarkbage How does this look:

JSXGeneratorExpression :

- ObjectLiteral
- FunctionExpression
- ClassExpression
- GeneratorExpression
- AsyncFunctionExpression
- [lookahead &#8713; { `{` }] StatementList<sub>[+Yield]</sub>

Seems like we might need the additional GeneratorExpression and AsyncFunctionExpression here because those can also be unnamed?

I'm going to put up a new proposal based on the feedback in this thread, but please feel free to keep discussing this one here.

clemmy commented 6 years ago

Wondering what people's thoughts are on keeping the old JSXExpression semantics when using { ... }, and having explicit syntax for do generators in the form of *{ ... }, which corresponds to the proposal. I made the PR here: https://github.com/facebook/jsx/pull/99

ljharb commented 6 years ago

@clemmy i think adding support in jsx for something that’s not even stage 1 yet (ie, it might not make it into the language at all) is premature; but i like that much much better than implicit alternatives.

clemmy commented 6 years ago

Closing this issue for now in lieu oF explicit generator expressions.