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

Aggressive JSX Do Expression Proposal #88

Open
sebmarkbage opened this issue Aug 22, 2017 · 20 comments
Open

Aggressive JSX Do Expression Proposal #88

sebmarkbage opened this issue Aug 22, 2017 · 20 comments
Labels
Proposal 2.0 Proposals considerable for JSX 2.0

Comments

@sebmarkbage
Copy link
Contributor

sebmarkbage commented Aug 22, 2017

This is my current take on #39

Do expressions in JS are moving quite slowly because it's not very pretty but there seems to be a direction that is sort of shaping up and compromises that occur. JSX users are also the ones that want this the most.

Therefore, I'm proposing that we change up how we do things a bit and instead of pursuing the JS route first, we become an early experiment surface for a more aggressive proposal.

The major rule of this proposal is that we should be backwards compatible with existing JSX for anything but edge cases that nobody would write. That helps us avoid a much larger conversation of #65.

The proposal that all JSX containers are implicit do-expressions with a few tweaks:

  1. If it starts with {, then that is an ObjectLiteral, not a BlockStatement. (Ignoring leading comments and open parenthesis.)

  2. The completion value of a for, for...in, for...of, do or while loop is a newly created array with the completion value of each iteration in it. If it's a continue then the completion is undefined. The semantics of this doesn't need to be specified in this spec but it needs to have the syntactic mechanisms in the spec to allow implementations and ASTs to be able to do this in a reasonable way.

  3. continue, break are not allowed at the top scope of these do-expressions. They're allowed in nested blocks but a break and continue inside a nested block is not allowed to reference a label outside the inner most do-expression. If you break out of the last statement, that completion value is undefined.

  4. return is not allowed inside a do-expression other than inside nested functions.

  5. throw expressions are allowed per https://github.com/rbuckton/proposal-throw-expressions but not specified separately.

Are there any other tweaks we'd have to do to remain backwards compatible with the expression form?

@syranide
Copy link
Contributor

syranide commented Aug 23, 2017

2+4; it seems weird to allow continue here but not return inside the do-expression. Surely they're conceptually the same thing? If you allow return then it's in a sense weird to not be able to conceptually continue-return too.


2+4; it means you cannot move logic from outside of elements to inside of elements and vice-versa without refactoring the code.

if (cond) {
  return <Foo />;
}
let user = getUser();
return <User foo={user} />;
<div>{
  if (cond) {
    <Foo />;
  } else {
    let user = getUser();
    <User foo={user} />;
  }
}</div>

It's certainly not the end of the world, but in the second example it would be nice if I could just copy-paste the code into a function and be done with it. In the first example I can just call the function as <div>{foobar()}</div> and it's done. Due to the different semantics you can't even reliably wrap it in a do-expression. If "rewriting code" is a valid concern then it is obviously a larger discussion, but it's obviously a complex subject as it's discussion about how JSX should be written.

Could it be an idea to assume/advocate a do-expression/everything-inside-a-fragment style for writing JSX?

function render() {
  return (
    <div>{
      if (cond) {
        <Foo />;
      } else {
        let user = getUser();
        <User foo={user} />;
      }
    }</div>
  );
}

Can now be trivially refactored into:

function foobar() {
  return <>{
    if (cond) {
      <Foo />;
    } else {
      let user = getUser();
      <User foo={user} />;
    }
  }</>;
}
function render() {
  return (
    <div>
      {foobar()}
    </div>
  );
}

Where foobar() could now be directly moved into its own component as a render() method too, no need to rewrite its code. Primarily this decision is interesting in that if one would like to move in this direction, JSX has a very clear goal; all use-cases must be able to written in that style and still be readable. It would then be clear what further quality-of-life improvements need/can be made, whereas now the default is always "just rewrite it as plain statements".

This is rather off-topic in some sense and it may not be a good idea to continue this particular discussion in here. So I leave that up to you so that this issue doesn't derail from its technical questions.

@sebmarkbage
Copy link
Contributor Author

To phrase it differently return, continue and break are all the same in that you're not allowed to use them to escape the do-expression. The only thing special about return is that it doesn't have a way to stay inside the do-expression.

I assume that return inside of do would return from the outer function - not just break out of the do-expression. That's what's confusing and controversial. So I think we should disable it.

I think the fragment solution is neat. It does allow a way to opt in to this new style. However, my hope is that it doesn't end there but that we'll able to have a more general solution to this. Either by making JSX expressions in last position have implicit returns like that other thread, or do-arrows.

let Foo = () => do {
  ...
};

@syranide
Copy link
Contributor

I assume that return inside of do would return from the outer function - not just break out of the do-expression. That's what's confusing and controversial. So I think we should disable it.

I would assume the opposite actually; you can't function-return inside an expression so why should you be able to function-return inside a do-expression. But I guess reusing return may be confusing either way.

I think the fragment solution is neat. It does allow a way to opt in to this new style. However, my hope is that it doesn't end there but that we'll able to have a more general solution to this. Either by making JSX expressions in last position have implicit returns like that other thread, or do-arrows.

Depending on the exact details of these JSX features, your do-arrow above would be equivalent to <> in my example no? If that's the case, I think there's an upside to my example in that you can afford some unorthodox behaviors without too much fuzz inside such a JSX-specific syntax.

Having the last statement be always be implicit return inside functions is kind of strange, but useful. Inside <> (doesn't have to be that syntax) it's not really all that strange at all, for-loops can push to arrays etc, it wouldn't really be that strange. But obviously, it depends on the specifics, but I think there's something tidy about making JSX code obvious rather than behave magically among other JS.

@loganfsmyth
Copy link

Is there precedent for disallowing return/continue/break in other languages with expression logic like this? Like I know Rust is totally fine with those in nested blocks. I'm not sure why that restriction would be desirable in the general case. You can already do those operations inside nested blocks in JS and it's no different when inside JSX expression blocks.

For the loop behavior, personally I think that it's a little too far to jump to looks collecting their results into an array. At that point you're introducing your own comprehension syntax, except it looks exactly like normal JS code other than being in completion location. Why is this more desirable than the existing .map approach?

I may be missing something, but why is calling out throw important? If it's a general do expression, throw will already be allowed as a statement.

@sebmarkbage
Copy link
Contributor Author

sebmarkbage commented Aug 23, 2017

Is there precedent for disallowing return/continue/break in other languages with expression logic like this?

Rust has a different semantic than what @syranide expected and thought was the more useful semantic. It seems like there are at least three plausible options here but starting disallowed at least opens up for making the other choice later. Especially given that this use case isn't critical nor even that common in the context of JSX.

Why is this more desirable than the existing .map approach?

Basically there's two control flows from templating languages that people are missing. <if /> and <for />. People seem to have a stylistic aversion to ternaries and the .map approach. I can see that. It may not be absolutely critical to solve that but neither is any of JSX. JSX is all about style preference. This comes up a lot in design of syntax sugar. If you compromise too much on the style, then the whole value of syntax sugar disappears.

Now I think that what people really ask for is to express these things as elements. I'm not sure if that's because there's inherent value is the brackets or if that's just because lack of other ideas. Handlebars and such don't use elements for this but has specialized syntax. I do think that element based control flow is a bit too far away from the spirit of JSX. So this is an attempt at solving it without going down that route. That said, this is a kind of compromise that may not solve the core of the problem.

For the loop behavior, personally I think that it's a little too far to jump to looks collecting their results into an array. At that point you're introducing your own comprehension syntax, ...

I agree, it's a big leap. I was considering comprehensions but I think that is a much bigger can of worms. It also is much further way from what people consider intuitive syntax. Meanwhile whenever we talk about do-expressions, people seem to assume that for-loops will work so it seems it is strangely intuitive.

if conditionals are nice but if we don't have a solution to loops, I don't think it's really worth doing at all since it's only half the story.

I may be missing something, but why is calling out throw important?

Since other control-flow is not allowed, it may seems like throw would fall into that category but it is special because it doesn't suffer from the same confusing cases as the others, and is already defined in expression forms so it has known semantics.

@sebmarkbage
Copy link
Contributor Author

I'm getting more convinced that for returning an array is not the right way.

The argument for comprehensions is usually to make it easy to write in a functional compositional style and make it approachable. E.g. nicer than writing filter and map. The counter-argument is that once you do anything complex in this style you end up needing filter, map, etc. and it actually ends up looking better.

So why push better for loops support? Because we see them all the time, but the reason isn't because people write in the functional compositional style, it's the opposite. When a loop gets complex, people break out of filter/map and switch to imperative loops since they seem easier to reason about.

I think the problem to solve is to make the transition from simple loops to imperative loops easy. With temporary variables. I'm not sure implicit return values from for loops solves that use case well... but maybe?

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

->

<div>{
  let active = [];
  let inactive = [];
  for (let item of items) {
    if (item.active) {
      active.push(<ActiveItem item={item} />);
    } else {
      inactive.push(<Item item={item} />);
    }
  }
  <>
    {active}
    {inactive}
  <>
}</div>

This works but the edge cases are pretty confusing because 1) you can't reference the variables active and inactive in other JSX expressions. I.e. outside the block. 2) You might accidentally return the for loop by doing something like:

let active = [];
let inactive = [];
<div>
  {
    for (let item of items) {
      if (item.active) {
        active.push(<ActiveItem item={item} />);
      } else {
        inactive.push(<Item item={item} />);
      }
    }
  }
  {active}
  {inactive}
</div>

However, the alternative of forcing people to write the .push manually is also not very palatable for simple loops.

<div>{
  let allItems = [];
  for (let item of items) {
    allItems.push(<Item item={item} />);
  }
  allItems;
}</div>

I wish this could be simplified just a little bit to make the transition to the complex example more natural while still letting the simple example remain concise.

@sebmarkbage
Copy link
Contributor Author

sebmarkbage commented Oct 10, 2017

Maybe they should be implicit immediately invoked generator functions?

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

Where the completion value is an implicit return at the end.

@syranide
Copy link
Contributor

syranide commented Oct 10, 2017

Maybe they should be implicit immediately invoked generator functions?

@sebmarkbage Something along those lines makes most sense to me, having to allocate the array and return it is what adds most overhead. I still think being explicit about when you're adding to the array is for the better, IMHO it's not really nuisance either. Being able to effortlessly push, or in your example yield, to an implicit array is the big thing.

On a tangent, I personally would apply a similar reasoning to the discussed do-expressions as well. I wouldn't mind having to write return if there is more than one expression/statement. I.e. {foobar}, {let foobar = 1; return foobar;}, or separate syntax. Essentially, either it's one expression, or many statements wrapped in an implicit self-executing fat arrow function, I think there are many benefits to it. The nuisance is not writing return, it's that you have to wrap it in a self-executing fat arrow function.

I'm not personally a fan of the functional style for templates; it tends to only be helpful to a point which rarely covers the vast majority of the cases IMHO.

Maybe they should be implicit immediately invoked generator functions?

Self-executing or called by React?

PS. Did you just thumbs up your own comment? 😄

@kasperpeulen
Copy link

@sebmarkbage
Copy link
Contributor Author

sebmarkbage commented Oct 24, 2017

The revised proposal is now these semantics for React:

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

(This is not the code we'd ever actually generate since we could optimize away the generator completely since we know it'll be immediately invoked and added to an array. So it would compile to .push().)

Note how it either uses the yields or the completion value conditionally. So you can continue just avoiding the yield keyword for single items and switch to using it for lists.

It is a little weird that conditionally executing a yield keyword either produces and array or undefined, but that's mostly fine for React since undefined are most equivalent.

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

You could even use yields to explicitly return a value.

<div className={if (foo) yield 'bar'} />

However this value would be wrapped in an array. We could potentially unwrap it for single items. Unfortunately that can have consequences for React key semantics, since this would not preserve state when switching between an array of one item vs. two items:

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

PS. Did you just thumbs up your own comment? 😄

Yes. I got excited about this. :)

