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

JSXStatement - What does it mean? #87

Open sebmarkbage opened 7 years ago

sebmarkbage commented 7 years ago

This is building on top of discussions in #85 and #86. However those include a lot of meta discussion, I'd like to clarify something much more narrow before we go into how it fits and whether it's a good idea.

I think there is a reasonable motivation for doing implicit return contrary to other JS contexts requiring explicit return. An arrow function without curly braces with a do expression isn't much different. I could argue that if anything JS should've had this for all functions and this is just a special case of fixing it. I see the points of making the language difficult to understand overall but there's already confusion why you can't just do this. JSX is seen as something slightly different. Not necessarily as just expressions. We don't have to decide on this now.

The semantics of JSX expression nor a statement is not up to this specification. It can be implemented differently in different transpilers and such. This repo is just meant to define the syntax without semantics.

JSX is already allowed as an expression statement. I'm not so concerned about breaking changes in this position at this point.

What I'd would like to clarify exactly what syntax differences would making a JSXStatement with no other changes include at this point?

@syranide you mentioned that it would affect ASI. Can you elaborate on that?

cc @bmeck

syranide commented 7 years ago

@sebmarkbage Can you clarify what you mean by ASI?

sebmarkbage commented 7 years ago

Automatic Semicolon Insertion

syranide commented 7 years ago

@sebmarkbage Ah ofc. What I posted in that issue:

foo()
<bar /> // implying implicit return

Would get tokenized to "foo" "()" "<" "bar" "/>" and thus normally parsed as foo() < bar .... AFAIK the only way around that is to special-case is somehow, either through ASI-rules or by making exceptions in the parser. So it can be worked around, but not "solved". Unless I'm mistaken?

bmeck commented 7 years ago

Can you clarify why foo() and <bar /> are related. Currently ASI would keep them as separate ExpressionStatements:

http://astexplorer.net/#/gist/8fcdde72ae0863f9a0947fb81774338e/cd254fb0b8c50f7711dc75f7f10b5398303b6ea9

syranide commented 7 years ago

@bmeck Remove the semicolon, this is about implicit semicolon: http://astexplorer.net/#/gist/8fcdde72ae0863f9a0947fb81774338e/56027b28ba324ac2a492fd62e3302aeff8834a3f

bmeck commented 7 years ago

Ah, so it fails to parse when it would be in a position that is potentially given an ASI. I would state not omitting semicolons would be a good thing since it doesn't have this problem.

Since this is an early error and doesn't allow code to evaluate/parse. I would throw this on the pile of ASI oddities unless there are good reasons to fix it?

Edit: the same class of oddities that need you to add semicolons before leading operators, [, and (.

sebmarkbage commented 7 years ago

We would need a spec mechanism to fix it while being backwards compatible. Since things like this is already valid JS:

foo()
<i>text</i></i>
bar()
bmeck commented 7 years ago

@sebmarkbage that is a good example for backwards compatibility. Nested and adjacent JSXElements always have >< I think though. Is there an example of nested/adjacent grammar collision?

foo()
<i>
<i>text</i></i>
bar()

Fails.

Also, is there an example that is in a completion position given to a function body where implicit return is being looked at.

syranide commented 7 years ago

I think there is a reasonable motivation for doing implicit return contrary to other JS contexts requiring explicit return. An arrow function without curly braces with a do expression isn't much different. I could argue that if anything JS should've had this for all functions and this is just a special case of fixing it. I see the points of making the language difficult to understand overall but there's already confusion why you can't just do this. JSX is seen as something slightly different. Not necessarily as just expressions. We don't have to decide on this now.

@sebmarkbage I'm not sure if I fully understand what part of the previous discussions you're referring to here. That return <A /> and <A /> are the same? Or that <A>{if (cond) <B/>}</A> should implicitly return <B />? If it's the first then I don't think I understand your reasoning. If it's the second then I fully agree unless the it turns out to be very problematic, as per earlier discussions I think JSX could greatly benefit from (something like) it.

sebmarkbage commented 7 years ago

@syranide I don't even want to have that discussion here yet. I'm not proposing any semantics yet. I'm just saying that's plausible and therefore we could consider carving out a JSXStatement in the syntax without semantics to allow that experiment and discussion to happen elsewhere. That could mean throwing, returning, goto, or whatever a transpiler thinks it should do.

syranide commented 7 years ago

@sebmarkbage Ah ok, now I'm with you. :+1:

sebmarkbage commented 7 years ago

@bmeck What I'm referring to is JS without JSX has that as valid syntax. Even if it's technically not ambiguous it would require a lot of look ahead to see if it is and cover grammar doesn't work for these.

syranide commented 7 years ago

What I'm referring to is JS without JSX has that as valid syntax. Even if it's technically not ambiguous it would require a lot of look ahead to see if it is and cover grammar doesn't work for these.

@sebmarkbage I would assume the solution here would simply be to special-case < after an ASI-candidate newline; if there's < and a valid JSX-identifier (without space) then it is a JSXStatement (preceded by an ASI). I don't see any other way to disambiguate these two that would actually make sense. Looking further ahead and trying to see if it's valid JSX-element syntax is bad for all kinds of reasons (even if we could do it). You could possibly stretch it to check for the next token after the JSXIdentifier too, but I don't think it would really matter.

It's not horrible, but it's not very nice either.

EDIT:

foo()
<bar /> // JSXStatement
foo()
< bar /> // Comparison and unexpected token
foo();<bar /> // JSXStatement
foo()<bar /> // Comparison and unexpected token

EDIT: So minifiers would be required to insert an explicit semicolon for it to be safely recognized as a JSXStatement.

sebmarkbage commented 7 years ago

Minifiers/printers sometimes insert line breaks at pretty arbitrary places just to avoid too giant lines. That code wouldn't have any spaces in it. I could see this being a breaking change that would mean that we couldn't parse some existing code using the same parser.

I think this would simply not be treated as JSX without the preceding semi-colon. Which is the same as today. So no change needed.

not-an-aardvark commented 7 years ago

I found JSX easy to learn after knowing JS because "A JSXElement is just an expression" is a very simple mental model to understand. To me, it seems like introducing additional semantics such as implicit return would lead to increased confusion over what the syntax is doing, with no clear justification other than saving a few keystrokes from typing return. I think it's unlikely that something like foo.map(X => { <X />; }) is a big point of confusion for people when writing JSX in particular, because people who are confused by that would also end up confused by something like foo.map(value => { value * 2; }) anyway.

sebmarkbage commented 7 years ago

It seems to me that this is an issue for JSX in do-expressions too.

It also seems like it's non-trivial to try to fix this. It might be that you simply have to add a semi-colon for this use case.

If there is nothing we can do to fix this then do we really need JSXStatement as a special syntactical form. Isn't JSXElement in an ExpressionStatement enough?