facebook / jsx

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

JSX Statements & Implicit returns #85

Closed jamiebuilds closed 7 years ago

jamiebuilds commented 7 years ago

I raised this idea awhile ago with @sebmarkbage, I figured I'd write it down since I keep wanting it.

Right now you can only place JSX in PrimaryExpression positions (it is a parse error elsewhere, at least in Babel's implementation). If we created a JSXStatement we could add an implicit return.

Before:

function Component() {
  return (
    <div/>
  );
}

After:

function Component() {
  <div/>
}

At the top level this makes if-statements a lot nicer:

function Component(props) {
  if (props.items.length < 1) {
    <Empty/>
  } else {
    <List>...</List>
  }
}

This is a bit of a separate tangent, but...

By combining JSX statements with implicit returns with implicit do expressions (#65) it feels much more template like.

function Component(props) {
  if (props.items.length < 1) {
    <Empty/>
  } else {
    <List>{
      items.map(item => {
        <Item key={item.id}>{`Item: ${item.name}`}</Item>
      })
    }</List>
  }
}

At which point why even have the {...} in between tags?

function Component(props) {
  if (props.items.length < 1) {
    <Empty/>
  } else {
    <List>
      items.map(item => {
        <Item key=item.id>`Item: ${item.name}`</Item>
      })
    </List>
  }
}

Comparing that to the original, it is much nicer:

function Component(props) {
  if (props.items.length < 1) {
    return <Empty/>
  } else {
    return (
      <List>{items.map(item => (
        <Item key={item.id}>{`Item: ${item.name}`}</Item>
      ))}</List>
    );
  }
}

@kittens actually implemented something like this once I believe.

sebmarkbage commented 7 years ago

The most problematic part isn't actually the core proposal but the examples using for (...) { }. It's problematic making those return an implicit array. That's not how do-expressions work. They return the last item. This is also an issue for making JSX containers be do-expressions by default. That's a pretty confusing part of making this look more template like.

sebmarkbage commented 7 years ago

Actually, I'm not sure what your proposal is in terms of the JSX spec. Where would be the entry point in the ECMAScript spec for the JSXStatement? Why isn't PrimaryExpression enough?

syranide commented 7 years ago

Isn't this a weird thing to do when JS functions normally don't work like this? Special-casing just JSX elements to behave like this seems very wrong. I don't see why JSX should behave any different.

You also run the risk of creating some bad ambiguities in the language:

foo()
<bar />

Gets tokenized to "foo" "()" "<" "bar" "/>", i.e. foo() < bar .... Sure you can argue that no one would ever write comparisons like that, but JS supports it and avoiding it would require further special-casing JSX-parsing/JS newline rules (this should also be a problem for implicit do-expressions, but maybe less so).

Also, comparing apples to apples:

function Component(props) {
  if (props.items.length < 1) {
    <Empty/>
  } else {
    <List>{items.map(item => (
      <Item key={item.id}>{`Item: ${item.name}`}</Item>
    ))}</List>
  }
}
function Component(props) {
  if (props.items.length < 1) {
    return <Empty/>;
  } else {
    return <List>{items.map(item => (
      <Item key={item.id}>{`Item: ${item.name}`}</Item>
    ))}</List>;
  }
}

Is that really so bad? The behavior of this code is obvious to anyone and all you need to add is "return" at the front. Which also signals that you intended to return it and didn't just forget an assignment or a similar mistake.

tl;dr Personally, I feel that it is fundamentally wrong to special-case behaviors around JSX-elements like this, they should behave like any other JS-value. There is nothing to gain here by removing return other than it looks neater, but you lose readability which is far worse.

By combining JSX statements with implicit returns with implicit do expressions (#65) it feels much more template like.

function Component(props) {
  if (props.items.length < 1) {
    <Empty/>
  } else {
    <List>{
      for (let item of items) {
        <Item key={item.id}>{`Item: ${item.name}`}</Item>
      }
    }</List>
  }
}

If we assume array comprehensions will be introduced then you can already achieve really similar syntax while retaining the full expressiveness of the language:

function Component(props) {
  if (props.items.length < 1) {
    <Empty/>
  } else {
    <List>{
      return [for (let item of items) {
        <Item key={item.id}>{`Item: ${item.name}`}</Item>
      }];
    }</List>
  }
}

Mixing the spurious nice-to-have features of a "template language" into a programming language doesn't seem like something you should take lightly (i.e. just because it looks neater) considering the long-term implications for the language. You can easily make the same arguments around any other language feature then (which will easily block or interfere with other language features).

Like simplified template languages in general, the experience tends to degrade horribly once you try to do even just slightly more complex behavior and not just simple loops at which point it is now significantly more complex for someone to understand how to do it rather than having learned the slightly more verbose syntax to start.

But if there's an interest in it, why not just put it inside it's own scope keyword, jsxtemplate { ... } similar to do-expressions, inside there you can have it behave any which way you like, it's fine. Correctly designed it would also allow you to escape it when you need additional functionality. Although contextually changing semantics is a big problem in a general, it makes it hard to understand what's going on. And it's questionable what you've really won, you now have the base language and another language on-top of that, if you don't understand both intimately you're prone to make errors or misunderstand other peoples code. You've effectively made it much more complex to learn while only really making it simpler/more concise when you've mastered it.

jamiebuilds commented 7 years ago

but the examples using for (...) { }. It's problematic making those return an implicit array.

@sebmarkbage Ah yeah, I originally wrote them using .map but switched them over last minute to for-loops without thinking. I've updated the examples above.

Where would be the entry point in the ECMAScript spec for the JSXStatement? Why isn't PrimaryExpression enough?

I'm not entirely sure where exactly this fits into the ECMAScript spec, but assuming implicit do-expressions (and dropping {...}), I think you'd actually start treating all JSX expressions as statements. A nested JSX tag would also work as a statement since it'd technically be in a statement position.

<Statement>
  <Statement>
    if (foo) {
      <Statement/>
    }
  </Statement>
</Statement>

But without do-expressions, it make sense to have a JSXStatement or you'd be adding semantics to ExpressionStatement or something.

You also run the risk of creating some bad ambiguities in the language:

@syranide your example already doesn't parse, I'm not sure how you could create any set of tokens that would currently parse because of </ or />.

If we assume array comprehensions will be introduced

Array comprehensions were shot down previously and last I heard no one wanted them anymore.

Mixing the spurious nice-to-have features of a "template language" into a programming language doesn't seem like something you should take lightly (i.e. just because it looks neater)

That's literally all JSX is. But since you're being dismissive instead of considering it more carefully, here's a better example:


With the syntax we have right now, people will write JSX like this:

<div>{items.map(item => (
  item.children.length
    ? (
      <div>
        {item.children.map(child => (
          <span key={child.id}>{child.name}</span>
        ))}
      </div>
    )
    : (
      <div>no children</div>
    )
)}</div>

Reading this sucks, but the bigger problem comes when you need to do something like declare a variable inside your nested JSX. JavaScript provides you with no way to declare a variable in an expression position so you need to refactor the syntax of your component.

<div>{
  items.map(item => {
    let value = calculateValue(item);
    if (item.children.length) {
      return (
        <div>
          {item.children.map(child => (
            <span key={child.id}>{child.name}</span>
          ))}
        </div>
      );
    } else {
      return (
        <div>no children</div>
      );
    }
  )}
</div>

Instead, if we had JSX statements that were automatically returned, you could just write it like this all the time and if you need to declare a variable or something, it's a one line change.

<div>
  items.map(item => {
    if (item.children.length) {
      <div>
        item.children.map(child => {
          <span>`${child.name}</span>
        })
      </div>
    } else {
      <div>"no children"</div>
    }
  })
</div>

And this is still just a pretty small component with only four JSX elements.

bmeck commented 7 years ago

I would like to see a few search results for:

Before making any decisions.

I feel like there is probably someone out there which is using them in ExpressionStatement position already for odd effects, but would like to see how many / if they are prominent.

https://github.com/ChALkeR/Gzemnid can be used to get you a very rough guess to start, which is probably enough.

jamiebuilds commented 7 years ago

@bmeck I'm downloading the registry right now using gzemnid, it'll take a little while considering I'm on Australian internet though...

syranide commented 7 years ago

@syranide your example already doesn't parse, I'm not sure how you could create any set of tokens that would currently parse because of </ or />.

What? That's the syntax you proposed, being able to put a JSX-element by itself and have it be returned.

The JSX element syntax itself obviously works rights now, the difference is that JSX elements are currently only recognized as values which means they cannot be confused with comparisons at the moment. You're moving them to the statement-level where it does create conflict because of implicit semicolons rules in JS.

That's literally all JSX is. But since you're being dismissive instead of considering it more carefully, here's a better example:

I think there's merit, but I also think there's a way bigger picture here than just being simpler. Everything has to be considered and IMHO you're only presenting the immediate positives, not the short- and long-term negatives of the proposed solution.

Just the simple fact that you're introducing an entire new sub language is a potential problem in itself, it has a non-trivial learning curve and can fracture the community to some extent (personal preferences for which syntax to use).

Instead, if we had JSX statements that were automatically returned, you could just write it like this all the time and if you need to declare a variable or something, it's a one line change.

You're not comparing apples to apples. This is what you propose:

<div>
  items.map(item => {
    if (item.children.length) {
      <div>
        item.children.map(child => {
          <span>{child.name}</span>
        })
      </div>
    } else {
      <div>no children</div>
    }
  })
</div>

In this syntax there is no way to have text-elements, you've replaced it with implicit do-expressions. But sure, that can be resolved by reintroducing the curly braces but keeping the implicit do-expressions.

<div>{
  items.map(item => {
    if (item.children.length) {
      <div>{
        item.children.map(child => {
          <span>{child.name}</span>
        })
      }</div>
    } else {
      <div>no children</div>
    }
  })
}</div>

But then it doesn't seem far off to just add back the returns.

return <div>{
  items.map(item => {
    if (item.children.length) {
      return <div>{
        item.children.map(child => {
          <span>{child.name}</span>
        })
      }</div>
    } else {
      return <div>no children</div>
    }
  })
}</div>

And we're now back with plain old JS that everyone understands and it isn't much different from what you proposed. So from my perspective; what you're proposing is a big change from the current JS(X), so if the advantage isn't as big or bigger than the disadvantages of your proposal it intuitively doesn't seem like a good trade. I think JSX leaves a lot to be desired, but changes/improvements has to be considered as a whole.

jamiebuilds commented 7 years ago

@syranide I'm going to stop engaging with you now because you're coming off hostile.

syranide commented 7 years ago

@thejameskyle Also, note that your syntax is actually not functional.

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

<B /> would be discarded in the above example as "implicit return" would return upon seeing <A />, thus <B /> would be dead-code.

bmeck commented 7 years ago

@syranide ok, if that is dead-code that just means we need to see if any rules w/ similar effects to the goal of this proposal work as intended.

jamiebuilds commented 7 years ago

I'm closing this because I'm not going to deal with this dude. Someone else open a new issue if you care to see it happen.

Daniel15 commented 7 years ago

For what it's worth, this looks very similar to C# Razor syntax: https://docs.microsoft.com/en-us/aspnet/core/mvc/views/razor#control-structures. It could be viable given the prior art with a similar syntax.