@sophiebits
Copy link
Contributor

To what extent is it possible to special-case single expressions that have no semicolon? I think in my ideal world…

  • {a} would evaluate to a
  • {a;} and {a;b} and {a\nb} would evaluate to nothing
  • if (1) yield x; y; would evaluate to [x]
  • if (0) yield x; y; would evaluate to nothing <-- it seems so confusing that it would be y with your proposal

So the completion value basically has no effect. This does require parser magic to treat ASI differently.

@sebmarkbage
Copy link
Contributor Author

Semi colon is probably a no-go.

I think the way you would solve this is to have a linter guarantee that the completion value is always undefined if there is a yield.

The JS way of solving this is adding a * sigil but it's so ugly.

@sophiebits
Copy link
Contributor

Why can we forbid it in the linter but not here? What about in our babel transform?

@sophiebits
Copy link
Contributor

I guess it can’t happen in the transform if the tree is the same.

@sebmarkbage
Copy link
Contributor Author

We could do this at the compiler level where we detect if there is a yield keyword at all in there to switch between generator or completion value.

The nice part of doing it at runtime is that it can be expressed as a generic JSX feature without semantic meaning. The semantics can be completely a simple runtime hook.

@syranide
Copy link
Contributor

However this value would be wrapped in an array. We could potentially unwrap it for single items. Unfortunately that can have consequences for React key semantics, since this would not preserve state when switching between an array of one item vs. two items:

Isn't there precedent for this? React already has a special-case for this because of the single-item optimization, i.e. <a>{<b/>}</a> reconciles with <a>{[<b/>]}</a>. Although <a><x/>{<b/>}</a> does NOT reconcile with <a><x/>{[<b/>]}</a>, which is still something I think should be looked at, but there's precedent (whether good or bad).

@clemmy
Copy link
Contributor

clemmy commented Oct 25, 2017

@sebmarkbage I think you're missing a bracket in
$ReactJSXExpression(function*() { return do { ... } })())

I really like this proposal though. One thing I'm wondering about is the difference in styles for doing conditionals:

{
  if (1) yield a;
  else yield b;
}

v.s.

{
  if (1) a;
  else b;
}

One would return an array, and the other would return a single element, right?

@ghost
Copy link

ghost commented Jan 3, 2018

Apologies I just made this comment on #39 but it looks like the discussion has moved over here.

An additional comment to what's below: I'm worried that in proposing this we're adding complexity to the language in an attempt to compensate for complex code without fully embracing the existing language functionality. That is, by doing this we're making things more complex in the name of simplicity, which is probably the wrong way to go.

Original Comment
This seems like a good idea at first glance but I'm concerned it encourages spaghetti code.

I can't think of a use case that couldn't already be implemented in a better way, predominantly through encapsulation. In simple cases bool expressions are fine (the classic {foo && <bar foo={foo} />}), in more complicated cases encapsulation provides cleaner code and better reusability (for better or worse).

Taking the example from the linked tweet in the original post, we can tidy it up through encapsulation as:

function UserMenuItem(props) {
  return props.user ? <UserInfo user={user} /> : <LoginForm />;
}

class Menu {
  render() {
    return <div>
      <h2>Menu</h2>
      <UserMenuItem user={this.props.user} />
    </div>;
  }
}

We could go a step further and make Menu itself a function-style component, further simplifying the code.

Edit: This is the tweet referred to above: https://twitter.com/vslinko/status/619581969903550464

@ghost
Copy link

ghost commented Jan 3, 2018

In short when code becomes complex within JSX markup it's best to move the code out of the JSX and replace it with an element, not make it easier to put more code in the JSX markup. We don't want JSX to become the new PHP. ;-)

@syranide
Copy link
Contributor

syranide commented Jan 3, 2018

@ogwh Over-abstraction is also a serious readability problem, simple problems should be simple to understand at a glance. The current situation is that even a lot of should-be-simple cases become bloated code-wise, this is what the syntax tries to solve. The intended benefits far outweigh the problem of adding another way to shoot yourself in the foot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Proposal 2.0 Proposals considerable for JSX 2.0
Projects
None yet
Development

No branches or pull requests

7 participants